Automatic merge of gitlab-org/gitlab master

app/controllers/admin/groups_controller.rb

@@ -30,9 +30,11 @@ class Admin::GroupsController < Admin::ApplicationController
 
   def new
     @group = Group.new
+    @group.build_admin_note
   end
 
   def edit
+    @group.build_admin_note unless @group.admin_note
   end
 
   def create
@@ -49,6 +51,8 @@ class Admin::GroupsController < Admin::ApplicationController
   end
 
   def update
+    @group.build_admin_note unless @group.admin_note
+
     if @group.update(group_params)
       redirect_to [:admin, @group], notice: _('Group was successfully updated.')
     else
@@ -105,7 +109,10 @@ class Admin::GroupsController < Admin::ApplicationController
       :require_two_factor_authentication,
       :two_factor_grace_period,
       :project_creation_level,
-      :subgroup_creation_level
+      :subgroup_creation_level,
+      admin_note_attributes: [
+        :note
+      ]
     ]
   end
 end

app/models/namespace.rb

@@ -46,6 +46,9 @@ class Namespace < ApplicationRecord
   has_one :aggregation_schedule, class_name: 'Namespace::AggregationSchedule'
   has_one :package_setting_relation, inverse_of: :namespace, class_name: 'PackageSetting'
 
+  has_one :admin_note, inverse_of: :namespace
+  accepts_nested_attributes_for :admin_note, update_only: true
+
   validates :owner, presence: true, unless: ->(n) { n.type == "Group" }
   validates :name,
     presence: true,

app/models/namespace/admin_note.rb

+# frozen_string_literal: true
+
+class Namespace::AdminNote < ApplicationRecord
+  belongs_to :namespace, inverse_of: :admin_note
+  validates :namespace, presence: true
+  validates :note, length: { maximum: 1000 }
+end

app/views/admin/groups/_form.html.haml

@@ -3,6 +3,8 @@
   = render 'shared/group_form', f: f
   = render 'shared/group_form_description', f: f
 
+  = render 'shared/admin/admin_note_form', f: f
+
   = render_if_exists 'shared/old_repository_size_limit_setting', form: f, type: :group
   = render_if_exists 'admin/namespace_plan', f: f
 

app/views/admin/groups/show.html.haml

@@ -103,6 +103,8 @@
                 %span.monospace= project.full_path + '.git'
 
   .col-md-6
+    = render 'shared/admin/admin_note'
+
     - if can?(current_user, :admin_group_member, @group)
       .card
         .card-header

app/views/admin/users/_admin_notes.html.haml

@@ -2,6 +2,6 @@
   %legend= _('Admin notes')
   .form-group.row
     .col-sm-2.col-form-label
-      = f.label :note, s_('AdminNote|Note')
+      = f.label :note, s_('Admin|Note')
     .col-sm-10
       = f.text_area :note, class: 'form-control gl-form-input gl-form-textarea'

app/views/shared/admin/_admin_note.html.haml

+- if @group.admin_note.present?
+  - text = @group.admin_note.note
+  .card.border-info
+    .card-header.bg-info.gl-text-white
+      = s_('Admin|Admin notes')
+    .card-body
+      %p= text

app/views/shared/admin/_admin_note_form.html.haml

+.form-group.row
+  = f.fields_for :admin_note do |an|
+    .col-sm-2.col-form-label.gl-text-right
+      = an.label :note, s_('Admin|Admin notes')
+    .col-sm-10
+      = an.text_area :note, class: 'form-control'

changelogs/unreleased/321662-validate_not_null_constraint_on_gitlab_subscriptions_namespace_id.yml

+---
+title: Validate NOT NULL constraint on gitlab_subscriptions namespace_id
+merge_request: 57113
+author:
+type: other

changelogs/unreleased/admin-group-notes.yml

+---
+title: Allow admin users to define admin notes on groups.
+merge_request: 47825
+author:
+type: added

db/migrate/20201218194311_create_admin_notes.rb

