From fd2c0fe446c7f761b845c91307ef8110d869e8e8 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Wed, 11 Nov 2015 15:12:51 +0200 Subject: award emoji --- app/assets/javascripts/awards_handler.coffee | 55 ++++++ app/assets/javascripts/notes.js.coffee | 9 +- app/assets/stylesheets/pages/issuable.scss | 42 +++++ app/controllers/projects/issues_controller.rb | 2 +- .../projects/merge_requests_controller.rb | 2 +- app/controllers/projects/notes_controller.rb | 25 ++- app/helpers/issues_helper.rb | 5 + app/models/concerns/issuable.rb | 49 ------ app/models/note.rb | 46 +---- app/services/notes/create_service.rb | 17 +- app/services/notification_service.rb | 1 + app/views/projects/issues/_issue.html.haml | 2 - .../merge_requests/_merge_request.html.haml | 2 - app/views/projects/notes/_note.html.haml | 20 --- app/views/votes/_votes_block.html.haml | 43 +++-- app/views/votes/_votes_inline.html.haml | 9 - config/routes.rb | 4 + db/migrate/20151106000015_add_is_award_to_notes.rb | 5 + db/schema.rb | 3 +- doc/api/merge_requests.md | 12 -- doc/api/notes.md | 8 +- lib/api/entities.rb | 4 +- spec/lib/votes_spec.rb | 188 --------------------- spec/models/project_spec.rb | 11 -- 24 files changed, 203 insertions(+), 361 deletions(-) create mode 100644 app/assets/javascripts/awards_handler.coffee delete mode 100644 app/views/votes/_votes_inline.html.haml create mode 100644 db/migrate/20151106000015_add_is_award_to_notes.rb delete mode 100644 spec/lib/votes_spec.rb diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee new file mode 100644 index 00000000000..aab3179f10e --- /dev/null +++ b/app/assets/javascripts/awards_handler.coffee @@ -0,0 +1,55 @@ +class @AwardsHandler + constructor: (@post_emoji_url, @noteable_type, @noteable_id) -> + + addAward: (emoji) -> + @postEmoji emoji, => + if @exist(emoji) + if @isActive(emoji) + @decrementCounter(emoji) + else + counter = $(".icon." + emoji).siblings(".counter") + counter.text(parseInt(counter.text()) + 1) + counter.parent().addClass("active") + else + @createEmoji(emoji) + + + exist: (emoji) -> + $(".icon").hasClass(emoji) + + isActive: (emoji) -> + $(".icon." + emoji).parent().hasClass("active") + + decrementCounter: (emoji) -> + counter = $(".icon." + emoji).siblings(".counter") + + if parseInt(counter.text()) > 1 + counter.text(parseInt(counter.text()) - 1) + counter.parent().removeClass("active") + else + counter.parent().remove() + + + createEmoji: (emoji) -> + nodes = [] + nodes.push("
") + nodes.push("
") + nodes.push(@getImage(emoji)) + nodes.push("
") + nodes.push("
1") + nodes.push("
") + + $(".awards").append(nodes.join("\n")) + + getImage: (emoji) -> + $("li." + emoji).html() + + postEmoji: (emoji, callback) -> + emoji = emoji.replace("emoji-", "") + $.post @post_emoji_url, { + emoji: emoji + noteable_type: @noteable_type + noteable_id: @noteable_id + },(data) -> + if data.ok + callback.call() \ No newline at end of file diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index ea75c656bcc..cd27b20dd7c 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -113,13 +113,16 @@ class @Notes renderNote: (note) -> # render note if it not present in loaded list # or skip if rendered - if @isNewNote(note) + if @isNewNote(note) && !note.award @note_ids.push(note.id) $('ul.main-notes-list'). append(note.html). syntaxHighlight() @initTaskList() + if note.award + awards_handler.addAward("emoji-" + note.note) + ### Check if note does not exists on page ### @@ -255,7 +258,6 @@ class @Notes ### addNote: (xhr, note, status) => @renderNote(note) - @updateVotes() ### Called in response to the new note form being submitted @@ -473,9 +475,6 @@ class @Notes form = $(e.target).closest(".js-discussion-note-form") @removeDiscussionNoteForm(form) - updateVotes: -> - true - ### Called after an attachment file has been selected. diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index abc27a19e32..efeb2393165 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -101,3 +101,45 @@ background-color: $background-color; } } + +.awards { + .award { + border: 1px solid; + padding: 1px 3px; + width: 50px; + border-radius: 5px; + float:left; + margin: 0 3px; + border-color: #ccc; + cursor: pointer; + + &.active { + border-color: rgba(79,176,252,.4); + background-color: rgba(79,176,252,.08); + + .counter { + font-weight: bold; + } + } + + .icon { + float: left; + margin-right: 10px; + } + } + + #add-award { + font-size: 20px; + border-radius: 5px; + float: left; + width: 50px; + font-weight: bold; + } + + .awards-menu{ + li { + float: left; + margin: 3px; + } + } +} diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index e74c2905e48..5250a0f5e67 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -60,7 +60,7 @@ class Projects::IssuesController < Projects::ApplicationController def show @participants = @issue.participants(current_user) @note = @project.notes.new(noteable: @issue) - @notes = @issue.notes.with_associations.fresh + @notes = @issue.notes.nonawards.with_associations.fresh @noteable = @issue respond_with(@issue) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 188f0cc4cea..a0468c65d5a 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -254,7 +254,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController # Build a note object for comment form @note = @project.notes.new(noteable: @merge_request) - @notes = @merge_request.mr_and_commit_notes.inc_author.fresh + @notes = @merge_request.nonawards.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 41cd08c93c6..357b292980d 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] + before_action :find_current_user_notes, except: [:destroy, :delete_attachment, :award_toggle]] def index current_fetched_at = Time.now.to_i @@ -58,6 +58,27 @@ class Projects::NotesController < Projects::ApplicationController end end + def award_toggle + noteable = params[:noteable_type] == "Issue" ? Issue : MergeRequest + noteable = noteable.find(params[:noteable_id]) + data = { + noteable: noteable, + author: current_user, + is_award: true, + note: params[:emoji] + } + + note = project.notes.find_by(data) + + if note + note.destroy + else + project.notes.create(data) + end + + render json: {ok: true} + end + private def note @@ -111,6 +132,8 @@ class Projects::NotesController < Projects::ApplicationController 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) } diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index beb083d82dc..ff3e0911954 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -87,6 +87,11 @@ module IssuesHelper merge_requests.map(&:to_reference).to_sentence(last_word_connector: ', or ') end + def url_to_emoji(name) + emoji_path = "emoji/#{Emoji.emoji_filename(name)}.png" + url_to_image(emoji_path) + end + # Required for Gitlab::Markdown::IssueReferenceFilter module_function :url_for_issue end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 492a026add9..91da6797df7 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -89,41 +89,6 @@ module Issuable opened? || reopened? end - # - # Votes - # - - # Return the number of -1 comments (downvotes) - def downvotes - filter_superceded_votes(notes.select(&:downvote?), notes).size - end - - def downvotes_in_percent - if votes_count.zero? - 0 - else - 100.0 - upvotes_in_percent - end - end - - # Return the number of +1 comments (upvotes) - def upvotes - filter_superceded_votes(notes.select(&:upvote?), notes).size - end - - def upvotes_in_percent - if votes_count.zero? - 0 - else - 100.0 / votes_count * upvotes - end - end - - # Return the total number of votes - def votes_count - upvotes + downvotes - end - def subscribed?(user) subscription = subscriptions.find_by_user_id(user.id) @@ -183,18 +148,4 @@ module Issuable def notes_with_associations notes.includes(:author, :project) end - - private - - def filter_superceded_votes(votes, notes) - filteredvotes = [] + votes - - votes.each do |vote| - if vote.superceded?(notes) - filteredvotes.delete(vote) - end - end - - filteredvotes - end end diff --git a/app/models/note.rb b/app/models/note.rb index 0b3aa30abd7..458d433211c 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -50,6 +50,8 @@ class Note < ActiveRecord::Base mount_uploader :attachment, AttachmentUploader # Scopes + scope :awards, ->{ where("is_award IS TRUE") } + scope :nonawards, ->{ where("is_award IS 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, '']) } @@ -97,6 +99,12 @@ class Note < ActiveRecord::Base def search(query) where("LOWER(note) like :query", query: "%#{query.downcase}%") end + + def grouped_awards + select(:note).distinct.map do |note| + [ note.note, where(note: note.note) ] + end + end end def cross_reference? @@ -288,44 +296,6 @@ class Note < ActiveRecord::Base nil end - DOWNVOTES = %w(-1 :-1: :thumbsdown: :thumbs_down_sign:) - - # Check if the note is a downvote - def downvote? - votable? && note.start_with?(*DOWNVOTES) - end - - UPVOTES = %w(+1 :+1: :thumbsup: :thumbs_up_sign:) - - # Check if the note is an upvote - def upvote? - votable? && note.start_with?(*UPVOTES) - end - - def superceded?(notes) - return false unless vote? - - notes.each do |note| - next if note == self - - if note.vote? && - self[:author_id] == note[:author_id] && - self[:created_at] <= note[:created_at] - return true - end - end - - false - end - - def vote? - upvote? || downvote? - end - - def votable? - for_issue? || (for_merge_request? && !for_diff_line?) - end - # Mentionable override. def gfm_reference(from_project = nil) noteable.gfm_reference(from_project) diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 2001dc89c33..f448f61cc86 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -5,11 +5,16 @@ module Notes note.author = current_user note.system = false + if contains_emoji_only?(params[:note]) + note.is_award = true + note.note = emoji_name(params[:note]) + end + if note.save notification_service.new_note(note) - # Skip system notes, like status changes and cross-references. - unless note.system + # Skip system notes, like status changes and cross-references and awards + unless note.system || note.is_award event_service.leave_note(note, note.author) note.create_cross_references! execute_hooks(note) @@ -28,5 +33,13 @@ module Notes note.project.execute_hooks(note_data, :note_hooks) note.project.execute_services(note_data, :note_hooks) end + + def contains_emoji_only?(note) + note =~ /^:[-_+[:alnum:]]*:\s?/ + end + + def emoji_name(note) + note.match(/\A:([-_+[:alnum:]]*):\s?/)[1] + end end end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index bbfe755f44a..d6550fbb555 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -102,6 +102,7 @@ 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/views/projects/issues/_issue.html.haml b/app/views/projects/issues/_issue.html.haml index 55ce912829d..d7657ee7e40 100644 --- a/app/views/projects/issues/_issue.html.haml +++ b/app/views/projects/issues/_issue.html.haml @@ -29,8 +29,6 @@ .issue-info = "#{issue.to_reference} opened #{time_ago_with_tooltip(issue.created_at, placement: 'bottom')} by #{link_to_member(@project, issue.author, avatar: false)}".html_safe - - if issue.votes_count > 0 - = render 'votes/votes_inline', votable: issue - if issue.milestone   %span diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml index c5234c0618c..83e8ad11989 100644 --- a/app/views/projects/merge_requests/_merge_request.html.haml +++ b/app/views/projects/merge_requests/_merge_request.html.haml @@ -34,8 +34,6 @@ .merge-request-info = "##{merge_request.iid} opened #{time_ago_with_tooltip(merge_request.created_at, placement: 'bottom')} by #{link_to_member(@project, merge_request.author, avatar: false)}".html_safe - - if merge_request.votes_count > 0 - = render 'votes/votes_inline', votable: merge_request - if merge_request.milestone_id?   %span diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index efa7dd01cc2..dd0abc8c746 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -35,26 +35,6 @@ - if note.updated_by && note.updated_by != note.author by #{link_to_member(note.project, note.updated_by, avatar: false, author_class: nil)} - - if note.superceded?(@notes) - - if note.upvote? - %span.vote.upvote.label.label-gray.strikethrough - = icon('thumbs-up') - \+1 - - if note.downvote? - %span.vote.downvote.label.label-gray.strikethrough - = icon('thumbs-down') - \-1 - - else - - if note.upvote? - %span.vote.upvote.label.label-success - = icon('thumbs-up') - \+1 - - if note.downvote? - %span.vote.downvote.label.label-danger - = icon('thumbs-down') - \-1 - - .note-body{class: note_editable?(note) ? 'js-task-list-container' : ''} .note-text = preserve do diff --git a/app/views/votes/_votes_block.html.haml b/app/views/votes/_votes_block.html.haml index 36ea6742064..3c3ae9dd0b2 100644 --- a/app/views/votes/_votes_block.html.haml +++ b/app/views/votes/_votes_block.html.haml @@ -1,10 +1,33 @@ -.votes.votes-block - .btn-group - - unless votable.upvotes.zero? - .btn.btn-sm.disabled.cgreen - %i.fa.fa-thumbs-up - = votable.upvotes - - unless votable.downvotes.zero? - .btn.btn-sm.disabled.cred - %i.fa.fa-thumbs-down - = votable.downvotes +.awards.votes-block + - votable.notes.awards.grouped_awards.each do | vote | + .award{class: ("active" if vote.last.pluck(:author_id).include?(current_user.id))} + .icon{class: "emoji-#{vote.first}"} + = image_tag url_to_emoji(vote.first), height: "20px", width: "20px" + .counter + = vote.last.count + + %button.dropdown + %a#add-award{"data-toggle" => "dropdown", "data-target" => "#", "href" => "#"} + + %ul.dropdown-menu.awards-menu + - ["100", "blush", "heart", "smile", "rage", "beers", "thumbsup", "disappointed", "ok_hand", "helicopter"].each do |emoji| + %li{class: "emoji-#{emoji}"}= image_tag url_to_emoji(emoji), height: "20px", width: "20px" + + +:coffeescript + post_emoji_url = "#{award_toggle_namespace_project_notes_path(@project.namespace, @project)}" + noteable_type = "Issue" + noteable_id = #{@issue.id} + awards_handler = new AwardsHandler(post_emoji_url, noteable_type, noteable_id) + + $ -> + $(".awards-menu li").click (e)-> + emoji = $(this).attr("class") + awards_handler.addAward(emoji) + + $(".awards").on "click", ".award", (e)-> + emoji = /(emoji-\S*)/.exec($(this).find(".icon").attr("class"))[0] + awards_handler.addAward(emoji) + + + + \ No newline at end of file diff --git a/app/views/votes/_votes_inline.html.haml b/app/views/votes/_votes_inline.html.haml deleted file mode 100644 index 2cb3ae04e1a..00000000000 --- a/app/views/votes/_votes_inline.html.haml +++ /dev/null @@ -1,9 +0,0 @@ -.votes.votes-inline - - unless votable.upvotes.zero? - %span.upvotes.cgreen - + #{votable.upvotes} - - unless votable.downvotes.zero? - \/ - - unless votable.downvotes.zero? - %span.downvotes.cred - \- #{votable.downvotes} diff --git a/config/routes.rb b/config/routes.rb index 0bc2c173453..ac81a2aac76 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -664,6 +664,10 @@ Gitlab::Application.routes.draw do member do delete :delete_attachment end + + collection do + post :award_toggle + end end resources :uploads, only: [:create] do diff --git a/db/migrate/20151106000015_add_is_award_to_notes.rb b/db/migrate/20151106000015_add_is_award_to_notes.rb new file mode 100644 index 00000000000..bffe85df3da --- /dev/null +++ b/db/migrate/20151106000015_add_is_award_to_notes.rb @@ -0,0 +1,5 @@ +class AddIsAwardToNotes < ActiveRecord::Migration + def change + add_column :notes, :is_award, :boolean, default: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 440a33e2006..6c322d33682 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -531,7 +531,7 @@ ActiveRecord::Schema.define(version: 20151116144118) do t.string "type" t.string "description", default: "", null: false t.string "avatar" - t.boolean "public", default: false + t.boolean "visible", default: false end add_index "namespaces", ["created_at", "id"], name: "index_namespaces_on_created_at_and_id", using: :btree @@ -554,6 +554,7 @@ ActiveRecord::Schema.define(version: 20151116144118) do t.boolean "system", default: false, null: false t.text "st_diff" t.integer "updated_by_id" + t.boolean "is_award", default: false end add_index "notes", ["author_id"], name: "index_notes_on_author_id", using: :btree diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index ffa7f2cdf14..2f17d4ae06b 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -31,8 +31,6 @@ Parameters: "project_id": 3, "title": "test1", "state": "opened", - "upvotes": 0, - "downvotes": 0, "author": { "id": 1, "username": "admin", @@ -77,8 +75,6 @@ Parameters: "project_id": 3, "title": "test1", "state": "merged", - "upvotes": 0, - "downvotes": 0, "author": { "id": 1, "username": "admin", @@ -126,8 +122,6 @@ Parameters: "updated_at": "2015-02-02T20:08:49.959Z", "target_branch": "secret_token", "source_branch": "version-1-9", - "upvotes": 0, - "downvotes": 0, "author": { "name": "Chad Hamill", "username": "jarrett", @@ -198,8 +192,6 @@ Parameters: "project_id": 3, "title": "test1", "state": "opened", - "upvotes": 0, - "downvotes": 0, "author": { "id": 1, "username": "admin", @@ -250,8 +242,6 @@ Parameters: "title": "test1", "description": "description1", "state": "opened", - "upvotes": 0, - "downvotes": 0, "author": { "id": 1, "username": "admin", @@ -304,8 +294,6 @@ Parameters: "project_id": 3, "title": "test1", "state": "merged", - "upvotes": 0, - "downvotes": 0, "author": { "id": 1, "username": "admin", diff --git a/doc/api/notes.md b/doc/api/notes.md index c683cb883d4..bcba1f62151 100644 --- a/doc/api/notes.md +++ b/doc/api/notes.md @@ -32,9 +32,7 @@ Parameters: "created_at": "2013-09-30T13:46:01Z" }, "created_at": "2013-10-02T09:22:45Z", - "system": true, - "upvote": false, - "downvote": false + "system": true }, { "id": 305, @@ -49,9 +47,7 @@ Parameters: "created_at": "2013-09-30T13:46:01Z" }, "created_at": "2013-10-02T09:56:03Z", - "system": false, - "upvote": false, - "downvote": false + "system": false } ] ``` diff --git a/lib/api/entities.rb b/lib/api/entities.rb index d6aec03d7f5..3da6bc415d6 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -162,7 +162,7 @@ module API end class MergeRequest < ProjectEntity - expose :target_branch, :source_branch, :upvotes, :downvotes + expose :target_branch, :source_branch expose :author, :assignee, using: Entities::UserBasic expose :source_project_id, :target_project_id expose :label_names, as: :labels @@ -192,8 +192,6 @@ module API expose :author, using: Entities::UserBasic expose :created_at expose :system?, as: :system - expose :upvote?, as: :upvote - expose :downvote?, as: :downvote end class MRNote < Grape::Entity diff --git a/spec/lib/votes_spec.rb b/spec/lib/votes_spec.rb deleted file mode 100644 index 39e5d054e62..00000000000 --- a/spec/lib/votes_spec.rb +++ /dev/null @@ -1,188 +0,0 @@ -require 'spec_helper' - -describe Issue, 'Votes' do - let(:issue) { create(:issue) } - - describe "#upvotes" do - it "with no notes has a 0/0 score" do - expect(issue.upvotes).to eq(0) - end - - it "should recognize non-+1 notes" do - add_note "No +1 here" - expect(issue.notes.size).to eq(1) - expect(issue.notes.first.upvote?).to be_falsey - expect(issue.upvotes).to eq(0) - end - - it "should recognize a single +1 note" do - add_note "+1 This is awesome" - expect(issue.upvotes).to eq(1) - end - - it 'should recognize multiple +1 notes' do - add_note '+1 This is awesome', create(:user) - add_note '+1 I want this', create(:user) - expect(issue.upvotes).to eq(2) - end - - it 'should not count 2 +1 votes from the same user' do - add_note '+1 This is awesome' - add_note '+1 I want this' - expect(issue.upvotes).to eq(1) - end - end - - describe "#downvotes" do - it "with no notes has a 0/0 score" do - expect(issue.downvotes).to eq(0) - end - - it "should recognize non--1 notes" do - add_note "Almost got a -1" - expect(issue.notes.size).to eq(1) - expect(issue.notes.first.downvote?).to be_falsey - expect(issue.downvotes).to eq(0) - end - - it "should recognize a single -1 note" do - add_note "-1 This is bad" - expect(issue.downvotes).to eq(1) - end - - it "should recognize multiple -1 notes" do - add_note('-1 This is bad', create(:user)) - add_note('-1 Away with this', create(:user)) - expect(issue.downvotes).to eq(2) - end - end - - describe "#votes_count" do - it "with no notes has a 0/0 score" do - expect(issue.votes_count).to eq(0) - end - - it "should recognize non notes" do - add_note "No +1 here" - expect(issue.notes.size).to eq(1) - expect(issue.votes_count).to eq(0) - end - - it "should recognize a single +1 note" do - add_note "+1 This is awesome" - expect(issue.votes_count).to eq(1) - end - - it "should recognize a single -1 note" do - add_note "-1 This is bad" - expect(issue.votes_count).to eq(1) - end - - it "should recognize multiple notes" do - add_note('+1 This is awesome', create(:user)) - add_note('-1 This is bad', create(:user)) - add_note('+1 I want this', create(:user)) - expect(issue.votes_count).to eq(3) - end - - it 'should not count 2 -1 votes from the same user' do - add_note '-1 This is suspicious' - add_note '-1 This is bad' - expect(issue.votes_count).to eq(1) - end - end - - describe "#upvotes_in_percent" do - it "with no notes has a 0% score" do - expect(issue.upvotes_in_percent).to eq(0) - end - - it "should count a single 1 note as 100%" do - add_note "+1 This is awesome" - expect(issue.upvotes_in_percent).to eq(100) - end - - it 'should count multiple +1 notes as 100%' do - add_note('+1 This is awesome', create(:user)) - add_note('+1 I want this', create(:user)) - expect(issue.upvotes_in_percent).to eq(100) - end - - it 'should count fractions for multiple +1 and -1 notes correctly' do - add_note('+1 This is awesome', create(:user)) - add_note('+1 I want this', create(:user)) - add_note('-1 This is bad', create(:user)) - add_note('+1 me too', create(:user)) - expect(issue.upvotes_in_percent).to eq(75) - end - end - - describe "#downvotes_in_percent" do - it "with no notes has a 0% score" do - expect(issue.downvotes_in_percent).to eq(0) - end - - it "should count a single -1 note as 100%" do - add_note "-1 This is bad" - expect(issue.downvotes_in_percent).to eq(100) - end - - it 'should count multiple -1 notes as 100%' do - add_note('-1 This is bad', create(:user)) - add_note('-1 Away with this', create(:user)) - expect(issue.downvotes_in_percent).to eq(100) - end - - it 'should count fractions for multiple +1 and -1 notes correctly' do - add_note('+1 This is awesome', create(:user)) - add_note('+1 I want this', create(:user)) - add_note('-1 This is bad', create(:user)) - add_note('+1 me too', create(:user)) - expect(issue.downvotes_in_percent).to eq(25) - end - end - - describe '#filter_superceded_votes' do - - it 'should count a users vote only once amongst multiple votes' do - add_note('-1 This needs work before I will accept it') - add_note('+1 I want this', create(:user)) - add_note('+1 This is is awesome', create(:user)) - add_note('+1 this looks good now') - add_note('+1 This is awesome', create(:user)) - add_note('+1 me too', create(:user)) - expect(issue.downvotes).to eq(0) - expect(issue.upvotes).to eq(5) - end - - it 'should count each users vote only once' do - add_note '-1 This needs work before it will be accepted' - add_note '+1 I like this' - add_note '+1 I still like this' - add_note '+1 I really like this' - add_note '+1 Give me this now!!!!' - expect(issue.downvotes).to eq(0) - expect(issue.upvotes).to eq(1) - end - - it 'should count a users vote only once without caring about comments' do - add_note '-1 This needs work before it will be accepted' - add_note 'Comment 1' - add_note 'Another comment' - add_note '+1 vote' - add_note 'final comment' - expect(issue.downvotes).to eq(0) - expect(issue.upvotes).to eq(1) - end - - end - - def add_note(text, author = issue.author) - created_at = Time.now - 1.hour + Note.count.seconds - issue.notes << create(:note, - note: text, - project: issue.project, - author_id: author.id, - created_at: created_at) - end -end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 8d7e6e76766..3c537220106 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -345,17 +345,6 @@ describe Project do expect(project1.star_count).to eq(0) expect(project2.star_count).to eq(0) end - - it 'is decremented when an upvoter account is deleted' do - user = create :user - project = create :project, :public - user.toggle_star(project) - project.reload - expect(project.star_count).to eq(1) - user.destroy - project.reload - expect(project.star_count).to eq(0) - end end describe :avatar_type do -- cgit v1.2.1 From 06a4fd1035c58d89251fb979dafa8610ba8c5157 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Tue, 17 Nov 2015 13:16:16 +0200 Subject: css improvements --- app/assets/javascripts/awards_handler.coffee | 2 +- app/assets/stylesheets/pages/issuable.scss | 20 +++++++++++++++----- app/finders/notes_finder.rb | 4 ++-- app/views/votes/_votes_block.html.haml | 2 +- 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee index aab3179f10e..1ede7c317c8 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -39,7 +39,7 @@ class @AwardsHandler nodes.push("
1") nodes.push("
") - $(".awards").append(nodes.join("\n")) + $(".awards-controls").before(nodes.join("\n")) getImage: (emoji) -> $("li." + emoji).html() diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index efeb2393165..3f79d0d4967 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -128,12 +128,22 @@ } } - #add-award { - font-size: 20px; - border-radius: 5px; + .awards-controls { + height: 25px; + width: 28px; float: left; - width: 50px; - font-weight: bold; + padding: 0 0 5px 5px; + line-height: 1; + + #add-award { + font-size: 27px; + &:hover { + text-decoration: none; + } + &:link { + text-decoration: none; + } + } } .awards-menu{ diff --git a/app/finders/notes_finder.rb b/app/finders/notes_finder.rb index ab252821b52..fa4c635f55c 100644 --- a/app/finders/notes_finder.rb +++ b/app/finders/notes_finder.rb @@ -12,9 +12,9 @@ class NotesFinder when "commit" project.notes.for_commit_id(target_id).not_inline when "issue" - project.issues.find(target_id).notes.inc_author + project.issues.find(target_id).notes.nonawards.inc_author when "merge_request" - project.merge_requests.find(target_id).mr_and_commit_notes.inc_author + project.merge_requests.find(target_id).mr_and_commit_notes.nonawards.inc_author when "snippet", "project_snippet" project.snippets.find(target_id).notes else diff --git a/app/views/votes/_votes_block.html.haml b/app/views/votes/_votes_block.html.haml index 3c3ae9dd0b2..7f988160ad9 100644 --- a/app/views/votes/_votes_block.html.haml +++ b/app/views/votes/_votes_block.html.haml @@ -6,7 +6,7 @@ .counter = vote.last.count - %button.dropdown + .dropdown.awards-controls %a#add-award{"data-toggle" => "dropdown", "data-target" => "#", "href" => "#"} + %ul.dropdown-menu.awards-menu - ["100", "blush", "heart", "smile", "rage", "beers", "thumbsup", "disappointed", "ok_hand", "helicopter"].each do |emoji| -- cgit v1.2.1 From 36d0442e837cd520dec780590040c83108bc14e6 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Tue, 17 Nov 2015 16:44:58 +0200 Subject: replace emoji references from class name to data [ci skip] --- app/assets/javascripts/awards_handler.coffee | 36 +++++++++++++++------------- app/assets/javascripts/notes.js.coffee | 2 +- app/views/votes/_votes_block.html.haml | 10 ++++---- 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee index 1ede7c317c8..8803c0cca2d 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -3,25 +3,27 @@ class @AwardsHandler addAward: (emoji) -> @postEmoji emoji, => - if @exist(emoji) - if @isActive(emoji) - @decrementCounter(emoji) - else - counter = $(".icon." + emoji).siblings(".counter") - counter.text(parseInt(counter.text()) + 1) - counter.parent().addClass("active") - else - @createEmoji(emoji) + @addAwardToEmojiBar(emoji) + addAwardToEmojiBar: (emoji) -> + if @exist(emoji) + if @isActive(emoji) + @decrementCounter(emoji) + else + counter = @findEmojiIcon(emoji).siblings(".counter") + counter.text(parseInt(counter.text()) + 1) + counter.parent().addClass("active") + else + @createEmoji(emoji) exist: (emoji) -> - $(".icon").hasClass(emoji) + @findEmojiIcon(emoji).length > 0 isActive: (emoji) -> - $(".icon." + emoji).parent().hasClass("active") + @findEmojiIcon(emoji).parent().hasClass("active") decrementCounter: (emoji) -> - counter = $(".icon." + emoji).siblings(".counter") + counter = @findEmojiIcon(emoji).siblings(".counter") if parseInt(counter.text()) > 1 counter.text(parseInt(counter.text()) - 1) @@ -33,7 +35,7 @@ class @AwardsHandler createEmoji: (emoji) -> nodes = [] nodes.push("
") - nodes.push("
") + nodes.push("
") nodes.push(@getImage(emoji)) nodes.push("
") nodes.push("
1") @@ -42,14 +44,16 @@ class @AwardsHandler $(".awards-controls").before(nodes.join("\n")) getImage: (emoji) -> - $("li." + emoji).html() + $("li[data-emoji='" + emoji + "'").html() postEmoji: (emoji, callback) -> - emoji = emoji.replace("emoji-", "") $.post @post_emoji_url, { emoji: emoji noteable_type: @noteable_type noteable_id: @noteable_id },(data) -> if data.ok - callback.call() \ No newline at end of file + callback.call() + + findEmojiIcon: (emoji) -> + $(".icon[data-emoji='" + emoji + "'") \ No newline at end of file diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index cd27b20dd7c..73a95c455e8 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -121,7 +121,7 @@ class @Notes @initTaskList() if note.award - awards_handler.addAward("emoji-" + note.note) + awards_handler.addAwardToEmojiBar(note.note) ### Check if note does not exists on page diff --git a/app/views/votes/_votes_block.html.haml b/app/views/votes/_votes_block.html.haml index 7f988160ad9..118a095181f 100644 --- a/app/views/votes/_votes_block.html.haml +++ b/app/views/votes/_votes_block.html.haml @@ -1,7 +1,7 @@ .awards.votes-block - votable.notes.awards.grouped_awards.each do | vote | .award{class: ("active" if vote.last.pluck(:author_id).include?(current_user.id))} - .icon{class: "emoji-#{vote.first}"} + .icon{"data-emoji" => "#{vote.first}"} = image_tag url_to_emoji(vote.first), height: "20px", width: "20px" .counter = vote.last.count @@ -10,22 +10,22 @@ %a#add-award{"data-toggle" => "dropdown", "data-target" => "#", "href" => "#"} + %ul.dropdown-menu.awards-menu - ["100", "blush", "heart", "smile", "rage", "beers", "thumbsup", "disappointed", "ok_hand", "helicopter"].each do |emoji| - %li{class: "emoji-#{emoji}"}= image_tag url_to_emoji(emoji), height: "20px", width: "20px" + %li{"data-emoji" => "#{emoji}"}= image_tag url_to_emoji(emoji), height: "20px", width: "20px" :coffeescript post_emoji_url = "#{award_toggle_namespace_project_notes_path(@project.namespace, @project)}" noteable_type = "Issue" noteable_id = #{@issue.id} - awards_handler = new AwardsHandler(post_emoji_url, noteable_type, noteable_id) + window.awards_handler = new AwardsHandler(post_emoji_url, noteable_type, noteable_id) $ -> $(".awards-menu li").click (e)-> - emoji = $(this).attr("class") + emoji = $(this).data("emoji") awards_handler.addAward(emoji) $(".awards").on "click", ".award", (e)-> - emoji = /(emoji-\S*)/.exec($(this).find(".icon").attr("class"))[0] + emoji = $(this).find(".icon").data("emoji") awards_handler.addAward(emoji) -- cgit v1.2.1 From f021bc5c6aa79147940ee31e800f519962f4d806 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Wed, 18 Nov 2015 15:43:53 +0200 Subject: add stats on hover --- app/assets/javascripts/awards_handler.coffee | 32 ++++++++++++++++++++++++++-- app/helpers/issues_helper.rb | 8 +++++++ app/views/votes/_votes_block.html.haml | 4 +++- 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee index 8803c0cca2d..29b11b0cc58 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -13,6 +13,7 @@ class @AwardsHandler counter = @findEmojiIcon(emoji).siblings(".counter") counter.text(parseInt(counter.text()) + 1) counter.parent().addClass("active") + @addMeToAuthorList(emoji) else @createEmoji(emoji) @@ -28,13 +29,38 @@ class @AwardsHandler if parseInt(counter.text()) > 1 counter.text(parseInt(counter.text()) - 1) counter.parent().removeClass("active") + @removeMeFromAuthorList(emoji) else - counter.parent().remove() + award = counter.parent() + award.tooltip("destroy") + award.remove() + removeMeFromAuthorList: (emoji) -> + award_block = @findEmojiIcon(emoji).parent() + authors = award_block.attr("data-original-title").split(", ") + authors = _.without(authors, "me").join(", ") + award_block.attr("title", authors) + @resetTooltip(award_block) + + addMeToAuthorList: (emoji) -> + award_block = @findEmojiIcon(emoji).parent() + authors = award_block.attr("data-original-title").split(", ") + authors.push("me") + award_block.attr("title", authors.join(", ")) + @resetTooltip(award_block) + + resetTooltip: (award) -> + award.tooltip("destroy") + + # "destroy" call is asynchronous, this is why we need to set timeout. + setTimeout (-> + award.tooltip() + ), 200 + createEmoji: (emoji) -> nodes = [] - nodes.push("
") + nodes.push("
") nodes.push("
") nodes.push(@getImage(emoji)) nodes.push("
") @@ -43,6 +69,8 @@ class @AwardsHandler $(".awards-controls").before(nodes.join("\n")) + $(".award").tooltip() + getImage: (emoji) -> $("li[data-emoji='" + emoji + "'").html() diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index ff3e0911954..3aa16b66944 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -92,6 +92,14 @@ module IssuesHelper url_to_image(emoji_path) end + def emoji_author_list(notes, current_user) + list = notes.map do |note| + note.author == current_user ? "me" : note.author.username + end + + list.join(", ") + end + # Required for Gitlab::Markdown::IssueReferenceFilter module_function :url_for_issue end diff --git a/app/views/votes/_votes_block.html.haml b/app/views/votes/_votes_block.html.haml index 118a095181f..a2298f1813a 100644 --- a/app/views/votes/_votes_block.html.haml +++ b/app/views/votes/_votes_block.html.haml @@ -1,6 +1,6 @@ .awards.votes-block - votable.notes.awards.grouped_awards.each do | vote | - .award{class: ("active" if vote.last.pluck(:author_id).include?(current_user.id))} + .award{class: ("active" if vote.last.pluck(:author_id).include?(current_user.id)), title: emoji_author_list(vote.last, current_user)} .icon{"data-emoji" => "#{vote.first}"} = image_tag url_to_emoji(vote.first), height: "20px", width: "20px" .counter @@ -28,6 +28,8 @@ emoji = $(this).find(".icon").data("emoji") awards_handler.addAward(emoji) + $(".award").tooltip() + \ No newline at end of file -- cgit v1.2.1 From 2f6f99d300675b0794b2e96be564db9d405fac36 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Wed, 18 Nov 2015 16:48:37 +0200 Subject: award for merge requests[ci skip] --- app/assets/stylesheets/pages/issuable.scss | 4 ++-- app/controllers/projects/merge_requests_controller.rb | 2 +- app/views/votes/_votes_block.html.haml | 10 +++------- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index 3f79d0d4967..5b73e20df7d 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -114,8 +114,8 @@ cursor: pointer; &.active { - border-color: rgba(79,176,252,.4); - background-color: rgba(79,176,252,.08); + border-color: rgba(79,176,252,0.4); + background-color: rgba(79,176,252,0.1); .counter { font-weight: bold; diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index a0468c65d5a..6378a1f56b0 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -254,7 +254,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController # Build a note object for comment form @note = @project.notes.new(noteable: @merge_request) - @notes = @merge_request.nonawards.mr_and_commit_notes.inc_author.fresh + @notes = @merge_request.mr_and_commit_notes.nonawards.inc_author.fresh @discussions = Note.discussions_from_notes(@notes) @noteable = @merge_request diff --git a/app/views/votes/_votes_block.html.haml b/app/views/votes/_votes_block.html.haml index a2298f1813a..2ae832c31f7 100644 --- a/app/views/votes/_votes_block.html.haml +++ b/app/views/votes/_votes_block.html.haml @@ -15,8 +15,8 @@ :coffeescript post_emoji_url = "#{award_toggle_namespace_project_notes_path(@project.namespace, @project)}" - noteable_type = "Issue" - noteable_id = #{@issue.id} + noteable_type = #{votable} + noteable_id = #{votable.id} window.awards_handler = new AwardsHandler(post_emoji_url, noteable_type, noteable_id) $ -> @@ -28,8 +28,4 @@ emoji = $(this).find(".icon").data("emoji") awards_handler.addAward(emoji) - $(".award").tooltip() - - - - \ No newline at end of file + $(".award").tooltip() \ No newline at end of file -- cgit v1.2.1 From 2d1fcd802a291cc8e26fbbe5874e20316a5f93af Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Wed, 18 Nov 2015 17:38:15 +0200 Subject: Emoji: refactoring --- app/helpers/issues_helper.rb | 4 ++++ app/views/votes/_votes_block.html.haml | 2 +- lib/award_emoji.rb | 6 ++++++ 3 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 lib/award_emoji.rb diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 3aa16b66944..bca32322096 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -100,6 +100,10 @@ module IssuesHelper list.join(", ") end + def emoji_list + ::AwardEmoji::EMOJI_LIST + end + # Required for Gitlab::Markdown::IssueReferenceFilter module_function :url_for_issue end diff --git a/app/views/votes/_votes_block.html.haml b/app/views/votes/_votes_block.html.haml index 2ae832c31f7..02947f2979e 100644 --- a/app/views/votes/_votes_block.html.haml +++ b/app/views/votes/_votes_block.html.haml @@ -9,7 +9,7 @@ .dropdown.awards-controls %a#add-award{"data-toggle" => "dropdown", "data-target" => "#", "href" => "#"} + %ul.dropdown-menu.awards-menu - - ["100", "blush", "heart", "smile", "rage", "beers", "thumbsup", "disappointed", "ok_hand", "helicopter"].each do |emoji| + - emoji_list.each do |emoji| %li{"data-emoji" => "#{emoji}"}= image_tag url_to_emoji(emoji), height: "20px", width: "20px" diff --git a/lib/award_emoji.rb b/lib/award_emoji.rb new file mode 100644 index 00000000000..95b9c8f921a --- /dev/null +++ b/lib/award_emoji.rb @@ -0,0 +1,6 @@ +class AwardEmoji + EMOJI_LIST = ["+1", "-1", "100", "blush", "heart", "smile", "rage", + "beers", "disappointed", "ok_hand", + "helicopter", "shit", "airplane", "alarm_clock", + "ambulance", "anguished", "two_hearts", "wink"] +end \ No newline at end of file -- cgit v1.2.1 From d8676f18fcbd6b78af472b8b00c29f20820a74a9 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Wed, 18 Nov 2015 21:44:48 +0200 Subject: fix --- app/views/votes/_votes_block.html.haml | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/app/views/votes/_votes_block.html.haml b/app/views/votes/_votes_block.html.haml index 02947f2979e..afd1c2745a1 100644 --- a/app/views/votes/_votes_block.html.haml +++ b/app/views/votes/_votes_block.html.haml @@ -15,17 +15,16 @@ :coffeescript post_emoji_url = "#{award_toggle_namespace_project_notes_path(@project.namespace, @project)}" - noteable_type = #{votable} - noteable_id = #{votable.id} + noteable_type = "#{votable.class}" + noteable_id = "#{votable.id}" window.awards_handler = new AwardsHandler(post_emoji_url, noteable_type, noteable_id) - $ -> - $(".awards-menu li").click (e)-> - emoji = $(this).data("emoji") - awards_handler.addAward(emoji) + $(".awards-menu li").click (e)-> + emoji = $(this).data("emoji") + awards_handler.addAward(emoji) - $(".awards").on "click", ".award", (e)-> - emoji = $(this).find(".icon").data("emoji") - awards_handler.addAward(emoji) + $(".awards").on "click", ".award", (e)-> + emoji = $(this).find(".icon").data("emoji") + awards_handler.addAward(emoji) - $(".award").tooltip() \ No newline at end of file + $(".award").tooltip() \ No newline at end of file -- cgit v1.2.1 From 23df515fd09661a690c0c0a651e131bc3a6d0191 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Wed, 18 Nov 2015 23:59:58 +0200 Subject: Emoji: fix image of emoji when it is submitted via comment --- app/assets/javascripts/awards_handler.coffee | 16 ++++++++++------ app/assets/javascripts/notes.js.coffee | 2 +- app/controllers/projects/notes_controller.rb | 1 + app/helpers/issues_helper.rb | 2 +- app/views/votes/_votes_block.html.haml | 1 - lib/award_emoji.rb | 4 ++++ 6 files changed, 17 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee index 29b11b0cc58..cac9c10332a 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -5,7 +5,7 @@ class @AwardsHandler @postEmoji emoji, => @addAwardToEmojiBar(emoji) - addAwardToEmojiBar: (emoji) -> + addAwardToEmojiBar: (emoji, custom_path = '') -> if @exist(emoji) if @isActive(emoji) @decrementCounter(emoji) @@ -15,7 +15,7 @@ class @AwardsHandler counter.parent().addClass("active") @addMeToAuthorList(emoji) else - @createEmoji(emoji) + @createEmoji(emoji, custom_path) exist: (emoji) -> @findEmojiIcon(emoji).length > 0 @@ -58,11 +58,11 @@ class @AwardsHandler ), 200 - createEmoji: (emoji) -> + createEmoji: (emoji, custom_path) -> nodes = [] nodes.push("
") nodes.push("
") - nodes.push(@getImage(emoji)) + nodes.push(@getImage(emoji, custom_path)) nodes.push("
") nodes.push("
1") nodes.push("
") @@ -71,8 +71,12 @@ class @AwardsHandler $(".award").tooltip() - getImage: (emoji) -> - $("li[data-emoji='" + emoji + "'").html() + getImage: (emoji, custom_path) -> + if custom_path + $(".awards-menu li").first().html().replace(/emoji\/.*\.png/, custom_path) + else + $("li[data-emoji='" + emoji + "'").html() + postEmoji: (emoji, callback) -> $.post @post_emoji_url, { diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index 73a95c455e8..7de7632201d 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -121,7 +121,7 @@ class @Notes @initTaskList() if note.award - awards_handler.addAwardToEmojiBar(note.note) + awards_handler.addAwardToEmojiBar(note.note, note.emoji_path) ### Check if note does not exists on page diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index 357b292980d..98bf056a605 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -133,6 +133,7 @@ class Projects::NotesController < Projects::ApplicationController discussion_id: note.discussion_id, html: note_to_html(note), award: note.is_award, + emoji_path: note.is_award ? ::AwardEmoji.path_to_emoji_image(note.note) : "", note: note.note, discussion_html: note_to_discussion_html(note), discussion_with_diff_html: note_to_discussion_with_diff_html(note) diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index bca32322096..bf289c6db14 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -88,7 +88,7 @@ module IssuesHelper end def url_to_emoji(name) - emoji_path = "emoji/#{Emoji.emoji_filename(name)}.png" + emoji_path = ::AwardEmoji.path_to_emoji_image(name) url_to_image(emoji_path) end diff --git a/app/views/votes/_votes_block.html.haml b/app/views/votes/_votes_block.html.haml index afd1c2745a1..5392915b4dd 100644 --- a/app/views/votes/_votes_block.html.haml +++ b/app/views/votes/_votes_block.html.haml @@ -12,7 +12,6 @@ - emoji_list.each do |emoji| %li{"data-emoji" => "#{emoji}"}= image_tag url_to_emoji(emoji), height: "20px", width: "20px" - :coffeescript post_emoji_url = "#{award_toggle_namespace_project_notes_path(@project.namespace, @project)}" noteable_type = "#{votable.class}" diff --git a/lib/award_emoji.rb b/lib/award_emoji.rb index 95b9c8f921a..9e296f0bc3c 100644 --- a/lib/award_emoji.rb +++ b/lib/award_emoji.rb @@ -3,4 +3,8 @@ class AwardEmoji "beers", "disappointed", "ok_hand", "helicopter", "shit", "airplane", "alarm_clock", "ambulance", "anguished", "two_hearts", "wink"] + + def self.path_to_emoji_image(name) + "emoji/#{Emoji.emoji_filename(name)}.png" + end end \ No newline at end of file -- cgit v1.2.1 From db91ef3a2a46511da23dff780791a70594e57312 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Thu, 19 Nov 2015 00:03:45 +0200 Subject: better regexp --- app/services/notes/create_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index f448f61cc86..dbff58dfb9c 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -35,7 +35,7 @@ module Notes end def contains_emoji_only?(note) - note =~ /^:[-_+[:alnum:]]*:\s?/ + note =~ /\A:[-_+[:alnum:]]*:\s?\z/ end def emoji_name(note) -- cgit v1.2.1 From 92943580cb1647930dbfdd8d2957213326c134d9 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Thu, 19 Nov 2015 00:10:58 +0200 Subject: improve style --- app/assets/stylesheets/pages/issuable.scss | 4 +++- app/views/votes/_votes_block.html.haml | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index 5b73e20df7d..7dd4f239c78 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -135,8 +135,10 @@ padding: 0 0 5px 5px; line-height: 1; - #add-award { + .add-award { font-size: 27px; + color: #ccc; + &:hover { text-decoration: none; } diff --git a/app/views/votes/_votes_block.html.haml b/app/views/votes/_votes_block.html.haml index 5392915b4dd..fff74745919 100644 --- a/app/views/votes/_votes_block.html.haml +++ b/app/views/votes/_votes_block.html.haml @@ -7,7 +7,8 @@ = vote.last.count .dropdown.awards-controls - %a#add-award{"data-toggle" => "dropdown", "data-target" => "#", "href" => "#"} + + %a.add-award{"data-toggle" => "dropdown", "data-target" => "#", "href" => "#"} + = icon('plus-circle') %ul.dropdown-menu.awards-menu - emoji_list.each do |emoji| %li{"data-emoji" => "#{emoji}"}= image_tag url_to_emoji(emoji), height: "20px", width: "20px" -- cgit v1.2.1 From fdd5a8f2e16cc210f24d93334877f1ca7ce92ce9 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Thu, 19 Nov 2015 00:59:07 +0200 Subject: addressing comments --- app/helpers/issues_helper.rb | 4 ++++ app/models/note.rb | 4 ++-- app/views/votes/_votes_block.html.haml | 10 +++++----- db/migrate/20151106000015_add_is_award_to_notes.rb | 3 ++- db/schema.rb | 3 ++- 5 files changed, 15 insertions(+), 9 deletions(-) diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index bf289c6db14..3a238824c0d 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -104,6 +104,10 @@ module IssuesHelper ::AwardEmoji::EMOJI_LIST end + def note_active_class(notes, current_user) + notes.pluck(:author_id).include?(current_user.id) ? "active" : "" + end + # Required for Gitlab::Markdown::IssueReferenceFilter module_function :url_for_issue end diff --git a/app/models/note.rb b/app/models/note.rb index 458d433211c..d53f568a671 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -50,8 +50,8 @@ class Note < ActiveRecord::Base mount_uploader :attachment, AttachmentUploader # Scopes - scope :awards, ->{ where("is_award IS TRUE") } - scope :nonawards, ->{ where("is_award IS FALSE") } + 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, '']) } diff --git a/app/views/votes/_votes_block.html.haml b/app/views/votes/_votes_block.html.haml index fff74745919..3eadf209a59 100644 --- a/app/views/votes/_votes_block.html.haml +++ b/app/views/votes/_votes_block.html.haml @@ -1,10 +1,10 @@ .awards.votes-block - - votable.notes.awards.grouped_awards.each do | vote | - .award{class: ("active" if vote.last.pluck(:author_id).include?(current_user.id)), title: emoji_author_list(vote.last, current_user)} - .icon{"data-emoji" => "#{vote.first}"} - = image_tag url_to_emoji(vote.first), height: "20px", width: "20px" + - votable.notes.awards.grouped_awards.each do | note | + .award{class: (note_active_class(note.last, current_user)), title: emoji_author_list(note.last, current_user)} + .icon{"data-emoji" => "#{note.first}"} + = image_tag url_to_emoji(note.first), height: "20px", width: "20px" .counter - = vote.last.count + = note.last.count .dropdown.awards-controls %a.add-award{"data-toggle" => "dropdown", "data-target" => "#", "href" => "#"} diff --git a/db/migrate/20151106000015_add_is_award_to_notes.rb b/db/migrate/20151106000015_add_is_award_to_notes.rb index bffe85df3da..02b271637e9 100644 --- a/db/migrate/20151106000015_add_is_award_to_notes.rb +++ b/db/migrate/20151106000015_add_is_award_to_notes.rb @@ -1,5 +1,6 @@ class AddIsAwardToNotes < ActiveRecord::Migration def change - add_column :notes, :is_award, :boolean, default: false + add_column :notes, :is_award, :boolean, default: false, null: false + add_index :notes, :is_award end end diff --git a/db/schema.rb b/db/schema.rb index 6c322d33682..f5511ac1898 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -554,13 +554,14 @@ ActiveRecord::Schema.define(version: 20151116144118) do t.boolean "system", default: false, null: false t.text "st_diff" t.integer "updated_by_id" - t.boolean "is_award", default: false + 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 -- cgit v1.2.1 From 73bc9edc4410d228db0258c7f0c6fa84dd1d2c49 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Thu, 19 Nov 2015 01:00:56 +0200 Subject: add changelog --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 94f07a31689..6bc0d07b1cf 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -50,6 +50,7 @@ v 8.2.0 - Fix bug when milestone/label filter was empty for dashboard issues page - Add ability to create milestone in group projects from single form - Prevent the last owner of a group from being able to delete themselves by 'adding' themselves as a master (James Lopez) + - Add Award Emoji to issue and merge request pages v 8.1.4 - Fix bug where manually merged branches in a MR would end up with an empty diff (Stan Hu) -- cgit v1.2.1 From fa2ed94fe9aa9f4d43c2aec7d99103982976063f Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Thu, 19 Nov 2015 01:19:32 +0200 Subject: fix schema --- db/schema.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/schema.rb b/db/schema.rb index f5511ac1898..f77f6dfc66d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -531,7 +531,7 @@ ActiveRecord::Schema.define(version: 20151116144118) do t.string "type" t.string "description", default: "", null: false t.string "avatar" - t.boolean "visible", default: false + t.boolean "public", default: false end add_index "namespaces", ["created_at", "id"], name: "index_namespaces_on_created_at_and_id", using: :btree -- cgit v1.2.1 From a2912074be67deb6345a37787c14b7e640be26f8 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Thu, 19 Nov 2015 01:31:15 +0200 Subject: satisfy rubocop --- app/controllers/projects/notes_controller.rb | 4 ++-- db/schema.rb | 4 +--- lib/award_emoji.rb | 8 +++++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index 98bf056a605..8159cc50838 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, except: [:destroy, :delete_attachment, :award_toggle] def index current_fetched_at = Time.now.to_i @@ -76,7 +76,7 @@ class Projects::NotesController < Projects::ApplicationController project.notes.create(data) end - render json: {ok: true} + render json: { ok: true } end private diff --git a/db/schema.rb b/db/schema.rb index f77f6dfc66d..3062bf4d419 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -642,9 +642,7 @@ ActiveRecord::Schema.define(version: 20151116144118) do t.integer "star_count", default: 0, null: false t.string "import_type" t.string "import_source" - t.integer "commit_count", default: 0 - t.boolean "merge_requests_ff_only_enabled", default: false - t.text "issues_template" + t.integer "commit_count", default: 0 t.text "import_error" end diff --git a/lib/award_emoji.rb b/lib/award_emoji.rb index 9e296f0bc3c..d58a196c4ef 100644 --- a/lib/award_emoji.rb +++ b/lib/award_emoji.rb @@ -1,10 +1,12 @@ class AwardEmoji - EMOJI_LIST = ["+1", "-1", "100", "blush", "heart", "smile", "rage", + EMOJI_LIST = [ + "+1", "-1", "100", "blush", "heart", "smile", "rage", "beers", "disappointed", "ok_hand", "helicopter", "shit", "airplane", "alarm_clock", - "ambulance", "anguished", "two_hearts", "wink"] + "ambulance", "anguished", "two_hearts", "wink" + ] def self.path_to_emoji_image(name) "emoji/#{Emoji.emoji_filename(name)}.png" end -end \ No newline at end of file +end -- cgit v1.2.1 From 618b54910ef3183cd1a3bf71ffe6301e029973fb Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 19 Nov 2015 10:50:38 +0100 Subject: Improve UI for emoji awards Signed-off-by: Dmitriy Zaporozhets --- app/assets/stylesheets/pages/issuable.scss | 51 ++++++++++++++++--------- app/views/projects/issues/_discussion.html.haml | 2 +- app/views/votes/_votes_block.html.haml | 4 +- 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index 7dd4f239c78..affa34a5f83 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -103,19 +103,23 @@ } .awards { + @include clearfix; + line-height: 32px; + margin: 5px 0; + .award { + @include border-radius(5px); + border: 1px solid; - padding: 1px 3px; - width: 50px; - border-radius: 5px; - float:left; - margin: 0 3px; - border-color: #ccc; + padding: 0px 10px; + float: left; + margin: 0 5px; + border-color: $border-color; cursor: pointer; &.active { - border-color: rgba(79,176,252,0.4); - background-color: rgba(79,176,252,0.1); + border-color: $border-gray-light; + background-color: $gray-light; .counter { font-weight: bold; @@ -126,28 +130,39 @@ float: left; margin-right: 10px; } + + .counter { + float: left; + } } .awards-controls { - height: 25px; - width: 28px; + line-height: 32px; + margin-left: 10px; float: left; - padding: 0 0 5px 5px; - line-height: 1; .add-award { - font-size: 27px; - color: #ccc; + font-size: 24px; + color: $gl-gray; + position: relative; + top: 2px; - &:hover { - text-decoration: none; - } + &:hover, &:link { text-decoration: none; } } + + .awards-menu { + padding: $gl-padding; + min-width: 214px; + + > li { + margin: 5px; + } + } } - + .awards-menu{ li { float: left; diff --git a/app/views/projects/issues/_discussion.html.haml b/app/views/projects/issues/_discussion.html.haml index c5fd863ae99..020952dd001 100644 --- a/app/views/projects/issues/_discussion.html.haml +++ b/app/views/projects/issues/_discussion.html.haml @@ -7,7 +7,7 @@ = render 'shared/show_aside' -.gray-content-block.second-block +.gray-content-block.second-block.oneline-block .row .col-md-9 .votes-holder.pull-right diff --git a/app/views/votes/_votes_block.html.haml b/app/views/votes/_votes_block.html.haml index 3eadf209a59..f32e5193d1a 100644 --- a/app/views/votes/_votes_block.html.haml +++ b/app/views/votes/_votes_block.html.haml @@ -8,7 +8,7 @@ .dropdown.awards-controls %a.add-award{"data-toggle" => "dropdown", "data-target" => "#", "href" => "#"} - = icon('plus-circle') + = icon('plus-square-o') %ul.dropdown-menu.awards-menu - emoji_list.each do |emoji| %li{"data-emoji" => "#{emoji}"}= image_tag url_to_emoji(emoji), height: "20px", width: "20px" @@ -27,4 +27,4 @@ emoji = $(this).find(".icon").data("emoji") awards_handler.addAward(emoji) - $(".award").tooltip() \ No newline at end of file + $(".award").tooltip() -- cgit v1.2.1 From 2c7d8678623a9e207d54e2e39d7eef9e2f77cb47 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 19 Nov 2015 10:59:58 +0100 Subject: Few minor improvements to emoji awards UI Signed-off-by: Dmitriy Zaporozhets --- app/assets/stylesheets/pages/issuable.scss | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index affa34a5f83..3a08ee70bc7 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -104,8 +104,8 @@ .awards { @include clearfix; - line-height: 32px; - margin: 5px 0; + line-height: 34px; + margin: 2px 0; .award { @include border-radius(5px); @@ -137,7 +137,6 @@ } .awards-controls { - line-height: 32px; margin-left: 10px; float: left; -- cgit v1.2.1 From 23c5473cc0bd9b9034c5671fbea1356b0fb531e5 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Thu, 19 Nov 2015 13:20:09 +0200 Subject: added spinach tests --- app/assets/javascripts/awards_handler.coffee | 4 +- app/helpers/issues_helper.rb | 6 ++- app/models/note.rb | 1 + app/views/votes/_votes_block.html.haml | 38 +++++++------- features/project/issues/award_emoji.feature | 14 ++++++ features/steps/project/issues/award_emoji.rb | 41 +++++++++++++++ spec/models/note_spec.rb | 75 ---------------------------- 7 files changed, 83 insertions(+), 96 deletions(-) create mode 100644 features/project/issues/award_emoji.feature create mode 100644 features/steps/project/issues/award_emoji.rb diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee index cac9c10332a..f5b9adbe9e2 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -75,7 +75,7 @@ class @AwardsHandler if custom_path $(".awards-menu li").first().html().replace(/emoji\/.*\.png/, custom_path) else - $("li[data-emoji='" + emoji + "'").html() + $("li[data-emoji='" + emoji + "']").html() postEmoji: (emoji, callback) -> @@ -88,4 +88,4 @@ class @AwardsHandler callback.call() findEmojiIcon: (emoji) -> - $(".icon[data-emoji='" + emoji + "'") \ No newline at end of file + $(".icon[data-emoji='" + emoji + "']") \ No newline at end of file diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 3a238824c0d..2c791aa5682 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -105,7 +105,11 @@ module IssuesHelper end def note_active_class(notes, current_user) - notes.pluck(:author_id).include?(current_user.id) ? "active" : "" + if current_user && notes.pluck(:author_id).include?(current_user.id) + "active" + else + "" + end end # Required for Gitlab::Markdown::IssueReferenceFilter diff --git a/app/models/note.rb b/app/models/note.rb index d53f568a671..39645f8f1ad 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -46,6 +46,7 @@ class Note < ActiveRecord::Base validates :noteable_id, presence: true, if: ->(n) { n.noteable_type.present? && n.noteable_type != 'Commit' } validates :commit_id, presence: true, if: ->(n) { n.noteable_type == 'Commit' } + validates :author, presence: true, if: ->(n) { n.is_award } mount_uploader :attachment, AttachmentUploader diff --git a/app/views/votes/_votes_block.html.haml b/app/views/votes/_votes_block.html.haml index f32e5193d1a..04e3b5a3814 100644 --- a/app/views/votes/_votes_block.html.haml +++ b/app/views/votes/_votes_block.html.haml @@ -6,25 +6,27 @@ .counter = note.last.count - .dropdown.awards-controls - %a.add-award{"data-toggle" => "dropdown", "data-target" => "#", "href" => "#"} - = icon('plus-square-o') - %ul.dropdown-menu.awards-menu - - emoji_list.each do |emoji| - %li{"data-emoji" => "#{emoji}"}= image_tag url_to_emoji(emoji), height: "20px", width: "20px" + - if current_user + .dropdown.awards-controls + %a.add-award{"data-toggle" => "dropdown", "data-target" => "#", "href" => "#"} + = icon('plus-square-o') + %ul.dropdown-menu.awards-menu + - emoji_list.each do |emoji| + %li{"data-emoji" => "#{emoji}"}= image_tag url_to_emoji(emoji), height: "20px", width: "20px" -:coffeescript - post_emoji_url = "#{award_toggle_namespace_project_notes_path(@project.namespace, @project)}" - noteable_type = "#{votable.class}" - noteable_id = "#{votable.id}" - window.awards_handler = new AwardsHandler(post_emoji_url, noteable_type, noteable_id) +- if current_user + :coffeescript + post_emoji_url = "#{award_toggle_namespace_project_notes_path(@project.namespace, @project)}" + noteable_type = "#{votable.class}" + noteable_id = "#{votable.id}" + window.awards_handler = new AwardsHandler(post_emoji_url, noteable_type, noteable_id) - $(".awards-menu li").click (e)-> - emoji = $(this).data("emoji") - awards_handler.addAward(emoji) + $(".awards-menu li").click (e)-> + emoji = $(this).data("emoji") + awards_handler.addAward(emoji) - $(".awards").on "click", ".award", (e)-> - emoji = $(this).find(".icon").data("emoji") - awards_handler.addAward(emoji) + $(".awards").on "click", ".award", (e)-> + emoji = $(this).find(".icon").data("emoji") + awards_handler.addAward(emoji) - $(".award").tooltip() + $(".award").tooltip() diff --git a/features/project/issues/award_emoji.feature b/features/project/issues/award_emoji.feature new file mode 100644 index 00000000000..a9bc8ffb9bb --- /dev/null +++ b/features/project/issues/award_emoji.feature @@ -0,0 +1,14 @@ +Feature: Award Emoji + Background: + Given I sign in as a user + And I own project "Shop" + And project "Shop" has issue "Bugfix" + And I visit "Bugfix" issue page + + @javascript + Scenario: I add and remove award in the issue + Given I click to emoji-picker + And I click to emoji in the picker + Then I have award added + And I can remove it by clicking to icon + \ No newline at end of file diff --git a/features/steps/project/issues/award_emoji.rb b/features/steps/project/issues/award_emoji.rb new file mode 100644 index 00000000000..8f7a45dec0e --- /dev/null +++ b/features/steps/project/issues/award_emoji.rb @@ -0,0 +1,41 @@ +class Spinach::Features::AwardEmoji < Spinach::FeatureSteps + include SharedAuthentication + include SharedProject + include SharedPaths + include Select2Helper + + step 'I visit "Bugfix" issue page' do + visit namespace_project_issue_path(@project.namespace, @project, @issue) + end + + step 'I click to emoji-picker' do + page.within ".awards-controls" do + page.find(".add-award").click + end + end + + step 'I click to emoji in the picker' do + page.within ".awards-menu" do + page.first("img").click + end + end + + step 'I can remove it by clicking to icon' do + page.within ".awards" do + page.first(".award").click + expect(page).to_not have_selector ".award" + end + end + + step 'I have award added' do + page.within ".awards" do + expect(page).to have_selector ".award" + expect(page.find(".award .counter")).to have_content "1" + end + end + + step 'project "Shop" has issue "Bugfix"' do + @project = Project.find_by(name: "Shop") + @issue = create(:issue, title: "Bugfix", project: project) + end +end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 75564839dcf..6e37dae07d0 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -32,77 +32,6 @@ describe Note do it { is_expected.to validate_presence_of(:project) } end - describe '#votable?' do - it 'is true for issue notes' do - note = build(:note_on_issue) - expect(note).to be_votable - end - - it 'is true for merge request notes' do - note = build(:note_on_merge_request) - expect(note).to be_votable - end - - it 'is false for merge request diff notes' do - note = build(:note_on_merge_request_diff) - expect(note).not_to be_votable - end - - it 'is false for commit notes' do - note = build(:note_on_commit) - expect(note).not_to be_votable - end - - it 'is false for commit diff notes' do - note = build(:note_on_commit_diff) - expect(note).not_to be_votable - end - end - - describe 'voting score' do - it 'recognizes a neutral note' do - note = build(:votable_note, note: 'This is not a +1 note') - expect(note).not_to be_upvote - expect(note).not_to be_downvote - end - - it 'recognizes a neutral emoji note' do - note = build(:votable_note, note: "I would :+1: this, but I don't want to") - expect(note).not_to be_upvote - expect(note).not_to be_downvote - end - - it 'recognizes a +1 note' do - note = build(:votable_note, note: '+1 for this') - expect(note).to be_upvote - end - - it 'recognizes a +1 emoji as a vote' do - note = build(:votable_note, note: ':+1: for this') - expect(note).to be_upvote - end - - it 'recognizes a thumbsup emoji as a vote' do - note = build(:votable_note, note: ':thumbsup: for this') - expect(note).to be_upvote - end - - it 'recognizes a -1 note' do - note = build(:votable_note, note: '-1 for this') - expect(note).to be_downvote - end - - it 'recognizes a -1 emoji as a vote' do - note = build(:votable_note, note: ':-1: for this') - expect(note).to be_downvote - end - - it 'recognizes a thumbsdown emoji as a vote' do - note = build(:votable_note, note: ':thumbsdown: for this') - expect(note).to be_downvote - end - end - describe "Commit notes" do let!(:note) { create(:note_on_commit, note: "+1 from me") } let!(:commit) { note.noteable } @@ -139,10 +68,6 @@ describe Note do it "should be recognized by #for_commit_diff_line?" do expect(note).to be_for_commit_diff_line end - - it "should not be votable" do - expect(note).not_to be_votable - end end describe 'authorization' do -- cgit v1.2.1 From 671a49cfd53230b57acf579a609bab958e066982 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Thu, 19 Nov 2015 14:41:05 +0200 Subject: added specs --- spec/helpers/issues_helper_spec.rb | 26 +++++++++++++++++++++++ spec/models/note_spec.rb | 13 ++++++++++++ spec/services/notes/create_service_spec.rb | 34 ++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+) diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb index 78a6b631eb2..1f2c4ee77b5 100644 --- a/spec/helpers/issues_helper_spec.rb +++ b/spec/helpers/issues_helper_spec.rb @@ -127,4 +127,30 @@ describe IssuesHelper do it { is_expected.to eq("!1, !2, or !3") } end + describe "#url_to_emoji" do + it "returns url" do + expect(url_to_emoji("smile")).to include("emoji/1F604.png") + end + end + + describe "#emoji_list" do + it "returns url" do + expect(emoji_list).to be_kind_of(Array) + end + end + + describe "#note_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("") + end + + it "returns active string for author" do + expect(note_active_class(Note.all, @note.author)).to eq("active") + end + end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 6e37dae07d0..86b6c27a182 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -129,4 +129,17 @@ describe Note do it { expect(Note.search('wow')).to include(note) } end + + describe :grouped_awards do + before do + create :note, note: "smile" + create :note, note: "smile" + end + + it "returns grouped array of notes" do + grouped_array = Note.grouped_awards + expect(Note.grouped_awards.first.first).to eq("smile") + expect(Note.grouped_awards.first.last).to match_array(Note.all) + end + end end diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index f2ea0805b2f..066461f5a12 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -24,4 +24,38 @@ describe Notes::CreateService do it { expect(@note.note).to eq('Awesome comment') } end end + + describe "award emoji" do + before do + project.team << [user, :master] + end + + it "creates emoji note" do + opts = { + note: ':smile: ', + noteable_type: 'Issue', + 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 + end + + it "creates regular note if emoji name is invalid" do + opts = { + note: ':smile: moretext: ', + noteable_type: 'Issue', + noteable_id: issue.id + } + + @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 + end + end end -- cgit v1.2.1 From bdf4007cb7b18ed6892455d0a9adf78476188563 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Thu, 19 Nov 2015 18:12:17 +0200 Subject: adressing comments --- app/assets/javascripts/awards_handler.coffee | 6 +++--- app/controllers/projects/notes_controller.rb | 12 ++++++------ app/models/note.rb | 5 +++-- app/services/notes/create_service.rb | 4 ++-- app/views/votes/_votes_block.html.haml | 14 +++++++------- 5 files changed, 21 insertions(+), 20 deletions(-) diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee index f5b9adbe9e2..ae42e390c43 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -79,11 +79,11 @@ class @AwardsHandler postEmoji: (emoji, callback) -> - $.post @post_emoji_url, { - emoji: emoji + $.post @post_emoji_url, { note: { + note: emoji noteable_type: @noteable_type noteable_id: @noteable_id - },(data) -> + }},(data) -> if data.ok callback.call() diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index 8159cc50838..263b8b8d94e 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -59,21 +59,21 @@ class Projects::NotesController < Projects::ApplicationController end def award_toggle - noteable = params[:noteable_type] == "Issue" ? Issue : MergeRequest - noteable = noteable.find(params[:noteable_id]) + noteable = note_params[:noteable_type] == "issue" ? Issue : MergeRequest + noteable = noteable.find_by!(id: note_params[:noteable_id], project: project) + data = { - noteable: noteable, author: current_user, is_award: true, - note: params[:emoji] + note: note_params[:note] } - note = project.notes.find_by(data) + note = noteable.notes.find_by(data) if note note.destroy else - project.notes.create(data) + Notes::CreateService.new(project, current_user, note_params).execute end render json: { ok: true } diff --git a/app/models/note.rb b/app/models/note.rb index 39645f8f1ad..e30be444eb5 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -40,13 +40,14 @@ class Note < ActiveRecord::Base delegate :name, :email, to: :author, prefix: true validates :note, :project, presence: true + validates :note, uniqueness: { scope: [:author, :noteable_type, :noteable_id] }, if: ->(n) { n.is_award } validates :line_code, format: { with: /\A[a-z0-9]+_\d+_\d+\Z/ }, allow_blank: true # Attachments are deprecated and are handled by Markdown uploader validates :attachment, file_size: { maximum: :max_attachment_size } validates :noteable_id, presence: true, if: ->(n) { n.noteable_type.present? && n.noteable_type != 'Commit' } validates :commit_id, presence: true, if: ->(n) { n.noteable_type == 'Commit' } - validates :author, presence: true, if: ->(n) { n.is_award } + validates :author, presence: true mount_uploader :attachment, AttachmentUploader @@ -102,7 +103,7 @@ class Note < ActiveRecord::Base end def grouped_awards - select(:note).distinct.map do |note| + awards.select(:note).distinct.map do |note| [ note.note, where(note: note.note) ] end end diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index dbff58dfb9c..25a985df4d8 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -35,11 +35,11 @@ module Notes end def contains_emoji_only?(note) - note =~ /\A:[-_+[:alnum:]]*:\s?\z/ + note =~ /\A:?[-_+[:alnum:]]*:?\s?\z/ end def emoji_name(note) - note.match(/\A:([-_+[:alnum:]]*):\s?/)[1] + note.match(/\A:?([-_+[:alnum:]]*):?\s?/)[1] end end end diff --git a/app/views/votes/_votes_block.html.haml b/app/views/votes/_votes_block.html.haml index 04e3b5a3814..7eb27c12d33 100644 --- a/app/views/votes/_votes_block.html.haml +++ b/app/views/votes/_votes_block.html.haml @@ -1,15 +1,15 @@ .awards.votes-block - - votable.notes.awards.grouped_awards.each do | note | - .award{class: (note_active_class(note.last, current_user)), title: emoji_author_list(note.last, current_user)} - .icon{"data-emoji" => "#{note.first}"} - = image_tag url_to_emoji(note.first), height: "20px", width: "20px" + - votable.notes.awards.grouped_awards.each do |emoji, notes| + .award{class: (note_active_class(notes, current_user)), title: emoji_author_list(notes, current_user)} + .icon{"data-emoji" => "#{emoji}"} + = image_tag url_to_emoji(emoji), height: "20px", width: "20px" .counter - = note.last.count + = notes.count - if current_user .dropdown.awards-controls %a.add-award{"data-toggle" => "dropdown", "data-target" => "#", "href" => "#"} - = icon('plus-square-o') + = icon('smile-o') %ul.dropdown-menu.awards-menu - emoji_list.each do |emoji| %li{"data-emoji" => "#{emoji}"}= image_tag url_to_emoji(emoji), height: "20px", width: "20px" @@ -17,7 +17,7 @@ - if current_user :coffeescript post_emoji_url = "#{award_toggle_namespace_project_notes_path(@project.namespace, @project)}" - noteable_type = "#{votable.class}" + noteable_type = "#{votable.class.name.underscore}" noteable_id = "#{votable.id}" window.awards_handler = new AwardsHandler(post_emoji_url, noteable_type, noteable_id) -- cgit v1.2.1 From 22bbb379ae976d94d7ddd7addb7384e0b79ddfd9 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Thu, 19 Nov 2015 23:00:30 +0200 Subject: fox tests --- spec/models/note_spec.rb | 5 ++- spec/services/notes/create_service_spec.rb | 50 +++++++++++++++--------------- 2 files changed, 27 insertions(+), 28 deletions(-) diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 86b6c27a182..f347f537550 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -132,12 +132,11 @@ describe Note do describe :grouped_awards do before do - create :note, note: "smile" - create :note, note: "smile" + create :note, note: "smile", is_award: true + create :note, note: "smile", is_award: true end it "returns grouped array of notes" do - grouped_array = Note.grouped_awards expect(Note.grouped_awards.first.first).to eq("smile") expect(Note.grouped_awards.first.last).to match_array(Note.all) end diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index 066461f5a12..cc38d257792 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -26,36 +26,36 @@ describe Notes::CreateService do end describe "award emoji" do - before do - project.team << [user, :master] - end + before do + project.team << [user, :master] + end - it "creates emoji note" do - opts = { - note: ':smile: ', - noteable_type: 'Issue', - noteable_id: issue.id - } + it "creates emoji note" do + opts = { + note: ':smile: ', + noteable_type: 'Issue', + 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('smile') - expect(@note.is_award).to be_truthy - end + expect(@note).to be_valid + expect(@note.note).to eq('smile') + expect(@note.is_award).to be_truthy + end - it "creates regular note if emoji name is invalid" do - opts = { - note: ':smile: moretext: ', - noteable_type: 'Issue', - noteable_id: issue.id - } + it "creates regular note if emoji name is invalid" do + opts = { + note: ':smile: moretext: ', + noteable_type: 'Issue', + 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 - end + expect(@note).to be_valid + expect(@note.note).to eq(opts[:note]) + expect(@note.is_award).to be_falsy + end end end -- cgit v1.2.1