Add latest changes from gitlab-org/gitlab@master

13 files changed, 381 insertions, 38 deletions

diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb
index 2241a7cf590..4a5283f1127 100644
--- a/spec/controllers/projects/notes_controller_spec.rb
+++ b/spec/controllers/projects/notes_controller_spec.rb
@@ -775,32 +775,40 @@ RSpec.describe Projects::NotesController, type: :controller, feature_category: :
   end
 
   describe 'PUT update' do
+    let(:note_params) { { note: "New comment" } }
+
     let(:request_params) do
       {
         namespace_id: project.namespace,
         project_id: project,
         id: note,
         format: :json,
-        note: {
-          note: "New comment"
-        }
+        note: note_params
       }
     end
 
-    specify { expect(put(:update, params: request_params)).to have_request_urgency(:low) }
+    subject(:update_note) { put :update, params: request_params }
 
-    context "should update the note with a valid issue" do
-      before do
-        sign_in(note.author)
-        project.add_developer(note.author)
-      end
+    before do
+      sign_in(note.author)
+      project.add_developer(note.author)
+    end
+
+    specify { expect(update_note).to have_request_urgency(:low) }
 
+    context "when the note is valid" do
       it "updates the note" do
-        expect { put :update, params: request_params }.to change { note.reload.note }
+        expect { update_note }.to change { note.reload.note }
+      end
+
+      it "returns status 200" do
+        update_note
+
+        expect(response).to have_gitlab_http_status(:ok)
       end
     end
 
-    context "doesnt update the note" do
+    context "when the issue is confidential and the user has guest permissions" do
       let(:issue)   { create(:issue, :confidential, project: project) }
       let(:note)    { create(:note, noteable: issue, project: project) }
 
@@ -809,20 +817,38 @@ RSpec.describe Projects::NotesController, type: :controller, feature_category: :
         project.add_guest(user)
       end
 
-      it "disallows edits when the issue is confidential and the user has guest permissions" do
-        request_params = {
-          namespace_id: project.namespace,
-          project_id: project,
-          id: note,
-          format: :json,
-          note: {
-            note: "New comment"
-          }
-        }
-        expect { put :update, params: request_params }.not_to change { note.reload.note }
+      it "disallows edits" do
+        expect { update_note }.not_to change { note.reload.note }
+      end
+
+      it "returns status 404" do
+        update_note
+
         expect(response).to have_gitlab_http_status(:not_found)
       end
     end
+
+    context "when there are ActiveRecord validation errors" do
+      before do
+        allow(note).to receive_message_chain(:errors, :full_messages)
+          .and_return(['Error 1', 'Error 2'])
+
+        allow_next_instance_of(Notes::UpdateService) do |service|
+          allow(service).to receive(:execute).and_return(note)
+        end
+      end
+
+      it "does not update the note" do
+        expect { update_note }.not_to change { note.reload.note }
+      end
+
+      it "returns status 422", :aggregate_failures do
+        update_note
+
+        expect(response).to have_gitlab_http_status(:unprocessable_entity)
+        expect(response.body).to eq('{"errors":"Error 1 and Error 2"}')
+      end
+    end
   end
 
   describe 'DELETE destroy' do
diff --git a/spec/controllers/snippets/notes_controller_spec.rb b/spec/controllers/snippets/notes_controller_spec.rb
index 00d99b46d0b..578973d5b3d 100644
--- a/spec/controllers/snippets/notes_controller_spec.rb
+++ b/spec/controllers/snippets/notes_controller_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Snippets::NotesController do
+RSpec.describe Snippets::NotesController, feature_category: :team_planning do
   let(:user) { create(:user) }
 
   let(:private_snippet)  { create(:personal_snippet, :private) }
@@ -256,6 +256,59 @@ RSpec.describe Snippets::NotesController do
     end
   end
 
