Merge branch '227244-dast-site-profile-edit' into 'master'

DAST Site profile - Form MVC - Add edit capability - Frontend

See merge request gitlab-org/gitlab!38315

ee/app/assets/javascripts/dast_site_profiles_form/components/dast_site_profile_form.vue

 <script>
 import * as Sentry from '@sentry/browser';
+import { isEqual } from 'lodash';
 import { __, s__ } from '~/locale';
 import { isAbsolute, redirectTo } from '~/lib/utils/url_utility';
 import { GlAlert, GlButton, GlForm, GlFormGroup, GlFormInput, GlModal } from '@gitlab/ui';
 import dastSiteProfileCreateMutation from '../graphql/dast_site_profile_create.mutation.graphql';
+import dastSiteProfileUpdateMutation from '../graphql/dast_site_profile_update.mutation.graphql';
 
 const initField = value => ({
   value,
@@ -11,6 +13,9 @@ const initField = value => ({
   feedback: null,
 });
 
+const extractFormValues = form =>
+  Object.fromEntries(Object.entries(form).map(([key, { value }]) => [key, value]));
+
 export default {
   name: 'DastSiteProfileForm',
   components: {
@@ -30,33 +35,56 @@ export default {
       type: String,
       required: true,
     },
+    siteProfile: {
+      type: Object,
+      required: false,
+      default: null,
+    },
   },
   data() {
+    const { name = '', targetUrl = '' } = this.siteProfile || {};
+    const form = {
+      profileName: initField(name),
+      targetUrl: initField(targetUrl),
+    };
     return {
-      form: {
-        profileName: initField(''),
-        targetUrl: initField(''),
-      },
+      form,
+      initialFormValues: extractFormValues(form),
       loading: false,
       showAlert: false,
     };
   },
   computed: {
-    formData() {
+    isEdit() {
+      return Boolean(this.siteProfile?.id);
+    },
+    i18n() {
+      const { isEdit } = this;
       return {
-        fullPath: this.fullPath,
-        ...Object.fromEntries(Object.entries(this.form).map(([key, { value }]) => [key, value])),
+        title: isEdit
+          ? s__('DastProfiles|Edit site profile')
+          : s__('DastProfiles|New site profile'),
+        errorMessage: isEdit
+          ? s__('DastProfiles|Could not update the site profile. Please try again.')
+          : s__('DastProfiles|Could not create the site profile. Please try again.'),
+        modal: {
+          title: isEdit
+            ? s__('DastProfiles|Do you want to discard your changes?')
+            : s__('DastProfiles|Do you want to discard this site profile?'),
+          okTitle: __('Discard'),
+          cancelTitle: __('Cancel'),
+        },
       };
     },
+    formTouched() {
+      return !isEqual(extractFormValues(this.form), this.initialFormValues);
+    },
     formHasErrors() {
       return Object.values(this.form).some(({ state }) => state === false);
     },
     someFieldEmpty() {
       return Object.values(this.form).some(({ value }) => !value);
     },
-    everyFieldEmpty() {
-      return Object.values(this.form).every(({ value }) => !value);
-    },
     isSubmitDisabled() {
       return this.formHasErrors || this.someFieldEmpty;
     },
@@ -76,19 +104,32 @@ export default {
     onSubmit() {
       this.loading = true;
       this.hideErrors();
+
+      const variables = {
+        fullPath: this.fullPath,
+        ...(this.isEdit ? { id: this.siteProfile.id } : {}),
+        ...extractFormValues(this.form),
+      };
+
       this.$apollo
         .mutate({
-          mutation: dastSiteProfileCreateMutation,
-          variables: this.formData,
-        })
-        .then(({ data: { dastSiteProfileCreate: { errors } } }) => {
-          if (errors?.length > 0) {
-            this.showErrors(errors);
-            this.loading = false;
-          } else {
-            redirectTo(this.profilesLibraryPath);
-          }
+          mutation: this.isEdit ? dastSiteProfileUpdateMutation : dastSiteProfileCreateMutation,
+          variables,
         })
+        .then(
+          ({
+            data: {
+              [this.isEdit ? 'dastSiteProfileUpdate' : 'dastSiteProfileCreate']: { errors = [] },
+            },
+          }) => {
+            if (errors.length > 0) {
+              this.showErrors(errors);
+              this.loading = false;
+            } else {
+              redirectTo(this.profilesLibraryPath);
+            }
+          },
+        )
         .catch(e => {
           Sentry.captureException(e);
           this.showErrors();
@@ -96,7 +137,7 @@ export default {
         });
     },
     onCancelClicked() {
-      if (this.everyFieldEmpty) {
+      if (!this.formTouched) {
         this.discard();
       } else {
         this.$refs[this.$options.modalId].show();
@@ -115,22 +156,17 @@ export default {
     },
   },
   modalId: 'deleteDastProfileModal',
-  i18n: {
-    modalTitle: s__('DastProfiles|Do you want to discard this site profile?'),
-    modalOkTitle: __('Discard'),
-    modalCancelTitle: __('Cancel'),
-  },
 };
 </script>
 
 <template>
   <gl-form @submit.prevent="onSubmit">
     <h2 class="gl-mb-6">
-      {{ s__('DastProfiles|New site profile') }}
+      {{ i18n.title }}
     </h2>
 
     <gl-alert v-if="showAlert" variant="danger" class="gl-mb-5" @dismiss="hideErrors">
-      {{ s__('DastProfiles|Could not create the site profile. Please try again.') }}
+      {{ i18n.errorMessage }}
       <ul v-if="errors.length" class="gl-mt-3 gl-mb-0">
         <li v-for="error in errors" :key="error" v-text="error"></li>
       </ul>
@@ -182,9 +218,9 @@ export default {
     <gl-modal
       :ref="$options.modalId"
       :modal-id="$options.modalId"
-      :title="$options.i18n.modalTitle"
-      :ok-title="$options.i18n.modalOkTitle"
-      :cancel-title="$options.i18n.modalCancelTitle"
+      :title="i18n.modal.title"
+      :ok-title="i18n.modal.okTitle"
+      :cancel-title="i18n.modal.cancelTitle"
       ok-variant="danger"
       body-class="gl-display-none"
       data-testid="dast-site-profile-form-cancel-modal"

ee/app/assets/javascripts/dast_site_profiles_form/graphql/dast_site_profile_update.mutation.graphql

+mutation dastSiteProfileUpdate(
+  $id: ID!
+  $fullPath: ID!
+  $profileName: String!
+  $targetUrl: String
+) {
+  project(fullPath: $fullPath) {
+    dastSiteProfileUpdate(input: { id: $id, profileName: $profileName, targetUrl: $targetUrl }) {
+      id
+      errors
+    }
+  }
+}

ee/app/assets/javascripts/dast_site_profiles_form/index.js

 import Vue from 'vue';
+import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
 import apolloProvider from './graphql/provider';
 import DastSiteProfileForm from './components/dast_site_profile_form.vue';
 
@@ -10,16 +11,22 @@ export default () => {
 
   const { fullPath, profilesLibraryPath } = el.dataset;
 
+  const props = {
+    fullPath,
+    profilesLibraryPath,
+  };
+
+  if (el.dataset.siteProfile) {
+    props.siteProfile = convertObjectPropsToCamelCase(JSON.parse(el.dataset.siteProfile));
+  }
+
   // eslint-disable-next-line no-new
   new Vue({
     el,
     apolloProvider,
     render(h) {
       return h(DastSiteProfileForm, {
-        props: {
-          fullPath,
-          profilesLibraryPath,
-        },
+        props,
       });
     },
   });

ee/app/assets/javascripts/pages/projects/dast_site_profiles/edit/index.js

+import initDastSiteProfileForm from 'ee/dast_site_profiles_form';
+
+document.addEventListener('DOMContentLoaded', initDastSiteProfileForm);

ee/app/controllers/projects/dast_site_profiles_controller.rb

@@ -7,6 +7,13 @@ module Projects
     def new
     end
 
+    def edit
+      @site_profile = @project
+        .dast_site_profiles
+        .with_dast_site
+        .find(params[:id])
+    end
+
     private
 
     def authorize_read_on_demand_scans!

ee/app/views/projects/dast_site_profiles/edit.html.haml

+- add_to_breadcrumbs s_('OnDemandScans|On-demand Scans'), project_on_demand_scans_path(@project)
+- add_to_breadcrumbs s_('DastProfiles|Manage profiles'), project_profiles_path(@project)
+- breadcrumb_title s_('DastProfiles|Edit site profile')
+- page_title s_('DastProfiles|Edit site profile')
+
+.js-dast-site-profile-form{ data: { full_path: @project.path_with_namespace,
+profiles_library_path: project_profiles_path(@project),
+site_profile: { id: @site_profile.id, name: @site_profile.name, target_url: @site_profile.dast_site.url }.to_json } }

ee/config/routes/project.rb

@@ -97,7 +97,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
           root 'on_demand_scans#index', as: 'on_demand_scans'
           scope :profiles do
             root 'dast_profiles#index', as: 'profiles'
-            resources :dast_site_profiles, only: [:new]
+            resources :dast_site_profiles, only: [:new, :edit]
           end
         end
 

ee/spec/frontend/dast_site_profiles_form/components/dast_site_profiles_form_spec.js

 import merge from 'lodash/merge';
+import { within } from '@testing-library/dom';
 import { mount, shallowMount } from '@vue/test-utils';
 import { GlAlert, GlForm, GlModal } from '@gitlab/ui';
 import { TEST_HOST } from 'helpers/test_constants';
 import DastSiteProfileForm from 'ee/dast_site_profiles_form/components/dast_site_profile_form.vue';
 import dastSiteProfileCreateMutation from 'ee/dast_site_profiles_form/graphql/dast_site_profile_create.mutation.graphql';
+import dastSiteProfileUpdateMutation from 'ee/dast_site_profiles_form/graphql/dast_site_profile_update.mutation.graphql';
 import { redirectTo } from '~/lib/utils/url_utility';
 
 jest.mock('~/lib/utils/url_utility', () => ({
@@ -24,6 +26,8 @@ const defaultProps = {
 describe('OnDemandScansApp', () => {
   let wrapper;
 
+  const withinComponent = () => within(wrapper.element);
+
   const findForm = () => wrapper.find(GlForm);
   const findProfileNameInput = () => wrapper.find('[data-testid="profile-name-input"]');
   const findTargetUrlInput = () => wrapper.find('[data-testid="target-url-input"]');
@@ -115,117 +119,133 @@ describe('OnDemandScansApp', () => {
     });
   });
 
-  describe('submission', () => {
-    const createdProfileId = 30203;
-
-    describe('on success', () => {
-      beforeEach(() => {
-        createComponent();
-        jest
-          .spyOn(wrapper.vm.$apollo, 'mutate')
-          .mockResolvedValue({ data: { dastSiteProfileCreate: { id: createdProfileId } } });
-        findProfileNameInput().vm.$emit('input', profileName);
-        findTargetUrlInput().vm.$emit('input', targetUrl);
-        submitForm();
+  describe.each`
+    title                  | siteProfile                                 | mutation                         | mutationVars | mutationKind
+    ${'New site profile'}  | ${null}                                     | ${dastSiteProfileCreateMutation} | ${{}}        | ${'dastSiteProfileCreate'}
+    ${'Edit site profile'} | ${{ id: 1, name: 'foo', targetUrl: 'bar' }} | ${dastSiteProfileUpdateMutation} | ${{ id: 1 }} | ${'dastSiteProfileUpdate'}
+  `('$title', ({ siteProfile, title, mutation, mutationVars, mutationKind }) => {
+    beforeEach(() => {
+      createFullComponent({
+        propsData: {
+          siteProfile,
+        },
       });
+    });
 
-      it('sets loading state', () => {
-        expect(findSubmitButton().props('loading')).toBe(true);
-      });
+    it('sets the correct title', () => {
+      expect(withinComponent().getByRole('heading', { name: title })).not.toBeNull();
+    });
 
-      it('triggers GraphQL mutation', () => {
-        expect(wrapper.vm.$apollo.mutate).toHaveBeenCalledWith({
-          mutation: dastSiteProfileCreateMutation,
-          variables: {
-            profileName,
-            targetUrl,
-            fullPath,
-          },
+    it('populates the fields with the data passed in via the siteProfile prop', () => {
+      expect(findProfileNameInput().element.value).toBe(siteProfile?.name ?? '');
+    });
+
+    describe('submission', () => {
+      const createdProfileId = 30203;
+
+      describe('on success', () => {
+        beforeEach(() => {
+          jest
+            .spyOn(wrapper.vm.$apollo, 'mutate')
+            .mockResolvedValue({ data: { [mutationKind]: { id: createdProfileId } } });
+          findProfileNameInput().vm.$emit('input', profileName);
+          findTargetUrlInput().vm.$emit('input', targetUrl);
+          submitForm();
         });
-      });
 
-      it('redirects to the profiles library', () => {
-        expect(redirectTo).toHaveBeenCalledWith(profilesLibraryPath);
-      });
+        it('sets loading state', () => {
+          expect(findSubmitButton().props('loading')).toBe(true);
+        });
 
-      it('does not show an alert', () => {
-        expect(findAlert().exists()).toBe(false);
-      });
-    });
+        it('triggers GraphQL mutation', () => {
+          expect(wrapper.vm.$apollo.mutate).toHaveBeenCalledWith({
+            mutation,
+            variables: {
+              profileName,
+              targetUrl,
+              fullPath,
+              ...mutationVars,
+            },
+          });
+        });
 
-    describe('on top-level error', () => {
-      beforeEach(() => {
-        createComponent();
-        jest.spyOn(wrapper.vm.$apollo, 'mutate').mockRejectedValue();
-        const input = findTargetUrlInput();
-        input.vm.$emit('input', targetUrl);
-        submitForm();
-      });
+        it('redirects to the profiles library', () => {
+          expect(redirectTo).toHaveBeenCalledWith(profilesLibraryPath);
+        });
 
-      it('resets loading state', () => {
-        expect(findSubmitButton().props('loading')).toBe(false);
+        it('does not show an alert', () => {
+          expect(findAlert().exists()).toBe(false);
+        });
       });
 
-      it('shows an error alert', () => {
-        expect(findAlert().exists()).toBe(true);
-      });
-    });
+      describe('on top-level error', () => {
+        beforeEach(() => {
+          jest.spyOn(wrapper.vm.$apollo, 'mutate').mockRejectedValue();
+          const input = findTargetUrlInput();
+          input.vm.$emit('input', targetUrl);
+          submitForm();
+        });
 
-    describe('on errors as data', () => {
-      const errors = ['error#1', 'error#2', 'error#3'];
-
-      beforeEach(() => {
-        createComponent();
-        jest
-          .spyOn(wrapper.vm.$apollo, 'mutate')
-          .mockResolvedValue({ data: { dastSiteProfileCreate: { pipelineUrl: null, errors } } });
-        const input = findTargetUrlInput();
-        input.vm.$emit('input', targetUrl);
-        submitForm();
-      });
+        it('resets loading state', () => {
+          expect(findSubmitButton().props('loading')).toBe(false);
+        });
 
-      it('resets loading state', () => {
-        expect(findSubmitButton().props('loading')).toBe(false);
+        it('shows an error alert', () => {
+          expect(findAlert().exists()).toBe(true);
+        });
       });
 
-      it('shows an alert with the returned errors', () => {
-        const alert = findAlert();
+      describe('on errors as data', () => {
+        const errors = ['error#1', 'error#2', 'error#3'];
 
-        expect(alert.exists()).toBe(true);
-        errors.forEach(error => {
-          expect(alert.text()).toContain(error);
+        beforeEach(() => {
+          jest
+            .spyOn(wrapper.vm.$apollo, 'mutate')
+            .mockResolvedValue({ data: { [mutationKind]: { errors } } });
+          const input = findTargetUrlInput();
+          input.vm.$emit('input', targetUrl);
+          submitForm();
         });
-      });
-    });
-  });
 
-  describe('cancellation', () => {
-    beforeEach(() => {
-      createFullComponent();
-    });
+        it('resets loading state', () => {
+          expect(findSubmitButton().props('loading')).toBe(false);
+        });
+
+        it('shows an alert with the returned errors', () => {
+          const alert = findAlert();
 
-    describe('form empty', () => {
-      it('redirects to the profiles library', () => {
-        findCancelButton().vm.$emit('click');
-        expect(redirectTo).toHaveBeenCalledWith(profilesLibraryPath);
+          expect(alert.exists()).toBe(true);
+          errors.forEach(error => {
+            expect(alert.text()).toContain(error);
+          });
+        });
       });
     });
 
-    describe('form not empty', () => {
-      beforeEach(() => {
-        findTargetUrlInput().setValue(targetUrl);
-        findProfileNameInput().setValue(profileName);
+    describe('cancellation', () => {
+      describe('form unchanged', () => {
+        it('redirects to the profiles library', () => {
+          findCancelButton().vm.$emit('click');
+          expect(redirectTo).toHaveBeenCalledWith(profilesLibraryPath);
+        });
       });
 
-      it('asks the user to confirm the action', () => {
-        jest.spyOn(findCancelModal().vm, 'show').mockReturnValue();
-        findCancelButton().trigger('click');
-        expect(findCancelModal().vm.show).toHaveBeenCalled();
-      });
+      describe('form changed', () => {
+        beforeEach(() => {
+          findTargetUrlInput().setValue(targetUrl);
+          findProfileNameInput().setValue(profileName);
+        });
 
-      it('redirects to the profiles library if confirmed', () => {
-        findCancelModal().vm.$emit('ok');
-        expect(redirectTo).toHaveBeenCalledWith(profilesLibraryPath);
+        it('asks the user to confirm the action', () => {
+          jest.spyOn(findCancelModal().vm, 'show').mockReturnValue();
+          findCancelButton().trigger('click');
+          expect(findCancelModal().vm.show).toHaveBeenCalled();
+        });
+
+        it('redirects to the profiles library if confirmed', () => {
+          findCancelModal().vm.$emit('ok');
+          expect(redirectTo).toHaveBeenCalledWith(profilesLibraryPath);
+        });
       });
     });
   });

ee/spec/requests/projects/dast_site_profiles_controller_spec.rb

@@ -5,8 +5,9 @@ require 'spec_helper'
 RSpec.describe Projects::DastSiteProfilesController, type: :request do
   let(:project) { create(:project) }
   let(:user) { create(:user) }
+  let(:dast_site_profile) { create(:dast_site_profile, project: project) }
 
-  describe 'GET #new' do
+  shared_examples 'a GET request' do
     context 'feature available' do
       before do
         stub_feature_flags(security_on_demand_scans_feature_flag: true)
@@ -21,7 +22,7 @@ RSpec.describe Projects::DastSiteProfilesController, type: :request do
         end
 
         it 'can access page' do
-          get project_profiles_path(project)
+          get path
 
           expect(response).to have_gitlab_http_status(:ok)
         end
@@ -35,7 +36,7 @@ RSpec.describe Projects::DastSiteProfilesController, type: :request do
         end
 
         it 'sees a 404 error' do
-          get project_profiles_path(project)
+          get path
 
           expect(response).to have_gitlab_http_status(:not_found)
         end
@@ -53,7 +54,7 @@ RSpec.describe Projects::DastSiteProfilesController, type: :request do
         it 'sees a 404 error' do
           stub_feature_flags(security_on_demand_scans_feature_flag: false)
           stub_licensed_features(security_on_demand_scans: true)
-          get project_profiles_path(project)
+          get path
 
           expect(response).to have_gitlab_http_status(:not_found)
         end
@@ -63,11 +64,23 @@ RSpec.describe Projects::DastSiteProfilesController, type: :request do
         it 'sees a 404 error' do
           stub_feature_flags(security_on_demand_scans_feature_flag: true)
           stub_licensed_features(security_on_demand_scans: false)
-          get project_profiles_path(project)
+          get path
 
           expect(response).to have_gitlab_http_status(:not_found)
         end
       end
     end
   end
+
+  describe 'GET #new' do
+    it_behaves_like 'a GET request' do
+      let(:path) { new_project_dast_site_profile_path(project) }
+    end
+  end
+
+  describe 'GET #edit' do
+    it_behaves_like 'a GET request' do
+      let(:path) { edit_project_dast_site_profile_path(project, dast_site_profile) }
+    end
+  end
 end

locale/gitlab.pot

@@ -7548,12 +7548,21 @@ msgstr ""
 msgid "DastProfiles|Could not create the site profile. Please try again."
 msgstr ""
 
+msgid "DastProfiles|Could not update the site profile. Please try again."
+msgstr ""
+
 msgid "DastProfiles|Do you want to discard this site profile?"
 msgstr ""
 
+msgid "DastProfiles|Do you want to discard your changes?"
+msgstr ""
+
 msgid "DastProfiles|Edit feature will come soon. Please create a new profile if changes needed"
 msgstr ""
 
+msgid "DastProfiles|Edit site profile"
+msgstr ""
+
 msgid "DastProfiles|Error fetching the profiles list. Please check your network connection and try again."
 msgstr ""