Merge branch 'admin-group-notes' into 'master'

Admin Notes for Groups

See merge request gitlab-org/gitlab!47825

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/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/schema_migrations/20201218194311

+7c33bd30af66ebb9a837c72e2ced107f015d4a22c7b6393554a9299bf3907cc0
\ No newline at end of file

db/structure.sql

@@ -14640,6 +14640,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 +19528,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);
@@ -20895,6 +20915,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 +23145,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 +25989,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;
 

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