Merge branch 'differentiate-contributor-from-team-member' into 'master'

Include author in commit message from core team community members

See merge request gitlab-org/gitlab!69076

.gitlab/changelog_config.yml

@@ -11,6 +11,8 @@ categories:
   security: Security
   performance: Performance
   other: Other
+include_groups:
+  - gitlab-org/gitlab-core-team/community-members
 template: |
   {% if categories %}
   {% each categories %}
@@ -18,7 +20,7 @@ template: |
 
   {% each entries %}
   - [{{ title }}]({{ commit.reference }})\
-  {% if author.contributor %} by {{ author.reference }}{% end %}\
+  {% if author.credit %} by {{ author.reference }}{% end %}\
   {% if commit.trailers.MR %}\
    ([merge request]({{ commit.trailers.MR }}))\
   {% else %}\

app/services/repositories/changelog_service.rb

@@ -61,7 +61,7 @@ module Repositories
     # rubocop: enable Metrics/ParameterLists
 
     def execute
-      config = Gitlab::Changelog::Config.from_git(@project)
+      config = Gitlab::Changelog::Config.from_git(@project, @user)
       from = start_of_commit_range(config)
 
       # For every entry we want to only include the merge request that

doc/api/repositories.md

@@ -448,6 +448,10 @@ You can set the following variables in this file:
 - `template`: a custom template to use for generating the changelog data.
 - `categories`: a hash that maps raw category names to the names to use in the
   changelog.
+- `include_groups`: a list of group full paths containing users whose
+  contributions should be credited regardless of project membership. The user
+  generating the changelog must have access to each group or the members will
+  not be credited.
 
 Using the default settings, generating a changelog results in a section along
 the lines of the following:
@@ -508,7 +512,7 @@ follows:
 
 {% each entries %}
 - [{{ title }}]({{ commit.reference }})\
-{% if author.contributor %} by {{ author.reference }}{% end %}\
+{% if author.credit %} by {{ author.reference }}{% end %}\
 {% if merge_request %} ([merge request]({{ merge_request.reference }})){% end %}
 
 {% end %}
@@ -598,7 +602,7 @@ template: |
 
   {% each entries %}
   - [{{ title }}]({{ commit.reference }})\
-  {% if author.contributor %} by {{ author.reference }}{% end %}
+  {% if author.credit %} by {{ author.reference }}{% end %}
 
   {% end %}
 
@@ -634,8 +638,11 @@ In an entry, the following variables are available (here `foo.bar` means that
 - `commit.trailers`: an object containing all the Git trailers that were present
   in the commit body.
 - `author.reference`: a reference to the commit author (for example, `@alice`).
-- `author.contributor`: a boolean set to `true` when the author is an external
-  contributor, otherwise this is set to `false`.
+- `author.contributor`: a boolean set to `true` when the author is not a project
+  member, otherwise `false`.
+- `author.credit`: a boolean set to `true` when `author.contributor` is `true` or 
+  when `include_groups` is configured, and the author is a member of one of the
+  groups.
 - `merge_request.reference`: a reference to the merge request that first
   introduced the change (for example, `gitlab-org/gitlab!50063`).
 

lib/gitlab/changelog/config.rb

@@ -34,17 +34,17 @@ module Gitlab
         '(?:-(?P<pre>(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))' \
         '?(?:\+(?P<meta>[0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$'
 
-      attr_accessor :date_format, :categories, :template, :tag_regex
+      attr_accessor :date_format, :categories, :template, :tag_regex, :always_credit_user_ids
 
-      def self.from_git(project)
+      def self.from_git(project, user = nil)
         if (yaml = project.repository.changelog_config)
-          from_hash(project, YAML.safe_load(yaml))
+          from_hash(project, YAML.safe_load(yaml), user)
         else
           new(project)
         end
       end
 
-      def self.from_hash(project, hash)
+      def self.from_hash(project, hash, user = nil)
         config = new(project)
 
         if (date = hash['date_format'])
@@ -72,6 +72,14 @@ module Gitlab
           config.tag_regex = regex
         end
 
+        config.always_credit_user_ids = Set.new
+        if (group_paths = Array(hash['include_groups']))
+          group_paths.each do |group_path|
+            group = Group.find_by_full_path(group_path)
+            config.always_credit_user_ids.merge(group&.users_ids_of_direct_members&.compact) if user&.can?(:read_group, group)
+          end
+        end
+
         config
       end
 
@@ -92,6 +100,10 @@ module Gitlab
         @project.team.contributor?(user&.id)
       end
 
+      def always_credit_author?(user)
+        always_credit_user_ids&.include?(user&.id) || false
+      end
+
       def category(name)
         @categories[name] || name
       end

lib/gitlab/changelog/release.rb

@@ -42,6 +42,7 @@ module Gitlab
             'reference' => author.to_reference(full: true),
             'contributor' => @config.contributor?(author)
           }
