Refactor vulnerability modal
- Related issue: https://gitlab.com/gitlab-org/gitlab-ee/issues/8146
- CE backport: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/31022
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.
vulnerability_details
component less smart
2. Make the This component's template contains a heavy mix of v-for
s and v-if
s 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
- [-] Changelog entry
- [-] Documentation created/updated or follow-up review issue created
-
Code review guidelines - [-] Merge request performance guidelines
-
Style guides - [-] Database guides
-
Separation of EE specific content