From 3bdc57f0a710b3769381ecad7ea4098223ecff56 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Sat, 16 Apr 2016 21:09:08 +0200 Subject: Create table for award emoji --- app/controllers/concerns/toggle_award_emoji.rb | 20 ++++++ app/controllers/projects/issues_controller.rb | 4 +- .../projects/merge_requests_controller.rb | 6 +- app/controllers/projects/notes_controller.rb | 35 ++------- app/controllers/projects_controller.rb | 2 +- app/helpers/issues_helper.rb | 13 ++-- app/models/award_emoji.rb | 35 +++++++++ app/models/concerns/awardable.rb | 81 +++++++++++++++++++++ app/models/concerns/issuable.rb | 32 +-------- app/models/merge_request.rb | 1 + app/models/note.rb | 53 +++----------- app/models/user.rb | 1 + app/services/notes/create_service.rb | 5 ++ app/services/notes/post_process_service.rb | 2 +- app/services/notification_service.rb | 1 - app/services/todo_service.rb | 8 +++ app/services/toggle_award_emoji_service.rb | 21 ++++++ app/views/award_emoji/_awards_block.html.haml | 15 ++++ app/views/emojis/index.html.haml | 4 +- app/views/projects/issues/_issue.html.haml | 2 +- app/views/projects/issues/show.html.haml | 2 +- .../merge_requests/_merge_request.html.haml | 2 +- app/views/projects/merge_requests/_show.html.haml | 4 +- app/views/votes/_votes_block.html.haml | 8 +-- config/initializers/inflections.rb | 4 ++ config/routes.rb | 7 +- db/migrate/20160416180807_add_award_emoji.rb | 15 ++++ ...0416182152_convert_award_note_to_emoji_award.rb | 17 +++++ db/migrate/20160416190505_remove_note_is_award.rb | 5 ++ db/schema.rb | 53 ++++++++------ lib/api/entities.rb | 7 +- lib/award_emoji.rb | 80 --------------------- lib/gitlab/award_emoji.rb | 82 ++++++++++++++++++++++ spec/controllers/groups_controller_spec.rb | 12 ++-- spec/factories/award_emoji.rb | 7 ++ spec/factories/notes.rb | 6 -- spec/helpers/issues_helper_spec.rb | 11 ++- spec/lib/award_emoji_spec.rb | 26 ------- spec/lib/gitlab/award_emoji_spec.rb | 26 +++++++ spec/models/award_emoji_spec.rb | 31 ++++++++ spec/models/concerns/issuable_spec.rb | 14 ---- spec/models/note_spec.rb | 39 ---------- 42 files changed, 469 insertions(+), 330 deletions(-) create mode 100644 app/controllers/concerns/toggle_award_emoji.rb create mode 100644 app/models/award_emoji.rb create mode 100644 app/models/concerns/awardable.rb create mode 100644 app/services/toggle_award_emoji_service.rb create mode 100644 app/views/award_emoji/_awards_block.html.haml create mode 100644 db/migrate/20160416180807_add_award_emoji.rb create mode 100644 db/migrate/20160416182152_convert_award_note_to_emoji_award.rb create mode 100644 db/migrate/20160416190505_remove_note_is_award.rb delete mode 100644 lib/award_emoji.rb create mode 100644 lib/gitlab/award_emoji.rb create mode 100644 spec/factories/award_emoji.rb delete mode 100644 spec/lib/award_emoji_spec.rb create mode 100644 spec/lib/gitlab/award_emoji_spec.rb create mode 100644 spec/models/award_emoji_spec.rb diff --git a/app/controllers/concerns/toggle_award_emoji.rb b/app/controllers/concerns/toggle_award_emoji.rb new file mode 100644 index 00000000000..9cd522d1c30 --- /dev/null +++ b/app/controllers/concerns/toggle_award_emoji.rb @@ -0,0 +1,20 @@ +module ToggleAwardEmoji + extend ActiveSupport::Concern + + included do + before_action :authenticate_user!, only: [:toggle_award_emoji] + end + + def toggle_award_emoji + name = params.require(:name) + awardable.toggle_award_emoji(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 38214f04793..86ba40facc5 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -1,6 +1,7 @@ class Projects::IssuesController < Projects::ApplicationController include ToggleSubscriptionAction include IssuableActions + include ToggleAwardEmoji before_action :module_enabled before_action :issue, @@ -61,7 +62,7 @@ class Projects::IssuesController < Projects::ApplicationController def show @note = @project.notes.new(noteable: @issue) - @notes = @issue.notes.nonawards.with_associations.fresh + @notes = @issue.notes.with_associations.fresh @noteable = @issue respond_to do |format| @@ -158,6 +159,7 @@ class Projects::IssuesController < Projects::ApplicationController end alias_method :subscribable_resource, :issue alias_method :issuable, :issue + alias_method :awardable, :issue def authorize_read_issue! return render_404 unless can?(current_user, :read_issue, @issue) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 3e0cfc6aa65..9117f9242cd 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -2,6 +2,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController include ToggleSubscriptionAction include DiffHelper include IssuableActions + include ToggleAwardEmoji before_action :module_enabled before_action :merge_request, only: [ @@ -195,7 +196,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController if params[:merge_when_build_succeeds].present? && @merge_request.ci_commit && @merge_request.ci_commit.active? MergeRequests::MergeWhenBuildSucceedsService.new(@project, current_user, merge_params) - .execute(@merge_request) + .execute(@merge_request) @status = :merge_when_build_succeeds else MergeWorker.perform_async(@merge_request.id, current_user.id, params) @@ -264,6 +265,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController end alias_method :subscribable_resource, :merge_request alias_method :issuable, :merge_request + alias_method :awardable, :merge_request def closes_issues @closes_issues ||= @merge_request.closes_issues @@ -299,7 +301,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController 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 707a0d0e5c6..9000e0adf63 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -3,7 +3,7 @@ class Projects::NotesController < Projects::ApplicationController 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 @@ -22,8 +22,10 @@ class Projects::NotesController < Projects::ApplicationController def create @note = Notes::CreateService.new(project, current_user, note_params).execute + @note = note.is_a?(AwardEmoji) ? @note.to_note_json : note_json(@note) + respond_to do |format| - format.json { render json: note_json(@note) } + format.json { render json: @note } format.html { redirect_back_or_default } end end @@ -56,35 +58,12 @@ class Projects::NotesController < Projects::ApplicationController 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( @@ -137,7 +116,7 @@ class Projects::NotesController < Projects::ApplicationController id: note.id, discussion_id: note.discussion_id, html: note_to_html(note), - award: note.is_award, + award: false, note: note.note, discussion_html: note_to_discussion_html(note), discussion_with_diff_html: note_to_discussion_with_diff_html(note) @@ -145,7 +124,7 @@ class Projects::NotesController < Projects::ApplicationController else { valid: false, - award: note.is_award, + award: false, errors: note.errors } end diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 3768efe142a..85a987c2cb2 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -145,7 +145,7 @@ class ProjectsController < Projects::ApplicationController 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/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 4cb8adcebad..38de0b442ca 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -144,16 +144,17 @@ module IssuesHelper end 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" : award.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.find { |a| a.user_id == current_user.id } "active" else "" diff --git a/app/models/award_emoji.rb b/app/models/award_emoji.rb new file mode 100644 index 00000000000..44a9b55a8a6 --- /dev/null +++ b/app/models/award_emoji.rb @@ -0,0 +1,35 @@ +class AwardEmoji < 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 + + def to_note_json + { + valid: valid?, + award: true, + id: id, + name: name + } + end +end diff --git a/app/models/concerns/awardable.rb b/app/models/concerns/awardable.rb new file mode 100644 index 00000000000..b4e3e9eb3dd --- /dev/null +++ b/app/models/concerns/awardable.rb @@ -0,0 +1,81 @@ +module Awardable + extend ActiveSupport::Concern + + included do + has_many :award_emoji, as: :awardable, dependent: :destroy + + if self < Participable + participant :award_emoji + end + end + + module ClassMethods + def order_upvotes_desc + order_votes_desc(AwardEmoji::UPVOTE_NAME) + end + + def order_downvotes_desc + order_votes_desc(AwardEmoji::DOWNVOTE_NAME) + end + + def order_votes_desc(emoji_name) + awardable_table = self.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( + 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(award_emoji.id) DESC") + end + end + + def grouped_awards(with_thumbs = true) + awards = award_emoji.group_by(&:name) + + if with_thumbs + awards[AwardEmoji::UPVOTE_NAME] ||= AwardEmoji.none + awards[AwardEmoji::DOWNVOTE_NAME] ||= AwardEmoji.none + end + + awards + end + + def downvotes + award_emoji.where(name: AwardEmoji::DOWNVOTE_NAME).count + end + + def upvotes + award_emoji.where(name: AwardEmoji::UPVOTE_NAME).count + end + + def emoji_awardable? + true + end + + def awarded_emoji?(emoji_name, current_user) + award_emoji.where(name: emoji_name, user: current_user).exists? + end + + def create_award_emoji(name, current_user) + return unless emoji_awardable? + + award_emoji.create(name: name, user: current_user) + end + + def remove_award_emoji(name, current_user) + award_emoji.where(name: name, user: current_user).destroy_all + end + + def toggle_award_emoji(emoji_name, current_user) + if awarded_emoji?(emoji_name, current_user) + remove_award_emoji(emoji_name, current_user) + else + create_award_emoji(emoji_name, current_user) + end + end +end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index afa2ca039ae..6af76c97cd3 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -10,6 +10,7 @@ module Issuable include Mentionable include Subscribable include StripAttribute + include Awardable included do belongs_to :author, class_name: "User" @@ -99,29 +100,6 @@ module Issuable 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? @@ -144,14 +122,6 @@ module Issuable opened? || reopened? end - def downvotes - notes.awards.where(note: "thumbsdown").count - end - - def upvotes - notes.awards.where(note: "thumbsup").count - end - def subscribed_without_subscriptions?(user) participants(user).include?(user) end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index e410febdfff..2cb3e8b0176 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -36,6 +36,7 @@ class MergeRequest < ActiveRecord::Base include Referable include Sortable include Taskable + include Awardable belongs_to :target_project, foreign_key: :target_project_id, class_name: "Project" belongs_to :source_project, foreign_key: :source_project_id, class_name: "Project" diff --git a/app/models/note.rb b/app/models/note.rb index 87ced65c650..b992b2e76f0 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' @@ -43,12 +42,9 @@ class Note < ActiveRecord::Base delegate :name, to: :project, prefix: true delegate :name, :email, to: :author, prefix: true - before_validation :set_award! before_validation :clear_blank_line_code! 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 } @@ -60,8 +56,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) } @@ -119,19 +113,6 @@ class Note < ActiveRecord::Base where(table[:note].matches(pattern)) 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? @@ -347,37 +328,25 @@ class Note < ActiveRecord::Base Event.reset_event_cache_for(self) end - def downvote? - is_award && note == "thumbsdown" - end - - def upvote? - is_award && note == "thumbsup" + def system? + read_attribute(:system) end def editable? - !system? && !is_award + !system? end def cross_reference_not_visible_for?(user) cross_reference? && referenced_mentionables(user).empty? 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 award_emoji? + award_emoji_supported? && contains_emoji_only? end - private + def create_award_emoji + self.noteable.award_emoji(award_emoji_name, author) + end def clear_blank_line_code! self.line_code = nil if self.line_code.blank? @@ -389,8 +358,8 @@ class Note < ActiveRecord::Base diffs.find { |d| d.new_path == self.diff.new_path } end - def awards_supported? - (for_issue? || for_merge_request?) && !for_diff_line? + def award_emoji_supported? + noteable.is_a?(Awardable) && !for_diff_line? end def contains_emoji_only? @@ -399,6 +368,6 @@ class Note < ActiveRecord::Base def award_emoji_name original_name = note.match(Banzai::Filter::EmojiFilter.emoji_pattern)[1] - AwardEmoji.normilize_emoji_name(original_name) + Gitlab::AwardEmoji.normilize_emoji_name(original_name) end end diff --git a/app/models/user.rb b/app/models/user.rb index 031315debd7..52f2904f450 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -144,6 +144,7 @@ class User < ActiveRecord::Base has_many :builds, dependent: :nullify, class_name: 'Ci::Build' has_many :todos, dependent: :destroy has_many :notification_settings, dependent: :destroy + has_many :award_emoji, as: :awardable, dependent: :destroy # # Validations diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 2bb312bb252..c5be21ba897 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -5,6 +5,11 @@ module Notes note.author = current_user note.system = false + if note.award_emoji? + return ToggleAwardEmojiService.new(project, current_user, params). + execute(note.noteable, note.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 e818f58d13c..c1bf46bdfb3 100644 --- a/app/services/notes/post_process_service.rb +++ b/app/services/notes/post_process_service.rb @@ -8,7 +8,7 @@ module Notes 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/services/notification_service.rb b/app/services/notification_service.rb index 42ec1ac9e1a..703636658b7 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -131,7 +131,6 @@ class NotificationService # 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/app/services/todo_service.rb b/app/services/todo_service.rb index 42c5bca90fd..da1b77c0f9e 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -98,6 +98,14 @@ class TodoService handle_note(note, current_user) end + # When an emoji is awarded we should: + # + # * mark all pending todos related to the awardable for the current user as done + # + def new_award_emoji(awardable, current_user) + mark_pending_todos_as_done(awardable, 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 diff --git a/app/services/toggle_award_emoji_service.rb b/app/services/toggle_award_emoji_service.rb new file mode 100644 index 00000000000..b77b4e79bf2 --- /dev/null +++ b/app/services/toggle_award_emoji_service.rb @@ -0,0 +1,21 @@ +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) + todo_service.new_award_emoji(awardable, current_user) + + # Needed if its posted as a note containing only :+1: + emoji = award_emoji_name(emoji) if emoji.start_with? ':' + awardable.toggle_award_emoji(emoji, current_user) + end + + private + + def award_emoji_name(emoji) + original_name = emoji.match(Banzai::Filter::EmojiFilter.emoji_pattern)[1] + Gitlab::AwardEmoji.normalize_emoji_name(original_name) + end +end diff --git a/app/views/award_emoji/_awards_block.html.haml b/app/views/award_emoji/_awards_block.html.haml new file mode 100644 index 00000000000..63c953195fe --- /dev/null +++ b/app/views/award_emoji/_awards_block.html.haml @@ -0,0 +1,15 @@ +- grouped_awards = awardable.grouped_awards(inline) +.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_awards).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) + %span.award-control-text.js-counter + = awards.count + + - if current_user + .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 diff --git a/app/views/emojis/index.html.haml b/app/views/emojis/index.html.haml index 3443a8e2307..97401a2e618 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/app/views/projects/issues/_issue.html.haml b/app/views/projects/issues/_issue.html.haml index 7a8009f6da4..4aa92d0b39e 100644 --- a/app/views/projects/issues/_issue.html.haml +++ b/app/views/projects/issues/_issue.html.haml @@ -27,7 +27,7 @@ = icon('thumbs-down') = downvotes - - note_count = issue.notes.user.nonawards.count + - note_count = issue.notes.user.count - if note_count > 0 %li = link_to issue_path(issue) + "#notes" do diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 5fe5ddc0819..c4cdd4b3d43 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -72,7 +72,7 @@ .content-block.content-block-small = render 'new_branch' - = render 'votes/votes_block', votable: @issue + = render 'award_emoji/awards_block', awardable: @issue, inline: true .row %section.col-md-12 diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml index e740fe8c84d..391193eed6c 100644 --- a/app/views/projects/merge_requests/_merge_request.html.haml +++ b/app/views/projects/merge_requests/_merge_request.html.haml @@ -35,7 +35,7 @@ = icon('thumbs-down') = downvotes - - note_count = merge_request.mr_and_commit_notes.user.nonawards.count + - note_count = merge_request.mr_and_commit_notes.user.count - if note_count > 0 %li = link_to merge_request_path(merge_request) + "#notes" do diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 2c34f9c454b..e8cda51e759 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -50,7 +50,7 @@ %li.notes-tab = link_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: {target: 'div#notes', action: 'notes', toggle: 'tab'} do Discussion - %span.badge= @merge_request.mr_and_commit_notes.user.nonawards.count + %span.badge= @merge_request.mr_and_commit_notes.user.count %li.commits-tab = link_to commits_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: {target: 'div#commits', action: 'commits', toggle: 'tab'} do Commits @@ -68,7 +68,7 @@ .tab-content #notes.notes.tab-pane.voting_notes .content-block.content-block-small.oneline-block - = render 'votes/votes_block', votable: @merge_request + = render 'award_emoji/awards_block', awardable: @merge_request, inline: true .row %section.col-md-12 diff --git a/app/views/votes/_votes_block.html.haml b/app/views/votes/_votes_block.html.haml index dc249155b92..8692c1cccee 100644 --- a/app/views/votes/_votes_block.html.haml +++ b/app/views/votes/_votes_block.html.haml @@ -1,9 +1,9 @@ -.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)), data: {placement: "top", original_title: emoji_author_list(notes, current_user)}} +.awards.votes-block{data: { toggle_url: url_for([:toggle_award_emoji, @project.namespace.becomes(Namespace), @project, awardable]) }} + - awards_sort(awardable.grouped_awards).each do |emoji, awards| + %button.btn.award-control.js-emoji-btn.has-tooltip{class: (note_active_class(awards, current_user)), data: {placement: "top", original_title: emoji_author_list(awards, current_user)}} = emoji_icon(emoji, sprite: false) %span.award-control-text.js-counter - = notes.count + = awards.count - if current_user %div.award-menu-holder.js-award-holder diff --git a/config/initializers/inflections.rb b/config/initializers/inflections.rb index 9e8b0131f8f..3d1a41a4652 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 46a25262844..ecde83d8547 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -639,6 +639,7 @@ Rails.application.routes.draw do post :cancel_merge_when_build_succeeds get :ci_status post :toggle_subscription + post :toggle_award_emoji post :remove_wip end @@ -703,6 +704,7 @@ Rails.application.routes.draw do resources :issues, constraints: { id: /\d+/ } do member do post :toggle_subscription + post :toggle_award_emoji get :referenced_merge_requests get :related_branches end @@ -731,10 +733,7 @@ Rails.application.routes.draw do resources :notes, only: [:index, :create, :destroy, :update], constraints: { id: /\d+/ } do member do delete :delete_attachment - end - - collection do - post :award_toggle + post :toggle_award_emoji end end diff --git a/db/migrate/20160416180807_add_award_emoji.rb b/db/migrate/20160416180807_add_award_emoji.rb new file mode 100644 index 00000000000..3177b86a133 --- /dev/null +++ b/db/migrate/20160416180807_add_award_emoji.rb @@ -0,0 +1,15 @@ +class AddAwardEmoji < ActiveRecord::Migration + def change + create_table :award_emoji do |t| + t.string :name + t.references :user + t.references :awardable, polymorphic: true + + t.timestamps + end + + add_index :award_emoji, :user_id + add_index :award_emoji, :awardable_type + add_index :award_emoji, :awardable_id + end +end diff --git a/db/migrate/20160416182152_convert_award_note_to_emoji_award.rb b/db/migrate/20160416182152_convert_award_note_to_emoji_award.rb new file mode 100644 index 00000000000..76f4a3aa6ae --- /dev/null +++ b/db/migrate/20160416182152_convert_award_note_to_emoji_award.rb @@ -0,0 +1,17 @@ +class ConvertAwardNoteToEmojiAward < ActiveRecord::Migration + def change + def up + execute "INSERT INTO award_emoji (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 + execute <<-SQL + INSERT INTO notes (noteable_type, noteable_id, author_id, note, created_at, updated_at, is_award) + (SELECT awardable_type, awardable_id, user_id, name, created_at, updated_at, TRUE + FROM award_emoji + WHERE awardable_type IN ('Issue', 'MergeRequest') + ) + SQL + end + end +end diff --git a/db/migrate/20160416190505_remove_note_is_award.rb b/db/migrate/20160416190505_remove_note_is_award.rb new file mode 100644 index 00000000000..da16372a297 --- /dev/null +++ b/db/migrate/20160416190505_remove_note_is_award.rb @@ -0,0 +1,5 @@ +class RemoveNoteIsAward < ActiveRecord::Migration + def change + remove_column :notes, :is_award, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 42c261003bb..354d7390a5b 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: 20160412140240) do +ActiveRecord::Schema.define(version: 20160416190505) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -94,6 +94,19 @@ ActiveRecord::Schema.define(version: 20160412140240) do 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" @@ -622,14 +635,12 @@ ActiveRecord::Schema.define(version: 20160412140240) do 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", ["note"], name: "index_notes_on_note_trigram", using: :gin, opclasses: {"note"=>"gin_trgm_ops"} add_index "notes", ["noteable_id", "noteable_type"], name: "index_notes_on_noteable_id_and_noteable_type", using: :btree @@ -716,37 +727,37 @@ ActiveRecord::Schema.define(version: 20160412140240) do t.datetime "created_at" t.datetime "updated_at" t.integer "creator_id" - t.boolean "issues_enabled", default: true, null: false - t.boolean "wall_enabled", default: true, null: false - t.boolean "merge_requests_enabled", default: true, null: false - t.boolean "wiki_enabled", default: true, null: false + t.boolean "issues_enabled", default: true, null: false + t.boolean "wall_enabled", default: true, null: false + t.boolean "merge_requests_enabled", default: true, null: false + t.boolean "wiki_enabled", default: true, null: false t.integer "namespace_id" - t.string "issues_tracker", default: "gitlab", null: false + t.string "issues_tracker", default: "gitlab", null: false t.string "issues_tracker_id" - t.boolean "snippets_enabled", default: true, null: false + t.boolean "snippets_enabled", default: true, null: false t.datetime "last_activity_at" t.string "import_url" - t.integer "visibility_level", default: 0, null: false - t.boolean "archived", default: false, null: false + t.integer "visibility_level", default: 0, null: false + t.boolean "archived", default: false, null: false t.string "avatar" t.string "import_status" - t.float "repository_size", default: 0.0 - t.integer "star_count", default: 0, null: false + t.float "repository_size", default: 0.0 + t.integer "star_count", default: 0, null: false t.string "import_type" t.string "import_source" - t.integer "commit_count", default: 0 + t.integer "commit_count", default: 0 t.text "import_error" t.integer "ci_id" - t.boolean "builds_enabled", default: true, null: false - t.boolean "shared_runners_enabled", default: true, null: false + t.boolean "builds_enabled", default: true, null: false + t.boolean "shared_runners_enabled", default: true, null: false t.string "runners_token" t.string "build_coverage_regex" - t.boolean "build_allow_git_fetch", default: true, null: false - t.integer "build_timeout", default: 3600, null: false - t.boolean "pending_delete", default: false - t.boolean "public_builds", default: true, null: false + t.boolean "build_allow_git_fetch", default: true, null: false + t.integer "build_timeout", default: 3600, null: false + t.boolean "pending_delete", default: false + t.boolean "public_builds", default: true, null: false t.string "main_language" - t.integer "pushes_since_gc", default: 0 + t.integer "pushes_since_gc", default: 0 t.boolean "last_repository_check_failed" t.datetime "last_repository_check_at" end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 60b9f5e0ece..b3769ba9c2d 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -170,6 +170,7 @@ module API expose :label_names, as: :labels expose :milestone, using: Entities::Milestone expose :assignee, :author, using: Entities::UserBasic + expose :upvotes, :downvotes expose :subscribed do |issue, options| issue.subscribed?(options[:current_user]) @@ -178,7 +179,7 @@ module API 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 @@ -216,8 +217,8 @@ module API 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 deleted file mode 100644 index 5f8ff01b0a9..00000000000 --- a/lib/award_emoji.rb +++ /dev/null @@ -1,80 +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 - - CATEGORY_ALIASES = { - symbols: "objects_symbols", - foods: "food_drink", - travel: "travel_places" - }.with_indifferent_access - - def self.normilize_emoji_name(name) - aliases[name] || name - end - - def self.emoji_by_category - unless @emoji_by_category - @emoji_by_category = Hash.new { |h, key| h[key] = [] } - - emojis.each do |emoji_name, data| - data["name"] = emoji_name - - # Skip Fitzpatrick(tone) modifiers - next if data["category"] == "modifier" - - category = CATEGORY_ALIASES[data["category"]] || data["category"] - - @emoji_by_category[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 00000000000..0ae220a86bc --- /dev/null +++ b/lib/gitlab/award_emoji.rb @@ -0,0 +1,82 @@ +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 + + CATEGORY_ALIASES = { + symbols: "objects_symbols", + foods: "food_drink", + travel: "travel_places" + }.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 = Hash.new { |h, key| h[key] = [] } + + emojis.each do |emoji_name, data| + data["name"] = emoji_name + + # Skip Fitzpatrick(tone) modifiers + next if data["category"] == "modifier" + + category = CATEGORY_ALIASES[data["category"]] || data["category"] + + @emoji_by_category[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/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 465531b2b36..82b25702172 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -31,9 +31,9 @@ describe GroupsController do 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) + create_list(:award_emoji, 2, awardable: issue_1) + create_list(:award_emoji, 2, awardable: issue_2, name: "thumbsdown") sign_in(user) end @@ -56,9 +56,9 @@ describe GroupsController do 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) + create_list(:award_emoji, 2, awardable: merge_request_1) + 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 00000000000..a1173834b29 --- /dev/null +++ b/spec/factories/award_emoji.rb @@ -0,0 +1,7 @@ +FactoryGirl.define do + factory :award_emoji do + name "thumbsup" + user + awardable factory: :issue + end +end diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb index e5dcb159014..2bfc5effd78 100644 --- a/spec/factories/notes.rb +++ b/spec/factories/notes.rb @@ -36,8 +36,6 @@ FactoryGirl.define do 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 @@ FactoryGirl.define do system true end - trait :award do - is_award true - end - trait :downvote do note "thumbsdown" end diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb index 543593cf389..2d4d9c18c9d 100644 --- a/spec/helpers/issues_helper_spec.rb +++ b/spec/helpers/issues_helper_spec.rb @@ -127,18 +127,15 @@ describe IssuesHelper do it { is_expected.to eq("!1, !2, or !3") } end - describe "note_active_class" do - before do - @note = create :note - @note1 = create :note - end + describe '#award_active_class' do + let!(:upvote) { create(:award_emoji) } it "returns empty string for unauthenticated user" do - expect(note_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(note_active_class(Note.all, @note.author)).to eq("active") + expect(award_active_class(AwardEmoji.all, upvote.user)).to eq("active") end end diff --git a/spec/lib/award_emoji_spec.rb b/spec/lib/award_emoji_spec.rb deleted file mode 100644 index 88c22912950..00000000000 --- a/spec/lib/award_emoji_spec.rb +++ /dev/null @@ -1,26 +0,0 @@ -require 'spec_helper' - -describe AwardEmoji do - describe '.urls' do - subject { AwardEmoji.urls } - - it { is_expected.to be_an_instance_of(Array) } - it { is_expected.to_not be_empty } - - context 'every Hash in the Array' do - it 'has the correct keys and values' do - subject.each do |hash| - expect(hash[:name]).to be_an_instance_of(String) - expect(hash[:path]).to be_an_instance_of(String) - end - end - end - end - - describe '.emoji_by_category' do - it "only contains known categories" do - undefined_categories = AwardEmoji.emoji_by_category.keys - AwardEmoji::CATEGORIES.keys - expect(undefined_categories).to be_empty - end - end -end diff --git a/spec/lib/gitlab/award_emoji_spec.rb b/spec/lib/gitlab/award_emoji_spec.rb new file mode 100644 index 00000000000..4e6c04a11b9 --- /dev/null +++ b/spec/lib/gitlab/award_emoji_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +describe Gitlab::AwardEmoji do + describe '.urls' do + subject { Gitlab::AwardEmoji.urls } + + it { is_expected.to be_an_instance_of(Array) } + it { is_expected.to_not be_empty } + + context 'every Hash in the Array' do + it 'has the correct keys and values' do + subject.each do |hash| + expect(hash[:name]).to be_an_instance_of(String) + expect(hash[:path]).to be_an_instance_of(String) + end + end + end + end + + describe '.emoji_by_category' do + it "only contains known categories" do + undefined_categories = Gitlab::AwardEmoji.emoji_by_category.keys - Gitlab::AwardEmoji::CATEGORIES.keys + expect(undefined_categories).to be_empty + end + end +end diff --git a/spec/models/award_emoji_spec.rb b/spec/models/award_emoji_spec.rb new file mode 100644 index 00000000000..fd3712b7d43 --- /dev/null +++ b/spec/models/award_emoji_spec.rb @@ -0,0 +1,31 @@ +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 a bug 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) + new_award = AwardEmoji.new(user: user, awardable: issue, name: "thumbsup") + + expect(new_award).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 b16ccc6e305..d5435916ea1 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -198,18 +198,4 @@ describe Issue, "Issuable" do to eq({ 'Author' => 'Robert', 'Assignee' => 'Douwe' }) end end - - 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) - end - - it "returns correct values" do - 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 6b18936edb1..bb591e9cb53 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -152,23 +152,6 @@ describe Note, models: true do 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 @@ describe Note, models: true do 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 @@ describe Note, models: true do 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: ' ') -- cgit v1.2.3 From 4eb16290e4e95c0a9bcf3d01ecc8060d91eec021 Mon Sep 17 00:00:00 2001 From: Arinde Eniola Date: Mon, 25 Apr 2016 09:09:39 +0100 Subject: move frontend logic from previous MR to new MR --- app/assets/javascripts/awards_handler.coffee | 215 ++++++++++++--------- app/assets/javascripts/dispatcher.js.coffee | 2 + .../javascripts/lib/emoji_aliases.js.coffee.erb | 9 + app/assets/javascripts/notes.js.coffee | 4 +- app/assets/stylesheets/pages/awards.scss | 13 +- app/assets/stylesheets/pages/notes.scss | 41 +++- app/finders/notes_finder.rb | 4 +- app/views/award_emoji/_awards_block.html.haml | 6 +- 8 files changed, 192 insertions(+), 102 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 af4462ece38..4c0a274b793 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -1,63 +1,109 @@ class @AwardsHandler - constructor: (@get_emojis_url, @post_emoji_url, @noteable_type, @noteable_id, @aliases) -> - $(".js-add-award").on "click", (event) => - event.stopPropagation() - event.preventDefault() + constructor: -> + @aliases = gl.emoji.emojiAliases() - @showEmojiMenu() + $(document) + .off "click", ".js-add-award" + .on "click", ".js-add-award", (event) => + event.stopPropagation() + event.preventDefault() + + @showEmojiMenu $(event.currentTarget) $("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" - $(".awards") - .off "click" - .on "click", ".js-emoji-btn", @handleClick - - @renderFrequentlyUsedBlock() + $(document) + .off "click", ".js-emoji-btn" + .on "click", ".js-emoji-btn", (e) => @handleClick(e) handleClick: (e) -> e.preventDefault() - emoji = $(this) + $emojiBtn = $(e.currentTarget) + $addAwardBtn = $('.js-add-award.is-active') + $votesBlock = $($addAwardBtn.closest('.js-award-holder').data('target')) + + 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' + emoji = $emojiBtn .find(".icon") .data "emoji" - if emoji is "thumbsup" and awards_handler.didUserClickEmoji $(this), "thumbsdown" - awards_handler.addAward "thumbsdown" + if emoji is "thumbsup" and @didUserClickEmoji $emojiBtn, "thumbsdown" + @addAward awardUrl, "thumbsdown" + + else if emoji is "thumbsdown" and @didUserClickEmoji $emojiBtn, "thumbsup" + @addAward awardUrl, "thumbsup" - else if emoji is "thumbsdown" and awards_handler.didUserClickEmoji $(this), "thumbsup" - awards_handler.addAward "thumbsup" + @addAward awardUrl, emoji - awards_handler.addAward emoji + didUserClickEmoji: (emojiBtn, emoji) -> + if emojiBtn.siblings("button:has([data-emoji=#{emoji}])").attr("data-original-title") + emojiBtn.siblings("button:has([data-emoji=#{emoji}])").attr("data-original-title").indexOf('me') > -1 - didUserClickEmoji: (that, emoji) -> - if $(that).siblings("button:has([data-emoji=#{emoji}])").attr("data-original-title") - $(that).siblings("button:has([data-emoji=#{emoji}])").attr("data-original-title").indexOf('me') > -1 + showEmojiMenu: ($addBtn) -> + $menu = $('.emoji-menu') + if $menu.length + $holder = $addBtn.closest('.js-award-holder') - showEmojiMenu: -> - if $(".emoji-menu").length - 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 - $('.js-add-award').addClass "is-loading" - $.get @get_emojis_url, (response) => - $('.js-add-award').removeClass "is-loading" - $(".js-award-holder").append response + $addBtn.addClass "is-loading is-active" + $.get $addBtn.data('award-menu-url'), (response) => + $addBtn.removeClass "is-loading" + $('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 - addAward: (emoji) -> + 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 emoji, => + @postEmoji awardUrl, emoji, => @addAwardToEmojiBar(emoji) + $('.js-awards-block').removeClass 'js-awards-block-current' $(".emoji-menu").removeClass "is-visible" @@ -65,58 +111,60 @@ class @AwardsHandler @addEmojiToFrequentlyUsedList(emoji) emoji = @normilizeEmojiName(emoji) - if @exist(emoji) - if @isActive(emoji) - @decrementCounter(emoji) + $emojiBtn = @findEmojiIcon(emoji).parent() + + if $emojiBtn.length > 0 + if @isActive($emojiBtn) + @decrementCounter($emojiBtn, emoji) else - counter = @findEmojiIcon(emoji).siblings(".js-counter") - counter.text(parseInt(counter.text()) + 1) - counter.parent().addClass("active") - @addMeToAuthorList(emoji) + $counter = $emojiBtn.find('.js-counter') + $counter.text(parseInt($counter.text()) + 1) + $emojiBtn.addClass("active") + @addMeToUserList(emoji) else @createEmoji(emoji) - exist: (emoji) -> - @findEmojiIcon(emoji).length > 0 - - isActive: (emoji) -> - @findEmojiIcon(emoji).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") - @removeMeFromAuthorList(emoji) - else if emoji == "thumbsup" || emoji == "thumbsdown" - emojiIcon.tooltip("destroy") - counter.text(0) - emojiIcon.removeClass("active") - @removeMeFromAuthorList(emoji) + isActive: ($emojiBtn) -> + $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 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() - removeMeFromAuthorList: (emoji) -> - award_block = @findEmojiIcon(emoji).parent() + $emojiBtn.removeClass("active") + + removeMeFromUserList: ($emojiBtn, emoji) -> + award_block = $emojiBtn authors = award_block .attr("data-original-title") .split(", ") - authors.splice(authors.indexOf("me"),1) + authors.splice(authors.indexOf("me"), 1) award_block .closest(".js-emoji-btn") .attr("data-original-title", authors.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("data-original-title", authors.join(", ")) + users = origTitle.split(', ') + users.push("me") + award_block.attr("data-original-title", users.join(", ")) @resetTooltip(award_block) resetTooltip: (award) -> @@ -127,24 +175,24 @@ class @AwardsHandler award.tooltip() ), 200 - createEmoji: (emoji) -> emojiCssClass = @resolveNameToCssClass(emoji) - nodes = [] - nodes.push( - "" - ) + buttonHtml = "" - emoji_node = $(nodes.join("\n")) - .insertBefore(".js-award-holder") + emoji_node = $(buttonHtml) + .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,17 +204,13 @@ class @AwardsHandler "emoji-#{unicodeName}" - postEmoji: (emoji, callback) -> - $.post @post_emoji_url, { note: { - note: ":#{emoji}:" - noteable_type: @noteable_type - noteable_id: @noteable_id - }},(data) -> + postEmoji: (awardUrl, emoji, callback) -> + $.post awardUrl, { name: emoji }, (data) -> if data.ok 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({ @@ -189,16 +233,15 @@ class @AwardsHandler if $.cookie('frequently_used_emojis') frequently_used_emojis = @getFrequentlyUsedEmojis() - ul = $("