[go: up one dir, main page]

Skip to content

Have backend provide full list of versions on Merge Request changes tab

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

Summary

Related to: !26918 (closed)

In merge requests under the diffs tab, the compare dropdowns are mostly provided by the backend via:

  • fetchDiffFiles
  • fetchDiffFilesMeta

These arrays are manipulated on the front end to append base, HEAD, and latest version

In the code snippet below the following vue props are used to manipulate the data returned for the above purposes and make them actionable in the component:

  • merge-request-version
  • base-version-path
  • head-version-path
  • target-branch
  • target-head-branch
<template #source>
  <compare-versions-dropdown
    :other-versions="mergeRequestDiffs"
    :merge-request-version="mergeRequestDiff"
    :show-commit-count="true"
    class="mr-version-dropdown"
  />
</template>
<template #target>
  <compare-versions-dropdown
    :other-versions="comparableDiffs"
    :base-version-path="baseVersionPath"
    :head-version-path="headVersionPath"
    :start-version="startVersion"
    :target-branch="targetBranch"
    :target-head-branch="targetHeadBranch"
    class="mr-version-compare-dropdown"
  />
</template>

There is a lot of unnecessarily complicated logic to maintain state since the frontend is also concerned with constructing the array instead of just receiving it from the backend.

image

Ideally, the backend will provide the complete list of versions with flags for base and head something like isBase and isHead

Improvements

The frontend logic gets much simpler, and many test are no longer needed. Future work on these areas would be dramatically easier since the components would be mostly presentational at that point.

Risks

Risks should be minimal, but we are potentially shifting the burden of this logic on to the backend. I think these should be dedicated API calls though so the surface area would be pretty small.

Involved components

Frontend

  • app/assets/javascripts/diffs/components/app.vue
  • app/assets/javascripts/diffs/components/compare_versions.vue
  • app/assets/javascripts/diffs/components/compare_versions_dropdown.vue

Backend

  • TBD I need input from a backend engineer

Optional: Intended side effects

n/a

Optional: Missing test coverage

The new API calls will need tests to cover them. The removed logic on the frontend will need to have the tests removed.

Edited by 🤖 GitLab Bot 🤖