+# frozen_string_literal: true
+
+class CreateAdminNotes < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    create_table_with_constraints :namespace_admin_notes do |t|
+      t.timestamps_with_timezone
+      t.references :namespace, null: false, foreign_key: { on_delete: :cascade }
+      t.text :note
+
+      t.text_limit :note, 1000
+    end
+  end
+
+  def down
+    drop_table :namespace_admin_notes
+  end
+end

db/post_migrate/20210322115438_validate_not_null_constraint_on_gitlab_subscriptions_namespace_id.rb

+# frozen_string_literal: true
+
+class ValidateNotNullConstraintOnGitlabSubscriptionsNamespaceId < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    validate_not_null_constraint :gitlab_subscriptions, :namespace_id
+  end
+
+  def down
+    # no-op
+  end
+end

db/schema_migrations/20201218194311

+7c33bd30af66ebb9a837c72e2ced107f015d4a22c7b6393554a9299bf3907cc0
\ No newline at end of file

db/schema_migrations/20210322115438

+e177c2cc0b59eea54de10417445b391cea7dd308547077aea34054fac22b9e40
\ No newline at end of file

db/structure.sql

@@ -13147,7 +13147,8 @@ CREATE TABLE gitlab_subscriptions (
     auto_renew boolean,
     seats_in_use integer DEFAULT 0 NOT NULL,
     seats_owed integer DEFAULT 0 NOT NULL,
-    trial_extension_type smallint
+    trial_extension_type smallint,
+    CONSTRAINT check_77fea3f0e7 CHECK ((namespace_id IS NOT NULL))
 );
 
 CREATE SEQUENCE gitlab_subscriptions_id_seq
@@ -14640,6 +14641,24 @@ CREATE SEQUENCE milestones_id_seq
 
 ALTER SEQUENCE milestones_id_seq OWNED BY milestones.id;
 
+CREATE TABLE namespace_admin_notes (
+    id bigint NOT NULL,
+    created_at timestamp with time zone NOT NULL,
+    updated_at timestamp with time zone NOT NULL,
+    namespace_id bigint NOT NULL,
+    note text,
+    CONSTRAINT check_e9d2e71b5d CHECK ((char_length(note) <= 1000))
+);
+
+CREATE SEQUENCE namespace_admin_notes_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE namespace_admin_notes_id_seq OWNED BY namespace_admin_notes.id;
+
 CREATE TABLE namespace_aggregation_schedules (
     namespace_id integer NOT NULL
 );
@@ -19510,6 +19529,8 @@ ALTER TABLE ONLY metrics_users_starred_dashboards ALTER COLUMN id SET DEFAULT ne
 
 ALTER TABLE ONLY milestones ALTER COLUMN id SET DEFAULT nextval('milestones_id_seq'::regclass);
 
+ALTER TABLE ONLY namespace_admin_notes ALTER COLUMN id SET DEFAULT nextval('namespace_admin_notes_id_seq'::regclass);
+
 ALTER TABLE ONLY namespace_statistics ALTER COLUMN id SET DEFAULT nextval('namespace_statistics_id_seq'::regclass);
 
 ALTER TABLE ONLY namespaces ALTER COLUMN id SET DEFAULT nextval('namespaces_id_seq'::regclass);
@@ -20238,9 +20259,6 @@ ALTER TABLE ONLY chat_teams
 ALTER TABLE vulnerability_scanners
     ADD CONSTRAINT check_37608c9db5 CHECK ((char_length(vendor) <= 255)) NOT VALID;
 
-ALTER TABLE gitlab_subscriptions
-    ADD CONSTRAINT check_77fea3f0e7 CHECK ((namespace_id IS NOT NULL)) NOT VALID;
-
 ALTER TABLE sprints
     ADD CONSTRAINT check_ccd8a1eae0 CHECK ((start_date IS NOT NULL)) NOT VALID;
 
@@ -20895,6 +20913,9 @@ ALTER TABLE ONLY milestone_releases
 ALTER TABLE ONLY milestones
     ADD CONSTRAINT milestones_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY namespace_admin_notes
+    ADD CONSTRAINT namespace_admin_notes_pkey PRIMARY KEY (id);
+
 ALTER TABLE ONLY namespace_aggregation_schedules
     ADD CONSTRAINT namespace_aggregation_schedules_pkey PRIMARY KEY (namespace_id);
 
