Fix (most) broken tests

17 files changed, 96 insertions, 107 deletions

diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb
index 3b595d55506..886d32473a4 100644
--- a/app/controllers/projects/notes_controller.rb
+++ b/app/controllers/projects/notes_controller.rb
@@ -22,7 +22,7 @@ class Projects::NotesController < Projects::ApplicationController
   end
 
   def create
-    @note = Notes::CreateService.new(project, current_user, note_params.merge(create_award_emoji: true)).execute
+    @note = Notes::CreateService.new(project, current_user, note_params).execute
 
     respond_to do |format|
       format.json { render json: note_json(@note) }
diff --git a/app/models/note.rb b/app/models/note.rb
index d7dad56b7cc..84edfafba68 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -343,12 +343,12 @@ class Note < ActiveRecord::Base
     cross_reference? && referenced_mentionables(user).empty?
   end
 
-  def emoji_award?
+  def award_emoji?
     emoji_awards_supported? && contains_only_emoji?
   end
 
   def create_award_emoji
-    self.emoji_award = self.noteable.award_emoji(emoji_award_name, self.author)
+    self.award_emoji = self.noteable.award_emoji(emoji_award_name, self.author)
   end
 
   def emoji_awardable?
diff --git a/app/models/user.rb b/app/models/user.rb
index e4c8263cef5..bf1057db6f3 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -142,7 +142,7 @@ class User < ActiveRecord::Base
   has_one  :abuse_report,             dependent: :destroy
   has_many :spam_logs,                dependent: :destroy
   has_many :builds,                   dependent: :nullify, class_name: 'Ci::Build'
-  has_many :emoji_awards,             as: :awardable, dependent: :destroy
+  has_many :award_emoji,             as: :awardable, dependent: :destroy
   has_many :todos,                    dependent: :destroy
 
   #
diff --git a/app/services/create_award_emoji_service.rb b/app/services/create_award_emoji_service.rb
index 8bb2c6d715b..91cb2834b76 100644
--- a/app/services/create_award_emoji_service.rb
+++ b/app/services/create_award_emoji_service.rb
@@ -9,6 +9,7 @@ class CreateAwardEmojiService < BaseService
     todo_service.award_emoji(issuable, current_user) if issuable
 
     awardable.toggle_award_emoji(emoji, current_user)
+    awardable
   end
 
   private
diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb
index a5083bcd2d0..78a767946a6 100644
--- a/app/services/notes/create_service.rb
+++ b/app/services/notes/create_service.rb
@@ -1,16 +1,13 @@
 module Notes
   class CreateService < BaseService
     def execute
-      create_award_emoji = params.delete(:create_award_emoji)
-
       note = project.notes.new(params)
-
       note.author = current_user
       note.system = false
 
-      if create_award_emoji && note.emoji_award?
-        note.create_award_emoji
-        return note
+      if note.award_emoji?
+        return CreateAwardEmojiService.new(project, current_user, params).
+                                                  execute(note.noteable, note.note)
       end
 
       if note.save
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index eff0d96f93d..b198e706875 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -131,7 +131,6 @@ class NotificationService
     # ignore gitlab service messages
     return true if note.note.start_with?('Status changed to closed')
     return true if note.cross_reference? && note.system == true
-    return true if note.is_award
 
     target = note.noteable
 
diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb
index aff5ca676be..af46df2d0f4 100644
--- a/features/steps/project/issues/issues.rb
+++ b/features/steps/project/issues/issues.rb
@@ -192,14 +192,14 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps
 
   step 'issue "Release 0.4" have 2 upvotes and 1 downvote' do
     issue = Issue.find_by(title: 'Release 0.4')
-    create_list(:upvote_note, 2, project: project, noteable: issue)
-    create(:downvote_note, project: project, noteable: issue)
+    create_list(:award_emoji, :upvote, 2, awardable: issue)
+    create(:award_emoji, :downvote, awardable: issue)
   end
 
   step 'issue "Tweet control" have 1 upvote and 2 downvotes' do
     issue = Issue.find_by(title: 'Tweet control')
-    create(:upvote_note, project: project, noteable: issue)
-    create_list(:downvote_note, 2, project: project, noteable: issue)
+    create(:award_emoji, :upvote, awardable: issue)
+    create_list(:award_emoji, :downvote,  2, awardable: issue)
   end
 
   step 'The list should be sorted by "Least popular"' do
diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb
index a4f02b590ea..ad1b8ca83fc 100644
--- a/features/steps/project/merge_requests.rb
+++ b/features/steps/project/merge_requests.rb
@@ -175,14 +175,14 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
 
   step 'merge request "Bug NS-04" have 2 upvotes and 1 downvote' do
     merge_request = MergeRequest.find_by(title: 'Bug NS-04')
-    create_list(:upvote_note, 2, project: project, noteable: merge_request)
-    create(:downvote_note, project: project, noteable: merge_request)
+    create_list(:award_emoji, 2, awardable: merge_request, name: "thumbsup")
+    create(:award_emoji, :downvote, awardable: merge_request)
   end
 
   step 'merge request "Bug NS-06" have 1 upvote and 2 downvotes' do
     merge_request = MergeRequest.find_by(title: 'Bug NS-06')
-    create(:upvote_note, project: project, noteable: merge_request)
-    create_list(:downvote_note, 2, project: project, noteable: merge_request)
+    create(:award_emoji, :upvote, awardable: merge_request)
+    create_list(:award_emoji, 2, awardable: merge_request, name: "thumbsdown")
   end
 
   step 'The list should be sorted by "Least popular"' do
diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb
index 465531b2b36..3ca9e7a019c 100644
--- a/spec/controllers/groups_controller_spec.rb
+++ b/spec/controllers/groups_controller_spec.rb
@@ -31,9 +31,9 @@ describe GroupsController do
     let(:issue_2) { create(:issue, project: project) }
 
     before do
-      create_list(:upvote_note, 3, project: project, noteable: issue_2)
-      create_list(:upvote_note, 2, project: project, noteable: issue_1)
-      create_list(:downvote_note, 2, project: project, noteable: issue_2)
+      create_list(:award_emoji, 3, awardable: issue_2, name: "thumbsup")
+      create_list(:award_emoji, 2, awardable: issue_1, name: "thumbsup")
+      create_list(:award_emoji, 2, awardable: issue_2, name: "thumbsdown")
 
       sign_in(user)
     end
@@ -56,9 +56,9 @@ describe GroupsController do
     let(:merge_request_2) { create(:merge_request, :simple, source_project: project) }
 
     before do
-      create_list(:upvote_note, 3, project: project, noteable: merge_request_2)
-      create_list(:upvote_note, 2, project: project, noteable: merge_request_1)
-      create_list(:downvote_note, 2, project: project, noteable: merge_request_2)
+      create_list(:award_emoji, 3, awardable: merge_request_2, name: "thumbsup")
+      create_list(:award_emoji, 2, awardable: merge_request_1, name: "thumbsup")
+      create_list(:award_emoji, 2, awardable: merge_request_2, name: "thumbsdown")
 
       sign_in(user)
     end
diff --git a/spec/factories/award_emoji.rb b/spec/factories/award_emoji.rb
new file mode 100644
index 00000000000..b09f8b0bc74
--- /dev/null
+++ b/spec/factories/award_emoji.rb
@@ -0,0 +1,18 @@
+FactoryGirl.define do
+  factory :award_emoji do
+    name "thumbsup"
+    user
+    awardable factory: :issue
+
+    trait :thumbs_up
+    trait :upvote
+
+    trait :thumbs_down do
+      name "thumbsdown"
+    end
+
+    trait :downvote do
+      name "thumbsdown"
+    end
+  end
+end
diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb
index b1ac04fd0e9..ff5b0800ce4 100644
--- a/spec/factories/notes.rb
+++ b/spec/factories/notes.rb
@@ -36,8 +36,6 @@ FactoryGirl.define do
     factory :note_on_merge_request_diff, traits: [:on_merge_request, :on_diff]
     factory :note_on_project_snippet,    traits: [:on_project_snippet]
     factory :system_note,                traits: [:system]
-    factory :downvote_note,              traits: [:award, :downvote]
-    factory :upvote_note,                traits: [:award, :upvote]
 
     trait :on_commit do
       project
@@ -69,10 +67,6 @@ FactoryGirl.define do
       system true
     end
 
-    trait :award do
-      is_award true
-    end
-
     trait :downvote do
       note "thumbsdown"
     end
diff --git a/spec/lib/award_emoji_spec.rb b/spec/lib/gitlab/award_emoji_spec.rb
index 330678f7f16..cf63d3e9a17 100644
--- a/spec/lib/award_emoji_spec.rb
+++ b/spec/lib/gitlab/award_emoji_spec.rb
@@ -1,8 +1,8 @@
 require 'spec_helper'
 
