From 7a828915238d9aaa13b86a9ba2c561c1e39b0f2f Mon Sep 17 00:00:00 2001
From: Mycroft Kang <taet777@naver.com>
Date: Wed, 16 Sep 2020 18:57:31 +0900
Subject: [PATCH] Add Contributor and Author badges on notes
---
.../notes/components/note_actions.vue | 61 +++++++++++-
.../notes/components/noteable_note.vue | 4 +
app/controllers/concerns/renders_notes.rb | 10 +-
app/helpers/issuables_helper.rb | 6 ++
app/helpers/notes_helper.rb | 4 +
app/models/merge_request.rb | 2 +-
app/models/note.rb | 39 ++------
app/models/project_team.rb | 34 +++++++
app/serializers/note_entity.rb | 4 +
app/serializers/project_note_entity.rb | 8 ++
app/views/projects/notes/_actions.html.haml | 13 +--
...contributor-and-author-badges-on-notes.yml | 5 +
.../development/show_author_on_note.yml | 7 ++
.../development/show_contributor_on_note.yml | 7 ++
locale/gitlab.pot | 15 ++-
.../api/schemas/entities/discussion.json | 3 +
.../notes/components/note_actions_spec.js | 34 ++++++-
spec/models/note_spec.rb | 66 +++++++++----
spec/models/project_team_spec.rb | 95 ++++++++++++++++++-
19 files changed, 348 insertions(+), 69 deletions(-)
create mode 100644 changelogs/unreleased/display-contributor-and-author-badges-on-notes.yml
create mode 100644 config/feature_flags/development/show_author_on_note.yml
create mode 100644 config/feature_flags/development/show_contributor_on_note.yml
diff --git a/app/assets/javascripts/notes/components/note_actions.vue b/app/assets/javascripts/notes/components/note_actions.vue
index 643145651e53e4ca..a8057276f1a8854c 100644
--- a/app/assets/javascripts/notes/components/note_actions.vue
+++ b/app/assets/javascripts/notes/components/note_actions.vue
@@ -1,12 +1,13 @@
<script>
import { mapGetters } from 'vuex';
import { GlLoadingIcon, GlTooltipDirective, GlIcon } from '@gitlab/ui';
-import { __ } from '~/locale';
+import { __, sprintf } from '~/locale';
import resolvedStatusMixin from '~/batch_comments/mixins/resolved_status';
import ReplyButton from './note_actions/reply_button.vue';
import eventHub from '~/sidebar/event_hub';
import Api from '~/api';
import { deprecatedCreateFlash as flash } from '~/flash';
+import { splitCamelCase } from '../../lib/utils/text_utility';
export default {
name: 'NoteActions',
@@ -47,6 +48,26 @@ export default {
required: false,
default: null,
},
+ isAuthor: {
+ type: Boolean,
+ required: false,
+ default: false,
+ },
+ isContributor: {
+ type: Boolean,
+ required: false,
+ default: false,
+ },
+ noteableType: {
+ type: String,
+ required: false,
+ default: '',
+ },
+ projectName: {
+ type: String,
+ required: false,
+ default: '',
+ },
showReply: {
type: Boolean,
required: true,
@@ -121,6 +142,9 @@ export default {
targetType() {
return this.getNoteableData.targetType;
},
+ noteableDisplayName() {
+ return splitCamelCase(this.noteableType).toLowerCase();
+ },
assignees() {
return this.getNoteableData.assignees || [];
},
@@ -130,6 +154,22 @@ export default {
canAssign() {
return this.getNoteableData.current_user?.can_update && this.isIssue;
},
+ displayAuthorBadgeText() {
+ return sprintf(__('This user is the author of this %{noteable}.'), {
+ noteable: this.noteableDisplayName,
+ });
+ },
+ displayMemberBadgeText() {
+ return sprintf(__('This user is a %{access} of the %{name} project.'), {
+ access: this.accessLevel.toLowerCase(),
+ name: this.projectName,
+ });
+ },
+ displayContributorBadgeText() {
+ return sprintf(__('This user has previously committed to the %{name} project.'), {
+ name: this.projectName,
+ });
+ },
},
methods: {
onEdit() {
@@ -175,7 +215,24 @@ export default {
<template>
<div class="note-actions">
- <span v-if="accessLevel" class="note-role user-access-role">{{ accessLevel }}</span>
+ <span
+ v-if="isAuthor"
+ class="note-role user-access-role has-tooltip d-none d-md-inline-block"
+ :title="displayAuthorBadgeText"
+ >{{ __('Author') }}</span
+ >
+ <span
+ v-if="accessLevel"
+ class="note-role user-access-role has-tooltip"
+ :title="displayMemberBadgeText"
+ >{{ accessLevel }}</span
+ >
+ <span
+ v-else-if="isContributor"
+ class="note-role user-access-role has-tooltip"
+ :title="displayContributorBadgeText"
+ >{{ __('Contributor') }}</span
+ >
<div v-if="canResolve" class="note-actions-item">
<button
ref="resolveButton"
diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue
index 180d12c9e210d5cc..4f45fcb0062c6c6e 100644
--- a/app/assets/javascripts/notes/components/noteable_note.vue
+++ b/app/assets/javascripts/notes/components/noteable_note.vue
@@ -389,6 +389,10 @@ export default {
:note-id="note.id"
:note-url="note.noteable_note_url"
:access-level="note.human_access"
+ :is-contributor="note.is_contributor"
+ :is-author="note.is_noteable_author"
+ :project-name="note.project_name"
+ :noteable-type="note.noteable_type"
:show-reply="showReplyButton"
:can-edit="note.current_user.can_edit"
:can-award-emoji="note.current_user.can_award_emoji"
diff --git a/app/controllers/concerns/renders_notes.rb b/app/controllers/concerns/renders_notes.rb
index 18015b1de8850b97..f8e3717acee5f234 100644
--- a/app/controllers/concerns/renders_notes.rb
+++ b/app/controllers/concerns/renders_notes.rb
@@ -5,7 +5,6 @@ module RendersNotes
def prepare_notes_for_rendering(notes, noteable = nil)
preload_noteable_for_regular_notes(notes)
preload_max_access_for_authors(notes, @project)
- preload_first_time_contribution_for_authors(noteable, notes)
preload_author_status(notes)
Notes::RenderService.new(current_user).execute(notes)
@@ -19,7 +18,8 @@ def preload_max_access_for_authors(notes, project)
return unless project
user_ids = notes.map(&:author_id)
- project.team.max_member_access_for_user_ids(user_ids)
+ access = project.team.max_member_access_for_user_ids(user_ids).select { |k, v| v == Gitlab::Access::NO_ACCESS }.keys
+ project.team.contribution_check_for_user_ids(access)
end
# rubocop: disable CodeReuse/ActiveRecord
@@ -28,12 +28,6 @@ def preload_noteable_for_regular_notes(notes)
end
# rubocop: enable CodeReuse/ActiveRecord
- def preload_first_time_contribution_for_authors(noteable, notes)
- return unless noteable.is_a?(Issuable) && noteable.first_contribution?
-
- notes.each {|n| n.specialize_for_first_contribution!(noteable)}
- end
-
# rubocop: disable CodeReuse/ActiveRecord
def preload_author_status(notes)
ActiveRecord::Associations::Preloader.new.preload(notes, { author: :status })
diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb
index 398e76b6697339bf..931183ea0a2e3685 100644
--- a/app/helpers/issuables_helper.rb
+++ b/app/helpers/issuables_helper.rb
@@ -205,6 +205,12 @@ def issuable_meta(issuable, project, text)
author_output
end
+ if access = project.team.human_max_access(issuable.author_id)
+ output << content_tag(:span, access, class: "user-access-role has-tooltip d-none d-xl-inline-block gl-ml-3 ", title: _("This user is a %{access} of the %{name} project.") % { access: access.downcase, name: project.name })
+ elsif project.team.contributor?(issuable.author_id)
+ output << content_tag(:span, _("Contributor"), class: "user-access-role has-tooltip d-none d-xl-inline-block gl-ml-3", title: _("This user has previously committed to the %{name} project.") % { name: project.name })
+ end
+
output << content_tag(:span, (sprite_icon('first-contribution', css_class: 'gl-icon gl-vertical-align-middle') if issuable.first_contribution?), class: 'has-tooltip gl-ml-2', title: _('1st contribution!'))
output << content_tag(:span, (issuable.task_status if issuable.tasks?), id: "task_status", class: "d-none d-sm-none d-md-inline-block gl-ml-3")
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index 2ba1d841c2e25165..66a62f8d1fc8d541 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -85,6 +85,10 @@ def note_max_access_for_user(note)
note.project.team.max_member_access(note.author_id)
end
+ def note_human_max_access(note)
+ note.project.team.human_max_access(note.author_id)
+ end
+
def discussion_path(discussion)
if discussion.for_merge_request?
return unless discussion.diff_discussion?
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 1f0d618d52776fc1..b510e4707156e5ff 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -1600,7 +1600,7 @@ def update_project_counter_caches
def first_contribution?
return false if project.team.max_member_access(author_id) > Gitlab::Access::GUEST
- project.merge_requests.merged.where(author_id: author_id).empty?
+ !project.merge_requests.merged.exists?(author_id: author_id)
end
# TODO: remove once production database rename completes
diff --git a/app/models/note.rb b/app/models/note.rb
index 7330b97bd76102bd..812d77d5f86d7be8 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -20,20 +20,6 @@ class Note < ApplicationRecord
include ThrottledTouch
include FromUnion
- module SpecialRole
- FIRST_TIME_CONTRIBUTOR = :first_time_contributor
-
- class << self
- def values
- constants.map {|const| self.const_get(const, false)}
- end
-
- def value?(val)
- values.include?(val)
- end
- end
- end
-
cache_markdown_field :note, pipeline: :note, issuable_state_filter_enabled: true
redact_field :note
@@ -60,9 +46,6 @@ def value?(val)
# Attribute used to store the attributes that have been changed by quick actions.
attr_accessor :commands_changes
- # A special role that may be displayed on issuable's discussions
- attr_reader :special_role
-
default_value_for :system, false
attr_mentionable :note, pipeline: :note
@@ -220,10 +203,6 @@ def count_for_collection(ids, type)
.where(noteable_type: type, noteable_id: ids)
end
- def has_special_role?(role, note)
- note.special_role == role
- end
-
def search(query)
fuzzy_search(query, [:note])
end
@@ -342,20 +321,20 @@ def noteable_assignee_or_author?(user)
noteable.author_id == user.id
end
- def special_role=(role)
- raise "Role is undefined, #{role} not found in #{SpecialRole.values}" unless SpecialRole.value?(role)
+ def contributor?
+ return false unless ::Feature.enabled?(:show_contributor_on_note, project)
- @special_role = role
+ project&.team&.contributor?(self.author_id)
end
- def has_special_role?(role)
- self.class.has_special_role?(role, self)
- end
+ def noteable_author?(noteable)
+ return false unless ::Feature.enabled?(:show_author_on_note, project)
- def specialize_for_first_contribution!(noteable)
- return unless noteable.author_id == self.author_id
+ noteable.author == self.author
+ end
- self.special_role = Note::SpecialRole::FIRST_TIME_CONTRIBUTOR
+ def project_name
+ project&.name
end
def confidential?(include_noteable: false)
diff --git a/app/models/project_team.rb b/app/models/project_team.rb
index 072d281e5f8b4582..5b7eded00cd0f83f 100644
--- a/app/models/project_team.rb
+++ b/app/models/project_team.rb
@@ -178,6 +178,40 @@ def max_member_access(user_id)
max_member_access_for_user_ids([user_id])[user_id]
end
+ def contribution_check_for_user_ids(user_ids)
+ user_ids = user_ids.uniq
+ key = "contribution_check_for_users:#{project.id}"
+
+ Gitlab::SafeRequestStore[key] ||= {}
+ contributors = Gitlab::SafeRequestStore[key] || {}
+
+ user_ids -= contributors.keys
+
+ return contributors if user_ids.empty?
+
+ resource_contributors = project.merge_requests
+ .merged
+ .where(author_id: user_ids, target_branch: project.default_branch.to_s)
+ .pluck(:author_id)
+ .product([true]).to_h
+
+ contributors.merge!(resource_contributors)
+
+ missing_resource_ids = user_ids - resource_contributors.keys
+
+ missing_resource_ids.each do |resource_id|
+ contributors[resource_id] = false
+ end
+
+ contributors
+ end
+
+ def contributor?(user_id)
+ return false if max_member_access(user_id) >= Gitlab::Access::GUEST
+
+ contribution_check_for_user_ids([user_id])[user_id]
+ end
+
private
def fetch_members(level = nil)
diff --git a/app/serializers/note_entity.rb b/app/serializers/note_entity.rb
index c49dec2a93c27af3..ef305195e22b40e6 100644
--- a/app/serializers/note_entity.rb
+++ b/app/serializers/note_entity.rb
@@ -46,6 +46,10 @@ class NoteEntity < API::Entities::Note
SystemNoteHelper.system_note_icon_name(note)
end
+ expose :is_noteable_author do |note|
+ note.noteable_author?(request.noteable)
+ end
+
expose :discussion_id do |note|
note.discussion_id(request.noteable)
end
diff --git a/app/serializers/project_note_entity.rb b/app/serializers/project_note_entity.rb
index f6cdea1d8b5a5624..3cd34f6af0de8ca8 100644
--- a/app/serializers/project_note_entity.rb
+++ b/app/serializers/project_note_entity.rb
@@ -5,6 +5,14 @@ class ProjectNoteEntity < NoteEntity
note.project.team.human_max_access(note.author_id)
end
+ expose :is_contributor, if: -> (note, _) { note.project.present? } do |note|
+ note.contributor?
+ end
+
+ expose :project_name, if: -> (note, _) { note.project.present? } do |note|
+ note.project.name
+ end
+
expose :toggle_award_path, if: -> (note, _) { note.emoji_awardable? } do |note|
toggle_award_emoji_project_note_path(note.project, note.id)
end
diff --git a/app/views/projects/notes/_actions.html.haml b/app/views/projects/notes/_actions.html.haml
index 66721a28e62b3971..058366eb75d7c32b 100644
--- a/app/views/projects/notes/_actions.html.haml
+++ b/app/views/projects/notes/_actions.html.haml
@@ -1,9 +1,10 @@
-- access = note_max_access_for_user(note)
-- if note.has_special_role?(Note::SpecialRole::FIRST_TIME_CONTRIBUTOR)
- %span.note-role.note-role-special.has-tooltip{ title: _("This is the author's first Merge Request to this project.") }
- = sprite_icon('first-contribution', css_class: 'gl-icon gl-vertical-align-top')
-- if access.nonzero?
- %span.note-role.user-access-role= Gitlab::Access.human_access(access)
+- access = note_human_max_access(note)
+- if note.noteable_author?(@noteable)
+ %span{ class: 'note-role user-access-role has-tooltip d-none d-md-inline-block', title: _("This user is the author of this %{noteable}.") % { noteable: @noteable.human_class_name } }= _("Author")
+- if access
+ %span{ class: 'note-role user-access-role has-tooltip', title: _("This user is a %{access} of the %{name} project.") % { access: access.downcase, name: note.project_name } }= access
+- elsif note.contributor?
+ %span{ class: 'note-role user-access-role has-tooltip', title: _("This user has previously committed to the %{name} project.") % { name: note.project_name } }= _("Contributor")
- if note.resolvable?
- can_resolve = can?(current_user, :resolve_note, note)
diff --git a/changelogs/unreleased/display-contributor-and-author-badges-on-notes.yml b/changelogs/unreleased/display-contributor-and-author-badges-on-notes.yml
new file mode 100644
index 0000000000000000..0832052dd1767d8c
--- /dev/null
+++ b/changelogs/unreleased/display-contributor-and-author-badges-on-notes.yml
@@ -0,0 +1,5 @@
+---
+title: Display Contributor and Author badges on notes
+merge_request: 40198
+author: Mycroft Kang @TaehyeokKang
+type: added
diff --git a/config/feature_flags/development/show_author_on_note.yml b/config/feature_flags/development/show_author_on_note.yml
new file mode 100644
index 0000000000000000..1f67392a3068c646
--- /dev/null
+++ b/config/feature_flags/development/show_author_on_note.yml
@@ -0,0 +1,7 @@
+---
+name: show_author_on_note
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40198
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/250282
+group: group::project management
+type: development
+default_enabled: false
\ No newline at end of file
diff --git a/config/feature_flags/development/show_contributor_on_note.yml b/config/feature_flags/development/show_contributor_on_note.yml
new file mode 100644
index 0000000000000000..8953303724443330
--- /dev/null
+++ b/config/feature_flags/development/show_contributor_on_note.yml
@@ -0,0 +1,7 @@
+---
+name: show_contributor_on_note
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40198
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/249179
+group: group::project management
+type: development
+default_enabled: false
\ No newline at end of file
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index a43e40daecf30367..187d3196e51b12c6 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -6945,6 +6945,9 @@ msgstr ""
msgid "Contributions per group member"
msgstr ""
+msgid "Contributor"
+msgstr ""
+
msgid "Contributors"
msgstr ""
@@ -25612,9 +25615,6 @@ msgstr ""
msgid "This is a security log of important events involving your account."
msgstr ""
-msgid "This is the author's first Merge Request to this project."
-msgstr ""
-
msgid "This is the highest peak of users on your installation since the license started."
msgstr ""
@@ -25852,6 +25852,15 @@ msgstr ""
msgid "This user has no identities"
msgstr ""
+msgid "This user has previously committed to the %{name} project."
+msgstr ""
+
+msgid "This user is a %{access} of the %{name} project."
+msgstr ""
+
+msgid "This user is the author of this %{noteable}."
+msgstr ""
+
msgid "This user will be the author of all events in the activity feed that are the result of an update, like new branches being created or new commits being pushed to existing branches."
msgstr ""
diff --git a/spec/fixtures/api/schemas/entities/discussion.json b/spec/fixtures/api/schemas/entities/discussion.json
index 21d8efe0b2ba3910..2afabcc91957a403 100644
--- a/spec/fixtures/api/schemas/entities/discussion.json
+++ b/spec/fixtures/api/schemas/entities/discussion.json
@@ -60,6 +60,9 @@
"resolve_with_issue_path": { "type": "string" },
"cached_markdown_version": { "type": "integer" },
"human_access": { "type": ["string", "null"] },
+ "is_noteable_author": { "type": "boolean" },
+ "is_contributor": { "type": "boolean" },
+ "project_name": { "type": "string" },
"toggle_award_path": { "type": "string" },
"path": { "type": "string" },
"commands_changes": { "type": "object", "additionalProperties": true },
diff --git a/spec/frontend/notes/components/note_actions_spec.js b/spec/frontend/notes/components/note_actions_spec.js
index 97d1752726b1e585..a79c3bbacb70ff03 100644
--- a/spec/frontend/notes/components/note_actions_spec.js
+++ b/spec/frontend/notes/components/note_actions_spec.js
@@ -35,8 +35,12 @@ describe('noteActions', () => {
canEdit: true,
canAwardEmoji: true,
canReportAsAbuse: true,
+ isAuthor: true,
+ isContributor: false,
+ noteableType: 'MergeRequest',
noteId: '539',
noteUrl: `${TEST_HOST}/group/project/-/merge_requests/1#note_1`,
+ projectName: 'project',
reportAbusePath: `${TEST_HOST}/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F7%23note_539&user_id=26`,
showReply: false,
};
@@ -60,15 +64,43 @@ describe('noteActions', () => {
wrapper = shallowMountNoteActions(props);
});
+ it('should render noteable author badge', () => {
+ expect(
+ wrapper
+ .findAll('.note-role')
+ .at(0)
+ .text()
+ .trim(),
+ ).toEqual('Author');
+ });
+
it('should render access level badge', () => {
expect(
wrapper
- .find('.note-role')
+ .findAll('.note-role')
+ .at(1)
.text()
.trim(),
).toEqual(props.accessLevel);
});
+ it('should render contributor badge', () => {
+ wrapper.setProps({
+ accessLevel: null,
+ isContributor: true,
+ });
+
+ return wrapper.vm.$nextTick().then(() => {
+ expect(
+ wrapper
+ .findAll('.note-role')
+ .at(1)
+ .text()
+ .trim(),
+ ).toBe('Contributor');
+ });
+ });
+
it('should render emoji link', () => {
expect(wrapper.find('.js-add-award').exists()).toBe(true);
expect(wrapper.find('.js-add-award').attributes('data-position')).toBe('right');
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 3b453a1109066c15..a3417ee5fc77e3de 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -286,6 +286,56 @@ def retrieve_participants
end
end
+ describe "noteable_author?" do
+ let(:user1) { create(:user) }
+ let(:user2) { create(:user) }
+ let(:project) { create(:project, :public, :repository) }
+
+ context 'when note is on commit' do
+ let(:noteable) { create(:commit, project: project, author: user1) }
+
+ context 'if user is the noteable author' do
+ let(:note) { create(:discussion_note_on_commit, commit_id: noteable.id, project: project, author: user1) }
+ let(:diff_note) { create(:diff_note_on_commit, commit_id: noteable.id, project: project, author: user1) }
+
+ it 'returns true' do
+ expect(note.noteable_author?(noteable)).to be true
+ expect(diff_note.noteable_author?(noteable)).to be true
+ end
+ end
+
+ context 'if user is not the noteable author' do
+ let(:note) { create(:discussion_note_on_commit, commit_id: noteable.id, project: project, author: user2) }
+ let(:diff_note) { create(:diff_note_on_commit, commit_id: noteable.id, project: project, author: user2) }
+
+ it 'returns false' do
+ expect(note.noteable_author?(noteable)).to be false
+ expect(diff_note.noteable_author?(noteable)).to be false
+ end
+ end
+ end
+
+ context 'when note is on issue' do
+ let(:noteable) { create(:issue, project: project, author: user1) }
+
+ context 'if user is the noteable author' do
+ let(:note) { create(:note, noteable: noteable, author: user1, project: project) }
+
+ it 'returns true' do
+ expect(note.noteable_author?(noteable)).to be true
+ end
+ end
+
+ context 'if user is not the noteable author' do
+ let(:note) { create(:note, noteable: noteable, author: user2, project: project) }
+
+ it 'returns false' do
+ expect(note.noteable_author?(noteable)).to be false
+ end
+ end
+ end
+ end
+
describe "edited?" do
let(:note) { build(:note, updated_by_id: nil, created_at: Time.current, updated_at: Time.current + 5.hours) }
@@ -1228,22 +1278,6 @@ def expect_expiration(noteable)
end
end
- describe '#special_role=' do
- let(:role) { Note::SpecialRole::FIRST_TIME_CONTRIBUTOR }
-
- it 'assigns role' do
- subject.special_role = role
-
- expect(subject.special_role).to eq(role)
- end
-
- it 'does not assign unknown role' do
- expect { subject.special_role = :bogus }.to raise_error(/Role is undefined/)
-
- expect(subject.special_role).to be_nil
- end
- end
-
describe '#parent' do
it 'returns project for project notes' do
project = create(:project)
diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb
index 34ec856459ca3f89..bbc056889d6c2149 100644
--- a/spec/models/project_team_spec.rb
+++ b/spec/models/project_team_spec.rb
@@ -3,6 +3,8 @@
require "spec_helper"
RSpec.describe ProjectTeam do
+ include ProjectForksHelper
+
let(:maintainer) { create(:user) }
let(:reporter) { create(:user) }
let(:guest) { create(:user) }
@@ -237,6 +239,35 @@
end
end
+ describe '#contributor?' do
+ let(:project) { create(:project, :public, :repository) }
+
+ context 'when user is a member of project' do
+ before do
+ project.add_maintainer(maintainer)
+ project.add_reporter(reporter)
+ project.add_guest(guest)
+ end
+
+ it { expect(project.team.contributor?(maintainer.id)).to be false }
+ it { expect(project.team.contributor?(reporter.id)).to be false }
+ it { expect(project.team.contributor?(guest.id)).to be false }
+ end
+
+ context 'when user has at least one merge request merged into default_branch' do
+ let(:contributor) { create(:user) }
+ let(:user_without_access) { create(:user) }
+ let(:first_fork_project) { fork_project(project, contributor, repository: true) }
+
+ before do
+ create(:merge_request, :merged, author: contributor, target_project: project, source_project: first_fork_project, target_branch: project.default_branch.to_s)
+ end
+
+ it { expect(project.team.contributor?(contributor.id)).to be true }
+ it { expect(project.team.contributor?(user_without_access.id)).to be false }
+ end
+ end
+
describe '#max_member_access' do
let(:requester) { create(:user) }
@@ -366,6 +397,66 @@
end
end
+ describe '#contribution_check_for_user_ids', :request_store do
+ let(:project) { create(:project, :public, :repository) }
+ let(:contributor) { create(:user) }
+ let(:second_contributor) { create(:user) }
+ let(:user_without_access) { create(:user) }
+ let(:first_fork_project) { fork_project(project, contributor, repository: true) }
+ let(:second_fork_project) { fork_project(project, second_contributor, repository: true) }
+
+ let(:users) do
+ [contributor, second_contributor, user_without_access].map(&:id)
+ end
+
+ let(:expected) do
+ {
+ contributor.id => true,
+ second_contributor.id => true,
+ user_without_access.id => false
+ }
+ end
+
+ before do
+ create(:merge_request, :merged, author: contributor, target_project: project, source_project: first_fork_project, target_branch: project.default_branch.to_s)
+ create(:merge_request, :merged, author: second_contributor, target_project: project, source_project: second_fork_project, target_branch: project.default_branch.to_s)
+ end
+
+ def contributors(users)
+ project.team.contribution_check_for_user_ids(users)
+ end
+
+ it 'does not perform extra queries when asked for users who have already been found' do
+ contributors(users)
+
+ expect { contributors([contributor.id]) }.not_to exceed_query_limit(0)
+
+ expect(contributors([contributor.id])).to eq(expected)
+ end
+
+ it 'only requests the extra users when uncached users are passed' do
+ new_contributor = create(:user)
+ new_fork_project = fork_project(project, new_contributor, repository: true)
+ second_new_user = create(:user)
+ all_users = users + [new_contributor.id, second_new_user.id]
+ create(:merge_request, :merged, author: new_contributor, target_project: project, source_project: new_fork_project, target_branch: project.default_branch.to_s)
+
+ expected_all = expected.merge(new_contributor.id => true,
+ second_new_user.id => false)
+
+ contributors(users)
+
+ queries = ActiveRecord::QueryRecorder.new { contributors(all_users) }
+
+ expect(queries.count).to eq(1)
+ expect(contributors([new_contributor.id])).to eq(expected_all)
+ end
+
+ it 'returns correct contributors' do
+ expect(contributors(users)).to eq(expected)
+ end
+ end
+
shared_examples 'max member access for users' do
let(:project) { create(:project) }
let(:group) { create(:group) }
@@ -438,9 +529,9 @@ def access_levels(users)
it 'does not perform extra queries when asked for users who have already been found' do
access_levels(users)
- expect { access_levels(users) }.not_to exceed_query_limit(0)
+ expect { access_levels([maintainer.id]) }.not_to exceed_query_limit(0)
- expect(access_levels(users)).to eq(expected)
+ expect(access_levels([maintainer.id])).to eq(expected)
end
it 'only requests the extra users when uncached users are passed' do
--
GitLab