Automatic merge of gitlab-org/gitlab master

doc/ci/cloud_deployment/index.md

@@ -299,7 +299,7 @@ include:
   - template: Auto-DevOps.gitlab-ci.yml
 
 variables:
-  - AUTO_DEVOPS_PLATFORM_TARGET: EC2
+  AUTO_DEVOPS_PLATFORM_TARGET: EC2
 
 build_artifact:
   stage: build

ee/app/assets/javascripts/analytics/cycle_analytics/components/create_value_stream_form/constants.js

@@ -8,7 +8,7 @@ export const I18N = {
   FORM_CREATED: s__("CreateValueStreamForm|'%{name}' Value Stream created"),
   RECOVER_HIDDEN_STAGE: s__('CreateValueStreamForm|Recover hidden stage'),
   RESTORE_HIDDEN_STAGE: s__('CreateValueStreamForm|Restore stage'),
-  RESTORE_STAGES: s__('CreateValueStreamForm|Restore defaults'),
+  RESTORE_DEFAULTS: s__('CreateValueStreamForm|Restore defaults'),
   RECOVER_STAGE_TITLE: s__('CreateValueStreamForm|Default stages'),
   RECOVER_STAGES_VISIBLE: s__('CreateValueStreamForm|All default stages are currently visible'),
   SELECT_START_EVENT: s__('CreateValueStreamForm|Select start event'),

ee/app/assets/javascripts/analytics/cycle_analytics/store/actions.js

@@ -354,6 +354,24 @@ export const createValueStream = ({ commit, dispatch, getters }, data) => {
     });
 };
 