+  describe 'PUT update' do
+    let(:note_params) { { note: "New comment" } }
+
+    let(:request_params) do
+      {
+        snippet_id: public_snippet,
+        id: note_on_public,
+        format: :json,
+        note: note_params
+      }
+    end
+
+    before do
+      sign_in(note_on_public.author)
+    end
+
+    subject(:update_note) { put :update, params: request_params }
+
+    context "when the note is valid" do
+      it "updates the note" do
+        expect { update_note }.to change { note_on_public.reload.note }
+      end
+
+      it "returns status 200" do
+        post :create, params: request_params
+
+        expect(response).to have_gitlab_http_status(:ok)
+      end
+    end
+
+    context "when there are ActiveRecord validation errors" do
+      before do
+        allow(note_on_public).to receive_message_chain(:errors, :full_messages)
+          .and_return(['Error 1', 'Error 2'])
+
+        allow_next_instance_of(Notes::UpdateService) do |service|
+          allow(service).to receive(:execute).and_return(note_on_public)
+        end
+      end
+
+      it "does not update the note" do
+        expect { update_note }.not_to change { note_on_public.reload.note }
+      end
+
+      it "returns status 422", :aggregate_failures do
+        update_note
+
+        expect(response).to have_gitlab_http_status(:unprocessable_entity)
+        expect(response.body).to eq('{"errors":"Error 1 and Error 2"}')
+      end
+    end
+  end
+
   describe 'DELETE destroy' do
     let(:request_params) do
       {
diff --git a/spec/finders/groups/environment_scopes_finder_spec.rb b/spec/finders/groups/environment_scopes_finder_spec.rb
new file mode 100644
index 00000000000..dfa32725e4a
--- /dev/null
+++ b/spec/finders/groups/environment_scopes_finder_spec.rb
@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Groups::EnvironmentScopesFinder, feature_category: :secrets_management do
+  describe '#execute' do
+    let_it_be(:user) { create(:user) }
+    let_it_be(:group) { create(:group, :public) }
+
+    let!(:environment1) { create(:ci_group_variable, group: group, key: 'var1', environment_scope: 'environment1') }
+    let!(:environment2) { create(:ci_group_variable, group: group, key: 'var2', environment_scope: 'environment2') }
+    let!(:environment3) { create(:ci_group_variable, group: group, key: 'var2', environment_scope: 'environment3') }
+    let(:finder) { described_class.new(group: group, params: params) }
+
+    subject { finder.execute }
+
+    context 'with default no arguments' do
+      let(:params) { {} }
+
+      it do
+        expected_result = group.variables.environment_scope_names
+
+        expect(subject.map(&:name))
+          .to match_array(expected_result)
+      end
+    end
+
+    context 'with search' do
+      let(:params) { { search: 'ment1' } }
+
+      it do
+        expected_result = ['environment1']
+
+        expect(subject.map(&:name))
+          .to match_array(expected_result)
+      end
+    end
+
+    context 'with specific name' do
+      let(:params) { { name: 'environment3' } }
+
+      it do
+        expect(subject.map(&:name))
+          .to match_array([environment3.environment_scope])
+      end
+    end
+  end
+end
diff --git a/spec/frontend/comment_templates/components/__snapshots__/list_item_spec.js.snap b/spec/frontend/comment_templates/components/__snapshots__/list_item_spec.js.snap
index 807bb116f2e..8cad483e27e 100644
--- a/spec/frontend/comment_templates/components/__snapshots__/list_item_spec.js.snap
+++ b/spec/frontend/comment_templates/components/__snapshots__/list_item_spec.js.snap
@@ -58,7 +58,7 @@ exports[`Comment templates list item component renders list item 1`] = `
         </button>
          
         <div
-          class="gl-new-dropdown-panel gl-w-31! gl-absolute"
+          class="gl-new-dropdown-panel gl-w-31!"
           data-testid="base-dropdown-menu"
           id="base-dropdown-7"
         >
diff --git a/spec/frontend/issues/show/components/header_actions_spec.js b/spec/frontend/issues/show/components/header_actions_spec.js
index c14da65a236..9a503a2d882 100644
--- a/spec/frontend/issues/show/components/header_actions_spec.js
+++ b/spec/frontend/issues/show/components/header_actions_spec.js
@@ -103,7 +103,8 @@ describe('HeaderActions component', () => {
     },
   };
 
-  const findToggleIssueStateButton = () => wrapper.find(`[data-testid="toggle-button"]`);
+  const findToggleIssueStateButton = () =>
+    wrapper.find(`[data-testid="toggle-issue-state-button"]`);
   const findEditButton = () => wrapper.find(`[data-testid="edit-button"]`);
 
   const findDropdownBy = (dataTestId) => wrapper.find(`[data-testid="${dataTestId}"]`);
diff --git a/spec/frontend/notes/components/noteable_note_spec.js b/spec/frontend/notes/components/noteable_note_spec.js
index 5d81a7a9a0f..d50fb130a69 100644
--- a/spec/frontend/notes/components/noteable_note_spec.js
+++ b/spec/frontend/notes/components/noteable_note_spec.js
@@ -1,6 +1,7 @@
 import Vue, { nextTick } from 'vue';
 import Vuex from 'vuex';
 import { GlAvatar } from '@gitlab/ui';
+import { clone } from 'lodash';
 import { mountExtended } from 'helpers/vue_test_utils_helper';
 import waitForPromises from 'helpers/wait_for_promises';
 import DiffsModule from '~/diffs/store/modules';
@@ -10,9 +11,13 @@ import NoteHeader from '~/notes/components/note_header.vue';
 import issueNote from '~/notes/components/noteable_note.vue';
 import NotesModule from '~/notes/stores/modules';
 import { NOTEABLE_TYPE_MAPPING } from '~/notes/constants';
+import { createAlert } from '~/alert';
+import { UPDATE_COMMENT_FORM } from '~/notes/i18n';
+import { sprintf } from '~/locale';
 import { noteableDataMock, notesDataMock, note } from '../mock_data';
 
 Vue.use(Vuex);
+jest.mock('~/alert');
 
 const singleLineNotePosition = {
   line_range: {
@@ -54,10 +59,13 @@ describe('issue_note', () => {
     store.dispatch('setNoteableData', noteableDataMock);
     store.dispatch('setNotesData', notesDataMock);
 
+    // the component overwrites the `note` prop with every action, hence create a copy
+    const noteCopy = clone(props.note || note);
+
     wrapper = mountExtended(issueNote, {
       store,
       propsData: {
-        note,
+        note: noteCopy,
         ...props,
       },
       stubs: [
@@ -252,7 +260,7 @@ describe('issue_note', () => {
     });
 
     it('should render issue body', () => {
-      expect(findNoteBody().props().note).toBe(note);
+      expect(findNoteBody().props().note).toMatchObject(note);
       expect(findNoteBody().props().line).toBe(null);
       expect(findNoteBody().props().canEdit).toBe(note.current_user.can_edit);
       expect(findNoteBody().props().isEditing).toBe(false);
@@ -297,7 +305,7 @@ describe('issue_note', () => {
     });
 
     it('does not have internal note class for external notes', () => {
-      createWrapper({ note });
+      createWrapper();
 
       expect(wrapper.classes()).not.toContain('internal-note');
     });
@@ -327,7 +335,6 @@ describe('issue_note', () => {
       });
 
       await nextTick();
-
       expect(findNoteBody().props().note.note_html).toBe(`<p dir="auto">${updatedText}</p>\n`);
 
       findNoteBody().vm.$emit('cancelForm', {});
@@ -340,7 +347,7 @@ describe('issue_note', () => {
   describe('formUpdateHandler', () => {
     const updateNote = jest.fn();
     const params = {
-      noteText: '',
+      noteText: 'updated note text',
       parentElement: null,
       callback: jest.fn(),
       resolveDiscussion: false,
@@ -359,28 +366,38 @@ describe('issue_note', () => {
       });
     };
 
+    beforeEach(() => {
+      createWrapper();
+      updateActions();
+    });
+
     afterEach(() => updateNote.mockReset());
 
     it('responds to handleFormUpdate', () => {
-      createWrapper();
-      updateActions();
       findNoteBody().vm.$emit('handleFormUpdate', params);
+
       expect(wrapper.emitted('handleUpdateNote')).toHaveLength(1);
     });
 
+    it('updates note content', async () => {
+      findNoteBody().vm.$emit('handleFormUpdate', params);
+
+      await nextTick();
+
+      expect(findNoteBody().props().note.note_html).toBe(`<p dir="auto">${params.noteText}</p>\n`);
+      expect(findNoteBody().props('isEditing')).toBe(false);
+    });
+
     it('should not update note with sensitive token', () => {
       const sensitiveMessage = 'token: glpat-1234567890abcdefghij';
-
-      createWrapper();
-      updateActions();
       findNoteBody().vm.$emit('handleFormUpdate', { ...params, noteText: sensitiveMessage });
+
       expect(updateNote).not.toHaveBeenCalled();
     });
 
     it('does not stringify empty position', () => {
-      createWrapper();
-      updateActions();
       findNoteBody().vm.$emit('handleFormUpdate', params);
+
       expect(updateNote.mock.calls[0][1].note.note.position).toBeUndefined();
     });
 
@@ -388,10 +405,35 @@ describe('issue_note', () => {
       const position = { test: true };
       const expectation = JSON.stringify(position);
       createWrapper({ note: { ...note, position } });
+
       updateActions();
       findNoteBody().vm.$emit('handleFormUpdate', params);
+
       expect(updateNote.mock.calls[0][1].note.note.position).toBe(expectation);
     });
+
+    describe('when updateNote returns errors', () => {
+      beforeEach(() => {
+        updateNote.mockRejectedValue({
+          response: { status: 422, data: { errors: 'error 1 and error 2' } },
+        });
+      });
+
+      beforeEach(() => {
+        findNoteBody().vm.$emit('handleFormUpdate', { ...params, noteText: 'invalid note' });
+      });
+
+      it('renders error message and restores content of updated note', async () => {
+        await waitForPromises();
+        expect(createAlert).toHaveBeenCalledWith({
+          message: sprintf(UPDATE_COMMENT_FORM.error, { reason: 'error 1 and error 2' }, false),
+          parent: wrapper.vm.$el,
+        });
+
+        expect(findNoteBody().props('isEditing')).toBe(true);
+        expect(findNoteBody().props().note.note_html).toBe(note.note_html);
+      });
+    });
   });
 
   describe('diffFile', () => {
diff --git a/spec/graphql/resolvers/group_environment_scopes_resolver_spec.rb b/spec/graphql/resolvers/group_environment_scopes_resolver_spec.rb
new file mode 100644
index 00000000000..71561137356
--- /dev/null
+++ b/spec/graphql/resolvers/group_environment_scopes_resolver_spec.rb
@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Resolvers::GroupEnvironmentScopesResolver, feature_category: :secrets_management do
+  include GraphqlHelpers
+
+  let_it_be(:current_user) { create(:user) }
+  let(:group) { create(:group) }
+
+  context "with a group" do
+    let(:expected_environment_scopes) do
+      %w[environment1 environment2 environment3 environment4 environment5 environment6]
+    end
+
+    before do
+      group.add_developer(current_user)
+      expected_environment_scopes.each_with_index do |env, index|
+        create(:ci_group_variable, group: group, key: "var#{index + 1}", environment_scope: env)
+      end
+    end
+
+    describe '#resolve' do
+      it 'finds all environment scopes' do
+        expect(resolve_environment_scopes.map(&:name)).to match_array(
+          expected_environment_scopes
+        )
+      end
+    end
+  end
+
+  context 'without a group' do
+    describe '#resolve' do
+      it 'rails to find any environment scopes' do
+        expect(resolve_environment_scopes.map(&:name)).to match_array(
+          []
+        )
+      end
+    end
+  end
+
+  def resolve_environment_scopes(args = {}, context = { current_user: current_user })
+    resolve(described_class, obj: group, args: args, ctx: context)
+  end
+end
diff --git a/spec/graphql/types/ci/group_environment_scope_type_spec.rb b/spec/graphql/types/ci/group_environment_scope_type_spec.rb
new file mode 100644
index 00000000000..3e3f52ca4bb
--- /dev/null
+++ b/spec/graphql/types/ci/group_environment_scope_type_spec.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe GitlabSchema.types['CiGroupEnvironmentScope'], feature_category: :secrets_management do
+  specify do
+    expect(described_class).to have_graphql_fields(
+      :name
+    ).at_least
+  end
+end
diff --git a/spec/models/ci/group_variable_spec.rb b/spec/models/ci/group_variable_spec.rb
index a2751b9fb20..5a8a2b391e1 100644
--- a/spec/models/ci/group_variable_spec.rb
+++ b/spec/models/ci/group_variable_spec.rb
@@ -54,6 +54,36 @@ RSpec.describe Ci::GroupVariable, feature_category: :secrets_management do
     it { expect(described_class.for_groups([group.id])).to eq([group_variable]) }
   end
 
+  describe '.for_environment_scope_like' do
+    let_it_be(:group) { create(:group) }
+    let_it_be(:variable1_on_staging1) { create(:ci_group_variable, group: group, environment_scope: 'staging1') }
+    let_it_be(:variable2_on_staging2) { create(:ci_group_variable, group: group, environment_scope: 'staging2') }
+    let_it_be(:variable3_on_production) { create(:ci_group_variable, group: group, environment_scope: 'production') }
+
+    it {
+      expect(described_class.for_environment_scope_like('staging'))
+        .to match_array([variable1_on_staging1, variable2_on_staging2])
+    }
+
+    it {
+      expect(described_class.for_environment_scope_like('production'))
+        .to match_array([variable3_on_production])
+    }
+  end
+
+  describe '.environment_scope_names' do
+    let_it_be(:group) { create(:group) }
+    let_it_be(:variable1_on_staging1) { create(:ci_group_variable, group: group, environment_scope: 'staging1') }
+    let_it_be(:variable2_on_staging2) { create(:ci_group_variable, group: group, environment_scope: 'staging2') }
+    let_it_be(:variable3_on_staging2) { create(:ci_group_variable, group: group, environment_scope: 'staging2') }
+    let_it_be(:variable4_on_production) { create(:ci_group_variable, group: group, environment_scope: 'production') }
+
+    it 'groups and orders' do
+      expect(described_class.environment_scope_names)
+        .to match_array(%w[production staging1 staging2])
+    end
+  end
+
   it_behaves_like 'cleanup by a loose foreign key' do
     let!(:model) { create(:ci_group_variable) }
 
diff --git a/spec/requests/api/graphql/ci/group_environment_scopes_spec.rb b/spec/requests/api/graphql/ci/group_environment_scopes_spec.rb
new file mode 100644
index 00000000000..13a3a128979
--- /dev/null
+++ b/spec/requests/api/graphql/ci/group_environment_scopes_spec.rb
@@ -0,0 +1,68 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'Query.group(fullPath).environmentScopes', feature_category: :secrets_management do
+  include GraphqlHelpers
+
+  let_it_be(:user) { create(:user) }
+  let_it_be(:group) { create(:group) }
+  let(:expected_environment_scopes) do
+    %w[
+      group1_environment1
+      group1_environment2
+      group2_environment3
+      group2_environment4
+      group2_environment5
+      group2_environment6
+    ]
+  end
+
+  let(:query) do
+    %(
+      query {
+        group(fullPath: "#{group.full_path}") {
+          environmentScopes#{environment_scopes_params} {
+            nodes {
+              name
+            }
+          }
+        }
+      }
+    )
+  end
+
+  before do
+    group.add_developer(user)
+    expected_environment_scopes.each_with_index do |env, index|
+      create(:ci_group_variable, group: group, key: "var#{index + 1}", environment_scope: env)
+    end
+  end
+
+  context 'when query has no parameters' do
+    let(:environment_scopes_params) { "" }
+
+    it 'returns all avaiable environment scopes' do
+      post_graphql(query, current_user: user)
+
+      expect(graphql_data.dig('group', 'environmentScopes', 'nodes')).to eq(
+        expected_environment_scopes.map { |env_scope| { 'name' => env_scope } }
+      )
+    end
+  end
+
+  context 'when query has search parameters' do
+    let(:environment_scopes_params) { "(search: \"group1\")" }
+
+    it 'returns only environment scopes with group1 prefix' do
+      post_graphql(query, current_user: user)
+
+      expect(graphql_data.dig('group', 'environmentScopes', 'nodes')).to eq(
+        [
+          { 'name' => 'group1_environment1' },
+          { 'name' => 'group1_environment2' }
+        ]
+      )
+    end
+  end
+end
diff --git a/spec/services/notes/update_service_spec.rb b/spec/services/notes/update_service_spec.rb
index 01b61acb476..e109bfbcd0b 100644
--- a/spec/services/notes/update_service_spec.rb
+++ b/spec/services/notes/update_service_spec.rb
@@ -47,6 +47,24 @@ RSpec.describe Notes::UpdateService, feature_category: :team_planning do
       end
     end
 
+    context 'when the note is invalid' do
+      let(:edit_note_text) { { note: 'new text' } }
+
+      before do
+        allow(note).to receive(:valid?).and_return(false)
+      end
+
+      it 'does not update the note' do
+        travel_to(1.day.from_now) do
+          expect { update_note(edit_note_text) }.not_to change { note.reload.updated_at }
+        end
+      end
+
+      it 'returns the note' do
+        expect(update_note(edit_note_text)).to eq(note)
+      end
+    end
+
     describe 'event tracking', :snowplow do
       let(:event) { Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_COMMENT_EDITED }
 
diff --git a/spec/support/finder_collection_allowlist.yml b/spec/support/finder_collection_allowlist.yml
index 8fcb4ee7b9c..7ac7e88867a 100644
--- a/spec/support/finder_collection_allowlist.yml
+++ b/spec/support/finder_collection_allowlist.yml
@@ -5,6 +5,7 @@
 # FooFinder  # Reason: It uses a memory backend
 - Namespaces::BilledUsersFinder # Reason: There is no need to have anything else besides the ids is current structure
 - Namespaces::FreeUserCap::UsersFinder # Reason: There is no need to have anything else besides the count
+- Groups::EnvironmentScopesFinder # Reason: There is no need to have anything else besides the simple strucutre with the scope name
 
 # Temporary excludes (aka TODOs)
 # For example:
diff --git a/spec/support/view_component.rb b/spec/support/view_component.rb
index 912bfda6d33..cddd39bbb1e 100644
--- a/spec/support/view_component.rb
+++ b/spec/support/view_component.rb
@@ -7,7 +7,7 @@ RSpec.configure do |config|
   config.include Devise::Test::ControllerHelpers, type: :component
 
   config.before(:each, type: :component) do
-    @request = controller.request
+    @request = vc_test_controller.request
   end
 
   config.include_context 'when page has no HTML escapes', type: :component