-describe AwardEmoji do
+describe Gitlab::AwardEmoji do
   describe '.urls' do
-    subject { AwardEmoji.urls }
+    subject { Gitlab::AwardEmoji.urls }
 
     it { is_expected.to be_an_instance_of(Array) }
     it { is_expected.to_not be_empty }
diff --git a/spec/models/award_emoji_spec.rb b/spec/models/award_emoji_spec.rb
new file mode 100644
index 00000000000..d006ecc325d
--- /dev/null
+++ b/spec/models/award_emoji_spec.rb
@@ -0,0 +1,30 @@
+require 'spec_helper'
+
+describe AwardEmoji, models: true do
+  describe "Associations" do
+    it { is_expected.to belong_to(:awardable) }
+    it { is_expected.to belong_to(:user) }
+  end
+
+  describe 'modules' do
+    it { is_expected.to include_module(Participable) }
+  end
+
+  describe "validations" do
+    it { is_expected.to validate_presence_of(:awardable) }
+    it { is_expected.to validate_presence_of(:user) }
+    it { is_expected.to validate_presence_of(:name) }
+    it { is_expected.to validate_presence_of(:awardable) }
+
+    # To circumvent in the shoulda matchers
+    describe "scoped uniqueness validation" do
+      it "rejects duplicate award emoji" do
+        user  = create(:user)
+        issue = create(:issue)
+        create(:award_emoji, user: user, awardable: issue)
+
+        expect(build(:award_emoji, user: user, awardable: issue)).not_to be_valid
+      end
+    end
+  end
+end
diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb
index b16ccc6e305..68a3584e1d5 100644
--- a/spec/models/concerns/issuable_spec.rb
+++ b/spec/models/concerns/issuable_spec.rb
@@ -2,7 +2,15 @@ require 'spec_helper'
 
 describe Issue, "Issuable" do
   let(:issue) { create(:issue) }
-  let(:user) { create(:user) }
+  let(:user)  { create(:user) }
+
+  describe "modules" do
+    it { is_expected.to include_module(Participable) }
+    it { is_expected.to include_module(Mentionable) }
+    it { is_expected.to include_module(Subscribable) }
+    it { is_expected.to include_module(StripAttribute) }
+    it { is_expected.to include_module(Awardable) }
+  end
 
   describe "Associations" do
     it { is_expected.to belong_to(:project) }
@@ -201,15 +209,13 @@ describe Issue, "Issuable" do
 
   describe "votes" do
     before do
-      author = create :user
-      project = create :empty_project
-      issue.notes.awards.create!(note: "thumbsup", author: author, project: project)
-      issue.notes.awards.create!(note: "thumbsdown", author: author, project: project)
+      create(:award_emoji, :thumbs_up, awardable: issue)
+      create(:award_emoji, :thumbs_down, awardable: issue)
     end
 
     it "returns correct values" do
-      expect(issue.upvotes).to eq(1)
-      expect(issue.downvotes).to eq(1)
+      expect(issue.upvotes).to    eq(1)
+      expect(issue.downvotes).to  eq(1)
     end
   end
 end
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 6b18936edb1..bb591e9cb53 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -152,23 +152,6 @@ describe Note, models: true do
     end
   end
 
-  describe '.grouped_awards' do
-    before do
-      create :note, note: "smile", is_award: true
-      create :note, note: "smile", is_award: true
-    end
-
-    it "returns grouped hash of notes" do
-      expect(Note.grouped_awards.keys.size).to eq(3)
-      expect(Note.grouped_awards["smile"]).to match_array(Note.all)
-    end
-
-    it "returns thumbsup and thumbsdown always" do
-      expect(Note.grouped_awards["thumbsup"]).to match_array(Note.none)
-      expect(Note.grouped_awards["thumbsdown"]).to match_array(Note.none)
-    end
-  end
-
   describe '#active?' do
     it 'is always true when the note has no associated diff' do
       note = build(:note)
@@ -239,11 +222,6 @@ describe Note, models: true do
       note = build(:note, system: true)
       expect(note.editable?).to be_falsy
     end
-
-    it "returns false" do
-      note = build(:note, is_award: true, note: "smiley")
-      expect(note.editable?).to be_falsy
-    end
   end
 
   describe "cross_reference_not_visible_for?" do
