From 37b17fa61a1fb5efe5942ab2cb27b15685bf905e Mon Sep 17 00:00:00 2001 From: Luke Duncalfe Date: Tue, 18 Jun 2019 13:44:43 +1200 Subject: [PATCH] Add service classes for mutating AwardEmoji Adding, destroying and toggling emoji previously lacked services and instead were performed through methods called on Awardable models. This led to inconsistencies where relevant todos would be marked as done only when emoji were awarded through our controllers, but not through the API. Todos could also be marked as done when an emoji was being removed. Behaviour changes - Awarding emoji through the API will now mark a relevant Todo as done - Toggling an emoji off (destroying it) through our controllers will no longer mark a relevant Todo as done Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/63372 --- .../concerns/toggle_award_emoji.rb | 19 +--- app/finders/award_emojis_finder.rb | 55 ++++++++++ app/models/award_emoji.rb | 6 +- app/models/concerns/awardable.rb | 26 +---- app/services/award_emojis/add_service.rb | 42 +++++++ app/services/award_emojis/base_service.rb | 32 ++++++ app/services/award_emojis/destroy_service.rb | 21 ++++ app/services/award_emojis/toggle_service.rb | 13 +++ app/services/issuable_base_service.rb | 5 +- db/fixtures/development/15_award_emoji.rb | 35 ++---- lib/api/award_emoji.rb | 8 +- .../snippets/notes_controller_spec.rb | 6 +- spec/finders/award_emojis_finder_spec.rb | 49 +++++++++ spec/models/award_emoji_spec.rb | 23 ++++ spec/models/concerns/awardable_spec.rb | 10 -- .../services/award_emojis/add_service_spec.rb | 103 ++++++++++++++++++ .../award_emojis/destroy_service_spec.rb | 89 +++++++++++++++ .../award_emojis/toggle_service_spec.rb | 72 ++++++++++++ spec/services/projects/create_service_spec.rb | 1 + .../award_emoji_todo_shared_examples.rb | 59 ++++++++++ 20 files changed, 586 insertions(+), 88 deletions(-) create mode 100644 app/finders/award_emojis_finder.rb create mode 100644 app/services/award_emojis/add_service.rb create mode 100644 app/services/award_emojis/base_service.rb create mode 100644 app/services/award_emojis/destroy_service.rb create mode 100644 app/services/award_emojis/toggle_service.rb create mode 100644 spec/finders/award_emojis_finder_spec.rb create mode 100644 spec/services/award_emojis/add_service_spec.rb create mode 100644 spec/services/award_emojis/destroy_service_spec.rb create mode 100644 spec/services/award_emojis/toggle_service_spec.rb create mode 100644 spec/support/shared_examples/award_emoji_todo_shared_examples.rb diff --git a/app/controllers/concerns/toggle_award_emoji.rb b/app/controllers/concerns/toggle_award_emoji.rb index 97b343f8b1a..24d178781d6 100644 --- a/app/controllers/concerns/toggle_award_emoji.rb +++ b/app/controllers/concerns/toggle_award_emoji.rb @@ -7,12 +7,9 @@ module ToggleAwardEmoji authenticate_user! name = params.require(:name) - if awardable.user_can_award?(current_user) - awardable.toggle_award_emoji(name, current_user) - - todoable = to_todoable(awardable) - TodoService.new.new_award_emoji(todoable, current_user) if todoable + service = AwardEmojis::ToggleService.new(awardable, name, current_user).execute + if service[:status] == :success render json: { ok: true } else render json: { ok: false } @@ -21,18 +18,6 @@ module ToggleAwardEmoji private - def to_todoable(awardable) - case awardable - when Note - # we don't create todos for personal snippet comments for now - awardable.for_personal_snippet? ? nil : awardable.noteable - when MergeRequest, Issue - awardable - when Snippet - nil - end - end - def awardable raise NotImplementedError end diff --git a/app/finders/award_emojis_finder.rb b/app/finders/award_emojis_finder.rb new file mode 100644 index 00000000000..7320e035409 --- /dev/null +++ b/app/finders/award_emojis_finder.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +class AwardEmojisFinder + attr_reader :awardable, :params + + def initialize(awardable, params = {}) + @awardable = awardable + @params = params + + validate_params + end + + def execute + awards = awardable.award_emoji + awards = by_name(awards) + awards = by_awarded_by(awards) + awards + end + + private + + def by_name(awards) + return awards unless params[:name] + + awards.named(params[:name]) + end + + def by_awarded_by(awards) + return awards unless params[:awarded_by] + + awards.awarded_by(params[:awarded_by]) + end + + def validate_params + return unless params.present? + + validate_name_param + validate_awarded_by_param + end + + def validate_name_param + return unless params[:name] + + raise ArgumentError, 'Invalid name param' unless params[:name].in?(Gitlab::Emoji.emojis_names) + end + + def validate_awarded_by_param + return unless params[:awarded_by] + + # awarded_by can be a `User`, or an ID + unless params[:awarded_by].is_a?(User) || params[:awarded_by].to_s.match(/\A\d+\Z/) + raise ArgumentError, 'Invalid awarded_by param' + end + end +end diff --git a/app/models/award_emoji.rb b/app/models/award_emoji.rb index e26162f6151..0ab302a0f3e 100644 --- a/app/models/award_emoji.rb +++ b/app/models/award_emoji.rb @@ -16,8 +16,10 @@ class AwardEmoji < ApplicationRecord participant :user - scope :downvotes, -> { where(name: DOWNVOTE_NAME) } - scope :upvotes, -> { where(name: UPVOTE_NAME) } + scope :downvotes, -> { named(DOWNVOTE_NAME) } + scope :upvotes, -> { named(UPVOTE_NAME) } + scope :named, -> (names) { where(name: names) } + scope :awarded_by, -> (users) { where(user: users) } after_save :expire_etag_cache after_destroy :expire_etag_cache diff --git a/app/models/concerns/awardable.rb b/app/models/concerns/awardable.rb index 14bc56f0eee..f229b42ade6 100644 --- a/app/models/concerns/awardable.rb +++ b/app/models/concerns/awardable.rb @@ -106,30 +106,6 @@ module Awardable end def awarded_emoji?(emoji_name, current_user) - award_emoji.where(name: emoji_name, user: current_user).exists? - end - - def create_award_emoji(name, current_user) - return unless emoji_awardable? - - award_emoji.create(name: normalize_name(name), user: current_user) - end - - def remove_award_emoji(name, current_user) - award_emoji.where(name: name, user: current_user).destroy_all # rubocop: disable DestroyAll - end - - def toggle_award_emoji(emoji_name, current_user) - if awarded_emoji?(emoji_name, current_user) - remove_award_emoji(emoji_name, current_user) - else - create_award_emoji(emoji_name, current_user) - end - end - - private - - def normalize_name(name) - Gitlab::Emoji.normalize_emoji_name(name) + award_emoji.named(emoji_name).awarded_by(current_user).exists? end end diff --git a/app/services/award_emojis/add_service.rb b/app/services/award_emojis/add_service.rb new file mode 100644 index 00000000000..eac15dabbf0 --- /dev/null +++ b/app/services/award_emojis/add_service.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +module AwardEmojis + class AddService < AwardEmojis::BaseService + include Gitlab::Utils::StrongMemoize + + def execute + unless awardable.user_can_award?(current_user) + return error('User cannot award emoji to awardable', status: :forbidden) + end + + unless awardable.emoji_awardable? + return error('Awardable cannot be awarded emoji', status: :unprocessable_entity) + end + + award = awardable.award_emoji.create(name: name, user: current_user) + + if award.persisted? + TodoService.new.new_award_emoji(todoable, current_user) if todoable + success(award: award) + else + error(award.errors.full_messages, award: award) + end + end + + private + + def todoable + strong_memoize(:todoable) do + case awardable + when Note + # We don't create todos for personal snippet comments for now + awardable.noteable unless awardable.for_personal_snippet? + when MergeRequest, Issue + awardable + when Snippet + nil + end + end + end + end +end diff --git a/app/services/award_emojis/base_service.rb b/app/services/award_emojis/base_service.rb new file mode 100644 index 00000000000..a677d03a221 --- /dev/null +++ b/app/services/award_emojis/base_service.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module AwardEmojis + class BaseService < ::BaseService + attr_accessor :awardable, :name + + def initialize(awardable, name, current_user) + @awardable = awardable + @name = normalize_name(name) + + super(awardable.project, current_user) + end + + private + + def normalize_name(name) + Gitlab::Emoji.normalize_emoji_name(name) + end + + # Provide more error state data than what BaseService allows. + # - An array of errors + # - The `AwardEmoji` if present + def error(errors, award: nil, status: nil) + errors = Array.wrap(errors) + + super(errors.to_sentence.presence, status).merge({ + award: award, + errors: errors + }) + end + end +end diff --git a/app/services/award_emojis/destroy_service.rb b/app/services/award_emojis/destroy_service.rb new file mode 100644 index 00000000000..3789a8403bc --- /dev/null +++ b/app/services/award_emojis/destroy_service.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module AwardEmojis + class DestroyService < AwardEmojis::BaseService + def execute + unless awardable.user_can_award?(current_user) + return error('User cannot destroy emoji on the awardable', status: :forbidden) + end + + awards = AwardEmojisFinder.new(awardable, name: name, awarded_by: current_user).execute + + if awards.empty? + return error("User has not awarded emoji of type #{name} on the awardable", status: :forbidden) + end + + award = awards.destroy_all.first # rubocop: disable DestroyAll + + success(award: award) + end + end +end diff --git a/app/services/award_emojis/toggle_service.rb b/app/services/award_emojis/toggle_service.rb new file mode 100644 index 00000000000..812dd1c2889 --- /dev/null +++ b/app/services/award_emojis/toggle_service.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module AwardEmojis + class ToggleService < AwardEmojis::BaseService + def execute + if awardable.awarded_emoji?(name, current_user) + DestroyService.new(awardable, name, current_user).execute + else + AddService.new(awardable, name, current_user).execute + end + end + end +end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 77c2224ee3b..2ab6e88599f 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -344,10 +344,7 @@ class IssuableBaseService < BaseService 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 + AwardEmojis::ToggleService.new(issuable, award, current_user).execute if award end def associations_before_update(issuable) diff --git a/db/fixtures/development/15_award_emoji.rb b/db/fixtures/development/15_award_emoji.rb index 137a036edaf..a9dcc048586 100644 --- a/db/fixtures/development/15_award_emoji.rb +++ b/db/fixtures/development/15_award_emoji.rb @@ -1,35 +1,22 @@ require './spec/support/sidekiq' Gitlab::Seeder.quiet do - emoji = Gitlab::Emoji.emojis.keys + EMOJI = Gitlab::Emoji.emojis.keys - Issue.order(Gitlab::Database.random).limit(Issue.count / 2).each do |issue| - project = issue.project + def seed_award_emoji(klass) + klass.order(Gitlab::Database.random).limit(klass.count / 2).each do |awardable| + awardable.project.authorized_users.where('project_authorizations.access_level > ?', Gitlab::Access::GUEST).sample(2).each do |user| + AwardEmojis::AddService.new(awardable, EMOJI.sample, user).execute - project.team.users.sample(2).each do |user| - issue.create_award_emoji(emoji.sample, user) + awardable.notes.user.sample(2).each do |note| + AwardEmojis::AddService.new(note, EMOJI.sample, user).execute + end - issue.notes.sample(2).each do |note| - next if note.system? - note.create_award_emoji(emoji.sample, user) + print '.' end - - print '.' end end - MergeRequest.order(Gitlab::Database.random).limit(MergeRequest.count / 2).each do |mr| - project = mr.project - - project.team.users.sample(2).each do |user| - mr.create_award_emoji(emoji.sample, user) - - mr.notes.sample(2).each do |note| - next if note.system? - note.create_award_emoji(emoji.sample, user) - end - - print '.' - end - end + seed_award_emoji(Issue) + seed_award_emoji(MergeRequest) end diff --git a/lib/api/award_emoji.rb b/lib/api/award_emoji.rb index a1851ba3627..89b7e5c5e4b 100644 --- a/lib/api/award_emoji.rb +++ b/lib/api/award_emoji.rb @@ -69,12 +69,12 @@ module API post endpoint do not_found!('Award Emoji') unless can_read_awardable? && can_award_awardable? - award = awardable.create_award_emoji(params[:name], current_user) + service = AwardEmojis::AddService.new(awardable, params[:name], current_user).execute - if award.persisted? - present award, with: Entities::AwardEmoji + if service[:status] == :success + present service[:award], with: Entities::AwardEmoji else - not_found!("Award Emoji #{award.errors.messages}") + not_found!("Award Emoji #{service[:message]}") end end diff --git a/spec/controllers/snippets/notes_controller_spec.rb b/spec/controllers/snippets/notes_controller_spec.rb index 652533ac49f..fd4b95ce226 100644 --- a/spec/controllers/snippets/notes_controller_spec.rb +++ b/spec/controllers/snippets/notes_controller_spec.rb @@ -288,11 +288,13 @@ describe Snippets::NotesController do describe 'POST toggle_award_emoji' do let(:note) { create(:note_on_personal_snippet, noteable: public_snippet) } + let(:emoji_name) { 'thumbsup'} + before do sign_in(user) end - subject { post(:toggle_award_emoji, params: { snippet_id: public_snippet, id: note.id, name: "thumbsup" }) } + subject { post(:toggle_award_emoji, params: { snippet_id: public_snippet, id: note.id, name: emoji_name }) } it "toggles the award emoji" do expect { subject }.to change { note.award_emoji.count }.by(1) @@ -301,7 +303,7 @@ describe Snippets::NotesController do end it "removes the already awarded emoji when it exists" do - note.toggle_award_emoji('thumbsup', user) # create award emoji before + create(:award_emoji, awardable: note, name: emoji_name, user: user) expect { subject }.to change { AwardEmoji.count }.by(-1) diff --git a/spec/finders/award_emojis_finder_spec.rb b/spec/finders/award_emojis_finder_spec.rb new file mode 100644 index 00000000000..ccac475daad --- /dev/null +++ b/spec/finders/award_emojis_finder_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe AwardEmojisFinder do + set(:issue_1) { create(:issue) } + set(:issue_1_thumbsup) { create(:award_emoji, name: 'thumbsup', awardable: issue_1) } + set(:issue_1_thumbsdown) { create(:award_emoji, name: 'thumbsdown', awardable: issue_1) } + # Create a matching set of emoji for a second issue. + # These should never appear in our finder results + set(:issue_2) { create(:issue) } + set(:issue_2_thumbsup) { create(:award_emoji, name: 'thumbsup', awardable: issue_2) } + set(:issue_2_thumbsdown) { create(:award_emoji, name: 'thumbsdown', awardable: issue_2) } + + describe 'param validation' do + it 'raises an error if `name` is invalid' do + expect { described_class.new(issue_1, { name: 'invalid' }).execute }.to raise_error( + ArgumentError, + 'Invalid name param' + ) + end + + it 'raises an error if `awarded_by` is invalid' do + expectation = [ArgumentError, 'Invalid awarded_by param'] + + expect { described_class.new(issue_1, { awarded_by: issue_2 }).execute }.to raise_error(*expectation) + expect { described_class.new(issue_1, { awarded_by: 'not-an-id' }).execute }.to raise_error(*expectation) + expect { described_class.new(issue_1, { awarded_by: 1.123 }).execute }.to raise_error(*expectation) + end + end + + describe '#execute' do + it 'scopes to the awardable' do + expect(described_class.new(issue_1).execute).to contain_exactly( + issue_1_thumbsup, issue_1_thumbsdown + ) + end + + it 'filters by emoji name' do + expect(described_class.new(issue_1, { name: 'thumbsup' }).execute).to contain_exactly(issue_1_thumbsup) + expect(described_class.new(issue_1, { name: '8ball' }).execute).to be_empty + end + + it 'filters by user' do + expect(described_class.new(issue_1, { awarded_by: issue_1_thumbsup.user }).execute).to contain_exactly(issue_1_thumbsup) + expect(described_class.new(issue_1, { awarded_by: issue_2_thumbsup.user }).execute).to be_empty + end + end +end diff --git a/spec/models/award_emoji_spec.rb b/spec/models/award_emoji_spec.rb index 8452ac69734..b15b26b1630 100644 --- a/spec/models/award_emoji_spec.rb +++ b/spec/models/award_emoji_spec.rb @@ -44,6 +44,29 @@ describe AwardEmoji do end end + describe 'scopes' do + set(:thumbsup) { create(:award_emoji, name: 'thumbsup') } + set(:thumbsdown) { create(:award_emoji, name: 'thumbsdown') } + + describe '.upvotes' do + it { expect(described_class.upvotes).to contain_exactly(thumbsup) } + end + + describe '.downvotes' do + it { expect(described_class.downvotes).to contain_exactly(thumbsdown) } + end + + describe '.named' do + it { expect(described_class.named('thumbsup')).to contain_exactly(thumbsup) } + it { expect(described_class.named(%w[thumbsup thumbsdown])).to contain_exactly(thumbsup, thumbsdown) } + end + + describe '.awarded_by' do + it { expect(described_class.awarded_by(thumbsup.user)).to contain_exactly(thumbsup) } + it { expect(described_class.awarded_by([thumbsup.user, thumbsdown.user])).to contain_exactly(thumbsup, thumbsdown) } + end + end + describe 'expiring ETag cache' do context 'on a note' do let(:note) { create(:note_on_issue) } diff --git a/spec/models/concerns/awardable_spec.rb b/spec/models/concerns/awardable_spec.rb index 9e7106281ee..76da42cf243 100644 --- a/spec/models/concerns/awardable_spec.rb +++ b/spec/models/concerns/awardable_spec.rb @@ -82,16 +82,6 @@ describe Awardable do end end - describe "#toggle_award_emoji" do - it "adds an emoji if it isn't awarded yet" do - expect { issue.toggle_award_emoji("thumbsup", award_emoji.user) }.to change { AwardEmoji.count }.by(1) - end - - it "toggles already awarded emoji" do - expect { issue.toggle_award_emoji("thumbsdown", award_emoji.user) }.to change { AwardEmoji.count }.by(-1) - end - end - describe 'querying award_emoji on an Awardable' do let(:issue) { create(:issue) } diff --git a/spec/services/award_emojis/add_service_spec.rb b/spec/services/award_emojis/add_service_spec.rb new file mode 100644 index 00000000000..037db39ba80 --- /dev/null +++ b/spec/services/award_emojis/add_service_spec.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe AwardEmojis::AddService do + set(:user) { create(:user) } + set(:project) { create(:project) } + set(:awardable) { create(:note, project: project) } + let(:name) { 'thumbsup' } + subject(:service) { described_class.new(awardable, name, user) } + + describe '#execute' do + context 'when user is not authorized' do + it 'does not add an emoji' do + expect { service.execute }.not_to change { AwardEmoji.count } + end + + it 'returns an error state' do + result = service.execute + + expect(result[:status]).to eq(:error) + expect(result[:http_status]).to eq(:forbidden) + end + end + + context 'when user is authorized' do + before do + project.add_developer(user) + end + + it 'creates an award emoji' do + expect { service.execute }.to change { AwardEmoji.count }.by(1) + end + + it 'returns the award emoji' do + result = service.execute + + expect(result[:award]).to be_kind_of(AwardEmoji) + end + + it 'return a success status' do + result = service.execute + + expect(result[:status]).to eq(:success) + end + + it 'sets the correct properties on the award emoji' do + award = service.execute[:award] + + expect(award.name).to eq(name) + expect(award.user).to eq(user) + end + + describe 'marking Todos as done' do + subject { service.execute } + + include_examples 'creating award emojis marks Todos as done' + end + + context 'when the awardable cannot have emoji awarded to it' do + before do + expect(awardable).to receive(:emoji_awardable?).and_return(false) + end + + it 'does not add an emoji' do + expect { service.execute }.not_to change { AwardEmoji.count } + end + + it 'returns an error status' do + result = service.execute + + expect(result[:status]).to eq(:error) + expect(result[:http_status]).to eq(:unprocessable_entity) + end + end + + context 'when the awardable is invalid' do + before do + expect_next_instance_of(AwardEmoji) do |award| + expect(award).to receive(:valid?).and_return(false) + expect(award).to receive_message_chain(:errors, :full_messages).and_return(['Error 1', 'Error 2']) + end + end + + it 'does not add an emoji' do + expect { service.execute }.not_to change { AwardEmoji.count } + end + + it 'returns an error status' do + result = service.execute + + expect(result[:status]).to eq(:error) + end + + it 'returns an error message' do + result = service.execute + + expect(result[:message]).to eq('Error 1 and Error 2') + end + end + end + end +end diff --git a/spec/services/award_emojis/destroy_service_spec.rb b/spec/services/award_emojis/destroy_service_spec.rb new file mode 100644 index 00000000000..c4a7d5ec20e --- /dev/null +++ b/spec/services/award_emojis/destroy_service_spec.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe AwardEmojis::DestroyService do + set(:user) { create(:user) } + set(:awardable) { create(:note) } + set(:project) { awardable.project } + let(:name) { 'thumbsup' } + let!(:award_from_other_user) do + create(:award_emoji, name: name, awardable: awardable, user: create(:user)) + end + subject(:service) { described_class.new(awardable, name, user) } + + describe '#execute' do + shared_examples_for 'a service that does not authorize the user' do |error:| + it 'does not remove the emoji' do + expect { service.execute }.not_to change { AwardEmoji.count } + end + + it 'returns an error state' do + result = service.execute + + expect(result[:status]).to eq(:error) + expect(result[:http_status]).to eq(:forbidden) + end + + it 'returns a nil award' do + result = service.execute + + expect(result).to have_key(:award) + expect(result[:award]).to be_nil + end + + it 'returns the error' do + result = service.execute + + expect(result[:message]).to eq(error) + expect(result[:errors]).to eq([error]) + end + end + + context 'when user is not authorized' do + it_behaves_like 'a service that does not authorize the user', + error: 'User cannot destroy emoji on the awardable' + end + + context 'when the user is authorized' do + before do + project.add_developer(user) + end + + context 'when user has not awarded an emoji to the awardable' do + let!(:award_from_user) { create(:award_emoji, name: name, user: user) } + + it_behaves_like 'a service that does not authorize the user', + error: 'User has not awarded emoji of type thumbsup on the awardable' + end + + context 'when user has awarded an emoji to the awardable' do + let!(:award_from_user) { create(:award_emoji, name: name, awardable: awardable, user: user) } + + it 'removes the emoji' do + expect { service.execute }.to change { AwardEmoji.count }.by(-1) + end + + it 'returns a success status' do + result = service.execute + + expect(result[:status]).to eq(:success) + end + + it 'returns no errors' do + result = service.execute + + expect(result).not_to have_key(:error) + expect(result).not_to have_key(:errors) + end + + it 'returns the destroyed award' do + result = service.execute + + expect(result[:award]).to eq(award_from_user) + expect(result[:award]).to be_destroyed + end + end + end + end +end diff --git a/spec/services/award_emojis/toggle_service_spec.rb b/spec/services/award_emojis/toggle_service_spec.rb new file mode 100644 index 00000000000..972a1d5fc06 --- /dev/null +++ b/spec/services/award_emojis/toggle_service_spec.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe AwardEmojis::ToggleService do + set(:user) { create(:user) } + set(:project) { create(:project, :public) } + set(:awardable) { create(:note, project: project) } + let(:name) { 'thumbsup' } + subject(:service) { described_class.new(awardable, name, user) } + + describe '#execute' do + context 'when user has awarded an emoji' do + let!(:award_from_other_user) { create(:award_emoji, name: name, awardable: awardable, user: create(:user)) } + let!(:award) { create(:award_emoji, name: name, awardable: awardable, user: user) } + + it 'calls AwardEmojis::DestroyService' do + expect(AwardEmojis::AddService).not_to receive(:new) + + expect_next_instance_of(AwardEmojis::DestroyService) do |service| + expect(service).to receive(:execute) + end + + service.execute + end + + it 'destroys an AwardEmoji' do + expect { service.execute }.to change { AwardEmoji.count }.by(-1) + end + + it 'returns the result of DestroyService#execute' do + mock_result = double(foo: true) + + expect_next_instance_of(AwardEmojis::DestroyService) do |service| + expect(service).to receive(:execute).and_return(mock_result) + end + + result = service.execute + + expect(result).to eq(mock_result) + end + end + + context 'when user has not awarded an emoji' do + it 'calls AwardEmojis::AddService' do + expect_next_instance_of(AwardEmojis::AddService) do |service| + expect(service).to receive(:execute) + end + + expect(AwardEmojis::DestroyService).not_to receive(:new) + + service.execute + end + + it 'creates an AwardEmoji' do + expect { service.execute }.to change { AwardEmoji.count }.by(1) + end + + it 'returns the result of AddService#execute' do + mock_result = double(foo: true) + + expect_next_instance_of(AwardEmojis::AddService) do |service| + expect(service).to receive(:execute).and_return(mock_result) + end + + result = service.execute + + expect(result).to eq(mock_result) + end + end + end +end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index b0b74407812..d4fa62fa85d 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -78,6 +78,7 @@ describe Projects::CreateService, '#execute' do expect(project).to be_valid expect(project.owner).to eq(group) expect(project.namespace).to eq(group) + expect(project.team.owners).to include(user) expect(user.authorized_projects).to include(project) end end diff --git a/spec/support/shared_examples/award_emoji_todo_shared_examples.rb b/spec/support/shared_examples/award_emoji_todo_shared_examples.rb new file mode 100644 index 00000000000..88ad37d232f --- /dev/null +++ b/spec/support/shared_examples/award_emoji_todo_shared_examples.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +# Shared examples to that test code that creates AwardEmoji also mark Todos +# as done. +# +# The examples expect these to be defined in the calling spec: +# - `subject` the callable code that executes the creation of an AwardEmoji +# - `user` +# - `project` +RSpec.shared_examples 'creating award emojis marks Todos as done' do + using RSpec::Parameterized::TableSyntax + + before do + project.add_developer(user) + end + + where(:type, :expectation) do + :issue | true + :merge_request | true + :project_snippet | false + end + + with_them do + let(:project) { awardable.project } + let(:awardable) { create(type) } + let!(:todo) { create(:todo, target: awardable, project: project, user: user) } + + it do + subject + + expect(todo.reload.done?).to eq(expectation) + end + end + + # Notes have more complicated rules than other Todoables + describe 'for notes' do + let!(:todo) { create(:todo, target: awardable.noteable, project: project, user: user) } + + context 'regular Notes' do + let(:awardable) { create(:note, project: project) } + + it 'marks the Todo as done' do + subject + + expect(todo.reload.done?).to eq(true) + end + end + + context 'PersonalSnippet Notes' do + let(:awardable) { create(:note, noteable: create(:personal_snippet, author: user)) } + + it 'does not mark the Todo as done' do + subject + + expect(todo.reload.done?).to eq(false) + end + end + end +end -- GitLab