Merge branch 'tr-metric-image-delete' into 'master'

Adds delete functionality to metric images

See merge request gitlab-org/gitlab!50043

changelogs/unreleased/tr-metric-image-delete.yml

+---
+title: Add delete metric image REST API endpoint
+merge_request: 50043
+author:
+type: added

doc/api/issues.md

@@ -2211,3 +2211,26 @@ Example response:
     }
 ]
 ```
+
+## Delete metric image
+
+Available only for Incident issues.
+
+```plaintext
+DELETE /projects/:id/issues/:issue_iid/metric_images/:image_id
+```
+
+| Attribute   | Type    | Required | Description                          |
+|-------------|---------|----------|--------------------------------------|
+| `id`        | integer/string | yes      | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user  |
+| `issue_iid` | integer | yes      | The internal ID of a project's issue |
+| `image_id` | integer | yes      | The ID of the image |
+
+```shell
+curl --header "PRIVATE-TOKEN: <your_access_token>" --request DELETE "https://gitlab.example.com/api/v4/projects/5/issues/93/metric_images/1"
+```
+
+Can return the following status codes:
+
+- `204 No Content`, if the image was deleted successfully.
+- `400 Bad Request`, if the image could not be deleted.

ee/app/assets/javascripts/api.js

@@ -46,6 +46,8 @@ export default {
   projectDeploymentFrequencyAnalyticsPath:
     '/api/:version/projects/:id/analytics/deployment_frequency',
   issueMetricImagesPath: '/api/:version/projects/:id/issues/:issue_iid/metric_images',
+  issueMetricSingleImagePath:
+    '/api/:version/projects/:id/issues/:issue_iid/metric_images/:image_id',
 
   userSubscription(namespaceId) {
     const url = Api.buildUrl(this.subscriptionPath).replace(':id', encodeURIComponent(namespaceId));
@@ -367,4 +369,13 @@ export default {
 
     return axios.post(metricImagesUrl, formData, options);
   },
+
+  deleteMetricImage({ issueIid, id, imageId }) {
+    const individualMetricImageUrl = Api.buildUrl(this.issueMetricSingleImagePath)
+      .replace(':id', encodeURIComponent(id))
+      .replace(':issue_iid', encodeURIComponent(issueIid))
+      .replace(':image_id', encodeURIComponent(imageId));
+
+    return axios.delete(individualMetricImageUrl);
+  },
 };

ee/app/assets/javascripts/issue_show/components/incidents/metrics_image.vue

 <script>
-import { GlButton, GlCard, GlIcon, GlLink } from '@gitlab/ui';
+import { GlButton, GlCard, GlIcon, GlLink, GlModal, GlSprintf } from '@gitlab/ui';
+import { mapActions } from 'vuex';
+import { __, s__ } from '~/locale';
 
 export default {
+  i18n: {
+    modalDelete: __('Delete'),
+    modalDescription: s__('Incident|Are you sure you wish to delete this image?'),
+    modalCancel: __('Cancel'),
+    modalTitle: s__('Incident|Deleting %{filename}'),
+  },
   components: {
     GlButton,
     GlCard,
     GlIcon,
     GlLink,
+    GlModal,
+    GlSprintf,
   },
+  inject: ['canUpdate'],
   props: {
     id: {
       type: Number,
@@ -30,9 +41,22 @@ export default {
   data() {
     return {
       isCollapsed: false,
+      isDeleting: false,
+      modalVisible: false,
     };
   },
   computed: {
+    actionPrimaryProps() {
+      return {
+        text: this.$options.i18n.modalDelete,
+        attributes: {
+          loading: this.isDeleting,
+          disabled: this.isDeleting,
+          category: 'primary',
+          variant: 'danger',
+        },
+      };
+    },
     arrowIconName() {
       return this.isCollapsed ? 'chevron-right' : 'chevron-down';
     },
@@ -46,9 +70,19 @@ export default {
     },
   },
   methods: {
+    ...mapActions(['deleteImage']),
     toggleCollapsed() {
       this.isCollapsed = !this.isCollapsed;
     },
+    async onDelete() {
+      try {
+        this.isDeleting = true;
+        await this.deleteImage(this.id);
+      } finally {
+        this.isDeleting = false;
+        this.modalVisible = false;
+      }
+    },
   },
 };
 </script>
@@ -59,9 +93,28 @@ export default {
     header-class="gl-display-flex gl-align-items-center gl-border-b-0 gl-py-3"
     :body-class="bodyClass"
   >
+    <gl-modal
+      body-class="gl-pb-0! gl-min-h-6!"
+      modal-id="delete-metric-modal"
+      size="sm"
+      :visible="modalVisible"
+      :action-primary="actionPrimaryProps"
+      :action-cancel="{ text: $options.i18n.modalCancel }"
+      @primary.prevent="onDelete"
+      @hidden="modalVisible = false"
+    >
+      <template #modal-title>
+        <gl-sprintf :message="$options.i18n.modalTitle">
+          <template #filename>
+            {{ filename }}
+          </template>
+        </gl-sprintf>
+      </template>
+      <p>{{ $options.i18n.modalDescription }}</p>
+    </gl-modal>
     <template #header>
       <div class="gl-w-full gl-display-flex gl-flex-direction-row gl-justify-content-space-between">
-        <div class="gl-display-flex gl-flex-direction-row">
+        <div class="gl-display-flex gl-flex-direction-row gl-align-items-center gl-w-full">
           <gl-button
             class="collapsible-card-btn gl-display-flex gl-text-decoration-none gl-reset-color! gl-hover-text-blue-800! gl-shadow-none!"
             :aria-label="filename"
@@ -76,6 +129,13 @@ export default {
             {{ filename }}
           </gl-link>
           <span v-else>{{ filename }}</span>
+          <gl-button
+            v-if="canUpdate"
+            class="gl-ml-auto"
+            icon="remove"
+            data-testid="delete-button"
+            @click="modalVisible = true"
+          />
         </div>
       </div>
     </template>

ee/app/assets/javascripts/issue_show/components/incidents/metrics_tab.vue

@@ -89,7 +89,7 @@ export default {
       :action-cancel="{ text: $options.i18n.modalCancel }"
       :title="$options.i18n.modalTitle"
       :visible="modalVisible"
-      @canceled="clearInputs"
+      @hidden="clearInputs"
       @primary.prevent="onUpload"
     >
       <p>{{ $options.i18n.modalDescription }}</p>

ee/app/assets/javascripts/issue_show/components/incidents/service.js

@@ -10,3 +10,8 @@ export const uploadMetricImage = async (payload) => {
   const response = await Api.uploadIssueMetricImage(payload);
   return convertObjectPropsToCamelCase(response.data);
 };
+
+export const deleteMetricImage = async (payload) => {
+  const response = await Api.deleteMetricImage(payload);
+  return convertObjectPropsToCamelCase(response.data);
+};

ee/app/assets/javascripts/issue_show/components/incidents/store/actions.js

 import { s__ } from '~/locale';
 import createFlash from '~/flash';
 import * as types from './mutation_types';
-import { getMetricImages, uploadMetricImage } from '../service';
+import { deleteMetricImage, getMetricImages, uploadMetricImage } from '../service';
 
 export const fetchMetricImages = async ({ state, commit }) => {
   commit(types.REQUEST_METRIC_IMAGES);
@@ -31,6 +31,17 @@ export const uploadImage = async ({ state, commit }, { files, url }) => {
   }
 };
 
+export const deleteImage = async ({ state, commit }, imageId) => {
+  const { issueIid, projectId } = state;
+
+  try {
+    await deleteMetricImage({ imageId, id: projectId, issueIid });
+    commit(types.RECEIVE_METRIC_DELETE_SUCCESS, imageId);
+  } catch (error) {
+    createFlash({ message: s__('Incidents|There was an issue deleting the image.') });
+  }
+};
+
 export const setInitialData = ({ commit }, data) => {
   commit(types.SET_INITIAL_DATA, data);
 };

ee/app/assets/javascripts/issue_show/components/incidents/store/mutation_types.js

@@ -6,4 +6,6 @@ export const REQUEST_METRIC_UPLOAD = 'REQUEST_METRIC_UPLOAD';
 export const RECEIVE_METRIC_UPLOAD_SUCCESS = 'RECEIVE_METRIC_UPLOAD_SUCCESS';
 export const RECEIVE_METRIC_UPLOAD_ERROR = 'RECEIVE_METRIC_UPLOAD_ERROR';
 
+export const RECEIVE_METRIC_DELETE_SUCCESS = 'RECEIVE_METRIC_DELETE_SUCCESS';
+
 export const SET_INITIAL_DATA = 'SET_INITIAL_DATA';

ee/app/assets/javascripts/issue_show/components/incidents/store/mutations.js

@@ -21,6 +21,10 @@ export default {
   [types.RECEIVE_METRIC_UPLOAD_ERROR](state) {
     state.isUploadingImage = false;
   },
+  [types.RECEIVE_METRIC_DELETE_SUCCESS](state, imageId) {
+    const metricIndex = state.metricImages.findIndex((image) => image.id === imageId);
+    state.metricImages.splice(metricIndex, 1);
+  },
   [types.SET_INITIAL_DATA](state, { issueIid, projectId }) {
     state.issueIid = issueIid;
     state.projectId = projectId;

ee/lib/ee/api/issues.rb

@@ -70,6 +70,25 @@ module EE
                 render_api_error!('Issue not found', 404)
               end
             end
+
+            desc 'Remove a metric image for an issue' do
+              success Entities::IssuableMetricImage
+            end
+            params do
+              requires :metric_image_id, type: Integer, desc: 'The ID of metric image'
+            end
+            delete ':metric_image_id' do
+              issue = find_project_issue(params[:issue_iid])
+              metric_image = issue.metric_images.find_by_id(params[:metric_image_id])
+
+              render_api_error!('Metric image not found', 404) unless metric_image
+
+              if metric_image&.destroy
+                no_content!
+              else
+                render_api_error!('Metric image could not be deleted', 400)
+              end
+            end
           end
         end
 

ee/spec/frontend/issue_show/components/incidents/__snapshots__/metrics_image_spec.js.snap

@@ -7,6 +7,21 @@ exports[`Metrics upload item render the metrics image component 1`] = `
   footerclass=""
   headerclass="gl-display-flex gl-align-items-center gl-border-b-0 gl-py-3"
 >