@@ -270,23 +248,6 @@ describe Note, models: true do
     end
   end
 
-  describe "set_award!" do
-    let(:merge_request) { create :merge_request }
-
-    it "converts aliases to actual name" do
-      note = create(:note, note: ":+1:", noteable: merge_request)
-      expect(note.reload.note).to eq("thumbsup")
-    end
-
-    it "is not an award emoji when comment is on a diff" do
-      note = create(:note, note: ":blowfish:", noteable: merge_request, line_code: "11d5d2e667e9da4f7f610f81d86c974b146b13bd_0_2")
-      note = note.reload
-
-      expect(note.note).to eq(":blowfish:")
-      expect(note.is_award?).to be_falsy
-    end
-  end
-
   describe 'clear_blank_line_code!' do
     it 'clears a blank line code before validation' do
       note = build(:note, line_code: ' ')
diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb
index ff23f13e1cb..51ab9c9774e 100644
--- a/spec/services/notes/create_service_spec.rb
+++ b/spec/services/notes/create_service_spec.rb
@@ -7,19 +7,15 @@ describe Notes::CreateService, services: true do
 
   describe :execute do
     context "valid params" do
+      let(:opts) { { note: 'Awesome comment', noteable_type: 'Issue', noteable_id: issue.id } }
+      let(:note) { Notes::CreateService.new(project, user, opts).execute }
+
       before do
         project.team << [user, :master]
-        opts = {
-          note: 'Awesome comment',
-          noteable_type: 'Issue',
-          noteable_id: issue.id
-        }
-        
-        @note = Notes::CreateService.new(project, user, opts).execute
       end
 
-      it { expect(@note).to be_valid }
-      it { expect(@note.note).to eq('Awesome comment') }
+      it { expect(note).to be_valid }
+      it { expect(note.note).to eq('Awesome comment') }
     end
   end
 
@@ -28,18 +24,14 @@ describe Notes::CreateService, services: true do
       project.team << [user, :master]
     end
 
-    it "creates emoji note" do
+    it "skips awards emoji" do
       opts = {
         note: ':smile: ',
         noteable_type: 'Issue',
-        noteable_id: issue.id
+        noteable_id: issue.id,
       }
 
-      @note = Notes::CreateService.new(project, user, opts).execute
-
-      expect(@note).to be_valid
-      expect(@note.note).to eq('smile')
-      expect(@note.is_award).to be_truthy
+      expect { Notes::CreateService.new(project, user, opts).execute }.not_to change { Note.count }
     end
 
     it "creates regular note if emoji name is invalid" do
@@ -49,11 +41,10 @@ describe Notes::CreateService, services: true do
         noteable_id: issue.id
       }
 
-      @note = Notes::CreateService.new(project, user, opts).execute
+      note = Notes::CreateService.new(project, user, opts).execute
 
-      expect(@note).to be_valid
-      expect(@note.note).to eq(opts[:note])
-      expect(@note.is_award).to be_falsy
+      expect(note).to be_valid
+      expect(note.note).to eq(opts[:note])
     end
   end
 end
diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb
index 82b7fbfa816..a04dbc5d50c 100644
--- a/spec/services/todo_service_spec.rb
+++ b/spec/services/todo_service_spec.rb
@@ -137,7 +137,6 @@ describe TodoService, services: true do
       let(:note_on_commit) { create(:note_on_commit, project: project, author: john_doe, note: mentions) }
       let(:note_on_confidential_issue) { create(:note_on_issue, noteable: confidential_issue, project: project, note: mentions) }
       let(:note_on_project_snippet) { create(:note_on_project_snippet, project: project, author: john_doe, note: mentions) }
-      let(:award_note) { create(:note, :award, project: project, noteable: issue, author: john_doe, note: 'thumbsup') }
       let(:system_note) { create(:system_note, project: project, noteable: issue) }
 
       it 'mark related pending todos to the noteable for the note author as done' do
@@ -150,13 +149,6 @@ describe TodoService, services: true do
         expect(second_todo.reload).to be_done
       end
 
-      it 'mark related pending todos to the noteable for the award note author as done' do
-        service.new_note(award_note, john_doe)
-
-        expect(first_todo.reload).to be_done
-        expect(second_todo.reload).to be_done
-      end
-
       it 'does not mark related pending todos it is a system note' do
         service.new_note(system_note, john_doe)