+          entry['author']['credit'] = entry['author']['contributor'] || @config.always_credit_author?(author)
         end
 
         if merge_request

lib/gitlab/changelog/template.tpl

@@ -4,7 +4,7 @@
 
 {% each entries %}
 - [{{ title }}]({{ commit.reference }})\
-{% if author.contributor %} by {{ author.reference }}{% end %}\
+{% if author.credit %} by {{ author.reference }}{% end %}\
 {% if merge_request %} ([merge request]({{ merge_request.reference }})){% end %}
 
 {% end %}

spec/lib/gitlab/changelog/config_spec.rb

@@ -15,7 +15,7 @@ RSpec.describe Gitlab::Changelog::Config do
 
       expect(described_class)
         .to receive(:from_hash)
-        .with(project, 'date_format' => '%Y')
+        .with(project, { 'date_format' => '%Y' }, nil)
 
       described_class.from_git(project)
     end
@@ -35,12 +35,25 @@ RSpec.describe Gitlab::Changelog::Config do
 
   describe '.from_hash' do
     it 'sets the configuration according to a Hash' do
+      user1 = create(:user)
+      user2 = create(:user)
+      user3 = create(:user)
+      group = create(:group, path: 'group')
+      group2 = create(:group, path: 'group-path')
+      group.add_developer(user1)
+      group.add_developer(user2)
+      group2.add_developer(user3)
+
       config = described_class.from_hash(
         project,
+        {
           'date_format' => 'foo',
           'template' => 'bar',
           'categories' => { 'foo' => 'bar' },
-        'tag_regex' => 'foo'
+          'tag_regex' => 'foo',
+          'include_groups' => %w[group group-path non-existent-group]
+        },
+        user1
       )
 
       expect(config.date_format).to eq('foo')
@@ -49,6 +62,7 @@ RSpec.describe Gitlab::Changelog::Config do
 
       expect(config.categories).to eq({ 'foo' => 'bar' })
       expect(config.tag_regex).to eq('foo')
+      expect(config.always_credit_user_ids).to match_array([user1.id, user2.id, user3.id])
     end
 
     it 'raises Error when the categories are not a Hash' do
@@ -122,4 +136,55 @@ RSpec.describe Gitlab::Changelog::Config do
       expect(config.format_date(time)).to eq('2021-01-05')
     end
   end
+
+  describe '#always_credit_author?' do
+    let_it_be(:group_member) { create(:user) }
+    let_it_be(:non_group_member) { create(:user) }
+    let_it_be(:group) { create(:group, :private, path: 'group') }
+
+    before do
+      group.add_developer(group_member)
+    end
+
+    context 'when include_groups is defined' do
+      context 'when user generating changelog has access to group' do
+        it 'returns whether author should always be credited' do
+          config = described_class.from_hash(
+            project,
+            { 'include_groups' => ['group'] },
+            group_member
+          )
+
+          expect(config.always_credit_author?(group_member)).to eq(true)
+          expect(config.always_credit_author?(non_group_member)).to eq(false)
+        end
+      end
+
+      context 'when user generating changelog has no access to group' do
+        it 'always returns false' do
+          config = described_class.from_hash(
+            project,
+            { 'include_groups' => ['group'] },
+            non_group_member
+          )
+
+          expect(config.always_credit_author?(group_member)).to eq(false)
+          expect(config.always_credit_author?(non_group_member)).to eq(false)
+        end
+      end
+    end
+
+    context 'when include_groups is not defined' do
+      it 'always returns false' do
+        config = described_class.from_hash(
+          project,
+          {},
+          group_member
+        )
+
+        expect(config.always_credit_author?(group_member)).to eq(false)
+        expect(config.always_credit_author?(non_group_member)).to eq(false)
+      end
+    end
+  end
 end

spec/lib/gitlab/changelog/release_spec.rb

@@ -94,6 +94,30 @@ RSpec.describe Gitlab::Changelog::Release do
       end
     end
 
+    context 'when the author should always be credited' do
+      it 'includes the author' do
+        allow(config).to receive(:contributor?).with(author).and_return(false)
+        allow(config).to receive(:always_credit_author?).with(author).and_return(true)
+
+        release.add_entry(
+          title: 'Entry title',
+          commit: commit,
+          category: 'fixed',
+          author: author
+        )
+
+        expect(release.to_markdown).to eq(<<~OUT)
+          ## 1.0.0 (2021-01-05)
+
+          ### fixed (1 change)
+
+          - [Entry title](#{commit.to_reference(full: true)}) \
+          by #{author.to_reference(full: true)}
+
+        OUT
+      end
+    end
+
     context 'when a category has no entries' do
       it "isn't included in the output" do
         config.categories['kittens'] = 'Kittens'