@@ -23122,6 +23143,8 @@ CREATE INDEX index_mr_metrics_on_target_project_id_merged_at_nulls_last ON merge
 
 CREATE INDEX index_mr_metrics_on_target_project_id_merged_at_time_to_merge ON merge_request_metrics USING btree (target_project_id, merged_at, created_at) WHERE (merged_at > created_at);
 
+CREATE INDEX index_namespace_admin_notes_on_namespace_id ON namespace_admin_notes USING btree (namespace_id);
+
 CREATE UNIQUE INDEX index_namespace_aggregation_schedules_on_namespace_id ON namespace_aggregation_schedules USING btree (namespace_id);
 
 CREATE UNIQUE INDEX index_namespace_root_storage_statistics_on_namespace_id ON namespace_root_storage_statistics USING btree (namespace_id);
@@ -25964,6 +25987,9 @@ ALTER TABLE ONLY approval_merge_request_rules_approved_approvers
 ALTER TABLE ONLY operations_feature_flags_clients
     ADD CONSTRAINT fk_rails_6650ed902c FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY namespace_admin_notes
+    ADD CONSTRAINT fk_rails_666166ea7b FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY web_hook_logs
     ADD CONSTRAINT fk_rails_666826e111 FOREIGN KEY (web_hook_id) REFERENCES web_hooks(id) ON DELETE CASCADE;
 

doc/development/documentation/index.md

@@ -463,7 +463,8 @@ The following GitLab features are used among others:
 
 ## Testing
 
-For more information on documentation testing, see [Documentation testing](testing.md)
+For more information about documentation testing, see the [Documentation testing](testing.md)
+guide.
 
 ## Danger Bot
 

doc/development/documentation/structure.md

