Skip to content

Add an action that dynamically upgrades a `renamed`-type diff to its full content

Thomas Randolph requested to merge 205401-upgrade-renamed-diff-file into master

What does this MR do?

For #205401 (closed)

Adds a Vuex action (and an associated util helper) to take a Renamed Diff File and a set of context-less Diff Lines and:

  1. Upgrade the diff file to a text viewer (instead of the previous renamed)
  2. Add the lines - after being processed into the "official" frontend diff file line format - to the Diff File in state
  3. Trigger a queue to render un-rendered Diff Files, which the Diff File in question is

Why

In the future, we'll be adding UI to view the content of a renamed file.

Right now, you can only see that it was renamed, not what the content of that file is.
This is sort of baked into the data structure of a renamed file: it doesn't have lines, it has a viewer that doesn't render any content, etc.

This action "upgrades" that renamed file to a file in state that can be rendered, and then triggers a queue to render it.

Notes

  • This action is currently unused: there is no UX change in this MR.
  • The action starts out a lot like the fetchFullDiff action, but deviates from it a lot. I tried a bunch of ways to re-use that action, but they were all extremely complex (essentially just two separate logic branches, which made a separate action much simpler)
  • The util function has a big comment because I think it could easily be assumed that the function already in the utils file could be re-used. While they seem similar, they really have practically no overlap. I left a link out to the epic for cleaning up our data - which hopefully will drastically improve this utils file.
  • The test suite for the util function has four separate tests. In theory, it could just be one test (the last one tests the whole input => the whole output). However, I think when we're testing individual functions like this, we should be calling out the parts we care about: "Is it actually doing these particular tasks?" The "all-in-one" tests indirectly performs that test, but the four individual tests directly inform the next reader: "Here's what should be happening." I think that's valuable.

Screenshots

N/A, all ~backstage

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team
Edited by 🤖 GitLab Bot 🤖

Merge request reports