From 88ee8f63498c1322d0d9fe4b639d41babbccecc3 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 19 Feb 2016 18:11:45 -0800 Subject: [PATCH 01/22] WIP: Award emojis to comments --- app/assets/javascripts/awards_handler.coffee | 32 ++++---- app/assets/javascripts/notes.js.coffee | 4 +- .../concerns/toggle_emoji_award.rb | 20 +++++ app/controllers/projects/issues_controller.rb | 6 +- .../projects/merge_requests_controller.rb | 6 +- app/controllers/projects/notes_controller.rb | 44 ++++------- app/finders/notes_finder.rb | 4 +- app/helpers/issues_helper.rb | 13 ++-- app/models/concerns/awardable.rb | 74 +++++++++++++++++++ app/models/concerns/issuable.rb | 32 +------- app/models/emoji_award.rb | 27 +++++++ app/models/note.rb | 66 +++++------------ app/models/user.rb | 2 +- app/services/notes/create_service.rb | 8 ++ app/services/notes/post_process_service.rb | 2 +- .../emoji_awards/_awards_block.html.haml | 48 ++++++++++++ app/views/projects/issues/show.html.haml | 2 +- .../projects/merge_requests/_show.html.haml | 2 +- app/views/votes/_votes_block.html.haml | 48 ------------ config/routes.rb | 7 +- db/migrate/20160219152120_add_emoji_awards.rb | 15 ++++ ...55008_convert_award_note_to_emoji_award.rb | 9 +++ .../20160219155039_remove_note_is_award.rb | 5 ++ db/schema.rb | 17 ++++- lib/api/entities.rb | 7 +- lib/award_emoji.rb | 2 +- spec/factories/notes.rb | 2 +- spec/helpers/issues_helper_spec.rb | 6 +- 28 files changed, 307 insertions(+), 203 deletions(-) create mode 100644 app/controllers/concerns/toggle_emoji_award.rb create mode 100644 app/models/concerns/awardable.rb create mode 100644 app/models/emoji_award.rb create mode 100644 app/views/emoji_awards/_awards_block.html.haml delete mode 100644 app/views/votes/_votes_block.html.haml create mode 100644 db/migrate/20160219152120_add_emoji_awards.rb create mode 100644 db/migrate/20160219155008_convert_award_note_to_emoji_award.rb create mode 100644 db/migrate/20160219155039_remove_note_is_award.rb diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee index 360acb864f62..d0056ceae294 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -1,5 +1,5 @@ class @AwardsHandler - constructor: (@post_emoji_url, @noteable_type, @noteable_id, @aliases) -> + constructor: (@post_emoji_url, @aliases) -> $(".add-award").click (event)-> event.stopPropagation() event.preventDefault() @@ -32,7 +32,7 @@ class @AwardsHandler counter = @findEmojiIcon(emoji).siblings(".counter") counter.text(parseInt(counter.text()) + 1) counter.parent().addClass("active") - @addMeToAuthorList(emoji) + @addMeToUserList(emoji) else @createEmoji(emoji) @@ -48,31 +48,31 @@ class @AwardsHandler if parseInt(counter.text()) > 1 counter.text(parseInt(counter.text()) - 1) emojiIcon.removeClass("active") - @removeMeFromAuthorList(emoji) + @removeMeFromUserList(emoji) else if emoji == "thumbsup" || emoji == "thumbsdown" emojiIcon.tooltip("destroy") counter.text(0) emojiIcon.removeClass("active") - @removeMeFromAuthorList(emoji) + @removeMeFromUserList(emoji) else emojiIcon.tooltip("destroy") emojiIcon.remove() - removeMeFromAuthorList: (emoji) -> + removeMeFromUserList: (emoji) -> award_block = @findEmojiIcon(emoji).parent() - authors = award_block.attr("data-original-title").split(", ") - authors.splice(authors.indexOf("me"),1) - award_block.closest(".award").attr("data-original-title", authors.join(", ")) + users = award_block.attr("data-original-title").split(", ") + users.splice(users.indexOf("me"),1) + award_block.closest(".award").attr("data-original-title", users.join(", ")) @resetTooltip(award_block) - addMeToAuthorList: (emoji) -> + addMeToUserList: (emoji) -> award_block = @findEmojiIcon(emoji).parent() origTitle = award_block.attr("data-original-title").trim() - authors = [] + users = [] if origTitle - authors = origTitle.split(', ') - authors.push("me") - award_block.attr("title", authors.join(", ")) + users = origTitle.split(', ') + users.push("me") + award_block.attr("title", users.join(", ")) @resetTooltip(award_block) resetTooltip: (award) -> @@ -109,11 +109,7 @@ class @AwardsHandler "emoji-#{unicodeName}" postEmoji: (emoji, callback) -> - $.post @post_emoji_url, { note: { - note: ":#{emoji}:" - noteable_type: @noteable_type - noteable_id: @noteable_id - }},(data) -> + $.post @post_emoji_url, { name: emoji }, (data) -> if data.ok callback.call() diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index 3347ab65c902..91d7c4f8e78f 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -120,12 +120,12 @@ class @Notes renderNote: (note) -> unless note.valid if note.award - flash = new Flash('You have already used this award emoji!', 'alert') + flash = new Flash('You have already awarded this emoji!', 'alert') flash.pinTo('.header-content') return if note.award - awards_handler.addAwardToEmojiBar(note.note) + awards_handler.addAwardToEmojiBar(note.name) awards_handler.scrollToAwards() # render note if it not present in loaded list diff --git a/app/controllers/concerns/toggle_emoji_award.rb b/app/controllers/concerns/toggle_emoji_award.rb new file mode 100644 index 000000000000..a2dee8d585b1 --- /dev/null +++ b/app/controllers/concerns/toggle_emoji_award.rb @@ -0,0 +1,20 @@ +module ToggleEmojiAward + extend ActiveSupport::Concern + + included do + before_action :authenticate_user!, only: [:toggle_emoji_award] + end + + def toggle_emoji_award + name = params.require(:name) + awardable.toggle_emoji_award(name, current_user) + + render json: { ok: true } + end + + private + + def awardable + raise NotImplementedError + end +end diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 68244883803f..79d7ca8a350b 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -1,4 +1,6 @@ class Projects::IssuesController < Projects::ApplicationController + include ToggleEmojiAward + before_action :module_enabled before_action :issue, only: [:edit, :update, :show, :toggle_subscription] @@ -59,7 +61,7 @@ def edit def show @note = @project.notes.new(noteable: @issue) - @notes = @issue.notes.nonawards.with_associations.fresh + @notes = @issue.notes.with_associations.fresh @noteable = @issue @merge_requests = @issue.referenced_merge_requests(current_user) @@ -129,6 +131,8 @@ def issue end end + alias_method :awardable, :issue + def authorize_update_issue! return render_404 unless can?(current_user, :update_issue, @issue) end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 9d588c370aab..c54cd9d5da28 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -1,4 +1,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController + include ToggleEmojiAward + before_action :module_enabled before_action :merge_request, only: [ :edit, :update, :show, :diffs, :commits, :builds, :merge, :merge_check, @@ -247,6 +249,8 @@ def merge_request @merge_request ||= @project.merge_requests.find_by!(iid: params[:id]) end + alias_method :awardable, :merge_request + def closes_issues @closes_issues ||= @merge_request.closes_issues end @@ -281,7 +285,7 @@ def validates_merge_request def define_show_vars # Build a note object for comment form @note = @project.notes.new(noteable: @merge_request) - @notes = @merge_request.mr_and_commit_notes.nonawards.inc_author.fresh + @notes = @merge_request.mr_and_commit_notes.inc_author.fresh @discussions = Note.discussions_from_notes(@notes) @noteable = @merge_request diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index 1b9dd5680432..e5aa1b2a55bd 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -1,9 +1,11 @@ class Projects::NotesController < Projects::ApplicationController + include ToggleEmojiAward + # Authorize before_action :authorize_read_note! before_action :authorize_create_note!, only: [:create] before_action :authorize_admin_note!, only: [:update, :destroy] - before_action :find_current_user_notes, except: [:destroy, :delete_attachment, :award_toggle] + before_action :find_current_user_notes, only: [:index] def index current_fetched_at = Time.now.to_i @@ -20,7 +22,7 @@ def index end def create - @note = Notes::CreateService.new(project, current_user, note_params).execute + @note = Notes::CreateService.new(project, current_user, note_params.merge(create_emoji_awards: true)).execute respond_to do |format| format.json { render json: note_json(@note) } @@ -57,36 +59,14 @@ def delete_attachment end end - def award_toggle - noteable = if note_params[:noteable_type] == "issue" - project.issues.find(note_params[:noteable_id]) - else - project.merge_requests.find(note_params[:noteable_id]) - end - - data = { - author: current_user, - is_award: true, - note: note_params[:note].delete(":") - } - - note = noteable.notes.find_by(data) - - if note - note.destroy - else - Notes::CreateService.new(project, current_user, note_params).execute - end - - render json: { ok: true } - end - private def note @note ||= @project.notes.find(params[:id]) end + alias_method :awardable, :note + def note_to_html(note) render_to_string( "projects/notes/_note", @@ -132,21 +112,27 @@ def note_to_discussion_with_diff_html(note) end def note_json(note) - if note.valid? + if note.persisted? { valid: true, id: note.id, discussion_id: note.discussion_id, html: note_to_html(note), - award: note.is_award, note: note.note, discussion_html: note_to_discussion_html(note), discussion_with_diff_html: note_to_discussion_with_diff_html(note) } + elsif note.emoji_award? + emoji_award = note.emoji_award + { + valid: emoji_award.valid?, + award: true, + id: emoji_award.id, + name: emoji_award.name + } else { valid: false, - award: note.is_award, errors: note.errors } end diff --git a/app/finders/notes_finder.rb b/app/finders/notes_finder.rb index fa4c635f55cf..ab252821b521 100644 --- a/app/finders/notes_finder.rb +++ b/app/finders/notes_finder.rb @@ -12,9 +12,9 @@ def execute(project, current_user, params) when "commit" project.notes.for_commit_id(target_id).not_inline when "issue" - project.issues.find(target_id).notes.nonawards.inc_author + project.issues.find(target_id).notes.inc_author when "merge_request" - project.merge_requests.find(target_id).mr_and_commit_notes.nonawards.inc_author + project.merge_requests.find(target_id).mr_and_commit_notes.inc_author when "snippet", "project_snippet" project.snippets.find(target_id).notes else diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index ae4ebc0854a9..372607c4908b 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -111,16 +111,17 @@ def emoji_icon(name, unicode = nil, aliases = []) } end - def emoji_author_list(notes, current_user) - list = notes.map do |note| - note.author == current_user ? "me" : note.author.name - end + def award_user_list(awards, current_user) + list = + awards.map do |award| + award.user == current_user ? "me" : note.user.name + end list.join(", ") end - def note_active_class(notes, current_user) - if current_user && notes.pluck(:author_id).include?(current_user.id) + def award_active_class(awards, current_user) + if current_user && awards.map(&:user_id).include?(current_user.id) "active" else "" diff --git a/app/models/concerns/awardable.rb b/app/models/concerns/awardable.rb new file mode 100644 index 000000000000..129ed2bf29e4 --- /dev/null +++ b/app/models/concerns/awardable.rb @@ -0,0 +1,74 @@ +module Awardable + extend ActiveSupport::Concern + + included do + has_many :emoji_awards, as: :awardable, dependent: :destroy + end + + module ClassMethods + def order_upvotes_desc + order_votes_desc(EmojiAward::UPVOTE_NAME) + end + + def order_downvotes_desc + order_votes_desc(EmojiAward::DOWNVOTE_NAME) + end + + def order_votes_desc(emoji_name) + awardable_table = self.arel_table + awards_table = EmojiAward.arel_table + + join_clause = awardable_table.join(awards_table, Arel::Nodes::OuterJoin).on( + awards_table[:awardable_id].eq(awardable_table[:id]).and( + awards_table[:awardable_type].eq(self.name).and( + awards_table[:name].eq(emoji_name) + ) + ) + ).join_sources + + joins(join_clause).group(awardable_table[:id]).reorder("COUNT(emoji_awards.id) DESC") + end + end + + def grouped_awards + awards = emoji_awards.group_by(&:name) + + awards[EmojiAward::UPVOTE_NAME] ||= EmojiAward.none + awards[EmojiAward::DOWNVOTE_NAME] ||= EmojiAward.none + + awards + end + + def downvotes + emoji_awards.where(name: EmojiAward::DOWNVOTE_NAME).count + end + + def upvotes + emoji_awards.where(name: EmojiAward::UPVOTE_NAME).count + end + + def emoji_awardable? + true + end + + def awarded_emoji?(emoji_name, current_user) + emoji_awards.where(name: emoji_name, user: current_user).exists? + end + + def award_emoji(emoji_name, current_user) + return unless emoji_awardable? + emoji_awards.create(name: emoji_name, user: current_user) + end + + def remove_emoji_award(emoji_name, current_user) + emoji_awards.where(name: emoji_name, user: current_user).destroy_all + end + + def toggle_emoji_award(emoji_name, current_user) + if awarded_emoji?(emoji_name, current_user) + remove_emoji_award(emoji_name, current_user) + else + award_emoji(emoji_name, current_user) + end + end +end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index e5f089fb8a09..de17a5dbe222 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -9,6 +9,7 @@ module Issuable include Participable include Mentionable include StripAttribute + include Awardable included do belongs_to :author, class_name: "User" @@ -75,29 +76,6 @@ def sort(method) order_by(method) end end - - def order_downvotes_desc - order_votes_desc('thumbsdown') - end - - def order_upvotes_desc - order_votes_desc('thumbsup') - end - - def order_votes_desc(award_emoji_name) - issuable_table = self.arel_table - note_table = Note.arel_table - - join_clause = issuable_table.join(note_table, Arel::Nodes::OuterJoin).on( - note_table[:noteable_id].eq(issuable_table[:id]).and( - note_table[:noteable_type].eq(self.name).and( - note_table[:is_award].eq(true).and(note_table[:note].eq(award_emoji_name)) - ) - ) - ).join_sources - - joins(join_clause).group(issuable_table[:id]).reorder("COUNT(notes.id) DESC") - end end def today? @@ -120,14 +98,6 @@ def open? opened? || reopened? end - def downvotes - notes.awards.where(note: "thumbsdown").count - end - - def upvotes - notes.awards.where(note: "thumbsup").count - end - def subscribed?(user) subscription = subscriptions.find_by_user_id(user.id) diff --git a/app/models/emoji_award.rb b/app/models/emoji_award.rb new file mode 100644 index 000000000000..309f34e2e3c3 --- /dev/null +++ b/app/models/emoji_award.rb @@ -0,0 +1,27 @@ +class EmojiAward < ActiveRecord::Base + DOWNVOTE_NAME = "thumbsdown".freeze + UPVOTE_NAME = "thumbsup".freeze + + include Participable + + belongs_to :awardable, polymorphic: true + belongs_to :user + + validates :awardable, :user, presence: true + validates :name, presence: true, + inclusion: { in: Emoji.emojis_names } + validates :name, uniqueness: { scope: [:user, :awardable_type, :awardable_id] } + + participant :user + + scope :downvotes, -> { where(name: DOWNVOTE_NAME) } + scope :upvotes, -> { where(name: UPVOTE_NAME) } + + def downvote? + self.name == DOWNVOTE_NAME + end + + def upvote? + self.name == UPVOTE_NAME + end +end diff --git a/app/models/note.rb b/app/models/note.rb index 55255d22c2f4..d91cfd64c4bf 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -16,7 +16,6 @@ # system :boolean default(FALSE), not null # st_diff :text # updated_by_id :integer -# is_award :boolean default(FALSE), not null # require 'carrierwave/orm/activerecord' @@ -26,6 +25,9 @@ class Note < ActiveRecord::Base include Gitlab::CurrentSettings include Participable include Mentionable + include Awardable + + attr_accessor :emoji_award default_value_for :system, false @@ -40,11 +42,7 @@ class Note < ActiveRecord::Base delegate :name, to: :project, prefix: true delegate :name, :email, to: :author, prefix: true - before_validation :set_award! - validates :note, :project, presence: true - validates :note, uniqueness: { scope: [:author, :noteable_type, :noteable_id] }, if: ->(n) { n.is_award } - validates :note, inclusion: { in: Emoji.emojis_names }, if: ->(n) { n.is_award } validates :line_code, line_code: true, allow_blank: true # Attachments are deprecated and are handled by Markdown uploader validates :attachment, file_size: { maximum: :max_attachment_size } @@ -56,8 +54,6 @@ class Note < ActiveRecord::Base mount_uploader :attachment, AttachmentUploader # Scopes - scope :awards, ->{ where(is_award: true) } - scope :nonawards, ->{ where(is_award: false) } scope :for_commit_id, ->(commit_id) { where(noteable_type: "Commit", commit_id: commit_id) } scope :inline, ->{ where("line_code IS NOT NULL") } scope :not_inline, ->{ where(line_code: [nil, '']) } @@ -105,19 +101,6 @@ def build_discussion_id(type, id, line_code) def search(query) where("LOWER(note) like :query", query: "%#{query.downcase}%") end - - def grouped_awards - notes = {} - - awards.select(:note).distinct.map do |note| - notes[note.note] = where(note: note.note) - end - - notes["thumbsup"] ||= Note.none - notes["thumbsdown"] ||= Note.none - - notes - end end def cross_reference? @@ -350,47 +333,38 @@ def system? read_attribute(:system) end - def downvote? - is_award && note == "thumbsdown" + def editable? + !system? end - def upvote? - is_award && note == "thumbsup" + def cross_reference_not_visible_for?(user) + cross_reference? && referenced_mentionables(user).empty? end - def editable? - !system? && !is_award + def emoji_award? + emoji_awards_supported? && contains_only_emoji? end - def cross_reference_not_visible_for?(user) - cross_reference? && referenced_mentionables(user).empty? + def create_emoji_award + self.emoji_award = self.noteable.award_emoji(emoji_award_name, self.author) end - # Checks if note is an award added as a comment - # - # If note is an award, this method sets is_award to true - # and changes content of the note to award name. - # - # Method is executed as a before_validation callback. - # - def set_award! - return unless awards_supported? && contains_emoji_only? - self.is_award = true - self.note = award_emoji_name + def emoji_awardable? + !self.system end private - def awards_supported? - noteable.kind_of?(Issue) || noteable.is_a?(MergeRequest) + def emoji_awards_supported? + noteable.kind_of?(Awardable) end - def contains_emoji_only? - note =~ /\A#{Banzai::Filter::EmojiFilter.emoji_pattern}\s?\Z/ + def emoji_award_name + original_name = note.match(Banzai::Filter::EmojiFilter.emoji_pattern)[1] + AwardEmoji.normalize_emoji_name(original_name) end - def award_emoji_name - original_name = note.match(Banzai::Filter::EmojiFilter.emoji_pattern)[1] - AwardEmoji.normilize_emoji_name(original_name) + def contains_only_emoji? + note =~ /\A#{Banzai::Filter::EmojiFilter.emoji_pattern}\s?\Z/ end end diff --git a/app/models/user.rb b/app/models/user.rb index 9fe94b13e528..7a4059bdb283 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -140,7 +140,7 @@ class User < ActiveRecord::Base has_one :abuse_report, dependent: :destroy has_many :spam_logs, dependent: :destroy has_many :builds, dependent: :nullify, class_name: 'Ci::Build' - + has_many :emoji_awards, as: :awardable, dependent: :destroy # # Validations diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 8d9661167b52..9cea7c7d4dc6 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -1,10 +1,18 @@ module Notes class CreateService < BaseService def execute + create_emoji_awards = params.delete(:create_emoji_awards) + note = project.notes.new(params) + note.author = current_user note.system = false + if create_emoji_awards && note.emoji_award? + note.create_emoji_award + return note + end + if note.save # Finish the harder work in the background NewNoteWorker.perform_in(2.seconds, note.id, params) diff --git a/app/services/notes/post_process_service.rb b/app/services/notes/post_process_service.rb index f37d3c50cdd1..49f0ee0a13fc 100644 --- a/app/services/notes/post_process_service.rb +++ b/app/services/notes/post_process_service.rb @@ -9,7 +9,7 @@ def initialize(note) def execute # Skip system notes, like status changes and cross-references and awards - unless @note.system || @note.is_award + unless @note.system EventCreateService.new.leave_note(@note, @note.author) @note.create_cross_references! execute_note_hooks diff --git a/app/views/emoji_awards/_awards_block.html.haml b/app/views/emoji_awards/_awards_block.html.haml new file mode 100644 index 000000000000..34d7755e2be6 --- /dev/null +++ b/app/views/emoji_awards/_awards_block.html.haml @@ -0,0 +1,48 @@ +.awards.votes-block{data: { toggle_url: url_for([:toggle_emoji_award, @project.namespace.becomes(Namespace), @project, awardable]) }} + - awards_sort(awardable.grouped_awards).each do |emoji, awards| + .award{class: (award_active_class(awards, current_user)), title: award_user_list(awards, current_user)} + = emoji_icon(emoji) + .counter + = awards.count + + - if current_user + .awards-controls + %a.add-award{"href" => "#"} + = icon('smile-o') + .emoji-menu + .emoji-menu-content + = text_field_tag :emoji_search, "", class: "emoji-search search-input form-control" + - AwardEmoji.emoji_by_category.each do |category, emojis| + %h5= AwardEmoji::CATEGORIES[category] + %ul + - emojis.each do |emoji| + %li + = emoji_icon(emoji["name"], emoji["unicode"], emoji["aliases"]) + +- if current_user + :javascript + var aliases = #{AwardEmoji.aliases.to_json}; + + $(".awards").each(function() { + container = $(this) + awards_handler = new AwardsHandler( + container.data("toggleUrl"), + aliases + ); + + container.data("awardsHandler", awards_handler); + + container.on("click", ".emoji-menu-content li", function(e) { + var emoji = $(this).find(".emoji-icon").data("emoji"); + container.data("awardsHandler").addAward(emoji); + }); + + container.on("click", ".award", function(e) { + var emoji = $(this).find(".icon").data("emoji"); + container.data("awardsHandler").addAward(emoji); + }); + + container.find(".award").tooltip(); + }); + + $(".emoji-menu-content").niceScroll({cursorwidth: "7px", autohidemode: false}); diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 69a0e2a0c4dc..a5b39ff73037 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -53,7 +53,7 @@ = render 'merge_requests' .content-block - = render 'votes/votes_block', votable: @issue + = render 'emoji_awards/awards_block', awardable: @issue .row %section.col-md-12 diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index da67645bc2bf..747c7e92c2c7 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -67,7 +67,7 @@ .tab-content #notes.notes.tab-pane.voting_notes .content-block.oneline-block - = render 'votes/votes_block', votable: @merge_request + = render 'emoji_awards/awards_block', awardable: @merge_request .row %section.col-md-12 diff --git a/app/views/votes/_votes_block.html.haml b/app/views/votes/_votes_block.html.haml deleted file mode 100644 index 91c5b7eac5e0..000000000000 --- a/app/views/votes/_votes_block.html.haml +++ /dev/null @@ -1,48 +0,0 @@ -.awards.votes-block - - awards_sort(votable.notes.awards.grouped_awards).each do |emoji, notes| - .award{class: (note_active_class(notes, current_user)), title: emoji_author_list(notes, current_user)} - = emoji_icon(emoji) - .counter - = notes.count - - - if current_user - .awards-controls - %a.add-award{"href" => "#"} - = icon('smile-o') - .emoji-menu - .emoji-menu-content - = text_field_tag :emoji_search, "", class: "emoji-search search-input form-control" - - AwardEmoji.emoji_by_category.each do |category, emojis| - %h5= AwardEmoji::CATEGORIES[category] - %ul - - emojis.each do |emoji| - %li - = emoji_icon(emoji["name"], emoji["unicode"], emoji["aliases"]) - -- if current_user - :javascript - var post_emoji_url = "#{award_toggle_namespace_project_notes_path(@project.namespace, @project)}"; - var noteable_type = "#{votable.class.name.underscore}"; - var noteable_id = "#{votable.id}"; - var aliases = #{AwardEmoji.aliases.to_json}; - - window.awards_handler = new AwardsHandler( - post_emoji_url, - noteable_type, - noteable_id, - aliases - ); - - $(".awards").on("click", ".emoji-menu-content li", function(e) { - var emoji = $(this).find(".emoji-icon").data("emoji"); - awards_handler.addAward(emoji); - }); - - $(".awards").on("click", ".award", function(e) { - var emoji = $(this).find(".icon").data("emoji"); - awards_handler.addAward(emoji); - }); - - $(".award").tooltip(); - - $(".emoji-menu-content").niceScroll({cursorwidth: "7px", autohidemode: false}); diff --git a/config/routes.rb b/config/routes.rb index 507bcbc53d7d..67427c6aeb2a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -590,6 +590,7 @@ post :cancel_merge_when_build_succeeds get :ci_status post :toggle_subscription + post :toggle_emoji_award end collection do @@ -648,6 +649,7 @@ resources :issues, constraints: { id: /\d+/ }, except: [:destroy] do member do post :toggle_subscription + post :toggle_emoji_award end collection do post :bulk_update @@ -672,10 +674,7 @@ resources :notes, only: [:index, :create, :destroy, :update], constraints: { id: /\d+/ } do member do delete :delete_attachment - end - - collection do - post :award_toggle + post :toggle_emoji_award end end diff --git a/db/migrate/20160219152120_add_emoji_awards.rb b/db/migrate/20160219152120_add_emoji_awards.rb new file mode 100644 index 000000000000..c6a45e57da1f --- /dev/null +++ b/db/migrate/20160219152120_add_emoji_awards.rb @@ -0,0 +1,15 @@ +class AddEmojiAwards < ActiveRecord::Migration + def change + create_table :emoji_awards do |t| + t.string :name + t.references :user + t.references :awardable, polymorphic: true + + t.timestamps + end + + add_index :emoji_awards, :user_id + add_index :emoji_awards, :awardable_type + add_index :emoji_awards, :awardable_id + end +end diff --git a/db/migrate/20160219155008_convert_award_note_to_emoji_award.rb b/db/migrate/20160219155008_convert_award_note_to_emoji_award.rb new file mode 100644 index 000000000000..524a01b6fc72 --- /dev/null +++ b/db/migrate/20160219155008_convert_award_note_to_emoji_award.rb @@ -0,0 +1,9 @@ +class ConvertAwardNoteToEmojiAward < ActiveRecord::Migration + def up + execute "INSERT INTO emoji_awards (awardable_type, awardable_id, user_id, name, created_at, updated_at) (SELECT noteable_type, noteable_id, author_id, note, created_at, updated_at FROM notes WHERE is_award = true)" + end + + def down + # TODO + end +end diff --git a/db/migrate/20160219155039_remove_note_is_award.rb b/db/migrate/20160219155039_remove_note_is_award.rb new file mode 100644 index 000000000000..ad1699b646e5 --- /dev/null +++ b/db/migrate/20160219155039_remove_note_is_award.rb @@ -0,0 +1,5 @@ +class RemoveNoteIsAward < ActiveRecord::Migration + def change + remove_column :notes, :is_award + end +end diff --git a/db/schema.rb b/db/schema.rb index 689a8c3ecc52..ec7cb6ba91d3 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160209130428) do +ActiveRecord::Schema.define(version: 20160219155039) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -346,6 +346,19 @@ add_index "emails", ["email"], name: "index_emails_on_email", unique: true, using: :btree add_index "emails", ["user_id"], name: "index_emails_on_user_id", using: :btree + create_table "emoji_awards", force: :cascade do |t| + t.string "name" + t.integer "user_id" + t.integer "awardable_id" + t.string "awardable_type" + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index "emoji_awards", ["awardable_id"], name: "index_emoji_awards_on_awardable_id", using: :btree + add_index "emoji_awards", ["awardable_type"], name: "index_emoji_awards_on_awardable_type", using: :btree + add_index "emoji_awards", ["user_id"], name: "index_emoji_awards_on_user_id", using: :btree + create_table "events", force: :cascade do |t| t.string "target_type" t.integer "target_id" @@ -583,14 +596,12 @@ t.boolean "system", default: false, null: false t.text "st_diff" t.integer "updated_by_id" - t.boolean "is_award", default: false, null: false end add_index "notes", ["author_id"], name: "index_notes_on_author_id", using: :btree add_index "notes", ["commit_id"], name: "index_notes_on_commit_id", using: :btree add_index "notes", ["created_at", "id"], name: "index_notes_on_created_at_and_id", using: :btree add_index "notes", ["created_at"], name: "index_notes_on_created_at", using: :btree - add_index "notes", ["is_award"], name: "index_notes_on_is_award", using: :btree add_index "notes", ["line_code"], name: "index_notes_on_line_code", using: :btree add_index "notes", ["noteable_id", "noteable_type"], name: "index_notes_on_noteable_id_and_noteable_type", using: :btree add_index "notes", ["noteable_type"], name: "index_notes_on_noteable_type", using: :btree diff --git a/lib/api/entities.rb b/lib/api/entities.rb index a9c09ffdb311..dc3b8afaefb9 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -164,11 +164,12 @@ class Issue < ProjectEntity expose :label_names, as: :labels expose :milestone, using: Entities::Milestone expose :assignee, :author, using: Entities::UserBasic + expose :upvotes, :downvotes end class MergeRequest < ProjectEntity expose :target_branch, :source_branch - expose :upvotes, :downvotes + expose :upvotes, :downvotes expose :author, :assignee, using: Entities::UserBasic expose :source_project_id, :target_project_id expose :label_names, as: :labels @@ -202,8 +203,8 @@ class Note < Grape::Entity expose :system?, as: :system expose :noteable_id, :noteable_type # upvote? and downvote? are deprecated, always return false - expose :upvote?, as: :upvote - expose :downvote?, as: :downvote + expose(:upvote?) { |note| false } + expose(:downvote?) { |note| false } end class MRNote < Grape::Entity diff --git a/lib/award_emoji.rb b/lib/award_emoji.rb index 783fcfb61ad0..36f11b4eb8a0 100644 --- a/lib/award_emoji.rb +++ b/lib/award_emoji.rb @@ -14,7 +14,7 @@ class AwardEmoji food_drink: "Food" }.with_indifferent_access - def self.normilize_emoji_name(name) + def self.normalize_emoji_name(name) aliases[name] || name end diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb index 32c202891d83..c4dd8ebd4780 100644 --- a/spec/factories/notes.rb +++ b/spec/factories/notes.rb @@ -29,7 +29,7 @@ factory :note_on_commit, traits: [:on_commit] factory :note_on_commit_diff, traits: [:on_commit, :on_diff] - factory :note_on_issue, traits: [:on_issue], aliases: [:votable_note] + factory :note_on_issue, traits: [:on_issue], aliases: [:awardable_note] factory :note_on_merge_request, traits: [:on_merge_request] factory :note_on_merge_request_diff, traits: [:on_merge_request, :on_diff] factory :note_on_project_snippet, traits: [:on_project_snippet] diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb index ffd8ebae0297..27b4cad88e4e 100644 --- a/spec/helpers/issues_helper_spec.rb +++ b/spec/helpers/issues_helper_spec.rb @@ -127,18 +127,18 @@ it { is_expected.to eq("!1, !2, or !3") } end - describe "#note_active_class" do + describe "#award_active_class" do before do @note = create :note @note1 = create :note end it "returns empty string for unauthenticated user" do - expect(note_active_class(Note.all, nil)).to eq("") + expect(award_active_class(Note.all, nil)).to eq("") end it "returns active string for author" do - expect(note_active_class(Note.all, @note.author)).to eq("active") + expect(award_active_class(Note.all, @note.author)).to eq("active") end end -- GitLab From a0ce40d7065f0c97d4c2f3b6649e555689ff2432 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 31 Mar 2016 09:21:33 +0100 Subject: [PATCH 02/22] Refactored JS slightly to make things easier --- app/assets/javascripts/awards_handler.coffee | 31 ++++++------ .../emoji_awards/_awards_block.html.haml | 50 +++++-------------- 2 files changed, 27 insertions(+), 54 deletions(-) diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee index 3ca808a7fd57..762e817aa69c 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -1,5 +1,5 @@ class @AwardsHandler - constructor: (@post_emoji_url, @noteable_type, @noteable_id, @aliases) -> + constructor: (@aliases) -> $(".js-add-award").on "click", (event) => event.stopPropagation() event.preventDefault() @@ -17,12 +17,14 @@ class @AwardsHandler @renderFrequentlyUsedBlock() - handleClick: (e) -> + handleClick: (e) => e.preventDefault() - emoji = $(this) + $emojiBtn = $(e.currentTarget) + awardUrl = $emojiBtn.closest('.js-votes-block').data 'award-url' + emoji = $emojiBtn .find(".icon") .data "emoji" - awards_handler.addAward emoji + @addAward awardUrl, emoji showEmojiMenu: -> if $(".emoji-menu").length @@ -43,9 +45,9 @@ class @AwardsHandler @setupSearch() , 200 - addAward: (emoji) -> + addAward: (awardUrl, emoji) -> emoji = @normilizeEmojiName(emoji) - @postEmoji emoji, => + @postEmoji awardUrl, emoji, => @addAwardToEmojiBar(emoji) $(".emoji-menu").removeClass "is-visible" @@ -120,15 +122,12 @@ class @AwardsHandler createEmoji: (emoji) -> emojiCssClass = @resolveNameToCssClass(emoji) - nodes = [] - nodes.push( - "" - ) + buttonHtml = "" - emoji_node = $(nodes.join("\n")) + emoji_node = $(buttonHtml) .insertBefore(".js-award-holder") .find(".emoji-icon") .data("emoji", emoji) @@ -145,8 +144,8 @@ class @AwardsHandler "emoji-#{unicodeName}" - postEmoji: (emoji, callback) -> - $.post @post_emoji_url, { name: emoji }, (data) -> + postEmoji: (awardUrl, emoji, callback) -> + $.post awardUrl, { name: emoji }, (data) -> if data.ok callback.call() diff --git a/app/views/emoji_awards/_awards_block.html.haml b/app/views/emoji_awards/_awards_block.html.haml index 34d7755e2be6..da1de1c1debd 100644 --- a/app/views/emoji_awards/_awards_block.html.haml +++ b/app/views/emoji_awards/_awards_block.html.haml @@ -1,48 +1,22 @@ -.awards.votes-block{data: { toggle_url: url_for([:toggle_emoji_award, @project.namespace.becomes(Namespace), @project, awardable]) }} +.awards.votes-block.js-votes-block{ data: { award_url: url_for([:toggle_emoji_award, @project.namespace.becomes(Namespace), @project, awardable]) } } - awards_sort(awardable.grouped_awards).each do |emoji, awards| - .award{class: (award_active_class(awards, current_user)), title: award_user_list(awards, current_user)} + %button.btn.award-control.js-emoji-btn.has-tooltip{ type: "button", class: (award_active_class(awards, current_user)), title: award_user_list(awards, current_user) } = emoji_icon(emoji) - .counter + %span.award-control-text.js-counter = awards.count - if current_user - .awards-controls - %a.add-award{"href" => "#"} - = icon('smile-o') - .emoji-menu - .emoji-menu-content - = text_field_tag :emoji_search, "", class: "emoji-search search-input form-control" - - AwardEmoji.emoji_by_category.each do |category, emojis| - %h5= AwardEmoji::CATEGORIES[category] - %ul - - emojis.each do |emoji| - %li - = emoji_icon(emoji["name"], emoji["unicode"], emoji["aliases"]) + %div.award-menu-holder.js-award-holder + %button.btn.award-control.js-add-award{ type: "button" } + = icon('smile-o', {class: "award-control-icon"}) + = icon('spinner spin', {class: "award-control-icon award-control-icon-loading"}) + %span.award-control-text + Add - if current_user :javascript var aliases = #{AwardEmoji.aliases.to_json}; - $(".awards").each(function() { - container = $(this) - awards_handler = new AwardsHandler( - container.data("toggleUrl"), - aliases - ); - - container.data("awardsHandler", awards_handler); - - container.on("click", ".emoji-menu-content li", function(e) { - var emoji = $(this).find(".emoji-icon").data("emoji"); - container.data("awardsHandler").addAward(emoji); - }); - - container.on("click", ".award", function(e) { - var emoji = $(this).find(".icon").data("emoji"); - container.data("awardsHandler").addAward(emoji); - }); - - container.find(".award").tooltip(); - }); - - $(".emoji-menu-content").niceScroll({cursorwidth: "7px", autohidemode: false}); + window.awards_handler = new AwardsHandler( + aliases + ); -- GitLab From db5c8a353dd8c7845e6984d8c140eaae3b306306 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 31 Mar 2016 10:55:37 +0100 Subject: [PATCH 03/22] Uses the same emoji-menu and just moves it around depending where it should be viewed --- app/assets/javascripts/awards_handler.coffee | 39 +++++++++++-------- app/assets/javascripts/dispatcher.js.coffee | 2 + .../lib/emoji_aliases.js.coffee.erb | 2 + app/assets/stylesheets/pages/awards.scss | 2 +- app/assets/stylesheets/pages/notes.scss | 33 +++++++++++++++- app/helpers/issues_helper.rb | 2 +- .../emoji_awards/_awards_block.html.haml | 14 ++----- app/views/projects/notes/_note.html.haml | 6 +++ 8 files changed, 68 insertions(+), 32 deletions(-) create mode 100644 app/assets/javascripts/lib/emoji_aliases.js.coffee.erb diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee index 762e817aa69c..9e3d9f091261 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -1,22 +1,22 @@ class @AwardsHandler - constructor: (@aliases) -> - $(".js-add-award").on "click", (event) => + constructor: -> + @aliases = emojiAliases() + + $(document).on "click", ".js-add-award", (event) => event.stopPropagation() event.preventDefault() - @showEmojiMenu() + @showEmojiMenu $(event.currentTarget) $("html").on 'click', (event) -> if !$(event.target).closest(".emoji-menu").length if $(".emoji-menu").is(":visible") $(".emoji-menu").removeClass "is-visible" - $(".awards") - .off "click" + $(document) + .off "click", ".js-emoji-btn" .on "click", ".js-emoji-btn", @handleClick - @renderFrequentlyUsedBlock() - handleClick: (e) => e.preventDefault() $emojiBtn = $(e.currentTarget) @@ -26,8 +26,13 @@ class @AwardsHandler .data "emoji" @addAward awardUrl, emoji - showEmojiMenu: -> + showEmojiMenu: ($addBtn) -> if $(".emoji-menu").length + $holder = $addBtn.closest('.js-award-holder') + + if $holder.find('.emoji-menu').length is 0 + $(".emoji-menu").detach().appendTo $holder + if $(".emoji-menu").is ".is-visible" $(".emoji-menu").removeClass "is-visible" $("#emoji_search").blur() @@ -35,10 +40,11 @@ class @AwardsHandler $(".emoji-menu").addClass "is-visible" $("#emoji_search").focus() else - $('.js-add-award').addClass "is-loading" - $.get "/emojis", (response) => - $('.js-add-award').removeClass "is-loading" - $(".js-award-holder").append response + $addBtn.addClass "is-loading" + $.get $addBtn.data('award-menu-url'), (response) => + $addBtn.removeClass "is-loading" + $addBtn.closest('.js-award-holder').append response + @renderFrequentlyUsedBlock() setTimeout => $(".emoji-menu").addClass "is-visible" $("#emoji_search").focus() @@ -128,7 +134,7 @@ class @AwardsHandler " emoji_node = $(buttonHtml) - .insertBefore(".js-award-holder") + .insertBefore(".js-award-holder:not(.js-award-action-btn)") .find(".emoji-icon") .data("emoji", emoji) $('.award-control').tooltip() @@ -173,16 +179,15 @@ class @AwardsHandler if $.cookie('frequently_used_emojis') frequently_used_emojis = @getFrequentlyUsedEmojis() - ul = $("
    ") + ul = $("
      ") for emoji in frequently_used_emojis - do (emoji) -> - $(".emoji-menu-content [data-emoji='#{emoji}']").closest("li").clone().appendTo(ul) + $(".emoji-menu-content [data-emoji='#{emoji}']").closest("li").clone().appendTo(ul) $("input.emoji-search").after(ul).after($("
      ").text("Frequently used")) setupSearch: -> - $("input.emoji-search").keyup (ev) => + $("input.emoji-search").on 'keyup', (ev) => term = $(ev.target).val() # Clean previous search results diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee index f5e1ca9860da..96b5cff56ede 100644 --- a/app/assets/javascripts/dispatcher.js.coffee +++ b/app/assets/javascripts/dispatcher.js.coffee @@ -22,6 +22,7 @@ class Dispatcher new Issue() shortcut_handler = new ShortcutsIssuable() new ZenMode() + window.awards_handler = new AwardsHandler() when 'projects:milestones:show', 'groups:milestones:show', 'dashboard:milestones:show' new Milestone() when 'dashboard:todos:index' @@ -52,6 +53,7 @@ class Dispatcher new Diff() shortcut_handler = new ShortcutsIssuable(true) new ZenMode() + window.awards_handler = new AwardsHandler() when "projects:merge_requests:diffs" new Diff() new ZenMode() diff --git a/app/assets/javascripts/lib/emoji_aliases.js.coffee.erb b/app/assets/javascripts/lib/emoji_aliases.js.coffee.erb new file mode 100644 index 000000000000..66f640a3cb73 --- /dev/null +++ b/app/assets/javascripts/lib/emoji_aliases.js.coffee.erb @@ -0,0 +1,2 @@ +window.emojiAliases = -> + JSON.parse('<%= AwardEmoji.aliases.to_json %>') diff --git a/app/assets/stylesheets/pages/awards.scss b/app/assets/stylesheets/pages/awards.scss index 28994e60baa5..71a25ea0f07f 100644 --- a/app/assets/stylesheets/pages/awards.scss +++ b/app/assets/stylesheets/pages/awards.scss @@ -107,7 +107,7 @@ } &.is-loading { - .award-control-icon { + .award-control-icon-normal { display: none; } diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 4bd2016bdcfe..411cd3cd2302 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -113,8 +113,6 @@ ul.notes { } .note-body { - overflow: auto; - .note-text { overflow: auto; word-wrap: break-word; @@ -280,3 +278,34 @@ ul.notes { } } } + +.note-action-award-holder { + .emoji-menu { + left: auto; + right: -15px; + transform-origin: 100% -45px; + } +} + +.note-award-control { + display: block; + + &:hover, + &:focus { + text-decoration: none; + } + + .award-control-icon-loading { + display: none; + } + + &.is-loading { + .award-control-icon-normal { + display: none; + } + + .award-control-icon-loading { + display: block; + } + } +} diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 3f08435313a7..36b9f3777ccd 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -131,7 +131,7 @@ def emoji_icon(name, unicode = nil, aliases = []) def award_user_list(awards, current_user) list = awards.map do |award| - award.user == current_user ? "me" : note.user.name + award.user == current_user ? "me" : award.user.name end list.join(", ") diff --git a/app/views/emoji_awards/_awards_block.html.haml b/app/views/emoji_awards/_awards_block.html.haml index da1de1c1debd..79ebaa6777e4 100644 --- a/app/views/emoji_awards/_awards_block.html.haml +++ b/app/views/emoji_awards/_awards_block.html.haml @@ -6,17 +6,9 @@ = awards.count - if current_user - %div.award-menu-holder.js-award-holder - %button.btn.award-control.js-add-award{ type: "button" } - = icon('smile-o', {class: "award-control-icon"}) + .award-menu-holder.js-award-holder + %button.btn.award-control.js-add-award{ type: "button", data: { award_menu_url: emojis_path } } + = icon('smile-o', {class: "award-control-icon award-control-icon-normal"}) = icon('spinner spin', {class: "award-control-icon award-control-icon-loading"}) %span.award-control-text Add - -- if current_user - :javascript - var aliases = #{AwardEmoji.aliases.to_json}; - - window.awards_handler = new AwardsHandler( - aliases - ); diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index 2cf32e6093de..72d75ac61d01 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -10,6 +10,11 @@ = link_to '#', title: 'Edit comment', class: 'js-note-edit' do = icon('pencil-square-o') + .award-menu-holder.note-action-award-holder.js-award-holder.js-award-action-btn + = link_to '#', title: 'Award emoji', class: 'note-award-control js-add-award', data: { award_menu_url: emojis_path } do + = icon('smile-o', {class: "award-control-icon award-control-icon-normal"}) + = icon('spinner spin', {class: "award-control-icon award-control-icon-loading"}) + = link_to namespace_project_note_path(note.project.namespace, note.project, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'js-note-delete danger' do = icon('trash-o') @@ -33,6 +38,7 @@ = markdown(note.note, pipeline: :note, cache_key: [note, "note"]) - if note_editable?(note) = render 'projects/notes/edit_form', note: note + = render 'emoji_awards/awards_block', awardable: note = edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago', include_author: true) - if note.attachment.url -- GitLab From 3954b2c66f03b1e1533744ec6867d08a4c4b17c5 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 31 Mar 2016 12:08:31 +0100 Subject: [PATCH 04/22] Shows the bar on notes if a new award is added Correctly adds/removes awards in notes --- app/assets/javascripts/awards_handler.coffee | 33 +++++++++++++------ app/models/concerns/awardable.rb | 8 +++-- .../emoji_awards/_awards_block.html.haml | 5 +-- app/views/projects/issues/show.html.haml | 2 +- .../projects/merge_requests/_show.html.haml | 2 +- app/views/projects/notes/_note.html.haml | 10 +++--- 6 files changed, 38 insertions(+), 22 deletions(-) diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee index 9e3d9f091261..4ee0c6e215d9 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -20,7 +20,13 @@ class @AwardsHandler handleClick: (e) => e.preventDefault() $emojiBtn = $(e.currentTarget) - awardUrl = $emojiBtn.closest('.js-votes-block').data 'award-url' + $votesBlock = $($emojiBtn.closest('.js-award-holder').data('target')) + + if $votesBlock.length is 0 + $votesBlock = $emojiBtn.closest('.js-awards-block') + + $votesBlock.addClass 'js-awards-block-current' + awardUrl = $votesBlock.data 'award-url' emoji = $emojiBtn .find(".icon") .data "emoji" @@ -44,7 +50,9 @@ class @AwardsHandler $.get $addBtn.data('award-menu-url'), (response) => $addBtn.removeClass "is-loading" $addBtn.closest('.js-award-holder').append response + @renderFrequentlyUsedBlock() + setTimeout => $(".emoji-menu").addClass "is-visible" $("#emoji_search").focus() @@ -56,14 +64,18 @@ class @AwardsHandler @postEmoji awardUrl, emoji, => @addAwardToEmojiBar(emoji) + $('.js-awards-block-current').removeClass 'js-awards-block-current' + $(".emoji-menu").removeClass "is-visible" addAwardToEmojiBar: (emoji) -> @addEmojiToFrequentlyUsedList(emoji) emoji = @normilizeEmojiName(emoji) - if @exist(emoji) - if @isActive(emoji) + $emojiBtn = @findEmojiIcon(emoji) + + if $emojiBtn.length > 0 + if @isActive($emojiBtn) @decrementCounter(emoji) else counter = @findEmojiIcon(emoji).siblings(".js-counter") @@ -73,11 +85,8 @@ class @AwardsHandler else @createEmoji(emoji) - exist: (emoji) -> - @findEmojiIcon(emoji).length > 0 - - isActive: (emoji) -> - @findEmojiIcon(emoji).parent().hasClass("active") + isActive: ($emojiBtn) -> + $emojiBtn.parent().hasClass("active") decrementCounter: (emoji) -> counter = @findEmojiIcon(emoji).siblings(".js-counter") @@ -134,11 +143,15 @@ class @AwardsHandler " emoji_node = $(buttonHtml) - .insertBefore(".js-award-holder:not(.js-award-action-btn)") + .insertBefore(".js-awards-block-current .js-award-holder:not(.js-award-action-btn)") .find(".emoji-icon") .data("emoji", emoji) $('.award-control').tooltip() + $currentBlock = $('.js-awards-block-current') + if $currentBlock.is('.hidden') + $currentBlock.removeClass 'hidden' + resolveNameToCssClass: (emoji) -> emoji_icon = $(".emoji-menu-content [data-emoji='#{emoji}']") @@ -156,7 +169,7 @@ class @AwardsHandler callback.call() findEmojiIcon: (emoji) -> - $(".awards > .js-emoji-btn [data-emoji='#{emoji}']") + $(".js-awards-block-current.awards > .js-emoji-btn [data-emoji='#{emoji}']") scrollToAwards: -> $('body, html').animate({ diff --git a/app/models/concerns/awardable.rb b/app/models/concerns/awardable.rb index 129ed2bf29e4..41e34c5e3887 100644 --- a/app/models/concerns/awardable.rb +++ b/app/models/concerns/awardable.rb @@ -30,11 +30,13 @@ def order_votes_desc(emoji_name) end end - def grouped_awards + def grouped_awards(with_thumbs = true) awards = emoji_awards.group_by(&:name) - awards[EmojiAward::UPVOTE_NAME] ||= EmojiAward.none - awards[EmojiAward::DOWNVOTE_NAME] ||= EmojiAward.none + if with_thumbs + awards[EmojiAward::UPVOTE_NAME] ||= EmojiAward.none + awards[EmojiAward::DOWNVOTE_NAME] ||= EmojiAward.none + end awards end diff --git a/app/views/emoji_awards/_awards_block.html.haml b/app/views/emoji_awards/_awards_block.html.haml index 79ebaa6777e4..7df64253f25b 100644 --- a/app/views/emoji_awards/_awards_block.html.haml +++ b/app/views/emoji_awards/_awards_block.html.haml @@ -1,5 +1,6 @@ -.awards.votes-block.js-votes-block{ data: { award_url: url_for([:toggle_emoji_award, @project.namespace.becomes(Namespace), @project, awardable]) } } - - awards_sort(awardable.grouped_awards).each do |emoji, awards| +- grouped_emojis = awardable.grouped_awards(inline) +.awards.js-awards-block{ class: ("hidden" if !inline && grouped_emojis.size == 0), data: { award_url: url_for([:toggle_emoji_award, @project.namespace.becomes(Namespace), @project, awardable]) } } + - awards_sort(grouped_emojis).each do |emoji, awards| %button.btn.award-control.js-emoji-btn.has-tooltip{ type: "button", class: (award_active_class(awards, current_user)), title: award_user_list(awards, current_user) } = emoji_icon(emoji) %span.award-control-text.js-counter diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index be39a37802fa..7d8f55911a0f 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -70,7 +70,7 @@ .content-block.content-block-small = render 'new_branch' - = render 'emoji_awards/awards_block', awardable: @issue + = render 'emoji_awards/awards_block', awardable: @issue, inline: true .row %section.col-md-12 diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 501a8a986bfb..3d3279cca6a0 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -69,7 +69,7 @@ .tab-content #notes.notes.tab-pane.voting_notes .content-block.content-block-small.oneline-block - = render 'emoji_awards/awards_block', awardable: @merge_request + = render 'emoji_awards/awards_block', awardable: @merge_request, inline: true .row %section.col-md-12 diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index 72d75ac61d01..1c5992f2c25a 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -7,14 +7,14 @@ .note-header - if note_editable?(note) .note-actions - = link_to '#', title: 'Edit comment', class: 'js-note-edit' do - = icon('pencil-square-o') - - .award-menu-holder.note-action-award-holder.js-award-holder.js-award-action-btn + .award-menu-holder.note-action-award-holder.js-award-holder.js-award-action-btn{ data: { target: "##{dom_id(note)} .js-awards-block" } } = link_to '#', title: 'Award emoji', class: 'note-award-control js-add-award', data: { award_menu_url: emojis_path } do = icon('smile-o', {class: "award-control-icon award-control-icon-normal"}) = icon('spinner spin', {class: "award-control-icon award-control-icon-loading"}) + = link_to '#', title: 'Edit comment', class: 'js-note-edit' do + = icon('pencil-square-o') + = link_to namespace_project_note_path(note.project.namespace, note.project, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'js-note-delete danger' do = icon('trash-o') @@ -38,7 +38,7 @@ = markdown(note.note, pipeline: :note, cache_key: [note, "note"]) - if note_editable?(note) = render 'projects/notes/edit_form', note: note - = render 'emoji_awards/awards_block', awardable: note + = render 'emoji_awards/awards_block', awardable: note, inline: false = edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago', include_author: true) - if note.attachment.url -- GitLab From cdeb70e471512da08e18b55432cb2e95356abcee Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 31 Mar 2016 12:56:48 +0100 Subject: [PATCH 05/22] Removes buttons in notes body --- app/assets/javascripts/awards_handler.coffee | 56 +++++++++++--------- 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee index 4ee0c6e215d9..d4ce01354b46 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -2,11 +2,13 @@ class @AwardsHandler constructor: -> @aliases = emojiAliases() - $(document).on "click", ".js-add-award", (event) => - event.stopPropagation() - event.preventDefault() + $(document) + .off "click", ".js-add-award" + .on "click", ".js-add-award", (event) => + event.stopPropagation() + event.preventDefault() - @showEmojiMenu $(event.currentTarget) + @showEmojiMenu $(event.currentTarget) $("html").on 'click', (event) -> if !$(event.target).closest(".emoji-menu").length @@ -72,40 +74,42 @@ class @AwardsHandler @addEmojiToFrequentlyUsedList(emoji) emoji = @normilizeEmojiName(emoji) - $emojiBtn = @findEmojiIcon(emoji) + $emojiBtn = @findEmojiIcon(emoji).parent() if $emojiBtn.length > 0 if @isActive($emojiBtn) - @decrementCounter(emoji) + @decrementCounter($emojiBtn, emoji) else - counter = @findEmojiIcon(emoji).siblings(".js-counter") + counter = $emojiBtn.siblings(".js-counter") counter.text(parseInt(counter.text()) + 1) - counter.parent().addClass("active") + $emojiBtn.addClass("active") @addMeToUserList(emoji) else @createEmoji(emoji) isActive: ($emojiBtn) -> - $emojiBtn.parent().hasClass("active") - - decrementCounter: (emoji) -> - counter = @findEmojiIcon(emoji).siblings(".js-counter") - emojiIcon = counter.parent() - if parseInt(counter.text()) > 1 - counter.text(parseInt(counter.text()) - 1) - emojiIcon.removeClass("active") - @removeMeFromUserList(emoji) - else if emoji == "thumbsup" || emoji == "thumbsdown" - emojiIcon.tooltip("destroy") - counter.text(0) - emojiIcon.removeClass("active") - @removeMeFromUserList(emoji) + $emojiBtn.hasClass("active") + + decrementCounter: ($emojiBtn, emoji) -> + isntNoteBody = $emojiBtn.closest('.note-body').length is 0 + counter = $('.js-counter', $emojiBtn) + counterNumber = parseInt(counter.text()) + + if counterNumber > 1 + counter.text(counterNumber - 1) + @removeMeFromUserList($emojiBtn, emoji) + else if (emoji == "thumbsup" || emoji == "thumbsdown") && isntNoteBody + $emojiBtn.tooltip("destroy") + counter.text('0') + @removeMeFromUserList($emojiBtn, emoji) else - emojiIcon.tooltip("destroy") - emojiIcon.remove() + $emojiBtn.tooltip("destroy") + $emojiBtn.remove() - removeMeFromUserList: (emoji) -> - award_block = @findEmojiIcon(emoji).parent() + $emojiBtn.removeClass("active") + + removeMeFromUserList: ($emojiBtn, emoji) -> + award_block = $emojiBtn authors = award_block .attr("data-original-title") .split(", ") -- GitLab From 83e0d96dcfc6a5d52bb66e056797cb0324b50f7e Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 31 Mar 2016 13:17:57 +0100 Subject: [PATCH 06/22] Hides the row in notes body when empty of emojis --- app/assets/javascripts/awards_handler.coffee | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee index d4ce01354b46..fb880d28f2cc 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -95,6 +95,11 @@ class @AwardsHandler counter = $('.js-counter', $emojiBtn) counterNumber = parseInt(counter.text()) + if !isntNoteBody + console.log $emojiBtn.get(0) + # If this is a note body, we just hide the award emoji row like the initial state + $emojiBtn.closest('.js-awards-block').addClass 'hidden' + if counterNumber > 1 counter.text(counterNumber - 1) @removeMeFromUserList($emojiBtn, emoji) -- GitLab From b2b30f99df312269826062c99b8e22ae7fb12940 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 31 Mar 2016 13:20:53 +0100 Subject: [PATCH 07/22] Removed console log --- app/assets/javascripts/awards_handler.coffee | 1 - 1 file changed, 1 deletion(-) diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee index fb880d28f2cc..9d7ed87e5988 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -96,7 +96,6 @@ class @AwardsHandler counterNumber = parseInt(counter.text()) if !isntNoteBody - console.log $emojiBtn.get(0) # If this is a note body, we just hide the award emoji row like the initial state $emojiBtn.closest('.js-awards-block').addClass 'hidden' -- GitLab From b9a216a5c24ecc5b20b2aa4b9f5a3ee21b2b1126 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 31 Mar 2016 15:19:38 +0100 Subject: [PATCH 08/22] Changed design of inline award picker to be similar to designs --- app/assets/stylesheets/pages/awards.scss | 2 -- app/assets/stylesheets/pages/notes.scss | 12 ++++++++++ app/views/projects/notes/_note.html.haml | 3 ++- app/views/votes/_votes_block.html.haml | 28 ------------------------ 4 files changed, 14 insertions(+), 31 deletions(-) delete mode 100644 app/views/votes/_votes_block.html.haml diff --git a/app/assets/stylesheets/pages/awards.scss b/app/assets/stylesheets/pages/awards.scss index 71a25ea0f07f..5550b9856961 100644 --- a/app/assets/stylesheets/pages/awards.scss +++ b/app/assets/stylesheets/pages/awards.scss @@ -1,6 +1,4 @@ .awards { - line-height: 34px; - .emoji-icon { width: 20px; height: 20px; diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 411cd3cd2302..012799b22657 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -309,3 +309,15 @@ ul.notes { } } } + +.note-awards { + .awards { + padding-top: 10px; + } + + .award-control { + padding-top: 2px; + padding-bottom: 2px; + font-size: 13px; + } +} diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index 1c5992f2c25a..0c2775f1320d 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -38,7 +38,8 @@ = markdown(note.note, pipeline: :note, cache_key: [note, "note"]) - if note_editable?(note) = render 'projects/notes/edit_form', note: note - = render 'emoji_awards/awards_block', awardable: note, inline: false + .note-awards + = render 'emoji_awards/awards_block', awardable: note, inline: false = edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago', include_author: true) - if note.attachment.url diff --git a/app/views/votes/_votes_block.html.haml b/app/views/votes/_votes_block.html.haml deleted file mode 100644 index 026472297764..000000000000 --- a/app/views/votes/_votes_block.html.haml +++ /dev/null @@ -1,28 +0,0 @@ -.awards.votes-block - - awards_sort(votable.notes.awards.grouped_awards).each do |emoji, notes| - %button.btn.award-control.js-emoji-btn.has-tooltip{class: (note_active_class(notes, current_user)), title: emoji_author_list(notes, current_user), data: {placement: "top"}} - = emoji_icon(emoji) - %span.award-control-text.js-counter - = notes.count - - - if current_user - %div.award-menu-holder.js-award-holder - %a.btn.award-control.js-add-award{"href" => "#"} - = icon('smile-o', {class: "award-control-icon"}) - = icon('spinner spin', {class: "award-control-icon award-control-icon-loading"}) - %span.award-control-text - Add - -- if current_user - :javascript - var post_emoji_url = "#{award_toggle_namespace_project_notes_path(@project.namespace, @project)}"; - var noteable_type = "#{votable.class.name.underscore}"; - var noteable_id = "#{votable.id}"; - var aliases = #{AwardEmoji.aliases.to_json}; - - window.awards_handler = new AwardsHandler( - post_emoji_url, - noteable_type, - noteable_id, - aliases - ); -- GitLab From 4ed983f42961c6258ee56d8861764efa916016ec Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 31 Mar 2016 17:28:52 +0100 Subject: [PATCH 09/22] Fixed bug with counter not changing Fixed bug with bar hiding when more than 1 button --- app/assets/javascripts/awards_handler.coffee | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee index 9d7ed87e5988..d74c79adbf1c 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -80,8 +80,8 @@ class @AwardsHandler if @isActive($emojiBtn) @decrementCounter($emojiBtn, emoji) else - counter = $emojiBtn.siblings(".js-counter") - counter.text(parseInt(counter.text()) + 1) + $counter = $emojiBtn.find('.js-counter') + $counter.text(parseInt($counter.text()) + 1) $emojiBtn.addClass("active") @addMeToUserList(emoji) else @@ -91,14 +91,11 @@ class @AwardsHandler $emojiBtn.hasClass("active") decrementCounter: ($emojiBtn, emoji) -> + $awardsBlock = $emojiBtn.closest('.js-awards-block') isntNoteBody = $emojiBtn.closest('.note-body').length is 0 counter = $('.js-counter', $emojiBtn) counterNumber = parseInt(counter.text()) - if !isntNoteBody - # If this is a note body, we just hide the award emoji row like the initial state - $emojiBtn.closest('.js-awards-block').addClass 'hidden' - if counterNumber > 1 counter.text(counterNumber - 1) @removeMeFromUserList($emojiBtn, emoji) @@ -112,6 +109,10 @@ class @AwardsHandler $emojiBtn.removeClass("active") + if !isntNoteBody and $awardsBlock.children('.js-emoji-btn').length is 0 + # If this is a note body, we just hide the award emoji row like the initial state + $awardsBlock.addClass 'hidden' + removeMeFromUserList: ($emojiBtn, emoji) -> award_block = $emojiBtn authors = award_block -- GitLab From edfbb5b846e4405e9b43c3e8ed276419ccd22a20 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 31 Mar 2016 17:34:42 +0100 Subject: [PATCH 10/22] Hides award bar when editing a note --- app/assets/stylesheets/pages/notes.scss | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 012799b22657..e240847388ea 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -103,7 +103,8 @@ ul.notes { &.is-editting { .note-header, .note-text, - .edited-text { + .edited-text, + .note-awards { display: none; } -- GitLab From c40c56beed6984347a0c83aa88ebeb330a58924a Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 31 Mar 2016 18:19:38 +0100 Subject: [PATCH 11/22] Tooltip placement on award buttons --- app/assets/javascripts/awards_handler.coffee | 2 +- app/assets/stylesheets/pages/notes.scss | 1 + app/views/emoji_awards/_awards_block.html.haml | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee index d74c79adbf1c..ebd2bcb6354b 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -146,7 +146,7 @@ class @AwardsHandler createEmoji: (emoji) -> emojiCssClass = @resolveNameToCssClass(emoji) - buttonHtml = "" diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index e240847388ea..025897ca1ff1 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -319,6 +319,7 @@ ul.notes { .award-control { padding-top: 2px; padding-bottom: 2px; + color: #8f8f8f; font-size: 13px; } } diff --git a/app/views/emoji_awards/_awards_block.html.haml b/app/views/emoji_awards/_awards_block.html.haml index 7df64253f25b..1b840d887526 100644 --- a/app/views/emoji_awards/_awards_block.html.haml +++ b/app/views/emoji_awards/_awards_block.html.haml @@ -1,7 +1,7 @@ - grouped_emojis = awardable.grouped_awards(inline) .awards.js-awards-block{ class: ("hidden" if !inline && grouped_emojis.size == 0), data: { award_url: url_for([:toggle_emoji_award, @project.namespace.becomes(Namespace), @project, awardable]) } } - awards_sort(grouped_emojis).each do |emoji, awards| - %button.btn.award-control.js-emoji-btn.has-tooltip{ type: "button", class: (award_active_class(awards, current_user)), title: award_user_list(awards, current_user) } + %button.btn.award-control.js-emoji-btn.has-tooltip{ type: "button", class: (award_active_class(awards, current_user)), title: award_user_list(awards, current_user), data: { placement: "bottom" } } = emoji_icon(emoji) %span.award-control-text.js-counter = awards.count -- GitLab From 3edfb06b5da9239c9ea3ec36ba4c55906d098592 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Fri, 1 Apr 2016 10:32:38 +0200 Subject: [PATCH 12/22] Update migrations --- .../20160219155008_convert_award_note_to_emoji_award.rb | 2 +- db/migrate/20160219155039_remove_note_is_award.rb | 2 +- db/schema.rb | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/db/migrate/20160219155008_convert_award_note_to_emoji_award.rb b/db/migrate/20160219155008_convert_award_note_to_emoji_award.rb index 524a01b6fc72..ceb5bd420a17 100644 --- a/db/migrate/20160219155008_convert_award_note_to_emoji_award.rb +++ b/db/migrate/20160219155008_convert_award_note_to_emoji_award.rb @@ -4,6 +4,6 @@ def up end def down - # TODO + # Missing as the recommended way to restore data is to restore a backup end end diff --git a/db/migrate/20160219155039_remove_note_is_award.rb b/db/migrate/20160219155039_remove_note_is_award.rb index ad1699b646e5..da16372a297f 100644 --- a/db/migrate/20160219155039_remove_note_is_award.rb +++ b/db/migrate/20160219155039_remove_note_is_award.rb @@ -1,5 +1,5 @@ class RemoveNoteIsAward < ActiveRecord::Migration def change - remove_column :notes, :is_award + remove_column :notes, :is_award, :boolean end end diff --git a/db/schema.rb b/db/schema.rb index bd082f83fdf5..b071a1d8c656 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160320204112) do +ActiveRecord::Schema.define(version: 20160331133914) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -431,7 +431,7 @@ t.integer "iid" t.integer "updated_by_id" t.integer "moved_to_id" - t.boolean "confidential", default: false + t.boolean "confidential", default: false t.datetime "deleted_at" end -- GitLab From 38c677a2fbe0e7f1401d4b78e3d8c167b91a3471 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Fri, 1 Apr 2016 11:35:10 +0200 Subject: [PATCH 13/22] Awarding emoji on notes makes you participant --- app/models/concerns/awardable.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/models/concerns/awardable.rb b/app/models/concerns/awardable.rb index 41e34c5e3887..cd1fb229f539 100644 --- a/app/models/concerns/awardable.rb +++ b/app/models/concerns/awardable.rb @@ -3,6 +3,10 @@ module Awardable included do has_many :emoji_awards, as: :awardable, dependent: :destroy + + if self < Participable + participant :emoji_awards + end end module ClassMethods @@ -59,6 +63,7 @@ def awarded_emoji?(emoji_name, current_user) def award_emoji(emoji_name, current_user) return unless emoji_awardable? + emoji_awards.create(name: emoji_name, user: current_user) end -- GitLab From 35c8c2276fc057a717c8ed4e35e3ad7bbc64ade6 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 1 Apr 2016 17:25:28 +0100 Subject: [PATCH 14/22] Added tests for issues --- app/views/projects/notes/_note.html.haml | 6 +- spec/features/issues/award_spec.rb | 140 +++++++++++++++++++++++ 2 files changed, 143 insertions(+), 3 deletions(-) create mode 100644 spec/features/issues/award_spec.rb diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index 0c2775f1320d..0bad696cbb01 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -5,13 +5,13 @@ = image_tag avatar_icon(note.author), alt: '', class: 'avatar s40' .timeline-content .note-header - - if note_editable?(note) - .note-actions + .note-actions + - if current_user .award-menu-holder.note-action-award-holder.js-award-holder.js-award-action-btn{ data: { target: "##{dom_id(note)} .js-awards-block" } } = link_to '#', title: 'Award emoji', class: 'note-award-control js-add-award', data: { award_menu_url: emojis_path } do = icon('smile-o', {class: "award-control-icon award-control-icon-normal"}) = icon('spinner spin', {class: "award-control-icon award-control-icon-loading"}) - + - if note_editable?(note) = link_to '#', title: 'Edit comment', class: 'js-note-edit' do = icon('pencil-square-o') diff --git a/spec/features/issues/award_spec.rb b/spec/features/issues/award_spec.rb new file mode 100644 index 000000000000..46e1ed5d8cd3 --- /dev/null +++ b/spec/features/issues/award_spec.rb @@ -0,0 +1,140 @@ +require 'rails_helper' + +feature 'Issue awards', js: true, feature: true do + let(:user) { create(:user) } + let(:project) { create(:project, :public) } + let(:issue) { create(:issue, project: project) } + let!(:note) { create(:note_on_issue, project: project, noteable: issue, note: 'Looks good!') } + + describe 'logged in' do + before do + login_as(user) + visit namespace_project_issue_path(project.namespace, project, issue) + end + + it 'should add award to issue' do + first('.js-emoji-btn').click + expect(page).to have_selector('.js-emoji-btn.active') + expect(first('.js-emoji-btn')).to have_content '1' + end + + it 'should remove award from issue' do + first('.js-emoji-btn').click + find('.js-emoji-btn.active').click + expect(first('.js-emoji-btn')).to have_content '0' + end + + it 'should show award menu button in notes' do + page.within('.note') do + expect(page).to have_selector('.js-award-action-btn') + end + end + + it 'should not show award bar on note if no awards given' do + page.within('.note') do + expect(find('.js-awards-block', visible: false)).not_to be_visible + end + end + + it 'should be able to show award menu when clicking add award button in note' do + show_note_award_menu + end + + it 'should only have one menu on the page' do + first('.js-add-award').click + expect(page).to have_selector('.emoji-menu') + + page.within('.note') do + find('.js-add-award').click + expect(page).to have_selector('.emoji-menu', count: 1) + end + end + + it 'should add award to note' do + show_note_award_menu + award_on_note + + page.within('.note') do + expect(find('.js-awards-block')).to be_visible + expect(find('.js-awards-block')).to have_selector('.active') + end + end + + it 'should remove award from note' do + show_note_award_menu + award_on_note + + page.within('.note') do + expect(find('.js-awards-block')).to be_visible + expect(find('.js-awards-block')).to have_selector('.active') + end + + remove_award_on_note + sleep 0.5 + + page.within('.note') do + expect(find('.js-awards-block', visible: false)).not_to be_visible + expect(find('.js-awards-block', visible: false)).not_to have_selector('.active') + end + end + + it 'should not hide award bar on notes with more than 1 award' do + show_note_award_menu + award_on_note + + show_note_award_menu + award_on_note(2) + + page.within('.note') do + expect(find('.js-awards-block')).to be_visible + expect(find('.js-awards-block')).to have_selector('.active') + end + + remove_award_on_note + + page.within('.note') do + expect(find('.js-awards-block')).to be_visible + end + end + end + + describe 'logged out' do + before do + visit namespace_project_issue_path(project.namespace, project, issue) + end + + it 'should not see award menu button' do + expect(page).not_to have_selector('.js-award-holder') + end + + it 'should not see award menu button in note' do + page.within('.note') do + expect(page).not_to have_selector('.js-award-action-btn') + end + end + end + + def show_note_award_menu + page.within('.note') do + find('.js-add-award').click + expect(page).to have_selector('.emoji-menu') + end + end + + def award_on_note(index = 1) + page.within('.note') do + page.within('.emoji-menu') do + buttons = all('.js-emoji-btn') + buttons[index].click + end + end + end + + def remove_award_on_note + page.within('.note') do + page.within('.js-awards-block') do + first('.js-emoji-btn').click + end + end + end +end -- GitLab From 23eabc3512d9c31b203e3e49c73331bed779d35d Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 1 Apr 2016 17:35:10 +0100 Subject: [PATCH 15/22] Award spec for merge requests --- spec/features/merge_requests/award_spec.rb | 140 +++++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 spec/features/merge_requests/award_spec.rb diff --git a/spec/features/merge_requests/award_spec.rb b/spec/features/merge_requests/award_spec.rb new file mode 100644 index 000000000000..91e76e83c6f8 --- /dev/null +++ b/spec/features/merge_requests/award_spec.rb @@ -0,0 +1,140 @@ +require 'rails_helper' + +feature 'Merge request awards', js: true, feature: true do + let(:user) { create(:user) } + let(:project) { create(:project, :public) } + let(:merge_request) { create(:merge_request_with_diffs, source_project: project) } + let!(:note) { create(:note_on_merge_request, project: project, noteable: merge_request, note: 'Looks good!') } + + describe 'logged in' do + before do + login_as(user) + visit namespace_project_merge_request_path(project.namespace, project, merge_request) + end + + it 'should add award to merge request' do + first('.js-emoji-btn').click + expect(page).to have_selector('.js-emoji-btn.active') + expect(first('.js-emoji-btn')).to have_content '1' + end + + it 'should remove award from merge request' do + first('.js-emoji-btn').click + find('.js-emoji-btn.active').click + expect(first('.js-emoji-btn')).to have_content '0' + end + + it 'should show award menu button in notes' do + page.within('.note') do + expect(page).to have_selector('.js-award-action-btn') + end + end + + it 'should not show award bar on note if no awards given' do + page.within('.note') do + expect(find('.js-awards-block', visible: false)).not_to be_visible + end + end + + it 'should be able to show award menu when clicking add award button in note' do + show_note_award_menu + end + + it 'should only have one menu on the page' do + first('.js-add-award').click + expect(page).to have_selector('.emoji-menu') + + page.within('.note') do + find('.js-add-award').click + expect(page).to have_selector('.emoji-menu', count: 1) + end + end + + it 'should add award to note' do + show_note_award_menu + award_on_note + + page.within('.note') do + expect(find('.js-awards-block')).to be_visible + expect(find('.js-awards-block')).to have_selector('.active') + end + end + + it 'should remove award from note' do + show_note_award_menu + award_on_note + + page.within('.note') do + expect(find('.js-awards-block')).to be_visible + expect(find('.js-awards-block')).to have_selector('.active') + end + + remove_award_on_note + sleep 0.5 + + page.within('.note') do + expect(find('.js-awards-block', visible: false)).not_to be_visible + expect(find('.js-awards-block', visible: false)).not_to have_selector('.active') + end + end + + it 'should not hide award bar on notes with more than 1 award' do + show_note_award_menu + award_on_note + + show_note_award_menu + award_on_note(2) + + page.within('.note') do + expect(find('.js-awards-block')).to be_visible + expect(find('.js-awards-block')).to have_selector('.active') + end + + remove_award_on_note + + page.within('.note') do + expect(find('.js-awards-block')).to be_visible + end + end + end + + describe 'logged out' do + before do + visit namespace_project_merge_request_path(project.namespace, project, merge_request) + end + + it 'should not see award menu button' do + expect(page).not_to have_selector('.js-award-holder') + end + + it 'should not see award menu button in note' do + page.within('.note') do + expect(page).not_to have_selector('.js-award-action-btn') + end + end + end + + def show_note_award_menu + page.within('.note') do + find('.js-add-award').click + expect(page).to have_selector('.emoji-menu') + end + end + + def award_on_note(index = 1) + page.within('.note') do + page.within('.emoji-menu') do + buttons = all('.js-emoji-btn') + buttons[index].click + end + end + end + + def remove_award_on_note + page.within('.note') do + page.within('.js-awards-block') do + first('.js-emoji-btn').click + end + end + end +end -- GitLab From a6b748c0718e35b4edf1014c1df5428e1ae6cdb2 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 1 Apr 2016 18:18:45 +0100 Subject: [PATCH 16/22] Adds the emoji menu to the body and then re-positions it depending on which button clicked This spots bugs where the menu could be in a div that has overflow hidden on ie. diff comments --- app/assets/javascripts/awards_handler.coffee | 52 +++++++++++++++----- app/assets/stylesheets/pages/awards.scss | 9 ++-- app/assets/stylesheets/pages/notes.scss | 8 --- app/views/projects/notes/_note.html.haml | 2 +- spec/features/issues/award_spec.rb | 13 +++-- spec/features/merge_requests/award_spec.rb | 13 +++-- 6 files changed, 59 insertions(+), 38 deletions(-) diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee index ebd2bcb6354b..f4bc0ded6e30 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -13,6 +13,7 @@ class @AwardsHandler $("html").on 'click', (event) -> if !$(event.target).closest(".emoji-menu").length if $(".emoji-menu").is(":visible") + $('.js-add-award.is-active').removeClass 'is-active' $(".emoji-menu").removeClass "is-visible" $(document) @@ -22,10 +23,13 @@ class @AwardsHandler handleClick: (e) => e.preventDefault() $emojiBtn = $(e.currentTarget) - $votesBlock = $($emojiBtn.closest('.js-award-holder').data('target')) + $addAwardBtn = $('.js-add-award.is-active') + $votesBlock = $($addAwardBtn.closest('.js-award-holder').data('target')) - if $votesBlock.length is 0 + if $addAwardBtn.length is 0 $votesBlock = $emojiBtn.closest('.js-awards-block') + else if $votesBlock.length is 0 + $votesBlock = $addAwardBtn.closest('.js-awards-block') $votesBlock.addClass 'js-awards-block-current' awardUrl = $votesBlock.data 'award-url' @@ -35,32 +39,56 @@ class @AwardsHandler @addAward awardUrl, emoji showEmojiMenu: ($addBtn) -> - if $(".emoji-menu").length - $holder = $addBtn.closest('.js-award-holder') + $menu = $('.emoji-menu') - if $holder.find('.emoji-menu').length is 0 - $(".emoji-menu").detach().appendTo $holder + if $menu.length + $holder = $addBtn.closest('.js-award-holder') - if $(".emoji-menu").is ".is-visible" - $(".emoji-menu").removeClass "is-visible" + if $menu.is ".is-visible" + $addBtn.removeClass "is-active" + $menu.removeClass "is-visible" $("#emoji_search").blur() else - $(".emoji-menu").addClass "is-visible" + $addBtn.addClass "is-active" + @positionMenu($menu, $addBtn) + + $menu.addClass "is-visible" $("#emoji_search").focus() else - $addBtn.addClass "is-loading" + $addBtn.addClass "is-loading is-active" $.get $addBtn.data('award-menu-url'), (response) => $addBtn.removeClass "is-loading" - $addBtn.closest('.js-award-holder').append response + $('body').append response + + $menu = $(".emoji-menu") + + @positionMenu($menu, $addBtn) @renderFrequentlyUsedBlock() setTimeout => - $(".emoji-menu").addClass "is-visible" + $menu.addClass "is-visible" $("#emoji_search").focus() @setupSearch() , 200 + positionMenu: ($menu, $addBtn) -> + position = $addBtn.data('position') + + # The menu could potentially be off-screen or in a hidden overflow element + # So we position the element absolute in the body + css = + top: "#{$addBtn.offset().top + $addBtn.outerHeight()}px" + + if position? and position is 'right' + css.left = "#{($addBtn.offset().left - $menu.outerWidth()) + 20}px" + $menu.addClass "is-aligned-right" + else + css.left = "#{$addBtn.offset().left}px" + $menu.removeClass "is-aligned-right" + + $menu.css(css) + addAward: (awardUrl, emoji) -> emoji = @normilizeEmojiName(emoji) @postEmoji awardUrl, emoji, => diff --git a/app/assets/stylesheets/pages/awards.scss b/app/assets/stylesheets/pages/awards.scss index 5550b9856961..b17e299f4a19 100644 --- a/app/assets/stylesheets/pages/awards.scss +++ b/app/assets/stylesheets/pages/awards.scss @@ -7,8 +7,6 @@ .emoji-menu { position: absolute; - top: 100%; - left: 0; margin-top: 3px; z-index: 1000; min-width: 160px; @@ -21,7 +19,12 @@ opacity: 0; transform: scale(.2); transform-origin: 0 -45px; - transition: all .3s cubic-bezier(.87,-.41,.19,1.44); + transition: .3s cubic-bezier(.87,-.41,.19,1.44); + transition-property: transform, opacity; + + &.is-aligned-right { + transform-origin: 100% -45px; + } &.is-visible { pointer-events: all; diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 025897ca1ff1..feb42c36d313 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -280,14 +280,6 @@ ul.notes { } } -.note-action-award-holder { - .emoji-menu { - left: auto; - right: -15px; - transform-origin: 100% -45px; - } -} - .note-award-control { display: block; diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index 0bad696cbb01..6da419d733b2 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -8,7 +8,7 @@ .note-actions - if current_user .award-menu-holder.note-action-award-holder.js-award-holder.js-award-action-btn{ data: { target: "##{dom_id(note)} .js-awards-block" } } - = link_to '#', title: 'Award emoji', class: 'note-award-control js-add-award', data: { award_menu_url: emojis_path } do + = link_to '#', title: 'Award emoji', class: 'note-award-control js-add-award', data: { award_menu_url: emojis_path, position: "right" } do = icon('smile-o', {class: "award-control-icon award-control-icon-normal"}) = icon('spinner spin', {class: "award-control-icon award-control-icon-loading"}) - if note_editable?(note) diff --git a/spec/features/issues/award_spec.rb b/spec/features/issues/award_spec.rb index 46e1ed5d8cd3..209d1cca1761 100644 --- a/spec/features/issues/award_spec.rb +++ b/spec/features/issues/award_spec.rb @@ -46,8 +46,9 @@ page.within('.note') do find('.js-add-award').click - expect(page).to have_selector('.emoji-menu', count: 1) end + + expect(page).to have_selector('.emoji-menu', count: 1) end it 'should add award to note' do @@ -117,16 +118,14 @@ def show_note_award_menu page.within('.note') do find('.js-add-award').click - expect(page).to have_selector('.emoji-menu') end + expect(page).to have_selector('.emoji-menu') end def award_on_note(index = 1) - page.within('.note') do - page.within('.emoji-menu') do - buttons = all('.js-emoji-btn') - buttons[index].click - end + page.within('.emoji-menu') do + buttons = all('.js-emoji-btn') + buttons[index].click end end diff --git a/spec/features/merge_requests/award_spec.rb b/spec/features/merge_requests/award_spec.rb index 91e76e83c6f8..0f268bab5da7 100644 --- a/spec/features/merge_requests/award_spec.rb +++ b/spec/features/merge_requests/award_spec.rb @@ -46,8 +46,9 @@ page.within('.note') do find('.js-add-award').click - expect(page).to have_selector('.emoji-menu', count: 1) end + + expect(page).to have_selector('.emoji-menu', count: 1) end it 'should add award to note' do @@ -117,16 +118,14 @@ def show_note_award_menu page.within('.note') do find('.js-add-award').click - expect(page).to have_selector('.emoji-menu') end + expect(page).to have_selector('.emoji-menu') end def award_on_note(index = 1) - page.within('.note') do - page.within('.emoji-menu') do - buttons = all('.js-emoji-btn') - buttons[index].click - end + page.within('.emoji-menu') do + buttons = all('.js-emoji-btn') + buttons[index].click end end -- GitLab From 339bf652c2330c43e95f565fa218c9c25e20f0c5 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Wed, 6 Apr 2016 20:51:41 +0200 Subject: [PATCH 17/22] Create award emoji through a service --- .../concerns/toggle_emoji_award.rb | 2 +- app/helpers/issues_helper.rb | 2 +- app/services/create_emoji_award_service.rb | 26 +++++++++++++++++++ app/services/todo_service.rb | 8 ++++++ 4 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 app/services/create_emoji_award_service.rb diff --git a/app/controllers/concerns/toggle_emoji_award.rb b/app/controllers/concerns/toggle_emoji_award.rb index a2dee8d585b1..4ad9cbece211 100644 --- a/app/controllers/concerns/toggle_emoji_award.rb +++ b/app/controllers/concerns/toggle_emoji_award.rb @@ -7,7 +7,7 @@ module ToggleEmojiAward def toggle_emoji_award name = params.require(:name) - awardable.toggle_emoji_award(name, current_user) + CreateEmojiAwardService.new(project, current_user).execute(awardable, name) render json: { ok: true } end diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 36b9f3777ccd..f9283e0dfa3a 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -138,7 +138,7 @@ def award_user_list(awards, current_user) end def award_active_class(awards, current_user) - if current_user && awards.map(&:user_id).include?(current_user.id) + if current_user && awards.find { |a| a.user_id == current_user.id } "active" else "" diff --git a/app/services/create_emoji_award_service.rb b/app/services/create_emoji_award_service.rb new file mode 100644 index 000000000000..41dbb5b9f319 --- /dev/null +++ b/app/services/create_emoji_award_service.rb @@ -0,0 +1,26 @@ +require_relative 'base_service' + +class CreateEmojiAwardService < BaseService + # For an award emoji being posted we should: + # - Mark the TODO as done for this issuable (skip on snippets) + # - Save the award emoji + def execute(awardable, item) + issuable = to_issuable(awardable) + todo_service.award_emoji(issuable, current_user) if issuable + + awardable.toggle_emoji_award(item, current_user) + end + + private + + def to_issuable(awardable) + case awardable + when Note + awardable.noteable + when Issuable + awardable + when Snippet + nil + end + end +end diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index f2662922e905..fcf402309dc7 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -98,6 +98,14 @@ def update_note(note, current_user) handle_note(note, current_user) end + # When an award emoji is awarded we should: + # + # * mark all pending todos related to the awardable for the current user as done + # + def award_emoji(issuable, current_user) + mark_pending_todos_as_done(issuable, current_user) + end + # When marking pending todos as done we should: # # * mark all pending todos related to the target for the current user as done -- GitLab From 8b90502857fbc0a5e957f58c7e46554f3e14cc24 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Wed, 6 Apr 2016 22:13:06 +0200 Subject: [PATCH 18/22] Rename Emoji Award to Emoji Award To make this work lib/award_emoji had to be namespaced under Gitlab, over private channels DouweM mentioned this could later maybe combined with the model --- .../lib/emoji_aliases.js.coffee.erb | 2 +- ...e_emoji_award.rb => toggle_award_emoji.rb} | 8 +-- app/controllers/projects/issues_controller.rb | 2 +- .../projects/merge_requests_controller.rb | 2 +- app/controllers/projects/notes_controller.rb | 14 ++-- app/controllers/projects_controller.rb | 2 +- app/models/{emoji_award.rb => award_emoji.rb} | 2 +- app/models/concerns/awardable.rb | 36 +++++----- app/models/note.rb | 6 +- ...rvice.rb => create_award_emoji_service.rb} | 6 +- app/services/notes/create_service.rb | 6 +- .../emoji_awards/_awards_block.html.haml | 2 +- app/views/emojis/index.html.haml | 4 +- config/initializers/inflections.rb | 4 ++ config/routes.rb | 6 +- ...85700_rename_emoji_award_to_award_emoji.rb | 5 ++ db/schema.rb | 42 +++++------ lib/award_emoji.rb | 70 ------------------ lib/gitlab/award_emoji.rb | 72 +++++++++++++++++++ lib/tasks/gemojione.rake | 2 +- 20 files changed, 152 insertions(+), 141 deletions(-) rename app/controllers/concerns/{toggle_emoji_award.rb => toggle_award_emoji.rb} (53%) rename app/models/{emoji_award.rb => award_emoji.rb} (94%) rename app/services/{create_emoji_award_service.rb => create_award_emoji_service.rb} (77%) create mode 100644 db/migrate/20160406185700_rename_emoji_award_to_award_emoji.rb delete mode 100644 lib/award_emoji.rb create mode 100644 lib/gitlab/award_emoji.rb diff --git a/app/assets/javascripts/lib/emoji_aliases.js.coffee.erb b/app/assets/javascripts/lib/emoji_aliases.js.coffee.erb index 66f640a3cb73..97be65116e24 100644 --- a/app/assets/javascripts/lib/emoji_aliases.js.coffee.erb +++ b/app/assets/javascripts/lib/emoji_aliases.js.coffee.erb @@ -1,2 +1,2 @@ window.emojiAliases = -> - JSON.parse('<%= AwardEmoji.aliases.to_json %>') + JSON.parse('<%= Gitlab::AwardEmoji.aliases.to_json %>') diff --git a/app/controllers/concerns/toggle_emoji_award.rb b/app/controllers/concerns/toggle_award_emoji.rb similarity index 53% rename from app/controllers/concerns/toggle_emoji_award.rb rename to app/controllers/concerns/toggle_award_emoji.rb index 4ad9cbece211..561a84762f2e 100644 --- a/app/controllers/concerns/toggle_emoji_award.rb +++ b/app/controllers/concerns/toggle_award_emoji.rb @@ -1,13 +1,13 @@ -module ToggleEmojiAward +module ToggleAwardEmoji extend ActiveSupport::Concern included do - before_action :authenticate_user!, only: [:toggle_emoji_award] + before_action :authenticate_user!, only: [:toggle_award_emoji] end - def toggle_emoji_award + def toggle_award_emoji name = params.require(:name) - CreateEmojiAwardService.new(project, current_user).execute(awardable, name) + CreateAwardEmojiService.new(project, current_user).execute(awardable, name) render json: { ok: true } end diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 2ad6a71951c1..0f7b8c39060c 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -1,5 +1,5 @@ class Projects::IssuesController < Projects::ApplicationController - include ToggleEmojiAward + include ToggleAwardEmoji include ToggleSubscriptionAction include IssuableActions diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 4a939872ed65..1134f5860600 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -1,5 +1,5 @@ class Projects::MergeRequestsController < Projects::ApplicationController - include ToggleEmojiAward + include ToggleAwardEmoji include ToggleSubscriptionAction include DiffHelper include IssuableActions diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index e5aa1b2a55bd..2d3763b7f9ea 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -1,5 +1,5 @@ class Projects::NotesController < Projects::ApplicationController - include ToggleEmojiAward + include ToggleAwardEmoji # Authorize before_action :authorize_read_note! @@ -22,7 +22,7 @@ def index end def create - @note = Notes::CreateService.new(project, current_user, note_params.merge(create_emoji_awards: true)).execute + @note = Notes::CreateService.new(project, current_user, note_params.merge(create_award_emoji: true)).execute respond_to do |format| format.json { render json: note_json(@note) } @@ -122,13 +122,13 @@ def note_json(note) discussion_html: note_to_discussion_html(note), discussion_with_diff_html: note_to_discussion_with_diff_html(note) } - elsif note.emoji_award? - emoji_award = note.emoji_award + elsif note.award_emoji? + create_award_emoj = note.award_emoji { - valid: emoji_award.valid?, + valid: award_emoji.valid?, award: true, - id: emoji_award.id, - name: emoji_award.name + id: award_emoji.id, + name: award_emoji.name } else { diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 62f53664db31..6e9524576de2 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -138,7 +138,7 @@ def autocomplete_sources participants = ::Projects::ParticipantsService.new(@project, current_user).execute(note_type, note_id) @suggestions = { - emojis: AwardEmoji.urls, + emojis: Gitlab::AwardEmoji.urls, issues: autocomplete.issues, mergerequests: autocomplete.merge_requests, members: participants diff --git a/app/models/emoji_award.rb b/app/models/award_emoji.rb similarity index 94% rename from app/models/emoji_award.rb rename to app/models/award_emoji.rb index 309f34e2e3c3..9a382c30896c 100644 --- a/app/models/emoji_award.rb +++ b/app/models/award_emoji.rb @@ -1,4 +1,4 @@ -class EmojiAward < ActiveRecord::Base +class AwardEmoji < ActiveRecord::Base DOWNVOTE_NAME = "thumbsdown".freeze UPVOTE_NAME = "thumbsup".freeze diff --git a/app/models/concerns/awardable.rb b/app/models/concerns/awardable.rb index cd1fb229f539..b17bf689a2d3 100644 --- a/app/models/concerns/awardable.rb +++ b/app/models/concerns/awardable.rb @@ -2,25 +2,25 @@ module Awardable extend ActiveSupport::Concern included do - has_many :emoji_awards, as: :awardable, dependent: :destroy + has_many :award_emoji, as: :awardable, dependent: :destroy if self < Participable - participant :emoji_awards + participant :award_emoji end end module ClassMethods def order_upvotes_desc - order_votes_desc(EmojiAward::UPVOTE_NAME) + order_votes_desc(AwardEmoji::UPVOTE_NAME) end def order_downvotes_desc - order_votes_desc(EmojiAward::DOWNVOTE_NAME) + order_votes_desc(AwardEmoji::DOWNVOTE_NAME) end def order_votes_desc(emoji_name) awardable_table = self.arel_table - awards_table = EmojiAward.arel_table + awards_table = AwardEmoji.arel_table join_clause = awardable_table.join(awards_table, Arel::Nodes::OuterJoin).on( awards_table[:awardable_id].eq(awardable_table[:id]).and( @@ -30,27 +30,27 @@ def order_votes_desc(emoji_name) ) ).join_sources - joins(join_clause).group(awardable_table[:id]).reorder("COUNT(emoji_awards.id) DESC") + joins(join_clause).group(awardable_table[:id]).reorder("COUNT(award_emoji.id) DESC") end end def grouped_awards(with_thumbs = true) - awards = emoji_awards.group_by(&:name) + awards = award_emoji.group_by(&:name) if with_thumbs - awards[EmojiAward::UPVOTE_NAME] ||= EmojiAward.none - awards[EmojiAward::DOWNVOTE_NAME] ||= EmojiAward.none + awards[AwardEmoji::UPVOTE_NAME] ||= AwardEmoji.none + awards[AwardEmoji::DOWNVOTE_NAME] ||= AwardEmoji.none end awards end def downvotes - emoji_awards.where(name: EmojiAward::DOWNVOTE_NAME).count + award_emoji.where(name: AwardEmoji::DOWNVOTE_NAME).count end def upvotes - emoji_awards.where(name: EmojiAward::UPVOTE_NAME).count + award_emoji.where(name: AwardEmoji::UPVOTE_NAME).count end def emoji_awardable? @@ -58,24 +58,24 @@ def emoji_awardable? end def awarded_emoji?(emoji_name, current_user) - emoji_awards.where(name: emoji_name, user: current_user).exists? + award_emoji.where(name: emoji_name, user: current_user).exists? end - def award_emoji(emoji_name, current_user) + def add_award_emoji(emoji_name, current_user) return unless emoji_awardable? - emoji_awards.create(name: emoji_name, user: current_user) + award_emoji.create(name: emoji_name, user: current_user) end - def remove_emoji_award(emoji_name, current_user) - emoji_awards.where(name: emoji_name, user: current_user).destroy_all + def remove_award_emoji(emoji_name, current_user) + award_emoji.where(name: emoji_name, user: current_user).destroy_all end - def toggle_emoji_award(emoji_name, current_user) + def toggle_award_emoji(emoji_name, current_user) if awarded_emoji?(emoji_name, current_user) remove_emoji_award(emoji_name, current_user) else - award_emoji(emoji_name, current_user) + add_award_emoji(emoji_name, current_user) end end end diff --git a/app/models/note.rb b/app/models/note.rb index f613cb81c633..37ef2b0a793c 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -27,7 +27,7 @@ class Note < ActiveRecord::Base include Mentionable include Awardable - attr_accessor :emoji_award + attr_accessor :award_emoji default_value_for :system, false @@ -347,7 +347,7 @@ def emoji_award? emoji_awards_supported? && contains_only_emoji? end - def create_emoji_award + def create_award_emoji self.emoji_award = self.noteable.award_emoji(emoji_award_name, self.author) end @@ -373,7 +373,7 @@ def emoji_awards_supported? def emoji_award_name original_name = note.match(Banzai::Filter::EmojiFilter.emoji_pattern)[1] - AwardEmoji.normalize_emoji_name(original_name) + Gitlab::AwardEmoji.normalize_emoji_name(original_name) end def contains_only_emoji? diff --git a/app/services/create_emoji_award_service.rb b/app/services/create_award_emoji_service.rb similarity index 77% rename from app/services/create_emoji_award_service.rb rename to app/services/create_award_emoji_service.rb index 41dbb5b9f319..8bb2c6d715b6 100644 --- a/app/services/create_emoji_award_service.rb +++ b/app/services/create_award_emoji_service.rb @@ -1,14 +1,14 @@ require_relative 'base_service' -class CreateEmojiAwardService < BaseService +class CreateAwardEmojiService < BaseService # For an award emoji being posted we should: # - Mark the TODO as done for this issuable (skip on snippets) # - Save the award emoji - def execute(awardable, item) + def execute(awardable, emoji) issuable = to_issuable(awardable) todo_service.award_emoji(issuable, current_user) if issuable - awardable.toggle_emoji_award(item, current_user) + awardable.toggle_award_emoji(emoji, current_user) end private diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 5cdf25333b0d..a5083bcd2d06 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -1,15 +1,15 @@ module Notes class CreateService < BaseService def execute - create_emoji_awards = params.delete(:create_emoji_awards) + create_award_emoji = params.delete(:create_award_emoji) note = project.notes.new(params) note.author = current_user note.system = false - if create_emoji_awards && note.emoji_award? - note.create_emoji_award + if create_award_emoji && note.emoji_award? + note.create_award_emoji return note end diff --git a/app/views/emoji_awards/_awards_block.html.haml b/app/views/emoji_awards/_awards_block.html.haml index 1b840d887526..22ab96f61120 100644 --- a/app/views/emoji_awards/_awards_block.html.haml +++ b/app/views/emoji_awards/_awards_block.html.haml @@ -1,5 +1,5 @@ - grouped_emojis = awardable.grouped_awards(inline) -.awards.js-awards-block{ class: ("hidden" if !inline && grouped_emojis.size == 0), data: { award_url: url_for([:toggle_emoji_award, @project.namespace.becomes(Namespace), @project, awardable]) } } +.awards.js-awards-block{ class: ("hidden" if !inline && grouped_emojis.size == 0), data: { award_url: url_for([:toggle_award_emoji, @project.namespace.becomes(Namespace), @project, awardable]) } } - awards_sort(grouped_emojis).each do |emoji, awards| %button.btn.award-control.js-emoji-btn.has-tooltip{ type: "button", class: (award_active_class(awards, current_user)), title: award_user_list(awards, current_user), data: { placement: "bottom" } } = emoji_icon(emoji) diff --git a/app/views/emojis/index.html.haml b/app/views/emojis/index.html.haml index 3443a8e2307f..97401a2e618e 100644 --- a/app/views/emojis/index.html.haml +++ b/app/views/emojis/index.html.haml @@ -1,9 +1,9 @@ .emoji-menu .emoji-menu-content = text_field_tag :emoji_search, "", class: "emoji-search search-input form-control" - - AwardEmoji.emoji_by_category.each do |category, emojis| + - Gitlab::AwardEmoji.emoji_by_category.each do |category, emojis| %h5.emoji-menu-title - = AwardEmoji::CATEGORIES[category] + = Gitlab::AwardEmoji::CATEGORIES[category] %ul.clearfix.emoji-menu-list - emojis.each do |emoji| %li.pull-left.text-center.emoji-menu-list-item diff --git a/config/initializers/inflections.rb b/config/initializers/inflections.rb index 9e8b0131f8ff..0af616cdad64 100644 --- a/config/initializers/inflections.rb +++ b/config/initializers/inflections.rb @@ -8,3 +8,7 @@ # inflect.irregular 'person', 'people' # inflect.uncountable %w( fish sheep ) # end + +ActiveSupport::Inflector.inflections do |inflect| + inflect.uncountable %w(award_emoji) +end diff --git a/config/routes.rb b/config/routes.rb index 235e73f2e988..1f7343bfc915 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -634,7 +634,7 @@ post :cancel_merge_when_build_succeeds get :ci_status post :toggle_subscription - post :toggle_emoji_award + post :toggle_award_emoji post :remove_wip end @@ -699,7 +699,7 @@ resources :issues, constraints: { id: /\d+/ } do member do post :toggle_subscription - post :toggle_emoji_award + post :toggle_award_emoji end collection do post :bulk_update @@ -726,7 +726,7 @@ resources :notes, only: [:index, :create, :destroy, :update], constraints: { id: /\d+/ } do member do delete :delete_attachment - post :toggle_emoji_award + post :toggle_award_emoji end end diff --git a/db/migrate/20160406185700_rename_emoji_award_to_award_emoji.rb b/db/migrate/20160406185700_rename_emoji_award_to_award_emoji.rb new file mode 100644 index 000000000000..8d729167df04 --- /dev/null +++ b/db/migrate/20160406185700_rename_emoji_award_to_award_emoji.rb @@ -0,0 +1,5 @@ +class RenameEmojiAwardToAwardEmoji < ActiveRecord::Migration + def change + rename_table :emoji_awards, :award_emoji + end +end diff --git a/db/schema.rb b/db/schema.rb index b071a1d8c656..e4b0103cb505 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160331133914) do +ActiveRecord::Schema.define(version: 20160406185700) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -44,7 +44,6 @@ t.datetime "updated_at" t.string "home_page_url" t.integer "default_branch_protection", default: 2 - t.boolean "twitter_sharing_enabled", default: true t.text "restricted_visibility_levels" t.boolean "version_check_enabled", default: true t.integer "max_attachment_size", default: 10, null: false @@ -71,11 +70,11 @@ t.string "recaptcha_site_key" t.string "recaptcha_private_key" t.integer "metrics_port", default: 8089 + t.boolean "akismet_enabled", default: false + t.string "akismet_api_key" t.integer "metrics_sample_interval", default: 15 t.boolean "sentry_enabled", default: false t.string "sentry_dsn" - t.boolean "akismet_enabled", default: false - t.string "akismet_api_key" t.boolean "email_author_in_body", default: false t.integer "default_group_visibility" end @@ -94,6 +93,19 @@ add_index "audit_events", ["entity_id", "entity_type"], name: "index_audit_events_on_entity_id_and_entity_type", using: :btree add_index "audit_events", ["type"], name: "index_audit_events_on_type", using: :btree + create_table "award_emoji", force: :cascade do |t| + t.string "name" + t.integer "user_id" + t.integer "awardable_id" + t.string "awardable_type" + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index "award_emoji", ["awardable_id"], name: "index_award_emoji_on_awardable_id", using: :btree + add_index "award_emoji", ["awardable_type"], name: "index_award_emoji_on_awardable_type", using: :btree + add_index "award_emoji", ["user_id"], name: "index_award_emoji_on_user_id", using: :btree + create_table "broadcast_messages", force: :cascade do |t| t.text "message", null: false t.datetime "starts_at" @@ -364,19 +376,6 @@ add_index "emails", ["email"], name: "index_emails_on_email", unique: true, using: :btree add_index "emails", ["user_id"], name: "index_emails_on_user_id", using: :btree - create_table "emoji_awards", force: :cascade do |t| - t.string "name" - t.integer "user_id" - t.integer "awardable_id" - t.string "awardable_type" - t.datetime "created_at" - t.datetime "updated_at" - end - - add_index "emoji_awards", ["awardable_id"], name: "index_emoji_awards_on_awardable_id", using: :btree - add_index "emoji_awards", ["awardable_type"], name: "index_emoji_awards_on_awardable_type", using: :btree - add_index "emoji_awards", ["user_id"], name: "index_emoji_awards_on_user_id", using: :btree - create_table "events", force: :cascade do |t| t.string "target_type" t.integer "target_id" @@ -430,9 +429,9 @@ t.string "state" t.integer "iid" t.integer "updated_by_id" - t.integer "moved_to_id" t.boolean "confidential", default: false t.datetime "deleted_at" + t.integer "moved_to_id" end add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree @@ -756,6 +755,7 @@ add_index "projects", ["namespace_id"], name: "index_projects_on_namespace_id", using: :btree add_index "projects", ["path"], name: "index_projects_on_path", using: :btree add_index "projects", ["path"], name: "index_projects_on_path_trigram", using: :gin, opclasses: {"path"=>"gin_trgm_ops"} + add_index "projects", ["pending_delete"], name: "index_projects_on_pending_delete", using: :btree add_index "projects", ["runners_token"], name: "index_projects_on_runners_token", using: :btree add_index "projects", ["star_count"], name: "index_projects_on_star_count", using: :btree add_index "projects", ["visibility_level"], name: "index_projects_on_visibility_level", using: :btree @@ -797,9 +797,9 @@ t.string "type" t.string "title" t.integer "project_id" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.boolean "active", null: false + t.datetime "created_at" + t.datetime "updated_at" + t.boolean "active", default: false, null: false t.text "properties" t.boolean "template", default: false t.boolean "push_events", default: true diff --git a/lib/award_emoji.rb b/lib/award_emoji.rb deleted file mode 100644 index 7c9d7ccfee0e..000000000000 --- a/lib/award_emoji.rb +++ /dev/null @@ -1,70 +0,0 @@ -class AwardEmoji - CATEGORIES = { - other: "Other", - objects: "Objects", - places: "Places", - travel_places: "Travel", - emoticons: "Emoticons", - objects_symbols: "Symbols", - nature: "Nature", - celebration: "Celebration", - people: "People", - activity: "Activity", - flags: "Flags", - food_drink: "Food" - }.with_indifferent_access - - def self.normalize_emoji_name(name) - aliases[name] || name - end - - def self.emoji_by_category - unless @emoji_by_category - @emoji_by_category = {} - - emojis.each do |emoji_name, data| - data["name"] = emoji_name - - @emoji_by_category[data["category"]] ||= [] - @emoji_by_category[data["category"]] << data - end - - @emoji_by_category = @emoji_by_category.sort.to_h - end - - @emoji_by_category - end - - def self.emojis - @emojis ||= begin - json_path = File.join(Rails.root, 'fixtures', 'emojis', 'index.json' ) - JSON.parse(File.read(json_path)) - end - end - - def self.aliases - @aliases ||= begin - json_path = File.join(Rails.root, 'fixtures', 'emojis', 'aliases.json' ) - JSON.parse(File.read(json_path)) - end - end - - # Returns an Array of Emoji names and their asset URLs. - def self.urls - @urls ||= begin - path = File.join(Rails.root, 'fixtures', 'emojis', 'digests.json') - prefix = Gitlab::Application.config.assets.prefix - digest = Gitlab::Application.config.assets.digest - - JSON.parse(File.read(path)).map do |hash| - if digest - fname = "#{hash['unicode']}-#{hash['digest']}" - else - fname = hash['unicode'] - end - - { name: hash['name'], path: "#{prefix}/#{fname}.png" } - end - end - end -end diff --git a/lib/gitlab/award_emoji.rb b/lib/gitlab/award_emoji.rb new file mode 100644 index 000000000000..0adcafd795c3 --- /dev/null +++ b/lib/gitlab/award_emoji.rb @@ -0,0 +1,72 @@ +module Gitlab + class AwardEmoji + CATEGORIES = { + other: "Other", + objects: "Objects", + places: "Places", + travel_places: "Travel", + emoticons: "Emoticons", + objects_symbols: "Symbols", + nature: "Nature", + celebration: "Celebration", + people: "People", + activity: "Activity", + flags: "Flags", + food_drink: "Food" + }.with_indifferent_access + + def self.normalize_emoji_name(name) + aliases[name] || name + end + + def self.emoji_by_category + unless @emoji_by_category + @emoji_by_category = {} + + emojis.each do |emoji_name, data| + data["name"] = emoji_name + + @emoji_by_category[data["category"]] ||= [] + @emoji_by_category[data["category"]] << data + end + + @emoji_by_category = @emoji_by_category.sort.to_h + end + + @emoji_by_category + end + + def self.emojis + @emojis ||= begin + json_path = File.join(Rails.root, 'fixtures', 'emojis', 'index.json' ) + JSON.parse(File.read(json_path)) + end + end + + def self.aliases + @aliases ||= begin + json_path = File.join(Rails.root, 'fixtures', 'emojis', 'aliases.json' ) + JSON.parse(File.read(json_path)) + end + end + + # Returns an Array of Emoji names and their asset URLs. + def self.urls + @urls ||= begin + path = File.join(Rails.root, 'fixtures', 'emojis', 'digests.json') + prefix = Gitlab::Application.config.assets.prefix + digest = Gitlab::Application.config.assets.digest + + JSON.parse(File.read(path)).map do |hash| + if digest + fname = "#{hash['unicode']}-#{hash['digest']}" + else + fname = hash['unicode'] + end + + { name: hash['name'], path: "#{prefix}/#{fname}.png" } + end + end + end + end +end diff --git a/lib/tasks/gemojione.rake b/lib/tasks/gemojione.rake index 7ec00a898fd3..481285f80cc0 100644 --- a/lib/tasks/gemojione.rake +++ b/lib/tasks/gemojione.rake @@ -6,7 +6,7 @@ namespace :gemojione do dir = Gemojione.index.images_path - digests = AwardEmoji.emojis.map do |name, emoji_hash| + digests = Gitlab::AwardEmoji.emojis.map do |name, emoji_hash| fpath = File.join(dir, "#{emoji_hash['unicode']}.png") digest = Digest::SHA256.file(fpath).hexdigest -- GitLab From 673c4ec8e074efd24e96f09b0efbe1a9ddb039b3 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Thu, 7 Apr 2016 15:10:41 +0200 Subject: [PATCH 19/22] Fix (most) broken tests --- app/controllers/projects/notes_controller.rb | 2 +- app/models/note.rb | 4 +- app/models/user.rb | 2 +- app/services/create_award_emoji_service.rb | 1 + app/services/notes/create_service.rb | 9 ++--- app/services/notification_service.rb | 1 - features/steps/project/issues/issues.rb | 8 ++-- features/steps/project/merge_requests.rb | 8 ++-- spec/controllers/groups_controller_spec.rb | 12 +++--- spec/factories/award_emoji.rb | 18 +++++++++ spec/factories/notes.rb | 6 --- spec/lib/{ => gitlab}/award_emoji_spec.rb | 4 +- spec/models/award_emoji_spec.rb | 30 +++++++++++++++ spec/models/concerns/issuable_spec.rb | 20 ++++++---- spec/models/note_spec.rb | 39 -------------------- spec/services/notes/create_service_spec.rb | 31 ++++++---------- spec/services/todo_service_spec.rb | 8 ---- 17 files changed, 96 insertions(+), 107 deletions(-) create mode 100644 spec/factories/award_emoji.rb rename spec/lib/{ => gitlab}/award_emoji_spec.rb (85%) create mode 100644 spec/models/award_emoji_spec.rb diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index 3b595d555061..886d32473a43 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -22,7 +22,7 @@ def index end def create - @note = Notes::CreateService.new(project, current_user, note_params.merge(create_award_emoji: true)).execute + @note = Notes::CreateService.new(project, current_user, note_params).execute respond_to do |format| format.json { render json: note_json(@note) } diff --git a/app/models/note.rb b/app/models/note.rb index d7dad56b7cca..84edfafba680 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -343,12 +343,12 @@ def cross_reference_not_visible_for?(user) cross_reference? && referenced_mentionables(user).empty? end - def emoji_award? + def award_emoji? emoji_awards_supported? && contains_only_emoji? end def create_award_emoji - self.emoji_award = self.noteable.award_emoji(emoji_award_name, self.author) + self.award_emoji = self.noteable.award_emoji(emoji_award_name, self.author) end def emoji_awardable? diff --git a/app/models/user.rb b/app/models/user.rb index e4c8263cef59..bf1057db6f3c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -142,7 +142,7 @@ class User < ActiveRecord::Base has_one :abuse_report, dependent: :destroy has_many :spam_logs, dependent: :destroy has_many :builds, dependent: :nullify, class_name: 'Ci::Build' - has_many :emoji_awards, as: :awardable, dependent: :destroy + has_many :award_emoji, as: :awardable, dependent: :destroy has_many :todos, dependent: :destroy # diff --git a/app/services/create_award_emoji_service.rb b/app/services/create_award_emoji_service.rb index 8bb2c6d715b6..91cb2834b768 100644 --- a/app/services/create_award_emoji_service.rb +++ b/app/services/create_award_emoji_service.rb @@ -9,6 +9,7 @@ def execute(awardable, emoji) todo_service.award_emoji(issuable, current_user) if issuable awardable.toggle_award_emoji(emoji, current_user) + awardable end private diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index a5083bcd2d06..78a767946a6c 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -1,16 +1,13 @@ module Notes class CreateService < BaseService def execute - create_award_emoji = params.delete(:create_award_emoji) - note = project.notes.new(params) - note.author = current_user note.system = false - if create_award_emoji && note.emoji_award? - note.create_award_emoji - return note + if note.award_emoji? + return CreateAwardEmojiService.new(project, current_user, params). + execute(note.noteable, note.note) end if note.save diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index eff0d96f93dd..b198e7068755 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -131,7 +131,6 @@ def new_note(note) # ignore gitlab service messages return true if note.note.start_with?('Status changed to closed') return true if note.cross_reference? && note.system == true - return true if note.is_award target = note.noteable diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb index aff5ca676beb..af46df2d0f4f 100644 --- a/features/steps/project/issues/issues.rb +++ b/features/steps/project/issues/issues.rb @@ -192,14 +192,14 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps step 'issue "Release 0.4" have 2 upvotes and 1 downvote' do issue = Issue.find_by(title: 'Release 0.4') - create_list(:upvote_note, 2, project: project, noteable: issue) - create(:downvote_note, project: project, noteable: issue) + create_list(:award_emoji, :upvote, 2, awardable: issue) + create(:award_emoji, :downvote, awardable: issue) end step 'issue "Tweet control" have 1 upvote and 2 downvotes' do issue = Issue.find_by(title: 'Tweet control') - create(:upvote_note, project: project, noteable: issue) - create_list(:downvote_note, 2, project: project, noteable: issue) + create(:award_emoji, :upvote, awardable: issue) + create_list(:award_emoji, :downvote, 2, awardable: issue) end step 'The list should be sorted by "Least popular"' do diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index a4f02b590ea8..ad1b8ca83fca 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -175,14 +175,14 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps step 'merge request "Bug NS-04" have 2 upvotes and 1 downvote' do merge_request = MergeRequest.find_by(title: 'Bug NS-04') - create_list(:upvote_note, 2, project: project, noteable: merge_request) - create(:downvote_note, project: project, noteable: merge_request) + create_list(:award_emoji, 2, awardable: merge_request, name: "thumbsup") + create(:award_emoji, :downvote, awardable: merge_request) end step 'merge request "Bug NS-06" have 1 upvote and 2 downvotes' do merge_request = MergeRequest.find_by(title: 'Bug NS-06') - create(:upvote_note, project: project, noteable: merge_request) - create_list(:downvote_note, 2, project: project, noteable: merge_request) + create(:award_emoji, :upvote, awardable: merge_request) + create_list(:award_emoji, 2, awardable: merge_request, name: "thumbsdown") end step 'The list should be sorted by "Least popular"' do diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 465531b2b367..3ca9e7a019c8 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -31,9 +31,9 @@ let(:issue_2) { create(:issue, project: project) } before do - create_list(:upvote_note, 3, project: project, noteable: issue_2) - create_list(:upvote_note, 2, project: project, noteable: issue_1) - create_list(:downvote_note, 2, project: project, noteable: issue_2) + create_list(:award_emoji, 3, awardable: issue_2, name: "thumbsup") + create_list(:award_emoji, 2, awardable: issue_1, name: "thumbsup") + create_list(:award_emoji, 2, awardable: issue_2, name: "thumbsdown") sign_in(user) end @@ -56,9 +56,9 @@ let(:merge_request_2) { create(:merge_request, :simple, source_project: project) } before do - create_list(:upvote_note, 3, project: project, noteable: merge_request_2) - create_list(:upvote_note, 2, project: project, noteable: merge_request_1) - create_list(:downvote_note, 2, project: project, noteable: merge_request_2) + create_list(:award_emoji, 3, awardable: merge_request_2, name: "thumbsup") + create_list(:award_emoji, 2, awardable: merge_request_1, name: "thumbsup") + create_list(:award_emoji, 2, awardable: merge_request_2, name: "thumbsdown") sign_in(user) end diff --git a/spec/factories/award_emoji.rb b/spec/factories/award_emoji.rb new file mode 100644 index 000000000000..b09f8b0bc743 --- /dev/null +++ b/spec/factories/award_emoji.rb @@ -0,0 +1,18 @@ +FactoryGirl.define do + factory :award_emoji do + name "thumbsup" + user + awardable factory: :issue + + trait :thumbs_up + trait :upvote + + trait :thumbs_down do + name "thumbsdown" + end + + trait :downvote do + name "thumbsdown" + end + end +end diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb index b1ac04fd0e92..ff5b0800ce4b 100644 --- a/spec/factories/notes.rb +++ b/spec/factories/notes.rb @@ -36,8 +36,6 @@ factory :note_on_merge_request_diff, traits: [:on_merge_request, :on_diff] factory :note_on_project_snippet, traits: [:on_project_snippet] factory :system_note, traits: [:system] - factory :downvote_note, traits: [:award, :downvote] - factory :upvote_note, traits: [:award, :upvote] trait :on_commit do project @@ -69,10 +67,6 @@ system true end - trait :award do - is_award true - end - trait :downvote do note "thumbsdown" end diff --git a/spec/lib/award_emoji_spec.rb b/spec/lib/gitlab/award_emoji_spec.rb similarity index 85% rename from spec/lib/award_emoji_spec.rb rename to spec/lib/gitlab/award_emoji_spec.rb index 330678f7f164..cf63d3e9a17a 100644 --- a/spec/lib/award_emoji_spec.rb +++ b/spec/lib/gitlab/award_emoji_spec.rb @@ -1,8 +1,8 @@ require 'spec_helper' -describe AwardEmoji do +describe Gitlab::AwardEmoji do describe '.urls' do - subject { AwardEmoji.urls } + subject { Gitlab::AwardEmoji.urls } it { is_expected.to be_an_instance_of(Array) } it { is_expected.to_not be_empty } diff --git a/spec/models/award_emoji_spec.rb b/spec/models/award_emoji_spec.rb new file mode 100644 index 000000000000..d006ecc325d5 --- /dev/null +++ b/spec/models/award_emoji_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' + +describe AwardEmoji, models: true do + describe "Associations" do + it { is_expected.to belong_to(:awardable) } + it { is_expected.to belong_to(:user) } + end + + describe 'modules' do + it { is_expected.to include_module(Participable) } + end + + describe "validations" do + it { is_expected.to validate_presence_of(:awardable) } + it { is_expected.to validate_presence_of(:user) } + it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_presence_of(:awardable) } + + # To circumvent in the shoulda matchers + describe "scoped uniqueness validation" do + it "rejects duplicate award emoji" do + user = create(:user) + issue = create(:issue) + create(:award_emoji, user: user, awardable: issue) + + expect(build(:award_emoji, user: user, awardable: issue)).not_to be_valid + end + end + end +end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index b16ccc6e3052..68a3584e1d54 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -2,7 +2,15 @@ describe Issue, "Issuable" do let(:issue) { create(:issue) } - let(:user) { create(:user) } + let(:user) { create(:user) } + + describe "modules" do + it { is_expected.to include_module(Participable) } + it { is_expected.to include_module(Mentionable) } + it { is_expected.to include_module(Subscribable) } + it { is_expected.to include_module(StripAttribute) } + it { is_expected.to include_module(Awardable) } + end describe "Associations" do it { is_expected.to belong_to(:project) } @@ -201,15 +209,13 @@ describe "votes" do before do - author = create :user - project = create :empty_project - issue.notes.awards.create!(note: "thumbsup", author: author, project: project) - issue.notes.awards.create!(note: "thumbsdown", author: author, project: project) + create(:award_emoji, :thumbs_up, awardable: issue) + create(:award_emoji, :thumbs_down, awardable: issue) end it "returns correct values" do - expect(issue.upvotes).to eq(1) - expect(issue.downvotes).to eq(1) + expect(issue.upvotes).to eq(1) + expect(issue.downvotes).to eq(1) end end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 6b18936edb12..bb591e9cb536 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -152,23 +152,6 @@ end end - describe '.grouped_awards' do - before do - create :note, note: "smile", is_award: true - create :note, note: "smile", is_award: true - end - - it "returns grouped hash of notes" do - expect(Note.grouped_awards.keys.size).to eq(3) - expect(Note.grouped_awards["smile"]).to match_array(Note.all) - end - - it "returns thumbsup and thumbsdown always" do - expect(Note.grouped_awards["thumbsup"]).to match_array(Note.none) - expect(Note.grouped_awards["thumbsdown"]).to match_array(Note.none) - end - end - describe '#active?' do it 'is always true when the note has no associated diff' do note = build(:note) @@ -239,11 +222,6 @@ note = build(:note, system: true) expect(note.editable?).to be_falsy end - - it "returns false" do - note = build(:note, is_award: true, note: "smiley") - expect(note.editable?).to be_falsy - end end describe "cross_reference_not_visible_for?" do @@ -270,23 +248,6 @@ end end - describe "set_award!" do - let(:merge_request) { create :merge_request } - - it "converts aliases to actual name" do - note = create(:note, note: ":+1:", noteable: merge_request) - expect(note.reload.note).to eq("thumbsup") - end - - it "is not an award emoji when comment is on a diff" do - note = create(:note, note: ":blowfish:", noteable: merge_request, line_code: "11d5d2e667e9da4f7f610f81d86c974b146b13bd_0_2") - note = note.reload - - expect(note.note).to eq(":blowfish:") - expect(note.is_award?).to be_falsy - end - end - describe 'clear_blank_line_code!' do it 'clears a blank line code before validation' do note = build(:note, line_code: ' ') diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index ff23f13e1cb3..51ab9c9774e3 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -7,19 +7,15 @@ describe :execute do context "valid params" do + let(:opts) { { note: 'Awesome comment', noteable_type: 'Issue', noteable_id: issue.id } } + let(:note) { Notes::CreateService.new(project, user, opts).execute } + before do project.team << [user, :master] - opts = { - note: 'Awesome comment', - noteable_type: 'Issue', - noteable_id: issue.id - } - - @note = Notes::CreateService.new(project, user, opts).execute end - it { expect(@note).to be_valid } - it { expect(@note.note).to eq('Awesome comment') } + it { expect(note).to be_valid } + it { expect(note.note).to eq('Awesome comment') } end end @@ -28,18 +24,14 @@ project.team << [user, :master] end - it "creates emoji note" do + it "skips awards emoji" do opts = { note: ':smile: ', noteable_type: 'Issue', - noteable_id: issue.id + noteable_id: issue.id, } - @note = Notes::CreateService.new(project, user, opts).execute - - expect(@note).to be_valid - expect(@note.note).to eq('smile') - expect(@note.is_award).to be_truthy + expect { Notes::CreateService.new(project, user, opts).execute }.not_to change { Note.count } end it "creates regular note if emoji name is invalid" do @@ -49,11 +41,10 @@ noteable_id: issue.id } - @note = Notes::CreateService.new(project, user, opts).execute + note = Notes::CreateService.new(project, user, opts).execute - expect(@note).to be_valid - expect(@note.note).to eq(opts[:note]) - expect(@note.is_award).to be_falsy + expect(note).to be_valid + expect(note.note).to eq(opts[:note]) end end end diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 82b7fbfa8162..a04dbc5d50c9 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -137,7 +137,6 @@ let(:note_on_commit) { create(:note_on_commit, project: project, author: john_doe, note: mentions) } let(:note_on_confidential_issue) { create(:note_on_issue, noteable: confidential_issue, project: project, note: mentions) } let(:note_on_project_snippet) { create(:note_on_project_snippet, project: project, author: john_doe, note: mentions) } - let(:award_note) { create(:note, :award, project: project, noteable: issue, author: john_doe, note: 'thumbsup') } let(:system_note) { create(:system_note, project: project, noteable: issue) } it 'mark related pending todos to the noteable for the note author as done' do @@ -150,13 +149,6 @@ expect(second_todo.reload).to be_done end - it 'mark related pending todos to the noteable for the award note author as done' do - service.new_note(award_note, john_doe) - - expect(first_todo.reload).to be_done - expect(second_todo.reload).to be_done - end - it 'does not mark related pending todos it is a system note' do service.new_note(system_note, john_doe) -- GitLab From 37cdfe4f43225508f48f6352c0018c2ff508e8e8 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Thu, 7 Apr 2016 20:20:41 +0200 Subject: [PATCH 20/22] Award Emoji are also copied to the new location when moved --- app/models/note.rb | 2 -- app/services/issues/move_service.rb | 10 ++++++++++ app/views/projects/notes/_note.html.haml | 3 +-- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/app/models/note.rb b/app/models/note.rb index 84edfafba680..1697ed78f1c1 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -27,8 +27,6 @@ class Note < ActiveRecord::Base include Mentionable include Awardable - attr_accessor :award_emoji - default_value_for :system, false attr_mentionable :note, cache: true, pipeline: :note diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb index 82e7090f1ea5..e7088c0bb4f4 100644 --- a/app/services/issues/move_service.rb +++ b/app/services/issues/move_service.rb @@ -24,6 +24,7 @@ def execute(issue, new_project) @new_issue = create_new_issue rewrite_notes + rewrite_award_emoji(issue, @new_issue) add_note_moved_from # Old issue tasks @@ -58,6 +59,15 @@ def rewrite_notes updated_at: note.updated_at } new_note.update(new_params) + rewrite_award_emoji(note, new_note) + end + end + + def rewrite_award_emoji(awardable, new_awardable) + awardable.award_emoji.each do |award| + new_award = award.dup + new_award.awardable = new_awardable + new_award.save end end diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index d326acf9c41e..3cbbdf78fb98 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -11,8 +11,7 @@ = link_to '#', title: 'Award emoji', class: 'note-award-control js-add-award', data: { award_menu_url: emojis_path, position: "right" } do = icon('smile-o', {class: "award-control-icon award-control-icon-normal"}) = icon('spinner spin', {class: "award-control-icon award-control-icon-loading"}) - - if note_editable?(note) - = link_to '#', title: 'Edit comment', class: 'js-note-edit' do + = link_to_member(note.project, note.author, avatar: false) .inline.note-headline-light = "#{note.author.to_reference} commented" -- GitLab From 7810d5c8e054258e01fd6d45ba95fc95b8d7202b Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Wed, 13 Apr 2016 20:52:09 +0200 Subject: [PATCH 21/22] Notes that are award emoji are posted as such --- app/assets/javascripts/notes.js.coffee | 2 +- .../concerns/toggle_award_emoji.rb | 2 +- app/controllers/projects/notes_controller.rb | 45 +++++++++++------ app/helpers/issues_helper.rb | 2 +- app/models/concerns/awardable.rb | 2 +- app/models/note.rb | 13 +---- app/services/create_award_emoji_service.rb | 27 ---------- app/services/notes/create_service.rb | 2 +- app/services/todo_service.rb | 2 +- app/services/toggle_award_emoji_service.rb | 33 +++++++++++++ .../_awards_block.html.haml | 0 app/views/projects/issues/show.html.haml | 2 +- .../projects/merge_requests/_show.html.haml | 2 +- app/views/projects/notes/_note.html.haml | 2 +- features/project/issues/award_emoji.feature | 5 -- spec/helpers/issues_helper_spec.rb | 9 ++-- spec/models/award_emoji_spec.rb | 2 +- spec/models/concerns/awardable_spec.rb | 49 +++++++++++++++++++ spec/models/user_spec.rb | 1 + spec/services/issues/move_service_spec.rb | 6 +++ spec/services/todo_service_spec.rb | 12 ++++- .../toggle_award_emoji_service_spec.rb | 49 +++++++++++++++++++ 22 files changed, 193 insertions(+), 76 deletions(-) delete mode 100644 app/services/create_award_emoji_service.rb create mode 100644 app/services/toggle_award_emoji_service.rb rename app/views/{emoji_awards => award_emoji}/_awards_block.html.haml (100%) create mode 100644 spec/models/concerns/awardable_spec.rb create mode 100644 spec/services/toggle_award_emoji_service_spec.rb diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index bff129748b66..6a91abf87a40 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -150,7 +150,7 @@ class @Notes renderNote: (note) -> unless note.valid if note.award - flash = new Flash('You have already awarded this emoji!', 'alert') + flash = new Flash('You have already awarded this emoji, and it we\'ve removed it', 'alert') flash.pinTo('.header-content') return diff --git a/app/controllers/concerns/toggle_award_emoji.rb b/app/controllers/concerns/toggle_award_emoji.rb index 561a84762f2e..a2576f866227 100644 --- a/app/controllers/concerns/toggle_award_emoji.rb +++ b/app/controllers/concerns/toggle_award_emoji.rb @@ -7,7 +7,7 @@ module ToggleAwardEmoji def toggle_award_emoji name = params.require(:name) - CreateAwardEmojiService.new(project, current_user).execute(awardable, name) + ToggleAwardEmojiService.new(project, current_user).execute(awardable, name) render json: { ok: true } end diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index 886d32473a43..4957ed24c939 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -111,29 +111,42 @@ def note_to_discussion_with_diff_html(note) ) end + # When an ordinairy note is posted, this is straight forward + # But when the note is ":+1:" this toggles the emoji posted + # note is than either an AwardEmoji or an Array (its remove using a where clause) def note_json(note) - if note.persisted? + case note + when Note + if note.persisted? + { + valid: true, + id: note.id, + discussion_id: note.discussion_id, + html: note_to_html(note), + note: note.note, + discussion_html: note_to_discussion_html(note), + discussion_with_diff_html: note_to_discussion_with_diff_html(note) + } + else + { + valid: false, + errors: note.errors + } + end + when AwardEmoji { - valid: true, - id: note.id, - discussion_id: note.discussion_id, - html: note_to_html(note), - note: note.note, - discussion_html: note_to_discussion_html(note), - discussion_with_diff_html: note_to_discussion_with_diff_html(note) - } - elsif note.award_emoji? - award_emoji = note.award_emoji - { - valid: award_emoji.valid?, + valid: note.valid?, award: true, - id: award_emoji.id, - name: award_emoji.name + id: note.id, + name: note.name } else + note = note.first { valid: false, - errors: note.errors + award: true, + id: note.id, + name: note.name } end end diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index f9283e0dfa3a..55e859c2edd9 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -146,7 +146,7 @@ def award_active_class(awards, current_user) end def awards_sort(awards) - awards.sort_by do |award, notes| + awards.sort_by do |award, _| if award == "thumbsup" 0 elsif award == "thumbsdown" diff --git a/app/models/concerns/awardable.rb b/app/models/concerns/awardable.rb index b17bf689a2d3..77f13f1bef07 100644 --- a/app/models/concerns/awardable.rb +++ b/app/models/concerns/awardable.rb @@ -73,7 +73,7 @@ def remove_award_emoji(emoji_name, current_user) def toggle_award_emoji(emoji_name, current_user) if awarded_emoji?(emoji_name, current_user) - remove_emoji_award(emoji_name, current_user) + remove_award_emoji(emoji_name, current_user) else add_award_emoji(emoji_name, current_user) end diff --git a/app/models/note.rb b/app/models/note.rb index 1697ed78f1c1..bf97df2655a0 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -342,11 +342,7 @@ def cross_reference_not_visible_for?(user) end def award_emoji? - emoji_awards_supported? && contains_only_emoji? - end - - def create_award_emoji - self.award_emoji = self.noteable.award_emoji(emoji_award_name, self.author) + award_emoji_supported? && contains_only_emoji? end def emoji_awardable? @@ -365,15 +361,10 @@ def find_noteable_diff diffs.find { |d| d.new_path == self.diff.new_path } end - def emoji_awards_supported? + def award_emoji_supported? noteable.kind_of?(Awardable) end - def emoji_award_name - original_name = note.match(Banzai::Filter::EmojiFilter.emoji_pattern)[1] - Gitlab::AwardEmoji.normalize_emoji_name(original_name) - end - def contains_only_emoji? note =~ /\A#{Banzai::Filter::EmojiFilter.emoji_pattern}\s?\Z/ end diff --git a/app/services/create_award_emoji_service.rb b/app/services/create_award_emoji_service.rb deleted file mode 100644 index 91cb2834b768..000000000000 --- a/app/services/create_award_emoji_service.rb +++ /dev/null @@ -1,27 +0,0 @@ -require_relative 'base_service' - -class CreateAwardEmojiService < BaseService - # For an award emoji being posted we should: - # - Mark the TODO as done for this issuable (skip on snippets) - # - Save the award emoji - def execute(awardable, emoji) - issuable = to_issuable(awardable) - todo_service.award_emoji(issuable, current_user) if issuable - - awardable.toggle_award_emoji(emoji, current_user) - awardable - end - - private - - def to_issuable(awardable) - case awardable - when Note - awardable.noteable - when Issuable - awardable - when Snippet - nil - end - end -end diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 78a767946a6c..e716240cd8da 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -6,7 +6,7 @@ def execute note.system = false if note.award_emoji? - return CreateAwardEmojiService.new(project, current_user, params). + return ToggleAwardEmojiService.new(project, current_user, params). execute(note.noteable, note.note) end diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index 634f3346f781..9df7bc5339b5 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -102,7 +102,7 @@ def update_note(note, current_user) # # * mark all pending todos related to the awardable for the current user as done # - def award_emoji(issuable, current_user) + def new_award_emoji(issuable, current_user) mark_pending_todos_as_done(issuable, current_user) end diff --git a/app/services/toggle_award_emoji_service.rb b/app/services/toggle_award_emoji_service.rb new file mode 100644 index 000000000000..649acdb828f3 --- /dev/null +++ b/app/services/toggle_award_emoji_service.rb @@ -0,0 +1,33 @@ +require_relative 'base_service' + +class ToggleAwardEmojiService < BaseService + # For an award emoji being posted we should: + # - Mark the TODO as done for this issuable (skip on snippets) + # - Save the award emoji + def execute(awardable, emoji) + todoable = to_todoable(awardable) + todo_service.new_award_emoji(todoable, current_user) if todoable + + # Needed if a note is posted as :+1: + emoji = emoji_award_name(emoji) if emoji.start_with? ':' + awardable.toggle_award_emoji(emoji, current_user) + end + + private + + def emoji_award_name(emoji) + original_name = emoji.match(Banzai::Filter::EmojiFilter.emoji_pattern)[1] + Gitlab::AwardEmoji.normalize_emoji_name(original_name) + end + + def to_todoable(awardable) + case awardable + when Note + awardable.noteable + when Issuable + awardable + when Snippet + nil + end + end +end diff --git a/app/views/emoji_awards/_awards_block.html.haml b/app/views/award_emoji/_awards_block.html.haml similarity index 100% rename from app/views/emoji_awards/_awards_block.html.haml rename to app/views/award_emoji/_awards_block.html.haml diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 7d8f55911a0f..52274c3c6e73 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -70,7 +70,7 @@ .content-block.content-block-small = render 'new_branch' - = render 'emoji_awards/awards_block', awardable: @issue, inline: true + = render 'award_emoji/awards_block', awardable: @issue, inline: true .row %section.col-md-12 diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 3d3279cca6a0..99bb8b9bfbe3 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -69,7 +69,7 @@ .tab-content #notes.notes.tab-pane.voting_notes .content-block.content-block-small.oneline-block - = render 'emoji_awards/awards_block', awardable: @merge_request, inline: true + = render 'award_emoji/awards_block', awardable: @merge_request, inline: true .row %section.col-md-12 diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index 3cbbdf78fb98..e52de3cf08e8 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -34,7 +34,7 @@ - if note_editable?(note) = render 'projects/notes/edit_form', note: note .note-awards - = render 'emoji_awards/awards_block', awardable: note, inline: false + = render 'award_emoji/awards_block', awardable: note, inline: false = edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago', include_author: true) - if note.attachment.url diff --git a/features/project/issues/award_emoji.feature b/features/project/issues/award_emoji.feature index f0fd414a9f91..7c35e27194c3 100644 --- a/features/project/issues/award_emoji.feature +++ b/features/project/issues/award_emoji.feature @@ -38,8 +38,3 @@ Feature: Award Emoji And The search field is focused And I search "hand" Then I see search result for "hand" - - @javascript - Scenario: I add award emoji using regular comment - Given I leave comment with a single emoji - Then I have award added diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb index 27b4cad88e4e..532c8946caf0 100644 --- a/spec/helpers/issues_helper_spec.rb +++ b/spec/helpers/issues_helper_spec.rb @@ -128,17 +128,14 @@ end describe "#award_active_class" do - before do - @note = create :note - @note1 = create :note - end + let!(:award) { create(:award_emoji) } it "returns empty string for unauthenticated user" do - expect(award_active_class(Note.all, nil)).to eq("") + expect(award_active_class(AwardEmoji.all, nil)).to eq("") end it "returns active string for author" do - expect(award_active_class(Note.all, @note.author)).to eq("active") + expect(award_active_class(AwardEmoji.all, award.user)).to eq("active") end end diff --git a/spec/models/award_emoji_spec.rb b/spec/models/award_emoji_spec.rb index d006ecc325d5..7d9d99c09c43 100644 --- a/spec/models/award_emoji_spec.rb +++ b/spec/models/award_emoji_spec.rb @@ -16,7 +16,7 @@ it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_presence_of(:awardable) } - # To circumvent in the shoulda matchers + # To circumvent a bug in the shoulda matchers describe "scoped uniqueness validation" do it "rejects duplicate award emoji" do user = create(:user) diff --git a/spec/models/concerns/awardable_spec.rb b/spec/models/concerns/awardable_spec.rb new file mode 100644 index 000000000000..6851d0683674 --- /dev/null +++ b/spec/models/concerns/awardable_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe Issue, "Awardable" do + let!(:issue) { create(:issue) } + let!(:award_emoji) { create(:award_emoji, :downvote, awardable: issue) } + + describe "Associations" do + it { is_expected.to have_many(:award_emoji).dependent(:destroy) } + end + + describe "ClassMethods" do + let!(:issue2) { create(:issue) } + + before do + create(:award_emoji, awardable: issue2) + end + + it "orders on upvotes" do + expect(Issue.order_upvotes_desc.to_a).to eq [issue2, issue] + end + + it "orders on downvotes" do + expect(Issue.order_downvotes_desc.to_a).to eq [issue, issue2] + end + end + + describe "#upvotes" do + it "counts the number of upvotes" do + expect(issue.upvotes).to be 0 + end + end + + describe "#downvotes" do + it "counts the number of downvotes" do + expect(issue.downvotes).to be 1 + end + end + + describe "#toggle_award_emoji" do + it "adds an emoji if it isn't awarded yet" do + expect { issue.toggle_award_emoji("thumbsup", award_emoji.user) }.to change { AwardEmoji.count }.by 1 + end + + it "toggles already awarded emoji" do + + expect { issue.toggle_award_emoji("thumbsdown", award_emoji.user) }.to change { AwardEmoji.count }.by -1 + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 8b2fb77e28eb..de1a233dfffb 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -93,6 +93,7 @@ it { is_expected.to have_one(:abuse_report) } it { is_expected.to have_many(:spam_logs).dependent(:destroy) } it { is_expected.to have_many(:todos).dependent(:destroy) } + it { is_expected.to have_many(:award_emoji).dependent(:destroy) } end describe 'validations' do diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb index 2a5e4ac3ec47..4204783f7538 100644 --- a/spec/services/issues/move_service_spec.rb +++ b/spec/services/issues/move_service_spec.rb @@ -106,6 +106,8 @@ notes_params.each do |note| create(:note, note_params.merge(note)) end + + create(:award_emoji, awardable: old_issue) end include_context 'issue move executed' @@ -125,6 +127,10 @@ it 'preserves orignal author of comment' do expect(user_notes.pluck(:author_id)).to all(eq(author.id)) end + + it 'moves the award emoji too' do + expect(new_issue.award_emoji).to eq(old_issue.award_emoji) + end end context 'note that has been updated' do diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index a04dbc5d50c9..f82f26fc7b74 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -129,6 +129,16 @@ end end + describe "#new_award_emoji" do + let!(:todo) { create(:todo, :assigned, user: john_doe, project: project, target: issue, author: author) } + + it "marks all pending tasks as done" do + service.new_award_emoji(issue, john_doe) + + expect(todo.reload).to be_done + end + end + describe '#new_note' do let!(:first_todo) { create(:todo, :assigned, user: john_doe, project: project, target: issue, author: author) } let!(:second_todo) { create(:todo, :assigned, user: john_doe, project: project, target: issue, author: author) } @@ -139,7 +149,7 @@ let(:note_on_project_snippet) { create(:note_on_project_snippet, project: project, author: john_doe, note: mentions) } let(:system_note) { create(:system_note, project: project, noteable: issue) } - it 'mark related pending todos to the noteable for the note author as done' do + it 'marks related pending todos to the noteable for the note author as done' do first_todo = create(:todo, :assigned, user: john_doe, project: project, target: issue, author: author) second_todo = create(:todo, :assigned, user: john_doe, project: project, target: issue, author: author) diff --git a/spec/services/toggle_award_emoji_service_spec.rb b/spec/services/toggle_award_emoji_service_spec.rb new file mode 100644 index 000000000000..22660b9bb4f3 --- /dev/null +++ b/spec/services/toggle_award_emoji_service_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe ToggleAwardEmojiService, services: true do + let(:project) { create(:empty_project) } + let(:user) { create(:user) } + let(:issue) { create(:issue, project: project) } + let(:service) { ToggleAwardEmojiService.new(project, user) } + + describe :execute do + before do + project.team << [user, :master] + end + + context "on an Issue" do + + it "toggles the awarded emoji" do + service.execute(issue, "flag_nl") + expect(issue.award_emoji.count).to eq 1 + + service.execute(issue, "flag_nl") + expect(issue.reload.award_emoji.count).to eq 0 + end + + it "marks the todoable as done" do + expect_any_instance_of(TodoService).to receive(:new_award_emoji).with(issue, user) + + service.execute(issue, "flag_nl") + end + end + + context "on a Snippet" do + let(:note) { create(:note, noteable: issue) } + + it "toggles the awarded emoji" do + service.execute(note, "flag_nl") + expect(note.award_emoji.count).to eq 1 + + service.execute(note, "flag_nl") + expect(note.reload.award_emoji.count).to eq 0 + end + + it "doesn't mark the todoable as done" do + expect_any_instance_of(TodoService).not_to receive(:new_award_emoji).with(note, user) + + service.execute(note, "flag_nl") + end + end + end +end -- GitLab From cc1d631e8c538ed2140a0589f4082e688fb1607e Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Thu, 14 Apr 2016 09:09:28 +0200 Subject: [PATCH 22/22] Fix specs again --- features/steps/project/issues/issues.rb | 4 ++-- spec/services/issues/move_service_spec.rb | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb index af46df2d0f4f..5b5631c92fb3 100644 --- a/features/steps/project/issues/issues.rb +++ b/features/steps/project/issues/issues.rb @@ -192,14 +192,14 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps step 'issue "Release 0.4" have 2 upvotes and 1 downvote' do issue = Issue.find_by(title: 'Release 0.4') - create_list(:award_emoji, :upvote, 2, awardable: issue) + create_list(:award_emoji, 2, awardable: issue, name: "thumbsup") create(:award_emoji, :downvote, awardable: issue) end step 'issue "Tweet control" have 1 upvote and 2 downvotes' do issue = Issue.find_by(title: 'Tweet control') create(:award_emoji, :upvote, awardable: issue) - create_list(:award_emoji, :downvote, 2, awardable: issue) + create_list(:award_emoji, 2, awardable: issue, name: "thumbsdown") end step 'The list should be sorted by "Least popular"' do diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb index 4204783f7538..8bfc47011a5c 100644 --- a/spec/services/issues/move_service_spec.rb +++ b/spec/services/issues/move_service_spec.rb @@ -129,7 +129,11 @@ end it 'moves the award emoji too' do - expect(new_issue.award_emoji).to eq(old_issue.award_emoji) + new_issue.reload + first_emoji = new_issue.award_emoji.first + + expect(new_issue.award_emoji.count).to eq(old_issue.award_emoji.count) + expect(first_emoji.name).to eq(old_issue.award_emoji.first.name) end end -- GitLab