@@ -153,6 +153,14 @@ This issue occurs when...
 The workaround is...
 ```
 
+For the topic title:
+
+- Consider including at least a partial error message in the title.
+- Use fewer than 70 characters.
+
+Remember to include the complete error message in the topics content if it is
+not complete in the title.
+
 ## Other information on a topic
 
 Topics include other information.

doc/development/migration_style_guide.md

@@ -1038,7 +1038,7 @@ To identify a high-traffic table for GitLab.com the following measures are consi
 Note that the metrics linked here are GitLab-internal only:
 
 - [Read operations](https://thanos.gitlab.net/graph?g0.range_input=2h&g0.max_source_resolution=0s&g0.expr=topk(500%2C%20sum%20by%20(relname)%20(rate(pg_stat_user_tables_seq_tup_read%7Benvironment%3D%22gprd%22%7D%5B12h%5D)%20%2B%20rate(pg_stat_user_tables_idx_scan%7Benvironment%3D%22gprd%22%7D%5B12h%5D)%20%2B%20rate(pg_stat_user_tables_idx_tup_fetch%7Benvironment%3D%22gprd%22%7D%5B12h%5D)))&g0.tab=1)
-- [Number of records](https://thanos.gitlab.net/graph?g0.range_input=2h&g0.max_source_resolution=0s&g0.expr=topk(500%2C%20sum%20by%20(relname)%20(rate(pg_stat_user_tables_n_live_tup%7Benvironment%3D%22gprd%22%7D%5B12h%5D)))&g0.tab=1)
-- [Size](https://thanos.gitlab.net/graph?g0.range_input=2h&g0.max_source_resolution=0s&g0.expr=topk(500%2C%20sum%20by%20(relname)%20(rate(pg_total_relation_size_bytes%7Benvironment%3D%22gprd%22%7D%5B12h%5D)))&g0.tab=1) is greater than 10 GB
+- [Number of records](https://thanos.gitlab.net/graph?g0.range_input=2h&g0.max_source_resolution=0s&g0.expr=topk(500%2C%20max%20by%20(relname)%20(pg_stat_user_tables_n_live_tup%7Benvironment%3D%22gprd%22%7D))&g0.tab=1)
+- [Size](https://thanos.gitlab.net/graph?g0.range_input=2h&g0.max_source_resolution=0s&g0.expr=topk(500%2C%20max%20by%20(relname)%20(pg_total_relation_size_bytes%7Benvironment%3D%22gprd%22%7D))&g0.tab=1) is greater than 10 GB
 
 Any table which has some high read operation compared to current [high-traffic tables](https://gitlab.com/gitlab-org/gitlab/-/blob/master/rubocop/rubocop-migrations.yml#L4) might be a good candidate.

ee/app/assets/javascripts/analytics/devops_report/devops_adoption/components/devops_adoption_table.vue

 <script>
-import { GlTable, GlButton, GlModalDirective, GlTooltipDirective, GlIcon } from '@gitlab/ui';
+import {
+  GlTable,
+  GlButton,
+  GlModalDirective,
+  GlTooltipDirective,
+  GlIcon,
+  GlBadge,
+} from '@gitlab/ui';
 import LocalStorageSync from '~/vue_shared/components/local_storage_sync.vue';
 import {
   DEVOPS_ADOPTION_TABLE_TEST_IDS,
@@ -58,6 +65,7 @@ export default {
     LocalStorageSync,
     DevopsAdoptionDeleteModal,
     GlIcon,
+    GlBadge,
   },
   i18n,
   devopsSegmentModalId: DEVOPS_ADOPTION_SEGMENT_MODAL_ID,
@@ -66,6 +74,11 @@ export default {
     GlTooltip: GlTooltipDirective,
     GlModal: GlModalDirective,
   },
+  inject: {
+    groupGid: {
+      default: null,
+    },
+  },
   tableHeaderFields: [
     ...headers,
     {
@@ -102,6 +115,9 @@ export default {
     slotName(key) {
       return `head(${key})`;
     },
+    isCurrentGroup(item) {
+      return item.namespace?.id === this.groupGid;
+    },
   },
 };
 </script>
@@ -140,20 +156,16 @@ export default {
         </div>
       </template>
 
-      <template
-        #cell(name)="{
-          item: {
-            namespace: { fullName },
-            latestSnapshot,
-          },
-        }"
-      >
+      <template #cell(name)="{ item }">
         <div :data-testid="$options.testids.SEGMENT">
-          <strong v-if="latestSnapshot">{{ fullName }}</strong>
+          <strong v-if="item.latestSnapshot">{{ item.namespace.fullName }}</strong>
           <template v-else>
-            <span class="gl-text-gray-400">{{ fullName }}</span>
+            <span class="gl-text-gray-400">{{ item.namespace.fullName }}</span>
             <gl-icon name="hourglass" class="gl-text-gray-400" />
           </template>
+          <gl-badge v-if="isCurrentGroup(item)" class="gl-ml-1" variant="info">{{
+            __('This group')
+          }}</gl-badge>
         </div>
       </template>
 

ee/spec/frontend/analytics/devops_report/devops_adoption/components/devops_adoption_table_spec.js

-import { GlTable, GlButton, GlIcon } from '@gitlab/ui';
+import { GlTable, GlButton, GlIcon, GlBadge } from '@gitlab/ui';
 import { mount } from '@vue/test-utils';
 import { nextTick } from 'vue';
 import DevopsAdoptionDeleteModal from 'ee/analytics/devops_report/devops_adoption/components/devops_adoption_delete_modal.vue';
@@ -12,12 +12,15 @@ import { devopsAdoptionSegmentsData, devopsAdoptionTableHeaders } from '../mock_
 describe('DevopsAdoptionTable', () => {
   let wrapper;
 
-  const createComponent = () => {
+  const createComponent = (options = {}) => {
+    const { provide = {} } = options;
+
     wrapper = mount(DevopsAdoptionTable, {
       propsData: {
         segments: devopsAdoptionSegmentsData.nodes,
         selectedSegment: devopsAdoptionSegmentsData.nodes[0],
       },
+      provide,
       directives: {
         GlTooltip: createMockDirective(),
       },
@@ -26,7 +29,6 @@ describe('DevopsAdoptionTable', () => {
 
   beforeEach(() => {
     localStorage.clear();
-    createComponent();
   });
 
   afterEach(() => {
@@ -53,6 +55,7 @@ describe('DevopsAdoptionTable', () => {
     let headers;
 
     beforeEach(() => {
+      createComponent();
       headers = findTable().findAll(`[data-testid="${TEST_IDS.TABLE_HEADERS}"]`);
     });
 
@@ -97,10 +100,33 @@ describe('DevopsAdoptionTable', () => {
   describe('table fields', () => {
     describe('segment name', () => {
       it('displays the correct segment name', () => {
+        createComponent();
+
         expect(findCol(TEST_IDS.SEGMENT).text()).toBe('Group 1');
       });
 
+      describe('"This group" badge', () => {
+        const thisGroupGid = devopsAdoptionSegmentsData.nodes[0].namespace.id;
+
+        it.each`
+          scenario                            | expected | provide
+          ${'is not shown by default'}        | ${false} | ${null}
+          ${'is not shown for other groups'}  | ${false} | ${{ groupGid: 'anotherGroupGid' }}
+          ${'is shown for the current group'} | ${true}  | ${{ groupGid: thisGroupGid }}
+        `('$scenario', ({ expected, provide }) => {
+          createComponent({ provide });
+
+          const badge = findColSubComponent(TEST_IDS.SEGMENT, GlBadge);
+
+          expect(badge.exists()).toBe(expected);
+        });
+      });
+
       describe('pending state (no snapshot data available)', () => {
+        beforeEach(() => {
+          createComponent();
+        });
+
         it('grays the text out', () => {
           const name = findColRowChild(TEST_IDS.SEGMENT, 1, 'span');
 
@@ -132,12 +158,16 @@ describe('DevopsAdoptionTable', () => {
       ${TEST_IDS.DEPLOYS}   | ${'deploys'}   | ${false}
       ${TEST_IDS.SCANNING}  | ${'scanning'}  | ${false}
     `('displays the correct $field snapshot value', ({ id, flag }) => {
+      createComponent();
+
       const booleanFlag = findColSubComponent(id, DevopsAdoptionTableCellFlag);
 
       expect(booleanFlag.props('enabled')).toBe(flag);
     });
 
     it('displays the actions icon', () => {
+      createComponent();
+
       const button = findColSubComponent(TEST_IDS.ACTIONS, GlButton);
 
       expect(button.exists()).toBe(true);
@@ -147,6 +177,10 @@ describe('DevopsAdoptionTable', () => {
   });
 
   describe('delete modal integration', () => {
+    beforeEach(() => {
+      createComponent();
+    });
+
     it('re emits trackModalOpenState with the given value', async () => {
       findDeleteModal().vm.$emit('trackModalOpenState', true);
 
@@ -158,6 +192,7 @@ describe('DevopsAdoptionTable', () => {
     let headers;
 
     beforeEach(() => {
+      createComponent();
       headers = findTable().findAll(`[data-testid="${TEST_IDS.TABLE_HEADERS}"]`);
     });
 

locale/gitlab.pot

@@ -2238,9 +2238,6 @@ msgstr ""
 msgid "AdminDashboard|Error loading the statistics. Please try again"
 msgstr ""
 
-msgid "AdminNote|Note"
-msgstr ""
-
 msgid "AdminProjects| You’re about to permanently delete the project %{projectName}, its repository, and all related resources including issues, merge requests, etc..  Once you confirm and press %{strong_start}Delete project%{strong_end}, it cannot be undone or recovered."
 msgstr ""
 
@@ -2676,6 +2673,12 @@ msgstr ""
 msgid "Admin|Additional users must be reviewed and approved by a system administrator. Learn more about %{help_link_start}usage caps%{help_link_end}."
 msgstr ""
 
+msgid "Admin|Admin notes"
+msgstr ""
+
+msgid "Admin|Note"
+msgstr ""
+
 msgid "Admin|View pending user approvals"
 msgstr ""
 

spec/controllers/admin/groups_controller_spec.rb

@@ -37,6 +37,12 @@ RSpec.describe Admin::GroupsController do
         post :create, params: { group: {  path: 'test', name: 'test' } }
       end.to change { NamespaceSetting.count }.by(1)
     end
+
+    it 'creates admin_note for group' do
+      expect do
+        post :create, params: { group: {  path: 'test', name: 'test', admin_note_attributes: { note: 'test' } } }
+      end.to change { Namespace::AdminNote.count }.by(1)
+    end
   end
 
   describe 'PUT #members_update' do

spec/features/admin/admin_groups_spec.rb

@@ -35,6 +35,7 @@ RSpec.describe 'Admin Groups' do
         expect(page).to have_field('group_path')
         expect(page).to have_field('group_visibility_level_0')
         expect(page).to have_field('description')
+        expect(page).to have_field('group_admin_note_attributes_note')
       end
     end
 
@@ -47,10 +48,12 @@ RSpec.describe 'Admin Groups' do
       path_component = 'gitlab'
       group_name = 'GitLab group name'
       group_description = 'Description of group for GitLab'
+      group_admin_note = 'A note about this group by an admin'
 
       fill_in 'group_path', with: path_component
       fill_in 'group_name', with: group_name
       fill_in 'group_description', with: group_description
+      fill_in 'group_admin_note_attributes_note', with: group_admin_note
       click_button "Create group"
 
       expect(current_path).to eq admin_group_path(Group.find_by(path: path_component))
@@ -61,6 +64,8 @@ RSpec.describe 'Admin Groups' do
       expect(li_texts).to match group_name
       expect(li_texts).to match path_component
       expect(li_texts).to match group_description
+      p_texts = content.all('p').collect(&:text).join('/n')
+      expect(p_texts).to match group_admin_note
     end
 
     it 'shows the visibility level radio populated with the default value' do
@@ -116,6 +121,16 @@ RSpec.describe 'Admin Groups' do
 
       expect(page).to have_link(group.name, href: group_path(group))
     end
+
+    it 'has a note if one is available' do
+      group = create(:group, :private)
+      note_text = 'A group administrator note'
+      group.update!(admin_note_attributes: { note: note_text })
+
+      visit admin_group_path(group)
+
+      expect(page).to have_text(note_text)
+    end
   end
 
   describe 'group edit' do
@@ -145,6 +160,36 @@ RSpec.describe 'Admin Groups' do
 
       expect(name_field.value).to eq original_name
     end
+
+    it 'adding an admin note to group without one' do
+      group = create(:group, :private)
+      expect(group.admin_note).to be_nil
+
+      visit admin_group_edit_path(group)
+      admin_note_text = 'A note by an administrator'
+
+      fill_in 'group_admin_note_attributes_note', with: admin_note_text
+      click_button 'Save changes'
+
+      expect(page).to have_content(admin_note_text)
+    end
+
+    it 'editing an existing group admin note' do
+      admin_note_text = 'A note by an administrator'
+      new_admin_note_text = 'A new note by an administrator'
+      group = create(:group, :private)
+      group.create_admin_note(note: admin_note_text)
+
+      visit admin_group_edit_path(group)
+
+      admin_note_field = find('#group_admin_note_attributes_note')
+      expect(admin_note_field.value).to eq(admin_note_text)
+
+      fill_in 'group_admin_note_attributes_note', with: new_admin_note_text
+      click_button 'Save changes'
+
+      expect(page).to have_content(new_admin_note_text)
+    end
   end
 
   describe 'add user into a group', :js do

spec/models/namespace/admin_note_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Namespace::AdminNote, type: :model do
+  let!(:namespace) { create(:namespace) }
+
+  describe 'associations' do
+    it { is_expected.to belong_to :namespace }
+  end
+
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:namespace) }
+    it { is_expected.to validate_length_of(:note).is_at_most(1000) }
+  end
+end

spec/models/namespace_spec.rb

@@ -21,6 +21,7 @@ RSpec.describe Namespace do
     it { is_expected.to have_many :custom_emoji }
     it { is_expected.to have_one :package_setting_relation }
     it { is_expected.to have_one :onboarding_progress }
+    it { is_expected.to have_one :admin_note }
   end
 
   describe 'validations' do