+  <gl-modal-stub
+    actioncancel="[object Object]"
+    actionprimary="[object Object]"
+    body-class="gl-pb-0! gl-min-h-6!"
+    dismisslabel="Close"
+    modalclass=""
+    modalid="delete-metric-modal"
+    size="sm"
+    titletag="h4"
+  >
+     
+    <p>
+      Are you sure you wish to delete this image?
+    </p>
+  </gl-modal-stub>
     
   <div
     class="gl-display-flex gl-flex-direction-column"

ee/spec/frontend/issue_show/components/incidents/metrics_image_spec.js

-import { shallowMount, mount } from '@vue/test-utils';
-import { GlLink } from '@gitlab/ui';
+import Vuex from 'vuex';
+import { createLocalVue, shallowMount, mount } from '@vue/test-utils';
+import { GlLink, GlModal } from '@gitlab/ui';
+import merge from 'lodash/merge';
 import waitForPromises from 'helpers/wait_for_promises';
+import createStore from 'ee/issue_show/components/incidents/store';
 import MetricsImage from 'ee/issue_show/components/incidents/metrics_image.vue';
 
 const defaultProps = {
@@ -9,16 +12,32 @@ const defaultProps = {
   filename: 'test_file_name',
 };
 
+const mockEvent = { preventDefault: jest.fn() };
+
+const localVue = createLocalVue();
+localVue.use(Vuex);
+
 describe('Metrics upload item', () => {
   let wrapper;
+  let store;
+
+  const mountComponent = (options = {}, mountMethod = mount) => {
+    store = createStore();
 
-  const mountComponent = (propsData = {}, mountMethod = mount) => {
-    wrapper = mountMethod(MetricsImage, {
+    wrapper = mountMethod(
+      MetricsImage,
+      merge(
+        {
+          localVue,
+          store,
           propsData: {
             ...defaultProps,
-        ...propsData,
           },
-    });
+          provide: { canUpdate: true },
+        },
+        options,
+      ),
+    );
   };
 
   afterEach(() => {
@@ -31,6 +50,12 @@ describe('Metrics upload item', () => {
   const findImageLink = () => wrapper.find(GlLink);
   const findCollapseButton = () => wrapper.find('[data-testid="collapse-button"]');
   const findMetricImageBody = () => wrapper.find('[data-testid="metric-image-body"]');
+  const findModal = () => wrapper.find(GlModal);
+  const findDeleteButton = () => wrapper.find('[data-testid="delete-button"]');
+
+  const closeModal = () => findModal().vm.$emit('hidden');
+  const submitModal = () => findModal().vm.$emit('primary', mockEvent);
+  const deleteImage = () => findDeleteButton().vm.$emit('click');
 
   it('render the metrics image component', () => {
     mountComponent({}, shallowMount);
@@ -40,7 +65,7 @@ describe('Metrics upload item', () => {
 
   it('shows a link with the correct url', () => {
     const testUrl = 'test_url';
-    mountComponent({ url: testUrl });
+    mountComponent({ propsData: { url: testUrl } });
 
     expect(findImageLink().attributes('href')).toBe(testUrl);
     expect(findImageLink().text()).toBe(defaultProps.filename);
@@ -63,4 +88,55 @@ describe('Metrics upload item', () => {
       expect(findMetricImageBody().isVisible()).toBe(false);
     });
   });
+
+  describe('delete functionality', () => {
+    it('should open the modal when clicked', async () => {
+      mountComponent({ stubs: { GlModal: true } });
+
+      deleteImage();
+
+      await waitForPromises();
+
+      expect(findModal().attributes('visible')).toBe('true');
+    });
+
+    describe('when the modal is open', () => {
+      beforeEach(() => {
+        mountComponent(
+          {
+            data() {
+              return { modalVisible: true };
+            },
+          },
+          shallowMount,
+        );
+      });
+
+      it('should close the modal when cancelled', async () => {
+        closeModal();
+
+        await waitForPromises();
+
+        expect(findModal().attributes('visible')).toBeFalsy();
+      });
+
+      it('should delete the image when selected', async () => {
+        const dispatchSpy = jest.spyOn(store, 'dispatch').mockImplementation(jest.fn());
+
+        submitModal();
+
+        await waitForPromises();
+
+        expect(dispatchSpy).toHaveBeenCalledWith('deleteImage', defaultProps.id);
+      });
+    });
+
+    describe('canUpdate permission', () => {
+      it('delete button is hidden when user lacks update permissions', () => {
+        mountComponent({ provide: { canUpdate: false } });
+
+        expect(findDeleteButton().exists()).toBe(false);
+      });
+    });
+  });
 });

ee/spec/frontend/issue_show/components/incidents/metrics_tab_spec.js

@@ -58,7 +58,7 @@ describe('Metrics tab', () => {
   const findImages = () => wrapper.findAll(MetricsImage);
   const findModal = () => wrapper.find(GlModal);
   const submitModal = () => findModal().vm.$emit('primary', mockEvent);
-  const cancelModal = () => findModal().vm.$emit('canceled');
+  const cancelModal = () => findModal().vm.$emit('hidden');
 
   describe('empty state', () => {
     beforeEach(() => {

ee/spec/frontend/issue_show/components/incidents/store/actions_spec.js

@@ -4,7 +4,11 @@ import testAction from 'helpers/vuex_action_helper';
 import createStore from 'ee/issue_show/components/incidents/store';
 import * as actions from 'ee/issue_show/components/incidents/store/actions';
 import * as types from 'ee/issue_show/components/incidents/store/mutation_types';
-import { getMetricImages, uploadMetricImage } from 'ee/issue_show/components/incidents/service';
+import {
+  getMetricImages,
+  uploadMetricImage,
+  deleteMetricImage,
+} from 'ee/issue_show/components/incidents/service';
 import createFlash from '~/flash';
 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
 import { fileList, initialData } from '../mock_data';
@@ -13,6 +17,7 @@ jest.mock('~/flash');
 jest.mock('ee/issue_show/components/incidents/service', () => ({
   getMetricImages: jest.fn(),
   uploadMetricImage: jest.fn(),
+  deleteMetricImage: jest.fn(),
 }));
 
 const defaultState = {
@@ -100,6 +105,21 @@ describe('Metrics tab store actions', () => {
     });
   });
 
+  describe('deleting a metric image', () => {
+    const payload = fileList[0].id;
+
+    it('should call success action when deleting an image', () => {
+      deleteMetricImage.mockImplementation(() => Promise.resolve());
+
+      testAction(actions.deleteImage, payload, state, [
+        {
+          type: types.RECEIVE_METRIC_DELETE_SUCCESS,
+          payload,
+        },
+      ]);
+    });
+  });
+
   describe('initial data', () => {
     it('should set the initial data correctly', () => {
       testAction(actions.setInitialData, initialData, state, [

ee/spec/frontend/issue_show/components/incidents/store/mutation_spec.js

@@ -10,8 +10,9 @@ const defaultState = {
 };
 
 const testImages = [
-  { filename: 'test.filename', id: 0, filePath: 'test/file/path', url: null },
-  { filename: 'second.filename', id: 1, filePath: 'second/file/path', url: 'test/url' },
+  { filename: 'test.filename', id: 5, filePath: 'test/file/path', url: null },
+  { filename: 'second.filename', id: 6, filePath: 'second/file/path', url: 'test/url' },
+  { filename: 'third.filename', id: 7, filePath: 'third/file/path', url: 'test/url' },
 ];
 
 describe('Metric images mutations', () => {
@@ -100,6 +101,20 @@ describe('Metric images mutations', () => {
     });
   });
 
+  describe('RECEIVE_METRIC_DELETE_SUCCESS', () => {
+    const deletedImageId = testImages[1].id;
+    const expectedResult = [testImages[0], testImages[2]];
+
+    beforeEach(() => {
+      createState({ metricImages: [...testImages] });
+      mutations[types.RECEIVE_METRIC_DELETE_SUCCESS](state, deletedImageId);
+    });
+
+    it('should remove the correct metric image', () => {
+      expect(state.metricImages).toEqual(expectedResult);
+    });
+  });
+
   describe('SET_INITIAL_DATA', () => {
     beforeEach(() => {
       mutations[types.SET_INITIAL_DATA](state, initialData);

ee/spec/requests/api/issues_spec.rb

@@ -696,6 +696,82 @@ RSpec.describe API::Issues, :mailer do
     end
   end
 
+  describe 'DELETE /projects/:id/issues/:issue_id/metric_images/:metric_image_id' do
+    using RSpec::Parameterized::TableSyntax
+
+    let_it_be(:project) do
+      create(:project, :private, creator_id: user.id, namespace: user.namespace)
+    end
+
+    let!(:image) { create(:issuable_metric_image, issue: issue) }
+
+    subject { delete api("/projects/#{project.id}/issues/#{issue.iid}/metric_images/#{image.id}", user2) }
+
+    shared_examples 'can_delete_metric_image' do
+      it 'can delete the metric images' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:no_content)
+        expect { image.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
+
+    shared_examples 'unauthorized_delete' do
+      it 'cannot delete the metric image' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:not_found)
+        expect(image.reload).to eq(image)
+      end
+    end
+
+    where(:user_role, :own_issue, :issue_confidential, :expected_status) do
+      :not_member | false | false | :unauthorized_delete
+      :not_member | true  | false | :unauthorized_delete
+      :not_member | true  | true  | :unauthorized_delete
+      :guest      | false | true  | :unauthorized_delete
+      :guest      | true  | false | :can_delete_metric_image
+      :guest      | false | false | :can_delete_metric_image
+      :reporter   | true  | false | :can_delete_metric_image
+      :reporter   | false | false | :can_delete_metric_image
+    end
+
+    with_them do
+      before do
+        stub_licensed_features(incident_metric_upload: true)
+        project.send("add_#{user_role}", user2) unless user_role == :not_member
+      end
+
+      let!(:issue) do
+        author = own_issue ? user2 : user
+        confidential = issue_confidential
+
+        create(:incident, project: project, confidential: confidential, author: author)
+      end
+
+      it_behaves_like "#{params[:expected_status]}"
+    end
+
+    context 'user has access' do
+      let(:issue) { create(:incident, project: project) }
+
+      before do
+        project.add_reporter(user2)
+      end
+
+      context 'metric image not found' do
+        subject { delete api("/projects/#{project.id}/issues/#{issue.iid}/metric_images/#{non_existing_record_id}", user2) }
+
+        it 'returns an error' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:not_found)
+          expect(json_response['message']).to eq('Metric image not found')
+        end
+      end
+    end
+  end
+
   private
 
   def epic_issue_response_for(epic_issue)

locale/gitlab.pot

@@ -15042,6 +15042,9 @@ msgstr ""
 msgid "Incidents|Must start with http or https"
 msgstr ""
 
+msgid "Incidents|There was an issue deleting the image."
+msgstr ""
+
 msgid "Incidents|There was an issue loading metric images."
 msgstr ""
 
@@ -15054,6 +15057,12 @@ msgstr ""
 msgid "Incident|Alert details"
 msgstr ""
 
+msgid "Incident|Are you sure you wish to delete this image?"
+msgstr ""
+
+msgid "Incident|Deleting %{filename}"
+msgstr ""
+
 msgid "Incident|Metrics"
 msgstr ""