Sign in or sign up before continuing. Don't have an account yet? Register now to get started.
Register now

Merge request approvals component can render empty component

The merge request widget component only renders a template if a provided property has length.

We can improve performance and not initialise the component at all, given that the same property is accessible in the parent component:

https://gitlab.com/gitlab-org/gitlab-ee/blob/master/ee/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals_footer.js#L67

approvedBy is a non-required property, which means that every time the parent component is rendered, even if approvedBy is either 0 or null, we will render the approvals_footer component without the need for it.

By moving this check to the parent component, by adding v-if="mr.approvals.approved_by"

<approvals-footer
  v-if="mr.approvals.approved_by"
  :mr="mr"
  :service="service"
  :user-can-approve="mr.approvals.user_can_approve"
  :user-has-approved="mr.approvals.user_has_approved"
  :approved-by="mr.approvals.approved_by"
  :approvals-left="mr.approvals.approvals_left"
  />

we avoid rendering an empty component.


EDIT:

We also need to improve test coverage for this component, I accidentally removed a v-if from the template and not one test broke: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2825/diffs#0d72452f8d3242e8f5eb3ec62bb8094c9356d450_87_89

Pipeline: https://gitlab.com/gitlab-org/gitlab-ee/pipelines/11439826


@jschatz1 can we schedule this?

Edited Sep 04, 2017 by Filipa Lacerda
Assignee Loading
Time tracking Loading