Merge branch '25437-just-emoji' into 'master'

#25437 Allow posting of just an emoji in comment; add /award slash command

Closes #25437

See merge request !9382

app/assets/javascripts/issuable/time_tracking/time_tracking_bundle.js.es6

@@ -39,8 +39,9 @@ require('../../subbable_resource');
           listenForSlashCommands() {
             $(document).on('ajax:success', '.gfm-form', (e, data) => {
               const subscribedCommands = ['spend_time', 'time_estimate'];
-              const changedCommands = data.commands_changes;
-
+              const changedCommands = data.commands_changes
+                ? Object.keys(data.commands_changes)
+                : [];
               if (changedCommands && _.intersection(subscribedCommands, changedCommands).length) {
                 this.fetchIssuable();
               }

app/assets/javascripts/notes.js

@@ -246,13 +246,22 @@ require('./task_list');
     };
 
     Notes.prototype.handleCreateChanges = function(note) {
+      var votesBlock;
       if (typeof note === 'undefined') {
         return;
       }
 
-      if (note.commands_changes && note.commands_changes.indexOf('merge') !== -1) {
+      if (note.commands_changes) {
+        if ('merge' in note.commands_changes) {
           $.get(mrRefreshWidgetUrl);
         }
+
+        if ('emoji_award' in note.commands_changes) {
+          votesBlock = $('.js-awards-block').eq(0);
+          gl.awardsHandler.addAwardToEmojiBar(votesBlock, note.commands_changes.emoji_award);
+          return gl.awardsHandler.scrollToAwards();
+        }
+      }
     };
 
     /*
@@ -262,26 +271,16 @@ require('./task_list');
      */
 
     Notes.prototype.renderNote = function(note) {
-      var $notesList, votesBlock;
+      var $notesList;
       if (!note.valid) {
-        if (note.award) {
-          new Flash('You have already awarded this emoji!', 'alert', this.parentTimeline);
-        }
-        else {
         if (note.errors.commands_only) {
           new Flash(note.errors.commands_only, 'notice', this.parentTimeline);
           this.refresh();
         }
-        }
         return;
       }
-      if (note.award) {
-        votesBlock = $('.js-awards-block').eq(0);
-        gl.awardsHandler.addAwardToEmojiBar(votesBlock, note.name);
-        return gl.awardsHandler.scrollToAwards();
-      // render note if it not present in loaded list
-      // or skip if rendered
-      } else if (this.isNewNote(note)) {
+
+      if (this.isNewNote(note)) {
         this.note_ids.push(note.id);
         $notesList = $('ul.main-notes-list');
         $notesList.append(note.html).syntaxHighlight();

app/controllers/projects/notes_controller.rb

@@ -148,17 +148,10 @@ class Projects::NotesController < Projects::ApplicationController
 
   def note_json(note)
     attrs = {
-      award: false,
       id: note.id
     }
 
-    if note.is_a?(AwardEmoji)
-      attrs.merge!(
-        valid:  note.valid?,
-        award:  true,
-        name:   note.name
-      )
-    elsif note.persisted?
+    if note.persisted?
       Banzai::NoteRenderer.render([note], @project, current_user)
 
       attrs.merge!(
@@ -198,7 +191,7 @@ class Projects::NotesController < Projects::ApplicationController
       )
     end
 
-    attrs[:commands_changes] = note.commands_changes unless attrs[:award]
+    attrs[:commands_changes] = note.commands_changes
     attrs
   end
 

app/models/note.rb

@@ -231,10 +231,6 @@ class Note < ActiveRecord::Base
     note =~ /\A#{Banzai::Filter::EmojiFilter.emoji_pattern}\s?\Z/
   end
 
-  def award_emoji_name
-    note.match(Banzai::Filter::EmojiFilter.emoji_pattern)[1]
-  end
-
   def to_ability_name
     for_personal_snippet? ? 'personal_snippet' : noteable_type.underscore
   end

app/services/issuable_base_service.rb

@@ -203,6 +203,7 @@ class IssuableBaseService < BaseService
     change_state(issuable)
     change_subscription(issuable)
     change_todo(issuable)
+    toggle_award(issuable)
     filter_params(issuable)
     old_labels = issuable.labels.to_a
     old_mentioned_users = issuable.mentioned_users.to_a
@@ -263,6 +264,14 @@ class IssuableBaseService < BaseService
     end
   end
 
+  def toggle_award(issuable)
+    award = params.delete(:emoji_award)
+    if award
+      todo_service.new_award_emoji(issuable, current_user)
+      issuable.toggle_award_emoji(award, current_user)
+    end
+  end
+
   def has_changes?(issuable, old_labels: [])
     valid_attrs = [:title, :description, :assignee_id, :milestone_id, :target_branch]
 

app/services/notes/create_service.rb

@@ -8,14 +8,6 @@ module Notes
       note.author  = current_user
       note.system  = false
 
-      if note.award_emoji?
-        noteable = note.noteable
-        if noteable.user_can_award?(current_user, note.award_emoji_name)
-          todo_service.new_award_emoji(noteable, current_user)
-          return noteable.create_award_emoji(note.award_emoji_name, current_user)
-        end
-      end
-
       # We execute commands (extracted from `params[:note]`) on the noteable
       # **before** we save the note because if the note consists of commands
       # only, there is no need be create a note!
@@ -48,7 +40,7 @@ module Notes
           note.errors.add(:commands_only, 'Commands applied')
         end
 
-        note.commands_changes = command_params.keys
+        note.commands_changes = command_params
       end
 
       note

app/services/slash_commands/interpret_service.rb

@@ -255,6 +255,18 @@ module SlashCommands
       @updates[:wip_event] = issuable.work_in_progress? ? 'unwip' : 'wip'
     end
 
+    desc 'Toggle emoji reward'
+    params ':emoji:'
+    condition do
+      issuable.persisted?
+    end
+    command :award do |emoji|
+      name = award_emoji_name(emoji)
+      if name && issuable.user_can_award?(current_user, name)
+        @updates[:emoji_award] = name
+      end
+    end
+
     desc 'Set time estimate'
     params '<1w 3d 2h 14m>'
     condition do
@@ -329,5 +341,10 @@ module SlashCommands
 
       ext.references(type)
     end
+
+    def award_emoji_name(emoji)
+      match = emoji.match(Banzai::Filter::EmojiFilter.emoji_pattern)
+      match[1] if match
+    end
   end
 end

changelogs/unreleased/25437-just-emoji.yml

+---
+title: Introduce /award slash command; Allow posting of just an emoji in comment
+merge_request: 9382
+author: mhasbini

doc/user/project/slash_commands.md

@@ -35,3 +35,4 @@ do.
 | <code>/spend &lt;1h 30m &#124; -1h 5m&gt;</code> | Add or subtract spent time |
 | `/remove_time_spent`       | Remove time spent |
 | `/target_branch <Branch Name>` | Set target branch for current merge request |
+| `/award :emoji:`  | Toggle award for :emoji: |

features/project/issues/award_emoji.feature

@@ -42,4 +42,4 @@ Feature: Award Emoji
   @javascript
   Scenario: I add award emoji using regular comment
     Given I leave comment with a single emoji
-    Then I have award added
+    Then I have new comment with emoji added

features/steps/project/issues/award_emoji.rb

@@ -44,6 +44,10 @@ class Spinach::Features::AwardEmoji < Spinach::FeatureSteps
     end
   end
 
+  step 'I have new comment with emoji added' do
+    expect(page).to have_selector ".emoji[title=':smile:']"
+  end
+
   step 'I have award added' do
     page.within '.awards' do
       expect(page).to have_selector '.js-emoji-btn'

spec/features/issues/award_emoji_spec.rb

@@ -67,6 +67,18 @@ describe 'Awards Emoji', feature: true do
           expect(page).not_to have_selector(emoji_counter)
         end
       end
+
+      context 'execute /award slash command' do
+        it 'toggles the emoji award on noteable', js: true do
+          execute_slash_command('/award :100:')
+
+          expect(find(noteable_award_counter)).to have_text("1")
+
+          execute_slash_command('/award :100:')
+
+          expect(page).not_to have_selector(noteable_award_counter)
+        end
+      end
     end
   end
 
@@ -80,6 +92,15 @@ describe 'Awards Emoji', feature: true do
     end
   end
 
+  def execute_slash_command(cmd)
+    within('.js-main-target-form') do
+      fill_in 'note[note]', with: cmd
+      click_button 'Comment'
+    end
+
+    wait_for_ajax
+  end
+
   def thumbsup_emoji
     page.all(emoji_counter).first
   end
@@ -92,6 +113,10 @@ describe 'Awards Emoji', feature: true do
     'span.js-counter'
   end
 
+  def noteable_award_counter
+    ".awards .active"
+  end
+
   def toggle_smiley_emoji(status)
     within('.note') do
       find('.note-emoji-button').click

spec/requests/api/notes_spec.rb

@@ -225,11 +225,11 @@ describe API::Notes, api: true  do
       context 'when the user is posting an award emoji on an issue created by someone else' do
         let(:issue2) { create(:issue, project: project) }
 
-        it 'returns an award emoji' do
+        it 'creates a new issue note' do
           post api("/projects/#{project.id}/issues/#{issue2.id}/notes", user), body: ':+1:'
 
           expect(response).to have_http_status(201)
-          expect(json_response['awardable_id']).to eq issue2.id
+          expect(json_response['body']).to eq(':+1:')
         end
       end
 

spec/requests/api/v3/notes_spec.rb

@@ -228,11 +228,11 @@ describe API::V3::Notes, api: true  do
       context 'when the user is posting an award emoji on an issue created by someone else' do
         let(:issue2) { create(:issue, project: project) }
 
-        it 'returns an award emoji' do
+        it 'creates a new issue note' do
           post v3_api("/projects/#{project.id}/issues/#{issue2.id}/notes", user), body: ':+1:'
 
           expect(response).to have_http_status(201)
-          expect(json_response['awardable_id']).to eq issue2.id
+          expect(json_response['body']).to eq(':+1:')
         end
       end
 

spec/services/notes/create_service_spec.rb

@@ -102,14 +102,9 @@ describe Notes::CreateService, services: true do
         expect(subject.note).to eq(params[:note])
       end
     end
-  end
-
-  describe "award emoji" do
-    before do
-      project.team << [user, :master]
-    end
 
-    it "creates an award emoji" do
+    describe 'note with emoji only' do
+      it 'creates regular note' do
         opts = {
           note: ':smile: ',
           noteable_type: 'Issue',
@@ -118,31 +113,8 @@ describe Notes::CreateService, services: true do
         note = described_class.new(project, user, opts).execute
 
         expect(note).to be_valid
-      expect(note.name).to eq('smile')
+        expect(note.note).to eq(':smile:')
       end
-
-    it "creates regular note if emoji name is invalid" do
-      opts = {
-        note: ':smile: moretext:',
-        noteable_type: 'Issue',
-        noteable_id: issue.id
-      }
-      note = described_class.new(project, user, opts).execute
-
-      expect(note).to be_valid
-      expect(note.note).to eq(opts[:note])
-    end
-
-    it "normalizes the emoji name" do
-      opts = {
-        note: ':+1:',
-        noteable_type: 'Issue',
-        noteable_id: issue.id
-      }
-
-      expect_any_instance_of(TodoService).to receive(:new_award_emoji).with(issue, user)
-
-      described_class.new(project, user, opts).execute
     end
   end
 end

spec/services/slash_commands/interpret_service_spec.rb

@@ -267,6 +267,14 @@ describe SlashCommands::InterpretService, services: true do
       end
     end
 
+    shared_examples 'award command' do
+      it 'toggle award 100 emoji if content containts /award :100:' do
+        _, updates = service.execute(content, issuable)
+
+        expect(updates).to eq(emoji_award: "100")
+      end
+    end
+
     it_behaves_like 'reopen command' do
       let(:content) { '/reopen' }
       let(:issuable) { issue }
@@ -654,6 +662,37 @@ describe SlashCommands::InterpretService, services: true do
       end
     end
 
+    context '/award command' do
+      it_behaves_like 'award command' do
+        let(:content) { '/award :100:' }
+        let(:issuable) { issue }
+      end
+
+      it_behaves_like 'award command' do
+        let(:content) { '/award :100:' }
+        let(:issuable) { merge_request }
+      end
+
+      context 'ignores command with no argument' do
+        it_behaves_like 'empty command' do
+          let(:content) { '/award' }
+          let(:issuable) { issue }
+        end
+      end
+
+      context 'ignores non-existing / invalid  emojis' do
+        it_behaves_like 'empty command' do
+          let(:content) { '/award noop' }
+          let(:issuable) { issue }
+        end
+
+        it_behaves_like 'empty command' do
+          let(:content) { '/award :lorem_ipsum:' }
+          let(:issuable) { issue }
+        end
+      end
+    end
+
     context '/target_branch command' do
       let(:non_empty_project) { create(:project) }
       let(:another_merge_request) { create(:merge_request, author: developer, source_project: non_empty_project) }