Automatic merge of gitlab-org/gitlab master

app/assets/javascripts/boards/components/board_list_header.vue

@@ -10,7 +10,7 @@ import {
 } from '@gitlab/ui';
 import { mapActions, mapState } from 'vuex';
 import { isListDraggable } from '~/boards/boards_util';
-import { isScopedLabel } from '~/lib/utils/common_utils';
+import { isScopedLabel, parseBoolean } from '~/lib/utils/common_utils';
 import { BV_HIDE_TOOLTIP } from '~/lib/utils/constants';
 import { n__, s__, __ } from '~/locale';
 import sidebarEventHub from '~/sidebar/event_hub';
@@ -69,7 +69,7 @@ export default {
     },
   },
   computed: {
-    ...mapState(['activeId']),
+    ...mapState(['activeId', 'isEpicBoard']),
     isLoggedIn() {
       return Boolean(this.currentUserId);
     },
@@ -97,11 +97,14 @@ export default {
     showListDetails() {
       return !this.list.collapsed || !this.isSwimlanesHeader;
     },
-    issuesCount() {
+    itemsCount() {
       return this.list.issuesCount;
     },
-    issuesTooltipLabel() {
-      return n__(`%d issue`, `%d issues`, this.issuesCount);
+    countIcon() {
+      return 'issues';
+    },
+    itemsTooltipLabel() {
+      return n__(`%d issue`, `%d issues`, this.itemsCount);
     },
     chevronTooltip() {
       return this.list.collapsed ? this.$options.i18n.expand : this.$options.i18n.collapse;
@@ -110,7 +113,7 @@ export default {
       return this.list.collapsed ? 'chevron-down' : 'chevron-right';
     },
     isNewIssueShown() {
-      return this.listType === ListType.backlog || this.showListHeaderButton;
+      return (this.listType === ListType.backlog || this.showListHeaderButton) && !this.isEpicBoard;
     },
     isSettingsShown() {
       return (
@@ -131,8 +134,14 @@ export default {
       return !this.disabled && isListDraggable(this.list);
     },
   },
+  created() {
+    const localCollapsed = parseBoolean(localStorage.getItem(`${this.uniqueKey}.collapsed`));
+    if ((!this.isLoggedIn || this.isEpicBoard) && localCollapsed) {
+      this.toggleListCollapsed({ listId: this.list.id, collapsed: true });
+    }
+  },
   methods: {
-    ...mapActions(['updateList', 'setActiveId']),
+    ...mapActions(['updateList', 'setActiveId', 'toggleListCollapsed']),
     openSidebarSettings() {
       if (this.activeId === inactiveId) {
         sidebarEventHub.$emit('sidebar.closeAll');
@@ -148,10 +157,10 @@ export default {
       eventHub.$emit(`toggle-issue-form-${this.list.id}`);
     },
     toggleExpanded() {
-      // eslint-disable-next-line vue/no-mutating-props
-      this.list.collapsed = !this.list.collapsed;
+      const collapsed = !this.list.collapsed;
+      this.toggleListCollapsed({ listId: this.list.id, collapsed });
 
-      if (!this.isLoggedIn) {
+      if (!this.isLoggedIn || this.isEpicBoard) {
         this.addToLocalStorage();
       } else {
         this.updateListFunction();
@@ -163,7 +172,7 @@ export default {
     },
     addToLocalStorage() {
       if (AccessorUtilities.isLocalStorageAccessSafe()) {
-        localStorage.setItem(`${this.uniqueKey}.expanded`, !this.list.collapsed);
+        localStorage.setItem(`${this.uniqueKey}.collapsed`, this.list.collapsed);
       }
     },
     updateListFunction() {
@@ -203,6 +212,7 @@ export default {
         class="board-title-caret no-drag gl-cursor-pointer"
         category="tertiary"
         size="small"
+        data-testid="board-title-caret"
         @click="toggleExpanded"
       />
       <!-- EE start -->
@@ -301,11 +311,11 @@ export default {
         <div v-if="list.maxIssueCount !== 0">
           •
           <gl-sprintf :message="__('%{issuesSize} with a limit of %{maxIssueCount}')">
-            <template #issuesSize>{{ issuesTooltipLabel }}</template>
+            <template #issuesSize>{{ itemsTooltipLabel }}</template>
             <template #maxIssueCount>{{ list.maxIssueCount }}</template>
           </gl-sprintf>
         </div>
-        <div v-else>• {{ issuesTooltipLabel }}</div>
+        <div v-else>• {{ itemsTooltipLabel }}</div>
         <div v-if="weightFeatureAvailable">
           •
           <gl-sprintf :message="__('%{totalWeight} total weight')">
@@ -323,13 +333,13 @@ export default {
         }"
       >
         <span class="gl-display-inline-flex">
-          <gl-tooltip :target="() => $refs.issueCount" :title="issuesTooltipLabel" />
-          <span ref="issueCount" class="issue-count-badge-count">
-            <gl-icon class="gl-mr-2" name="issues" />
-            <issue-count :issues-size="issuesCount" :max-issue-count="list.maxIssueCount" />
+          <gl-tooltip :target="() => $refs.itemCount" :title="itemsTooltipLabel" />
+          <span ref="itemCount" class="issue-count-badge-count">
+            <gl-icon class="gl-mr-2" :name="countIcon" />
+            <issue-count :issues-size="itemsCount" :max-issue-count="list.maxIssueCount" />
           </span>
           <!-- EE start -->
-          <template v-if="weightFeatureAvailable">
+          <template v-if="weightFeatureAvailable && !isEpicBoard">
             <gl-tooltip :target="() => $refs.weightTooltip" :title="weightCountToolTip" />
             <span ref="weightTooltip" class="gl-display-inline-flex gl-ml-3">
               <gl-icon class="gl-mr-2" name="weight" />

app/assets/javascripts/boards/stores/actions.js

@@ -256,6 +256,10 @@ export default {
       });
   },
 
+  toggleListCollapsed: ({ commit }, { listId, collapsed }) => {
+    commit(types.TOGGLE_LIST_COLLAPSED, { listId, collapsed });
+  },
+
   removeList: ({ state, commit }, listId) => {
     const listsBackup = { ...state.boardLists };
 

app/assets/javascripts/boards/stores/mutation_types.js

@@ -14,6 +14,7 @@ export const RECEIVE_ADD_LIST_SUCCESS = 'RECEIVE_ADD_LIST_SUCCESS';
 export const RECEIVE_ADD_LIST_ERROR = 'RECEIVE_ADD_LIST_ERROR';
 export const MOVE_LIST = 'MOVE_LIST';
 export const UPDATE_LIST_FAILURE = 'UPDATE_LIST_FAILURE';
+export const TOGGLE_LIST_COLLAPSED = 'TOGGLE_LIST_COLLAPSED';
 export const REMOVE_LIST = 'REMOVE_LIST';
 export const REMOVE_LIST_FAILURE = 'REMOVE_LIST_FAILURE';
 export const REQUEST_ITEMS_FOR_LIST = 'REQUEST_ITEMS_FOR_LIST';

app/assets/javascripts/boards/stores/mutations.js

@@ -105,6 +105,10 @@ export default {
     Vue.set(state, 'boardLists', backupList);
   },
 
+  [mutationTypes.TOGGLE_LIST_COLLAPSED]: (state, { listId, collapsed }) => {
+    Vue.set(state.boardLists[listId], 'collapsed', collapsed);
+  },
+
   [mutationTypes.REMOVE_LIST]: (state, listId) => {
     Vue.delete(state.boardLists, listId);
   },

app/assets/javascripts/snippets/components/snippet_visibility_edit.vue

@@ -55,7 +55,12 @@ export default {
         >
           <div class="d-flex align-items-center">
             <gl-icon :size="16" :name="option.icon" />
-            <span class="font-weight-bold ml-1 js-visibility-option">{{ option.label }}</span>
+            <span
+              class="font-weight-bold ml-1 js-visibility-option"
+              data-qa-selector="visibility_content"
+              :data-qa-visibility="option.label"
+              >{{ option.label }}</span
+            >
           </div>
           <template #help>{{
             isProjectSnippet && option.description_project

changelogs/unreleased/kassio-githubimporter-avoid-approval-exception.yml

+---
+title: 'GithubImporter: Add Merge request approval only if it does not exists yet'
+merge_request: 55376
+author:
+type: fixed

config/application.rb

@@ -193,7 +193,6 @@ module Gitlab
     config.assets.precompile << "page_bundles/import.css"
     config.assets.precompile << "page_bundles/incident_management_list.css"
     config.assets.precompile << "page_bundles/issues_list.css"
-    config.assets.precompile << "page_bundles/iterations.css"
     config.assets.precompile << "page_bundles/jira_connect.css"
     config.assets.precompile << "page_bundles/jira_connect_users.css"
     config.assets.precompile << "page_bundles/learn_gitlab.css"

ee/app/assets/javascripts/boards/components/board_list_header.vue

 <script>
+import { mapState } from 'vuex';
 // This is a false violation of @gitlab/no-runtime-template-compiler, since it
 // extends a valid Vue single file component.
 /* eslint-disable @gitlab/no-runtime-template-compiler */
 import BoardListHeaderFoss from '~/boards/components/board_list_header.vue';
-import { __, sprintf, s__ } from '~/locale';
+import { n__, __, sprintf, s__ } from '~/locale';
 
 export default {
   extends: BoardListHeaderFoss,
   inject: ['weightFeatureAvailable'],
   computed: {
-    issuesTooltip() {
+    ...mapState(['isEpicBoard']),
+    countIcon() {
+      return this.isEpicBoard ? 'epic' : 'issues';
+    },
+    itemsCount() {
+      return this.isEpicBoard ? this.list.epicsCount : this.list.issuesCount;
+    },
+    itemsTooltipLabel() {
       const { maxIssueCount } = this.list;
-
       if (maxIssueCount > 0) {
-        return sprintf(__('%{issuesCount} issues with a limit of %{maxIssueCount}'), {
-          issuesCount: this.issuesCount,
+        return sprintf(__('%{itemsCount} issues with a limit of %{maxIssueCount}'), {
+          itemsCount: this.itemsCount,
           maxIssueCount,
         });
       }
 
-      // TODO: Remove this pattern.
-      return BoardListHeaderFoss.computed.issuesTooltip.call(this);
+      return this.isEpicBoard
+        ? n__(`%d epic`, `%d epics`, this.itemsCount)
+        : n__(`%d issue`, `%d issues`, this.itemsCount);
     },
     weightCountToolTip() {
       const { totalWeight } = this.list;

ee/app/assets/javascripts/boards/components/boards_selector.vue

@@ -14,6 +14,9 @@ export default {
     showCreate() {
       return this.isEpicBoard || this.multipleIssueBoardsAvailable;
     },
+    showDelete() {
+      return this.boards.length > 1 && !this.isEpicBoard;
+    },
   },
   methods: {
     epicBoardUpdate(data) {

ee/app/assets/javascripts/boards/graphql/epic_board_list.fragment.graphql

@@ -5,6 +5,8 @@ fragment EpicBoardListFragment on EpicList {
   title
   position
   listType
+  collapsed
+  epicsCount
   label {
     ...Label
   }

ee/app/assets/javascripts/epic/components/epic_form.vue

@@ -158,12 +158,15 @@ export default {
           </template>
         </markdown-field>
       </gl-form-group>
-      <gl-form-group :label="__('Confidentiality')" label-for="epic-confidentiality">
+      <gl-form-group
+        :label="__('Confidentiality')"
+        label-for="epic-confidentiality"
+        data-qa-selector="confidential_epic_checkbox"
+      >
         <gl-form-checkbox
           id="epic-confidentiality"
           v-model="confidential"
           data-testid="epic-confidentiality"
-          data-qa-selector="confidential_epic_checkbox"
         >
           {{ $options.i18n.confidentialityLabel }}
         </gl-form-checkbox>

ee/app/assets/stylesheets/page_bundles/iterations.scss

-/* TODO Remove this CSS page bundle once this CSS override has been added to GitLab UI */
-
-.gl-alert .gl-label .gl-link {
-  text-decoration: none;
-}

ee/app/views/groups/iterations/show.html.haml

 - add_to_breadcrumbs _("Iterations"), group_iterations_path(@group)
 - breadcrumb_title params[:id]
 - page_title _("Iterations")
-- add_page_specific_style 'page_bundles/iterations'
 
 - if Feature.enabled?(:group_iterations, @group, default_enabled: true)
   .js-iteration{ data: { full_path: @group.full_path,

ee/app/views/projects/iterations/show.html.haml

 - add_to_breadcrumbs _("Iterations"), project_iterations_path(@project)
 - breadcrumb_title params[:id]
 - page_title _("Iteration")
-- add_page_specific_style 'page_bundles/iterations'
 
 .js-iteration{ data: { full_path: @project.full_path,
   can_edit: can?(current_user, :admin_iteration, @project).to_s,

ee/spec/features/epic_boards/epic_boards_spec.rb

@@ -10,8 +10,8 @@ RSpec.describe 'epic boards', :js do
   let_it_be(:label) { create(:group_label, group: group, name: 'Label1') }
   let_it_be(:label2) { create(:group_label, group: group, name: 'Label2') }
   let_it_be(:label_list) { create(:epic_list, epic_board: epic_board, label: label, position: 0) }
-  let_it_be(:backlog_list) { create(:epic_list, epic_board: epic_board, list_type: :closed) }
-  let_it_be(:closed_list) { create(:epic_list, epic_board: epic_board, list_type: :backlog) }
+  let_it_be(:backlog_list) { create(:epic_list, epic_board: epic_board, list_type: :backlog) }
+  let_it_be(:closed_list) { create(:epic_list, epic_board: epic_board, list_type: :closed) }
 
   let_it_be(:epic1) { create(:epic, group: group, labels: [label], title: 'Epic1') }
   let_it_be(:epic2) { create(:epic, group: group, title: 'Epic2') }
@@ -38,6 +38,8 @@ RSpec.describe 'epic boards', :js do
     end
 
     it 'displays two epics in Open list' do
+      expect(list_header(backlog_list)).to have_content('2')
+
       page.within("[data-board-type='backlog']") do
         expect(page).to have_selector('.board-card', count: 2)
         page.within(first('.board-card')) do
@@ -51,6 +53,8 @@ RSpec.describe 'epic boards', :js do
     end
 
     it 'displays one epic in Label list' do
+      expect(list_header(label_list)).to have_content('1')
+
       page.within("[data-board-type='label']") do
         expect(page).to have_selector('.board-card', count: 1)
         page.within(first('.board-card')) do
@@ -79,4 +83,8 @@ RSpec.describe 'epic boards', :js do
     visit group_epic_boards_path(group)
     wait_for_requests
   end
+
+  def list_header(list)
+    find(".board[data-id='gid://gitlab/Boards::EpicList/#{list.id}'] .board-header")
+  end
 end

lib/gitlab/github_import/importer/pull_request_review_importer.rb

@@ -77,12 +77,22 @@ module Gitlab
         def add_approval!(user_id)
           return unless review.review_type == 'APPROVED'
 
-          add_approval_system_note!(user_id)
-
-          merge_request.approvals.create!(
+          approval_attribues = {
+            merge_request_id: merge_request.id,
             user_id: user_id,
-            created_at: review.submitted_at
+            created_at: review.submitted_at,
+            updated_at: review.submitted_at
+          }
+
+          result = ::Approval.insert(
+            approval_attribues,
+            returning: [:id],
+            unique_by: [:user_id, :merge_request_id]
           )
+
+          if result.rows.present?
+            add_approval_system_note!(user_id)
+          end
         end
 
         def add_approval_system_note!(user_id)

locale/gitlab.pot

@@ -173,6 +173,11 @@ msgid_plural "%d days"
 msgstr[0] ""
 msgstr[1] ""
 
+msgid "%d epic"
+msgid_plural "%d epics"
+msgstr[0] ""
+msgstr[1] ""
+
 msgid "%d error"
 msgid_plural "%d errors"
 msgstr[0] ""
@@ -559,6 +564,9 @@ msgstr ""
 msgid "%{issuesSize} with a limit of %{maxIssueCount}"
 msgstr ""
 
+msgid "%{itemsCount} issues with a limit of %{maxIssueCount}"
+msgstr ""
+
 msgid "%{labelStart}Actual response:%{labelEnd} %{headers}"
 msgstr ""
 

qa/qa/ee/page/group/epic/new.rb

@@ -17,7 +17,7 @@ module QA
             end
 
             def enable_confidential_epic
-              click_element :confidential_epic_checkbox
+              find_element(:confidential_epic_checkbox, visible: false).click
             end
 
             def set_title(title)

qa/qa/page/component/new_snippet.rb

@@ -32,6 +32,10 @@ module QA
             # This 'element' is here only to ensure the changes in the view source aren't mistakenly changed
             element :_, "qa_selector = local_assigns.fetch(:qa_selector, '')" # rubocop:disable QA/ElementWithPattern
           end
+
+          base.view 'app/assets/javascripts/snippets/components/snippet_visibility_edit.vue' do
+            element :visibility_content
+          end
         end
 
         def fill_title(title)
@@ -44,7 +48,7 @@ module QA
         end
 
         def set_visibility(visibility)
-          choose visibility
+          click_element(:visibility_content, visibility: visibility)
         end
 
         def fill_file_name(name, file_number = nil)

spec/frontend/boards/components/board_list_header_spec.js

 import { shallowMount, createLocalVue } from '@vue/test-utils';
 import Vuex from 'vuex';
+import { extendedWrapper } from 'helpers/vue_test_utils_helper';
 
 import { mockLabelList } from 'jest/boards/mock_data';
 import BoardListHeader from '~/boards/components/board_list_header.vue';
@@ -14,6 +15,7 @@ describe('Board List Header Component', () => {
   let store;
 
   const updateListSpy = jest.fn();
+  const toggleListCollapsedSpy = jest.fn();
 
   afterEach(() => {
     wrapper.destroy();
@@ -43,38 +45,39 @@ describe('Board List Header Component', () => {
 
     if (withLocalStorage) {
       localStorage.setItem(
-        `boards.${boardId}.${listMock.listType}.${listMock.id}.expanded`,
-        (!collapsed).toString(),
+        `boards.${boardId}.${listMock.listType}.${listMock.id}.collapsed`,
+        collapsed.toString(),
       );
     }
 
     store = new Vuex.Store({
       state: {},
-      actions: { updateList: updateListSpy },
+      actions: { updateList: updateListSpy, toggleListCollapsed: toggleListCollapsedSpy },
       getters: {},
     });
 
-    wrapper = shallowMount(BoardListHeader, {
-      store,
-      localVue,
-      propsData: {
-        disabled: false,
-        list: listMock,
-      },
-      provide: {
-        boardId,
-        weightFeatureAvailable: false,
-        currentUserId,
-      },
-    });
+    wrapper = extendedWrapper(
+      shallowMount(BoardListHeader, {
+        store,
+        localVue,
+        propsData: {
+          disabled: false,
+          list: listMock,
+        },
+        provide: {
+          boardId,
+          weightFeatureAvailable: false,
+          currentUserId,
+        },
+      }),
+    );
   };
 
   const isCollapsed = () => wrapper.vm.list.collapsed;
-  const isExpanded = () => !isCollapsed;
 
   const findAddIssueButton = () => wrapper.find({ ref: 'newIssueBtn' });
   const findTitle = () => wrapper.find('.board-title');
-  const findCaret = () => wrapper.find('.board-title-caret');
+  const findCaret = () => wrapper.findByTestId('board-title-caret');
 
   describe('Add issue button', () => {
     const hasNoAddButton = [ListType.closed];
@@ -114,40 +117,29 @@ describe('Board List Header Component', () => {
   });
 
   describe('expanding / collapsing the column', () => {
-    it('does not collapse when clicking the header', async () => {
+    it('should display collapse icon when column is expanded', async () => {
       createComponent();
 
-      expect(isCollapsed()).toBe(false);
-
-      wrapper.find('[data-testid="board-list-header"]').trigger('click');
+      const icon = findCaret();
 
-      await wrapper.vm.$nextTick();
-
-      expect(isCollapsed()).toBe(false);
+      expect(icon.props('icon')).toBe('chevron-right');
     });
 
-    it('collapses expanded Column when clicking the collapse icon', async () => {
-      createComponent();
-
-      expect(isCollapsed()).toBe(false);
-
-      findCaret().vm.$emit('click');
+    it('should display expand icon when column is collapsed', async () => {
+      createComponent({ collapsed: true });
 
-      await wrapper.vm.$nextTick();
+      const icon = findCaret();
 
-      expect(isCollapsed()).toBe(true);
+      expect(icon.props('icon')).toBe('chevron-down');
     });
 
-    it('expands collapsed Column when clicking the expand icon', async () => {
-      createComponent({ collapsed: true });
-
-      expect(isCollapsed()).toBe(true);
+    it('should dispatch toggleListCollapse when clicking the collapse icon', async () => {
+      createComponent();
 
       findCaret().vm.$emit('click');
 
       await wrapper.vm.$nextTick();
-
-      expect(isCollapsed()).toBe(false);
+      expect(toggleListCollapsedSpy).toHaveBeenCalledTimes(1);
     });
 
     it("when logged in it calls list update and doesn't set localStorage", async () => {
@@ -157,7 +149,7 @@ describe('Board List Header Component', () => {
       await wrapper.vm.$nextTick();
 
       expect(updateListSpy).toHaveBeenCalledTimes(1);
-      expect(localStorage.getItem(`${wrapper.vm.uniqueKey}.expanded`)).toBe(null);
+      expect(localStorage.getItem(`${wrapper.vm.uniqueKey}.collapsed`)).toBe(null);
     });
 
     it("when logged out it doesn't call list update and sets localStorage", async () => {
@@ -167,7 +159,7 @@ describe('Board List Header Component', () => {
       await wrapper.vm.$nextTick();
 
       expect(updateListSpy).not.toHaveBeenCalled();
-      expect(localStorage.getItem(`${wrapper.vm.uniqueKey}.expanded`)).toBe(String(isExpanded()));
+      expect(localStorage.getItem(`${wrapper.vm.uniqueKey}.collapsed`)).toBe(String(isCollapsed()));
     });
   });
 

spec/frontend/boards/stores/actions_spec.js

@@ -452,6 +452,22 @@ describe('updateList', () => {
   });
 });
 
+describe('toggleListCollapsed', () => {
+  it('should commit TOGGLE_LIST_COLLAPSED mutation', async () => {
+    const payload = { listId: 'gid://gitlab/List/1', collapsed: true };
+    await testAction({
+      action: actions.toggleListCollapsed,
+      payload,
+      expectedMutations: [
+        {
+          type: types.TOGGLE_LIST_COLLAPSED,
+          payload,
+        },
+      ],
+    });
+  });
+});
+
 describe('removeList', () => {
   let state;
   const list = mockLists[0];

spec/frontend/boards/stores/mutations_spec.js

@@ -202,6 +202,24 @@ describe('Board Store Mutations', () => {
     });
   });
 
+  describe('TOGGLE_LIST_COLLAPSED', () => {
+    it('updates collapsed attribute of list in boardLists state', () => {
+      const listId = 'gid://gitlab/List/1';
+      state = {
+        ...state,
+        boardLists: {
+          [listId]: mockLists[0],
+        },
+      };
+
+      expect(state.boardLists[listId].collapsed).toEqual(false);
+
+      mutations.TOGGLE_LIST_COLLAPSED(state, { listId, collapsed: true });
+
+      expect(state.boardLists[listId].collapsed).toEqual(true);
+    });
+  });
+
   describe('REMOVE_LIST', () => {
     it('removes list from boardLists', () => {
       const [list, secondList] = mockLists;

spec/frontend/snippets/components/__snapshots__/snippet_visibility_edit_spec.js.snap

@@ -46,6 +46,8 @@ exports[`Snippet Visibility Edit component rendering matches the snapshot 1`] =
            
           <span
             class="font-weight-bold ml-1 js-visibility-option"
+            data-qa-selector="visibility_content"
+            data-qa-visibility="Private"
           >
             Private
           </span>
@@ -65,6 +67,8 @@ exports[`Snippet Visibility Edit component rendering matches the snapshot 1`] =
            
           <span
             class="font-weight-bold ml-1 js-visibility-option"
+            data-qa-selector="visibility_content"
+            data-qa-visibility="Internal"
           >
             Internal
           </span>
@@ -84,6 +88,8 @@ exports[`Snippet Visibility Edit component rendering matches the snapshot 1`] =
            
           <span
             class="font-weight-bold ml-1 js-visibility-option"
+            data-qa-selector="visibility_content"
+            data-qa-visibility="Public"
           >
             Public
           </span>

spec/lib/gitlab/github_import/importer/pull_request_review_importer_spec.rb

@@ -19,8 +19,10 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestReviewImporter, :clean
       context 'when the review is "APPROVED"' do
         let(:review) { create_review(type: 'APPROVED', note: '') }
 
-        it 'creates a note for the review' do
-          expect { subject.execute }.to change(Note, :count)
+        it 'creates a note for the review and approves the Merge Request' do
+          expect { subject.execute }
+            .to change(Note, :count).by(1)
+            .and change(Approval, :count).by(1)
 
           last_note = merge_request.notes.last
           expect(last_note.note).to eq('approved this merge request')
@@ -31,6 +33,14 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestReviewImporter, :clean
           expect(merge_request.approved_by_users.reload).to include(author)
           expect(merge_request.approvals.last.created_at).to eq(submitted_at)
         end
+
+        it 'does nothing if the user already approved the merge request' do
+          create(:approval, merge_request: merge_request, user: author)
+
+          expect { subject.execute }
+            .to change(Note, :count).by(0)
+            .and change(Approval, :count).by(0)
+        end
       end
 
       context 'when the review is "COMMENTED"' do