From a2718ebaffd431b1eec91f3ee467d9632309b5f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Fri, 4 Aug 2017 19:14:04 +0200 Subject: [PATCH 1/2] Make user/author use project.creator in most factories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- spec/factories/commits.rb | 2 +- spec/factories/deployments.rb | 3 ++- spec/factories/events.rb | 2 +- spec/factories/issues.rb | 2 +- spec/factories/merge_requests.rb | 2 +- spec/factories/notes.rb | 2 +- spec/factories/project_wikis.rb | 2 +- spec/factories/sent_notifications.rb | 2 +- spec/factories/snippets.rb | 1 + spec/factories/subscriptions.rb | 2 +- spec/factories/timelogs.rb | 2 +- spec/factories/todos.rb | 4 ++-- .../features/dashboard/merge_requests_spec.rb | 22 ++++++++++--------- .../merge_request/user_awards_emoji_spec.rb | 2 +- .../slash_commands/issue_search_spec.rb | 2 +- spec/policies/project_policy_spec.rb | 2 +- spec/services/issues/close_service_spec.rb | 2 +- spec/services/issues/update_service_spec.rb | 3 ++- .../merge_requests/close_service_spec.rb | 2 +- .../merge_requests/ff_merge_service_spec.rb | 3 ++- .../merge_requests/reopen_service_spec.rb | 2 +- .../merge_requests/update_service_spec.rb | 3 ++- spec/services/notification_service_spec.rb | 6 +++-- 23 files changed, 42 insertions(+), 33 deletions(-) diff --git a/spec/factories/commits.rb b/spec/factories/commits.rb index 84a8bc56640..d5d819d862a 100644 --- a/spec/factories/commits.rb +++ b/spec/factories/commits.rb @@ -23,7 +23,7 @@ FactoryBot.define do end after(:build) do |commit, evaluator| - allow(commit).to receive(:author).and_return(evaluator.author || build(:author)) + allow(commit).to receive(:author).and_return(evaluator.author || build_stubbed(:author)) end trait :without_author do diff --git a/spec/factories/deployments.rb b/spec/factories/deployments.rb index 9d7d5e56611..cac56695319 100644 --- a/spec/factories/deployments.rb +++ b/spec/factories/deployments.rb @@ -3,13 +3,14 @@ FactoryBot.define do sha '97de212e80737a608d939f648d959671fb0a0142' ref 'master' tag false - user + user nil project nil deployable factory: :ci_build environment factory: :environment after(:build) do |deployment, evaluator| deployment.project ||= deployment.environment.project + deployment.user ||= deployment.project.creator unless deployment.project.repository_exists? allow(deployment.project.repository).to receive(:create_ref) diff --git a/spec/factories/events.rb b/spec/factories/events.rb index ed275243ac9..5798b81ecad 100644 --- a/spec/factories/events.rb +++ b/spec/factories/events.rb @@ -1,7 +1,7 @@ FactoryBot.define do factory :event do project - author factory: :user + author(factory: :user) { project.creator } action Event::JOINED trait(:created) { action Event::CREATED } diff --git a/spec/factories/issues.rb b/spec/factories/issues.rb index 71dc169c6a2..998080a3dd5 100644 --- a/spec/factories/issues.rb +++ b/spec/factories/issues.rb @@ -1,8 +1,8 @@ FactoryBot.define do factory :issue do title { generate(:title) } - author project + author { project.creator } trait :confidential do confidential true diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index 40558c88d15..d26cb0c3417 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -1,9 +1,9 @@ FactoryBot.define do factory :merge_request do title { generate(:title) } - author association :source_project, :repository, factory: :project target_project { source_project } + author { source_project.creator } # $ git log --pretty=oneline feature..master # 5937ac0a7beb003549fc5fd26fc247adbce4a52e Add submodule from gitlab.com diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb index 707ecbd6be5..2defb4935ad 100644 --- a/spec/factories/notes.rb +++ b/spec/factories/notes.rb @@ -6,7 +6,7 @@ FactoryBot.define do factory :note do project note { generate(:title) } - author + author { project&.creator || create(:user) } on_issue factory :note_on_commit, traits: [:on_commit] diff --git a/spec/factories/project_wikis.rb b/spec/factories/project_wikis.rb index 89d8248f9f4..db2eb4fc863 100644 --- a/spec/factories/project_wikis.rb +++ b/spec/factories/project_wikis.rb @@ -3,7 +3,7 @@ FactoryBot.define do skip_create project - user factory: :user + user { project.creator } initialize_with { new(project, user) } end end diff --git a/spec/factories/sent_notifications.rb b/spec/factories/sent_notifications.rb index 80872067233..b0174dd06b7 100644 --- a/spec/factories/sent_notifications.rb +++ b/spec/factories/sent_notifications.rb @@ -1,7 +1,7 @@ FactoryBot.define do factory :sent_notification do project - recipient factory: :user + recipient { project.creator } noteable { create(:issue, project: project) } reply_key { SentNotification.reply_key } end diff --git a/spec/factories/snippets.rb b/spec/factories/snippets.rb index 2ab9a56d255..dc12b562108 100644 --- a/spec/factories/snippets.rb +++ b/spec/factories/snippets.rb @@ -21,6 +21,7 @@ FactoryBot.define do factory :project_snippet, parent: :snippet, class: :ProjectSnippet do project + author { project.creator } end factory :personal_snippet, parent: :snippet, class: :PersonalSnippet do diff --git a/spec/factories/subscriptions.rb b/spec/factories/subscriptions.rb index a4bc4e87b0a..8f7ab74ec70 100644 --- a/spec/factories/subscriptions.rb +++ b/spec/factories/subscriptions.rb @@ -1,7 +1,7 @@ FactoryBot.define do factory :subscription do - user project + user { project.creator } subscribable factory: :issue end end diff --git a/spec/factories/timelogs.rb b/spec/factories/timelogs.rb index af34b0681e2..b45f06b9a0a 100644 --- a/spec/factories/timelogs.rb +++ b/spec/factories/timelogs.rb @@ -3,7 +3,7 @@ FactoryBot.define do factory :timelog do time_spent 3600 - user issue + user { issue.project.creator } end end diff --git a/spec/factories/todos.rb b/spec/factories/todos.rb index 6a6de665dd1..94f8caedfa6 100644 --- a/spec/factories/todos.rb +++ b/spec/factories/todos.rb @@ -1,8 +1,8 @@ FactoryBot.define do factory :todo do project - author - user + author { project.creator } + user { project.creator } target factory: :issue action { Todo::ASSIGNED } diff --git a/spec/features/dashboard/merge_requests_spec.rb b/spec/features/dashboard/merge_requests_spec.rb index 991d360ccaf..744041ac425 100644 --- a/spec/features/dashboard/merge_requests_spec.rb +++ b/spec/features/dashboard/merge_requests_spec.rb @@ -44,36 +44,38 @@ feature 'Dashboard Merge Requests' do context 'merge requests exist' do let!(:assigned_merge_request) do - create(:merge_request, assignee: current_user, target_project: project, source_project: project) + create(:merge_request, + assignee: current_user, + source_project: project, + author: create(:user)) end let!(:assigned_merge_request_from_fork) do create(:merge_request, source_branch: 'markdown', assignee: current_user, - target_project: public_project, source_project: forked_project - ) + target_project: public_project, source_project: forked_project, + author: create(:user)) end let!(:authored_merge_request) do create(:merge_request, - source_branch: 'markdown', author: current_user, - target_project: project, source_project: project - ) + source_branch: 'markdown', + source_project: project, + author: current_user) end let!(:authored_merge_request_from_fork) do create(:merge_request, source_branch: 'feature_conflict', author: current_user, - target_project: public_project, source_project: forked_project - ) + target_project: public_project, source_project: forked_project) end let!(:other_merge_request) do create(:merge_request, source_branch: 'fix', - target_project: project, source_project: project - ) + source_project: project, + author: create(:user)) end before do diff --git a/spec/features/merge_request/user_awards_emoji_spec.rb b/spec/features/merge_request/user_awards_emoji_spec.rb index 15a0878fb16..2f24cfbd9e3 100644 --- a/spec/features/merge_request/user_awards_emoji_spec.rb +++ b/spec/features/merge_request/user_awards_emoji_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' describe 'Merge request > User awards emoji', :js do let(:project) { create(:project, :public, :repository) } let(:user) { project.creator } - let(:merge_request) { create(:merge_request, source_project: project) } + let(:merge_request) { create(:merge_request, source_project: project, author: create(:user)) } describe 'logged in' do before do diff --git a/spec/lib/gitlab/slash_commands/issue_search_spec.rb b/spec/lib/gitlab/slash_commands/issue_search_spec.rb index e41e5254dde..35d01efc1bd 100644 --- a/spec/lib/gitlab/slash_commands/issue_search_spec.rb +++ b/spec/lib/gitlab/slash_commands/issue_search_spec.rb @@ -5,7 +5,7 @@ describe Gitlab::SlashCommands::IssueSearch do let!(:issue) { create(:issue, project: project, title: 'find me') } let!(:confidential) { create(:issue, :confidential, project: project, title: 'mepmep find') } let(:project) { create(:project) } - let(:user) { issue.author } + let(:user) { create(:user) } let(:regex_match) { described_class.match("issue search find") } subject do diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index f2593a1a75c..129344f105f 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -92,7 +92,7 @@ describe ProjectPolicy do it 'does not include the read_issue permission when the issue author is not a member of the private project' do project = create(:project, :private) - issue = create(:issue, project: project) + issue = create(:issue, project: project, author: create(:user)) user = issue.author expect(project.team.member?(issue.author)).to be false diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb index 8897a64a138..47c1ebbeb81 100644 --- a/spec/services/issues/close_service_spec.rb +++ b/spec/services/issues/close_service_spec.rb @@ -4,7 +4,7 @@ describe Issues::CloseService do let(:user) { create(:user) } let(:user2) { create(:user) } let(:guest) { create(:user) } - let(:issue) { create(:issue, assignees: [user2]) } + let(:issue) { create(:issue, assignees: [user2], author: create(:user)) } let(:project) { issue.project } let!(:todo) { create(:todo, :assigned, user: user, project: project, target: issue, author: user2) } diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 1cb6f2e097f..41237dd7160 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -13,7 +13,8 @@ describe Issues::UpdateService, :mailer do create(:issue, title: 'Old title', description: "for #{user2.to_reference}", assignee_ids: [user3.id], - project: project) + project: project, + author: create(:user)) end before do diff --git a/spec/services/merge_requests/close_service_spec.rb b/spec/services/merge_requests/close_service_spec.rb index 4d12de3ecce..216e0cd4266 100644 --- a/spec/services/merge_requests/close_service_spec.rb +++ b/spec/services/merge_requests/close_service_spec.rb @@ -4,7 +4,7 @@ describe MergeRequests::CloseService do let(:user) { create(:user) } let(:user2) { create(:user) } let(:guest) { create(:user) } - let(:merge_request) { create(:merge_request, assignee: user2) } + let(:merge_request) { create(:merge_request, assignee: user2, author: create(:user)) } let(:project) { merge_request.project } let!(:todo) { create(:todo, :assigned, user: user, project: project, target: merge_request, author: user2) } diff --git a/spec/services/merge_requests/ff_merge_service_spec.rb b/spec/services/merge_requests/ff_merge_service_spec.rb index aa90feeef89..5ef6365fcc9 100644 --- a/spec/services/merge_requests/ff_merge_service_spec.rb +++ b/spec/services/merge_requests/ff_merge_service_spec.rb @@ -7,7 +7,8 @@ describe MergeRequests::FfMergeService do create(:merge_request, source_branch: 'flatten-dir', target_branch: 'improve/awesome', - assignee: user2) + assignee: user2, + author: create(:user)) end let(:project) { merge_request.project } diff --git a/spec/services/merge_requests/reopen_service_spec.rb b/spec/services/merge_requests/reopen_service_spec.rb index a44d63e5f9f..9ee37c51d95 100644 --- a/spec/services/merge_requests/reopen_service_spec.rb +++ b/spec/services/merge_requests/reopen_service_spec.rb @@ -4,7 +4,7 @@ describe MergeRequests::ReopenService do let(:user) { create(:user) } let(:user2) { create(:user) } let(:guest) { create(:user) } - let(:merge_request) { create(:merge_request, :closed, assignee: user2) } + let(:merge_request) { create(:merge_request, :closed, assignee: user2, author: create(:user)) } let(:project) { merge_request.project } before do diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 2238da2d14d..c31259239ee 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -12,7 +12,8 @@ describe MergeRequests::UpdateService, :mailer do create(:merge_request, :simple, title: 'Old title', description: "FYI #{user2.to_reference}", assignee_id: user3.id, - source_project: project) + source_project: project, + author: create(:user)) end before do diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 5c59455e3e1..35eb84e5e88 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -458,7 +458,7 @@ describe NotificationService, :mailer do context "merge request diff note" do let(:project) { create(:project, :repository) } let(:user) { create(:user) } - let(:merge_request) { create(:merge_request, source_project: project, assignee: user) } + let(:merge_request) { create(:merge_request, source_project: project, assignee: user, author: create(:user)) } let(:note) { create(:diff_note_on_merge_request, project: project, noteable: merge_request) } before do @@ -469,11 +469,13 @@ describe NotificationService, :mailer do describe '#new_note' do it "records sent notifications" do - # Ensure create SentNotification by noteable = merge_request 6 times, not noteable = note + # 3 SentNotification are sent: the MR assignee and author, and the @u_watcher expect(SentNotification).to receive(:record_note).with(note, any_args).exactly(3).times.and_call_original notification.new_note(note) + expect(SentNotification.last(3).map(&:recipient).map(&:id)) + .to contain_exactly(merge_request.assignee.id, merge_request.author.id, @u_watcher.id) expect(SentNotification.last.in_reply_to_discussion_id).to eq(note.discussion_id) end end From c6311207eef6cf66f77d410269efc7581288cb2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Wed, 31 Jan 2018 18:30:31 +0100 Subject: [PATCH 2/2] Fix a JSON schema that doesn't include enough fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- spec/fixtures/api/schemas/public_api/v4/user/basic.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/fixtures/api/schemas/public_api/v4/user/basic.json b/spec/fixtures/api/schemas/public_api/v4/user/basic.json index bf330d8278c..2d815be32a6 100644 --- a/spec/fixtures/api/schemas/public_api/v4/user/basic.json +++ b/spec/fixtures/api/schemas/public_api/v4/user/basic.json @@ -2,12 +2,16 @@ "type": ["object", "null"], "required": [ "id", + "name", + "username", "state", "avatar_url", "web_url" ], "properties": { "id": { "type": "integer" }, + "name": { "type": "string" }, + "username": { "type": "string" }, "state": { "type": "string" }, "avatar_url": { "type": "string" }, "web_url": { "type": "string" }