+export const updateValueStream = (
+  { commit, dispatch, getters },
+  { id: valueStreamId, ...data },
+) => {
+  const { currentGroupPath } = getters;
+  commit(types.REQUEST_UPDATE_VALUE_STREAM);
+
+  return Api.cycleAnalyticsUpdateValueStream({ groupId: currentGroupPath, valueStreamId, data })
+    .then(({ data: newValueStream }) => {
+      commit(types.RECEIVE_UPDATE_VALUE_STREAM_SUCCESS, newValueStream);
+      return dispatch('fetchCycleAnalyticsData');
+    })
+    .catch(({ response } = {}) => {
+      const { data: { message, payload: { errors } } = null } = response;
+      commit(types.RECEIVE_UPDATE_VALUE_STREAM_ERROR, { message, errors, data });
+    });
+};
+
 export const deleteValueStream = ({ commit, dispatch, getters }, valueStreamId) => {
   const { currentGroupPath } = getters;
   commit(types.REQUEST_DELETE_VALUE_STREAM);

ee/app/assets/javascripts/analytics/cycle_analytics/store/mutation_types.js

@@ -39,6 +39,10 @@ export const REQUEST_CREATE_VALUE_STREAM = 'REQUEST_CREATE_VALUE_STREAM';
 export const RECEIVE_CREATE_VALUE_STREAM_SUCCESS = 'RECEIVE_CREATE_VALUE_STREAM_SUCCESS';
 export const RECEIVE_CREATE_VALUE_STREAM_ERROR = 'RECEIVE_CREATE_VALUE_STREAM_ERROR';
 
+export const REQUEST_UPDATE_VALUE_STREAM = 'REQUEST_UPDATE_VALUE_STREAM';
+export const RECEIVE_UPDATE_VALUE_STREAM_SUCCESS = 'RECEIVE_UPDATE_VALUE_STREAM_SUCCESS';
+export const RECEIVE_UPDATE_VALUE_STREAM_ERROR = 'RECEIVE_UPDATE_VALUE_STREAM_ERROR';
+
 export const REQUEST_DELETE_VALUE_STREAM = 'REQUEST_DELETE_VALUE_STREAM';
 export const RECEIVE_DELETE_VALUE_STREAM_SUCCESS = 'RECEIVE_DELETE_VALUE_STREAM_SUCCESS';
 export const RECEIVE_DELETE_VALUE_STREAM_ERROR = 'RECEIVE_DELETE_VALUE_STREAM_ERROR';

ee/app/assets/javascripts/analytics/cycle_analytics/store/mutations.js

@@ -129,7 +129,21 @@ export default {
   [types.RECEIVE_CREATE_VALUE_STREAM_SUCCESS](state, valueStream) {
     state.isCreatingValueStream = false;
     state.createValueStreamErrors = {};
-    state.selectedValueStream = convertObjectPropsToCamelCase(valueStream);
+    state.selectedValueStream = convertObjectPropsToCamelCase(valueStream, { deep: true });
+  },
+  [types.REQUEST_UPDATE_VALUE_STREAM](state) {
+    state.isEditingValueStream = true;
+    state.createValueStreamErrors = {};
+  },
+  [types.RECEIVE_UPDATE_VALUE_STREAM_ERROR](state, { data: { stages = [] }, errors = {} }) {
+    const { stages: stageErrors = {}, ...rest } = errors;
+    state.createValueStreamErrors = { ...rest, stages: prepareStageErrors(stages, stageErrors) };
+    state.isEditingValueStream = false;
+  },
+  [types.RECEIVE_UPDATE_VALUE_STREAM_SUCCESS](state, valueStream) {
+    state.isEditingValueStream = false;
+    state.createValueStreamErrors = {};
+    state.selectedValueStream = convertObjectPropsToCamelCase(valueStream, { deep: true });
   },
   [types.REQUEST_DELETE_VALUE_STREAM](state) {
     state.isDeletingValueStream = true;
@@ -145,7 +159,7 @@ export default {
     state.selectedValueStream = null;
   },
   [types.SET_SELECTED_VALUE_STREAM](state, valueStream) {
-    state.selectedValueStream = convertObjectPropsToCamelCase(valueStream);
+    state.selectedValueStream = convertObjectPropsToCamelCase(valueStream, { deep: true });
   },
   [types.REQUEST_VALUE_STREAMS](state) {
     state.isLoadingValueStreams = true;

ee/app/assets/javascripts/analytics/cycle_analytics/store/state.js

@@ -22,6 +22,7 @@ export default () => ({
 
   isLoadingValueStreams: false,
   isCreatingValueStream: false,
+  isEditingValueStream: false,
   isDeletingValueStream: false,
 
   createValueStreamErrors: {},

ee/app/assets/javascripts/api.js

@@ -194,6 +194,13 @@ export default {
     return axios.post(url, data);
   },
 
+  cycleAnalyticsUpdateValueStream({ groupId, valueStreamId, data }) {
+    const url = Api.buildUrl(this.cycleAnalyticsValueStreamPath)
+      .replace(':id', groupId)
+      .replace(':value_stream_id', valueStreamId);
+    return axios.put(url, data);
+  },
+
   cycleAnalyticsDeleteValueStream(groupId, valueStreamId) {
     const url = Api.buildUrl(this.cycleAnalyticsValueStreamPath)
       .replace(':id', groupId)

ee/spec/frontend/analytics/cycle_analytics/store/actions_spec.js

@@ -16,6 +16,13 @@ import {
   valueStreams,
 } from '../mock_data';
 
+const mockStartEventIdentifier = 'issue_first_mentioned_in_commit';
+const mockEndEventIdentifier = 'issue_first_added_to_board';
+const mockEvents = {
+  startEventIdentifier: mockStartEventIdentifier,
+  endEventIdentifier: mockEndEventIdentifier,
+};
+
 const group = { fullPath: 'fake_group_full_path' };
 const milestonesPath = 'fake_milestones_path';
 const labelsPath = 'fake_labels_path';
@@ -904,7 +911,18 @@ describe('Value Stream Analytics actions', () => {
   });
 
   describe('createValueStream', () => {
-    const payload = { name: 'cool value stream' };
+    const payload = {
+      name: 'cool value stream',
+      stages: [
+        {
+          ...selectedStage,
+          ...mockEvents,
+          id: null,
+        },
+        { ...hiddenStage, ...mockEvents },
+      ],
+    };
+
     const createResp = { id: 'new value stream', is_custom: true, ...payload };
 
     beforeEach(() => {
@@ -957,6 +975,63 @@ describe('Value Stream Analytics actions', () => {
     });
   });
 
+  describe('updateValueStream', () => {
+    const payload = {
+      name: 'cool value stream',
+      stages: [
+        {
+          ...selectedStage,
+          ...mockEvents,
+          id: 'stage-1',
+        },
+        { ...hiddenStage, ...mockEvents },
+      ],
+    };
+    const updateResp = { id: 'new value stream', is_custom: true, ...payload };
+
+    beforeEach(() => {
+      state = { currentGroup };
+    });
+
+    describe('with no errors', () => {
+      beforeEach(() => {
+        mock.onPut(endpoints.valueStreamData).replyOnce(httpStatusCodes.OK, updateResp);
+      });
+
+      it(`commits the ${types.REQUEST_UPDATE_VALUE_STREAM} and ${types.RECEIVE_UPDATE_VALUE_STREAM_SUCCESS} actions`, () => {
+        return testAction(
+          actions.updateValueStream,
+          payload,
+          state,
+          [
+            { type: types.REQUEST_UPDATE_VALUE_STREAM },
+            { type: types.RECEIVE_UPDATE_VALUE_STREAM_SUCCESS, payload: updateResp },
+          ],
+          [{ type: 'fetchCycleAnalyticsData' }],
+        );
+      });
+    });
+
+    describe('with errors', () => {
+      const errors = { name: ['is taken'] };
+      const message = { message: 'error' };
+      const resp = { message, payload: { errors } };
+      beforeEach(() => {
+        mock.onPut(endpoints.valueStreamData).replyOnce(httpStatusCodes.NOT_FOUND, resp);
+      });
+
+      it(`commits the ${types.REQUEST_UPDATE_VALUE_STREAM} and ${types.RECEIVE_UPDATE_VALUE_STREAM_ERROR} actions `, () => {
+        return testAction(actions.updateValueStream, payload, state, [
+          { type: types.REQUEST_UPDATE_VALUE_STREAM },
+          {
+            type: types.RECEIVE_UPDATE_VALUE_STREAM_ERROR,
+            payload: { message, data: payload, errors },
+          },
+        ]);
+      });
+    });
+  });
+
   describe('deleteValueStream', () => {
     const payload = 'my-fake-value-stream';
 

ee/spec/frontend/analytics/cycle_analytics/store/mutations_spec.js

@@ -49,6 +49,10 @@ describe('Value Stream Analytics mutations', () => {
     ${types.RECEIVE_CREATE_VALUE_STREAM_SUCCESS} | ${'isCreatingValueStream'}   | ${false}
     ${types.REQUEST_CREATE_VALUE_STREAM}         | ${'createValueStreamErrors'} | ${{}}
     ${types.RECEIVE_CREATE_VALUE_STREAM_SUCCESS} | ${'createValueStreamErrors'} | ${{}}
+    ${types.REQUEST_UPDATE_VALUE_STREAM}         | ${'isEditingValueStream'}    | ${true}
+    ${types.RECEIVE_UPDATE_VALUE_STREAM_SUCCESS} | ${'isEditingValueStream'}    | ${false}
+    ${types.REQUEST_UPDATE_VALUE_STREAM}         | ${'createValueStreamErrors'} | ${{}}
+    ${types.RECEIVE_UPDATE_VALUE_STREAM_SUCCESS} | ${'createValueStreamErrors'} | ${{}}
     ${types.REQUEST_DELETE_VALUE_STREAM}         | ${'isDeletingValueStream'}   | ${true}
     ${types.RECEIVE_DELETE_VALUE_STREAM_SUCCESS} | ${'isDeletingValueStream'}   | ${false}
     ${types.REQUEST_DELETE_VALUE_STREAM}         | ${'deleteValueStreamError'}  | ${null}
@@ -61,17 +65,32 @@ describe('Value Stream Analytics mutations', () => {
     expect(state[stateKey]).toEqual(value);
   });
 
+  const valueStreamErrors = {
+    data: { stages },
+    errors: {
+      name: ['is required'],
+      stages: { 1: { name: "Can't be blank" } },
+    },
+  };
+
+  const expectedValueStreamErrors = {
+    name: ['is required'],
+    stages: [{}, { name: "Can't be blank" }, {}, {}, {}, {}, {}, {}],
+  };
+
   it.each`
-    mutation                                     | payload                                                    | expectedState
-    ${types.SET_FEATURE_FLAGS}                   | ${{ hasDurationChart: true }}                              | ${{ featureFlags: { hasDurationChart: true } }}
-    ${types.SET_SELECTED_PROJECTS}               | ${selectedProjects}                                        | ${{ selectedProjects }}
-    ${types.SET_DATE_RANGE}                      | ${{ startDate, endDate }}                                  | ${{ startDate, endDate }}
-    ${types.SET_SELECTED_STAGE}                  | ${{ id: 'first-stage' }}                                   | ${{ selectedStage: { id: 'first-stage' } }}
-    ${types.RECEIVE_CREATE_VALUE_STREAM_ERROR}   | ${{ data: { stages }, errors: { name: ['is required'] } }} | ${{ createValueStreamErrors: { name: ['is required'] }, isCreatingValueStream: false }}
-    ${types.RECEIVE_DELETE_VALUE_STREAM_ERROR}   | ${'Some error occurred'}                                   | ${{ deleteValueStreamError: 'Some error occurred' }}
-    ${types.RECEIVE_VALUE_STREAMS_SUCCESS}       | ${valueStreams}                                            | ${{ valueStreams, isLoadingValueStreams: false }}
-    ${types.SET_SELECTED_VALUE_STREAM}           | ${valueStreams[1].id}                                      | ${{ selectedValueStream: {} }}
-    ${types.RECEIVE_CREATE_VALUE_STREAM_SUCCESS} | ${valueStreams[1]}                                         | ${{ selectedValueStream: valueStreams[1] }}
+    mutation                                     | payload                       | expectedState
+    ${types.SET_FEATURE_FLAGS}                   | ${{ hasDurationChart: true }} | ${{ featureFlags: { hasDurationChart: true } }}
+    ${types.SET_SELECTED_PROJECTS}               | ${selectedProjects}           | ${{ selectedProjects }}
+    ${types.SET_DATE_RANGE}                      | ${{ startDate, endDate }}     | ${{ startDate, endDate }}
+    ${types.SET_SELECTED_STAGE}                  | ${{ id: 'first-stage' }}      | ${{ selectedStage: { id: 'first-stage' } }}
+    ${types.RECEIVE_CREATE_VALUE_STREAM_ERROR}   | ${valueStreamErrors}          | ${{ createValueStreamErrors: expectedValueStreamErrors, isCreatingValueStream: false }}
+    ${types.RECEIVE_UPDATE_VALUE_STREAM_ERROR}   | ${valueStreamErrors}          | ${{ createValueStreamErrors: expectedValueStreamErrors, isEditingValueStream: false }}
+    ${types.RECEIVE_DELETE_VALUE_STREAM_ERROR}   | ${'Some error occurred'}      | ${{ deleteValueStreamError: 'Some error occurred' }}
+    ${types.RECEIVE_VALUE_STREAMS_SUCCESS}       | ${valueStreams}               | ${{ valueStreams, isLoadingValueStreams: false }}
+    ${types.SET_SELECTED_VALUE_STREAM}           | ${valueStreams[1].id}         | ${{ selectedValueStream: {} }}
+    ${types.RECEIVE_CREATE_VALUE_STREAM_SUCCESS} | ${valueStreams[1]}            | ${{ selectedValueStream: valueStreams[1] }}
+    ${types.RECEIVE_UPDATE_VALUE_STREAM_SUCCESS} | ${valueStreams[1]}            | ${{ selectedValueStream: valueStreams[1] }}
   `(
     '$mutation with payload $payload will update state with $expectedState',
     ({ mutation, payload, expectedState }) => {

ee/spec/frontend/api_spec.js

@@ -374,6 +374,24 @@ describe('Api', () => {
       });
     });
 
+    describe('cycleAnalyticsUpdateValueStream', () => {
+      it('updates the custom value stream data', (done) => {
+        const response = {};
+        const customValueStream = { name: 'cool-value-stream-stage', stages: [] };
+        const expectedUrl = valueStreamBaseUrl({ resource: `value_streams/${valueStreamId}` });
+        mock.onPut(expectedUrl).reply(httpStatus.OK, response);
+
+        Api.cycleAnalyticsUpdateValueStream({ groupId, valueStreamId, data: customValueStream })
+          .then(({ data, config: { data: reqData, url } }) => {
+            expect(data).toEqual(response);
+            expect(JSON.parse(reqData)).toMatchObject(customValueStream);
+            expect(url).toEqual(expectedUrl);
+          })
+          .then(done)
+          .catch(done.fail);
+      });
+    });
+
     describe('cycleAnalyticsDeleteValueStream', () => {
       it('delete the custom value stream', (done) => {
         const response = {};

qa/qa/specs/features/api/1_manage/user_access_termination_spec.rb

+# frozen_string_literal: true
+
+module QA
+  RSpec.describe 'Manage' do
+    describe 'User', :requires_admin do
+      before(:all) do
+        admin_api_client = Runtime::API::Client.as_admin
+
+        @user = Resource::User.fabricate_via_api! do |user|
+          user.api_client = admin_api_client
+        end
+
+        @user_api_client = Runtime::API::Client.new(:gitlab, user: @user)
+
+        @group = Resource::Group.fabricate_via_api!
+
+        @group.sandbox.add_member(@user)
+
+        @project = Resource::Project.fabricate_via_api! do |project|
+          project.group = @group
+          project.name = "project-for-user-group-access-termination"
+          project.initialize_with_readme = true
+        end
+      end
+
+      context 'after parent group membership termination' do
+        before do
+          @group.sandbox.remove_member(@user)
+        end
+
+        it 'is not allowed to push code via the CLI', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/1660' do
+          expect do
+            Resource::Repository::Push.fabricate! do |push|
+              push.repository_http_uri = @project.repository_http_location.uri
+              push.file_name = 'test.txt'
+              push.file_content = "# This is a test project named #{@project.name}"
+              push.commit_message = 'Add test.txt'
+              push.branch_name = 'new_branch'
+              push.user = @user
+            end
+          end.to raise_error(QA::Support::Run::CommandError, /You are not allowed to push code to this project/)
+        end
+
+        it 'is not allowed to create a file via the API', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/1661' do
+          expect do
+            Resource::File.fabricate_via_api! do |file|
+              file.api_client = @user_api_client
+              file.project = @project
+              file.branch = 'new_branch'
+              file.commit_message = 'Add new file'
+              file.name = 'test.txt'
+              file.content = "New file"
+            end
+          end.to raise_error(Resource::ApiFabricator::ResourceFabricationFailedError, /403 Forbidden/)
+        end
+
+        it 'is not allowed to commit via the API', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/1662' do
+          expect do
+            Resource::Repository::Commit.fabricate_via_api! do |commit|
+              commit.api_client = @user_api_client
+              commit.project = @project
+              commit.branch = 'new_branch'
+              commit.start_branch = @project.default_branch
+              commit.commit_message = 'Add new file'
+              commit.add_files([
+                { file_path: 'test.txt', content: 'new file' }
+              ])
+            end
+          end.to raise_error(Resource::ApiFabricator::ResourceFabricationFailedError, /403 Forbidden - You are not allowed to push into this branch/)
+        end
+      end
+
+      after(:all) do
+        @user.remove_via_api!
+        @project.remove_via_api!
+        begin
+          @group.remove_via_api!
+        rescue Resource::ApiFabricator::ResourceNotDeletedError
+          # It is ok if the group is already marked for deletion by another test
+        end
+      end
+    end
+  end
+end

qa/qa/specs/features/browser_ui/1_manage/user/user_access_termination_spec.rb

+# frozen_string_literal: true
+
+module QA
+  RSpec.describe 'Manage' do
+    describe 'User', :requires_admin do
+      let(:admin_api_client) { Runtime::API::Client.as_admin }
+
+      let!(:user) do
+        Resource::User.fabricate_via_api! do |user|
+          user.api_client = admin_api_client
+        end
+      end
+
+      let!(:group) do
+        group = Resource::Group.fabricate_via_api!
+        group.sandbox.add_member(user)
+        group
+      end
+
+      let!(:project) do
+        Resource::Project.fabricate_via_api! do |project|
+          project.group = group
+          project.name = "project-for-user-access-termination"
+          project.initialize_with_readme = true
+        end
+      end
+
+      context 'after parent group membership termination' do
+        before do
+          Flow::Login.while_signed_in_as_admin do
+            group.sandbox.visit!
+
+            Page::Group::Menu.perform(&:click_group_members_item)
+            Page::Group::Members.perform do |members_page|
+              members_page.remove_member(user.username)
+            end
+          end
+        end
+
+        it 'is not allowed to edit the project files', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/1663' do
+          Flow::Login.sign_in(as: user)
+          project.visit!
+
+          Page::Project::Show.perform do |project|
+            project.click_file('README.md')
+          end
+
+          Page::File::Show.perform(&:click_edit)
+
+          expect(page).to have_text("You're not allowed to edit files in this project directly.")
+        end
+
+        after do
+          user.remove_via_api!
+          project.remove_via_api!
+          begin
+            group.remove_via_api!
+          rescue Resource::ApiFabricator::ResourceNotDeletedError
+            # It is ok if the group is already marked for deletion by another test
+          end
+        end
+      end
+    end
+  end
+end

qa/qa/specs/features/ee/api/1_manage/user/minimal_access_user_spec.rb

@@ -67,7 +67,11 @@ module QA
       after(:all) do
         @user_with_minimal_access.remove_via_api!
         @project.remove_via_api!
-        @group.remove_via_api!
+        begin
+          @group.remove_via_api!
+        rescue Resource::ApiFabricator::ResourceNotDeletedError
+          # It is ok if the group is already marked for deletion by another test
+        end
       end
     end
   end

qa/qa/specs/features/ee/browser_ui/1_manage/user/minimal_access_user_spec.rb

@@ -41,7 +41,11 @@ module QA
       after do
         user_with_minimal_access.remove_via_api!
         project.remove_via_api!
-        group.remove_via_api!
+        begin
+          group.remove_via_api!
+        rescue Resource::ApiFabricator::ResourceNotDeletedError
+          # It is ok if the group is already marked for deletion by another test
+        end
       end
     end
   end