Skip to content

Refactor vulnerability modal

Paul Gascou-Vaillancourt requested to merge 8146-vuln-modal-refactor into master

What does this MR do?

This MR is the first step for refactoring the vulnerability modal.

What's the plan?

1. Create a Vuex module dedicated to the vulnerability modal

Currently, the modal's data is fed by 2 different store modules depending on the context it's used in:

  • ee/app/assets/javascripts/security_dashboard/store/modules/vulnerabilities/state.js
  • ee/app/assets/javascripts/vue_shared/security_reports/store/state.js

In this first step, we'll move all state/actions/mutations/getters related to the modal into a dedicated Vuex module. This module will be dynamically registered in the contexts where it's needed.

Once this done, we might still end up with some duplicated code. For example, the dismissVulnerability action can be triggered from both the vulnerabilities list and the modal, which means we need to keep it in two different places in the store, for now at least. This could be addressed in some followup MR.

2. Make the vulnerability_details component less smart

This component's template contains a heavy mix of v-fors and v-ifs meant to properly display different properties from the store, but as it turns out, the information that's being displayed isn't that generic which makes the code hard to read for a very small benefit.

In the second step, we'll remove that logic and display each property manually.

3. Flatten the modal's state and move translations out of the store back into the component

In the same vein as step 2, the modal's state seems unnecessarily complex, eg:

{
  description: {
    value: null,
    text: s__('ciReport|Description'),
    isLink: false,
  },
  file: {
    value: null,
    url: null,
    text: s__('ciReport|File'),
    isLink: true,
  },
}

Each vulnerability data key is represented by an object containing some properties that were most likely meant for the component's template logic above. Once the 2 first steps are done, we could probably flatten the state, eg:

{
  description: null,
  file: null,
  fileUrl: null,
}

Thanks to the template refactor from step 2, we'll be able to move translations back into the component (which seems like best practice for this kind of things) and the isLink booleans won't be necessary anymore since there won't be the rendering loop anymore.

4. Something else? TBD

Steps 2 and 3 might be doable in a single MR, depending on the size of the resulting diff.

Does this MR meet the acceptance criteria?

Conformity

Performance and testing

Edited by Paul Gascou-Vaillancourt

Merge request reports