From 6075cbe07688f3d072b23b51eab87afd2df673ab Mon Sep 17 00:00:00 2001 From: Fernando Date: Tue, 4 Aug 2020 21:16:17 -0500 Subject: [PATCH 01/21] Implemeent Vuex module for fetching security configurations * Impleement actions, mutations, mutation types, and store Add unit tests * Add mutations, actions unit tests Implement code review suggestions * Tweak mutations and update unit tests Update actions spec to use async/await * Update tests First pass of approval rules Add different row states * Add template markup and logic * Add state for job enabled, but approval rule not configured * Add state for job disabled and rule not configured * Add state for approval rule configured Tweak rendering logic * Still show row when job not properly configured Run prettier and linter * Lint and clean up formatting Add doc links * Add GLSprintf component for doc links Run prettier and linter * clean up code formatting Add loading state Run prettier and linter --- .../components/modal_rule_create.vue | 21 +++- .../project_settings/project_rules.vue | 73 +++++++++++- .../approvals/components/rule_form.vue | 14 ++- .../unconfigured_security_rule.vue | 111 ++++++++++++++++++ .../approvals/mount_project_settings.js | 11 ++ .../javascripts/approvals/stores/index.js | 2 + .../modules/configuration/index.js | 4 +- .../modules/configuration/mutations.js | 1 + .../modules/configuration/state.js | 1 + ..._request_approvals_settings_form.html.haml | 5 +- 10 files changed, 234 insertions(+), 9 deletions(-) create mode 100644 ee/app/assets/javascripts/approvals/components/security_configuration/unconfigured_security_rule.vue diff --git a/ee/app/assets/javascripts/approvals/components/modal_rule_create.vue b/ee/app/assets/javascripts/approvals/components/modal_rule_create.vue index 4e05b4b229039c..f917a714be4d15 100644 --- a/ee/app/assets/javascripts/approvals/components/modal_rule_create.vue +++ b/ee/app/assets/javascripts/approvals/components/modal_rule_create.vue @@ -22,8 +22,20 @@ export default { }, computed: { ...mapState('createModal', { - rule: 'data', + rule(state) { + /* + * rule-form component expects undefined if we pre-populate the form input, + * otherwise populate with existing rule + */ + return state.data?.initRuleField ? undefined : state.data; + }, + originalData: 'data', }), + initRuleFieldName() { + return this.originalData?.initRuleField && this.originalData?.name + ? this.originalData.name + : ''; + }, title() { return this.rule ? __('Update approval rule') : __('Add approval rule'); }, @@ -47,6 +59,11 @@ export default { size="sm" @ok.prevent="submit" > - + diff --git a/ee/app/assets/javascripts/approvals/components/project_settings/project_rules.vue b/ee/app/assets/javascripts/approvals/components/project_settings/project_rules.vue index c252daaedc8cd4..047f1b58813519 100644 --- a/ee/app/assets/javascripts/approvals/components/project_settings/project_rules.vue +++ b/ee/app/assets/javascripts/approvals/components/project_settings/project_rules.vue @@ -1,6 +1,6 @@ + + diff --git a/ee/app/assets/javascripts/approvals/mount_project_settings.js b/ee/app/assets/javascripts/approvals/mount_project_settings.js index 91cd298738c020..f922d3fd6f7894 100644 --- a/ee/app/assets/javascripts/approvals/mount_project_settings.js +++ b/ee/app/assets/javascripts/approvals/mount_project_settings.js @@ -12,6 +12,12 @@ export default function mountProjectSettingsApprovals(el) { return null; } + const { + securityConfigurationPath, + vulnerabilityCheckHelpPagePath, + licenseCheckHelpPagePath, + } = el.dataset; + const store = createStore(projectSettingsModule(), { ...el.dataset, prefix: 'project-settings', @@ -22,6 +28,11 @@ export default function mountProjectSettingsApprovals(el) { return new Vue({ el, store, + provide: { + securityConfigurationPath, + vulnerabilityCheckHelpPagePath, + licenseCheckHelpPagePath, + }, render(h) { return h(ProjectSettingsApp); }, diff --git a/ee/app/assets/javascripts/approvals/stores/index.js b/ee/app/assets/javascripts/approvals/stores/index.js index b17514eab74678..f1e7afa8e7c75a 100644 --- a/ee/app/assets/javascripts/approvals/stores/index.js +++ b/ee/app/assets/javascripts/approvals/stores/index.js @@ -1,5 +1,6 @@ import Vuex from 'vuex'; import modalModule from '~/vuex_shared/modules/modal'; +import securityConfigurationModule from 'ee/security_configuration/modules/configuration'; import state from './state'; export const createStoreOptions = (approvalsModule, settings) => ({ @@ -8,6 +9,7 @@ export const createStoreOptions = (approvalsModule, settings) => ({ ...(approvalsModule ? { approvals: approvalsModule } : {}), createModal: modalModule(), deleteModal: modalModule(), + securityConfiguration: securityConfigurationModule(), }, }); diff --git a/ee/app/assets/javascripts/security_configuration/modules/configuration/index.js b/ee/app/assets/javascripts/security_configuration/modules/configuration/index.js index 68c81bb45096f1..636092ce1a9aab 100644 --- a/ee/app/assets/javascripts/security_configuration/modules/configuration/index.js +++ b/ee/app/assets/javascripts/security_configuration/modules/configuration/index.js @@ -2,9 +2,9 @@ import state from './state'; import mutations from './mutations'; import * as actions from './actions'; -export default { +export default () => ({ namespaced: true, state, mutations, actions, -}; +}); diff --git a/ee/app/assets/javascripts/security_configuration/modules/configuration/mutations.js b/ee/app/assets/javascripts/security_configuration/modules/configuration/mutations.js index a45bbbdaa838c7..b64962f5172206 100644 --- a/ee/app/assets/javascripts/security_configuration/modules/configuration/mutations.js +++ b/ee/app/assets/javascripts/security_configuration/modules/configuration/mutations.js @@ -10,6 +10,7 @@ export default { }, [types.RECEIVE_SECURITY_CONFIGURATION_SUCCESS](state, payload) { state.isLoading = false; + state.hasLoaded = true; state.errorLoading = false; state.configuration = payload; }, diff --git a/ee/app/assets/javascripts/security_configuration/modules/configuration/state.js b/ee/app/assets/javascripts/security_configuration/modules/configuration/state.js index 872f40846ad949..4a7221e549a626 100644 --- a/ee/app/assets/javascripts/security_configuration/modules/configuration/state.js +++ b/ee/app/assets/javascripts/security_configuration/modules/configuration/state.js @@ -1,6 +1,7 @@ export default () => ({ securityConfigurationPath: '', isLoading: false, + hasLoaded: false, errorLoading: false, configuration: {}, }); diff --git a/ee/app/views/projects/_merge_request_approvals_settings_form.html.haml b/ee/app/views/projects/_merge_request_approvals_settings_form.html.haml index f3ffd3c4fe557b..9a624515f54713 100644 --- a/ee/app/views/projects/_merge_request_approvals_settings_form.html.haml +++ b/ee/app/views/projects/_merge_request_approvals_settings_form.html.haml @@ -13,7 +13,10 @@ 'rules_path': expose_path(api_v4_projects_approval_settings_rules_path(id: @project.id)), 'allow_multi_rule': @project.multiple_approval_rules_available?.to_s, 'eligible_approvers_docs_path': help_page_path('user/project/merge_requests/merge_request_approvals', anchor: 'eligible-approvers'), - 'security_approvals_help_page_path': help_page_path('user/application_security/index.md', anchor: 'security-approvals-in-merge-requests-ultimate')} } + 'security_approvals_help_page_path': help_page_path('user/application_security/index.md', anchor: 'security-approvals-in-merge-requests-ultimate'), + 'security_configuration_path': project_security_configuration_path(@project), + 'vulnerability_check_help_page_path': help_page_path('user/application_security/index', anchor: 'enabling-security-approvals-within-a-project'), + 'license_check_help_page_path': help_page_path('user/application_security/index', anchor: 'enabling-license-approvals-within-a-project')} } .text-center.gl-mt-3 = sprite_icon('spinner', size: 24, css_class: 'gl-spinner') -- GitLab From 86c322d62a1dd5848ab716e8ca93ecbf988aed18 Mon Sep 17 00:00:00 2001 From: Fernando Date: Mon, 10 Aug 2020 00:51:03 -0500 Subject: [PATCH 02/21] Add approval_suggestions feature flag * Isolate changes behind feature flag [as much as possible]) --- app/controllers/projects_controller.rb | 3 +- .../components/modal_rule_create.vue | 25 ++++++-- .../project_settings/project_rules.vue | 35 +++++++---- .../approvals/components/rule_form.vue | 62 ++++++++++++++----- 4 files changed, 92 insertions(+), 33 deletions(-) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 5146a44de839ff..4f286cb301a981 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -35,10 +35,11 @@ class ProjectsController < Projects::ApplicationController before_action :export_rate_limit, only: [:export, :download_export, :generate_new_export] # Experiments - before_action only: [:new, :create] do + before_action only: [:new, :create, :edit] do frontend_experimentation_tracking_data(:new_create_project_ui, 'click_tab') push_frontend_feature_flag(:new_create_project_ui) if experiment_enabled?(:new_create_project_ui) push_frontend_feature_flag(:service_desk_custom_address, @project) + push_frontend_feature_flag(:approval_suggestions, project) end layout :determine_layout diff --git a/ee/app/assets/javascripts/approvals/components/modal_rule_create.vue b/ee/app/assets/javascripts/approvals/components/modal_rule_create.vue index f917a714be4d15..e1e95a7f72017e 100644 --- a/ee/app/assets/javascripts/approvals/components/modal_rule_create.vue +++ b/ee/app/assets/javascripts/approvals/components/modal_rule_create.vue @@ -1,4 +1,5 @@ - +
{{ matchRule.name }}
-- GitLab From b5888f5fd6e8188648bca042423b9f17044fffa1 Mon Sep 17 00:00:00 2001 From: Fernando Date: Sat, 15 Aug 2020 05:03:43 -0500 Subject: [PATCH 12/21] Move security rule specific logic into its own component * Isolate and extract logic into seperate wrapper component * Set endpoint url at store creation and not in Vue lifecycle hook * Remove uneeded 'this' in 'some' call * Remove rules.? --- .../project_settings/project_rules.vue | 94 ++----------------- .../unconfigured_security_rule.vue | 6 +- .../unconfigured_security_rules.vue | 81 ++++++++++++++++ .../approvals/mount_project_settings.js | 7 +- .../javascripts/approvals/stores/index.js | 4 +- .../modules/configuration/index.js | 6 +- .../modules/configuration/state.js | 4 +- 7 files changed, 103 insertions(+), 99 deletions(-) create mode 100644 ee/app/assets/javascripts/approvals/components/security_configuration/unconfigured_security_rules.vue diff --git a/ee/app/assets/javascripts/approvals/components/project_settings/project_rules.vue b/ee/app/assets/javascripts/approvals/components/project_settings/project_rules.vue index a283d802a23542..146d581abc0418 100644 --- a/ee/app/assets/javascripts/approvals/components/project_settings/project_rules.vue +++ b/ee/app/assets/javascripts/approvals/components/project_settings/project_rules.vue @@ -1,13 +1,8 @@ + + diff --git a/ee/app/assets/javascripts/approvals/mount_project_settings.js b/ee/app/assets/javascripts/approvals/mount_project_settings.js index f922d3fd6f7894..d08a630ca682de 100644 --- a/ee/app/assets/javascripts/approvals/mount_project_settings.js +++ b/ee/app/assets/javascripts/approvals/mount_project_settings.js @@ -12,11 +12,7 @@ export default function mountProjectSettingsApprovals(el) { return null; } - const { - securityConfigurationPath, - vulnerabilityCheckHelpPagePath, - licenseCheckHelpPagePath, - } = el.dataset; + const { vulnerabilityCheckHelpPagePath, licenseCheckHelpPagePath } = el.dataset; const store = createStore(projectSettingsModule(), { ...el.dataset, @@ -29,7 +25,6 @@ export default function mountProjectSettingsApprovals(el) { el, store, provide: { - securityConfigurationPath, vulnerabilityCheckHelpPagePath, licenseCheckHelpPagePath, }, diff --git a/ee/app/assets/javascripts/approvals/stores/index.js b/ee/app/assets/javascripts/approvals/stores/index.js index f1e7afa8e7c75a..e49a9bed016cf7 100644 --- a/ee/app/assets/javascripts/approvals/stores/index.js +++ b/ee/app/assets/javascripts/approvals/stores/index.js @@ -9,7 +9,9 @@ export const createStoreOptions = (approvalsModule, settings) => ({ ...(approvalsModule ? { approvals: approvalsModule } : {}), createModal: modalModule(), deleteModal: modalModule(), - securityConfiguration: securityConfigurationModule(), + securityConfiguration: securityConfigurationModule({ + securityConfigurationPath: settings.securityConfigurationPath, + }), }, }); diff --git a/ee/app/assets/javascripts/security_configuration/modules/configuration/index.js b/ee/app/assets/javascripts/security_configuration/modules/configuration/index.js index 636092ce1a9aab..b9849719c571f7 100644 --- a/ee/app/assets/javascripts/security_configuration/modules/configuration/index.js +++ b/ee/app/assets/javascripts/security_configuration/modules/configuration/index.js @@ -1,10 +1,10 @@ -import state from './state'; +import createState from './state'; import mutations from './mutations'; import * as actions from './actions'; -export default () => ({ +export default ({ securityConfigurationPath = '' }) => ({ namespaced: true, - state, + state: createState({ securityConfigurationPath }), mutations, actions, }); diff --git a/ee/app/assets/javascripts/security_configuration/modules/configuration/state.js b/ee/app/assets/javascripts/security_configuration/modules/configuration/state.js index 4a7221e549a626..a65636ddfbfc3f 100644 --- a/ee/app/assets/javascripts/security_configuration/modules/configuration/state.js +++ b/ee/app/assets/javascripts/security_configuration/modules/configuration/state.js @@ -1,5 +1,5 @@ -export default () => ({ - securityConfigurationPath: '', +export default ({ securityConfigurationPath }) => ({ + securityConfigurationPath, isLoading: false, hasLoaded: false, errorLoading: false, -- GitLab From 406e57ce58f026d9a6689624e964daa003442495 Mon Sep 17 00:00:00 2001 From: Fernando Date: Sat, 15 Aug 2020 05:31:57 -0500 Subject: [PATCH 13/21] Additonal code review changes * Removve uneeded rule-form feature flag check * Change project to @project in controller * Simplofy feature flag logic for data function --- app/controllers/projects_controller.rb | 2 +- .../components/modal_rule_create.vue | 6 ----- .../approvals/components/rule_form.vue | 24 ++++++------------- .../unconfigured_security_rule.vue | 3 +-- 4 files changed, 9 insertions(+), 26 deletions(-) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 4f286cb301a981..f125a59199d849 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -39,7 +39,7 @@ class ProjectsController < Projects::ApplicationController frontend_experimentation_tracking_data(:new_create_project_ui, 'click_tab') push_frontend_feature_flag(:new_create_project_ui) if experiment_enabled?(:new_create_project_ui) push_frontend_feature_flag(:service_desk_custom_address, @project) - push_frontend_feature_flag(:approval_suggestions, project) + push_frontend_feature_flag(:approval_suggestions, @project) end layout :determine_layout diff --git a/ee/app/assets/javascripts/approvals/components/modal_rule_create.vue b/ee/app/assets/javascripts/approvals/components/modal_rule_create.vue index 1031c0676b1dd7..3018062d184083 100644 --- a/ee/app/assets/javascripts/approvals/components/modal_rule_create.vue +++ b/ee/app/assets/javascripts/approvals/components/modal_rule_create.vue @@ -1,5 +1,4 @@