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