Improve security reports MR widget

We added several security reports in the past releases, Dast, Sast, Clair, Codeclimate and a Performance one too.

We were able to create a reusable vue component to show the reports, but there is some technical debt that should be addressed:

  • The main focus point is to abstract the data in the backend and make the frontend dumber. It would be great to have all the main logic in backend, keeping it similar to other areas.

Not only we need to clean the code, doing this would allow us almost no effort in adding more reports

frontend

  1. For some reports, we compare 2 JSON payloads (from 2 endpoints) to show the correct data. We could move this comparison to the backend and fetch only one report

  2. Although most reports show the same kind of information, each endpoint payload has a different object structure - each is being parsed on the frontend.

  3. Whether to know if each report can or not be rendered, we added a function: https://gitlab.com/gitlab-org/gitlab-ee/blob/master/ee/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js#L32 we could DRY this if for all the payloads the keys were the same.

  4. The same goes for the loading states, they are repeated for each: https://gitlab.com/gitlab-org/gitlab-ee/blob/master/ee/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js#L17, we could DRY if we didn't have to know which is which.

  5. And also for the requests and store methods. If each report had only one endpoint and the JSON structure and key naming were shared between all we could get rid of all the parsing methods in the store:

  6. https://gitlab.com/gitlab-org/gitlab-ee/blob/master/ee/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js#L88

  7. https://gitlab.com/gitlab-org/gitlab-ee/blob/master/ee/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js#L107

  8. https://gitlab.com/gitlab-org/gitlab-ee/blob/master/ee/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js#L118

  9. https://gitlab.com/gitlab-org/gitlab-ee/blob/master/ee/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js#L190

  10. https://gitlab.com/gitlab-org/gitlab-ee/blob/master/ee/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js#L228

  11. https://gitlab.com/gitlab-org/gitlab-ee/blob/master/ee/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js#L233

Component-specific:

  1. Improve issue component to be unaware of type:
  2. has priority check: Instead of checking the type, move this logic up and pass it as a prop: https://gitlab.com/gitlab-org/gitlab-ee/blob/master/ee/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_report_issues.vue#L50
  3. Use icon component instead of spriteIcon function

UX

  1. https://gitlab.com/gitlab-org/gitlab-ee/issues/4310

/cc @timzallmann @dzaporozhets @ayufan @dimitrieh

Edited Apr 04, 2020 by Fabien Catteau
Assignee Loading
Time tracking Loading