Skip to content

Extract finding a diff file to a dedicated module

What does this MR do?

Adds a single central ES module for managing Diff Files. It's in the store folder because that's where a bunch of the logic that would need it is (primarily actions and mutations), but the case could definitely be made for moving it somewhere more generic (like lib/utils).

In that module, exports a single function that supports finding a file in a list of files by an arbitrary identifier.

Why

Right now, we have two possible ways to identify a diff file: file_path and file_hash. This is troubling enough on its own, but worse, we have no central way to find a file by one of those identifiers from a list of files.

As I'm working to add (at least) one more identifier in support of #33867, I found it to be irresponsible to add a THIRD identifier scattered throughout the code. We cannot remove these existing ways to find files (in most cases), but we can centralize the entire process so updating or (if we must) adding new identifiers in the future can be done in just one location.

Future

This file looks a little bare at the moment, but that's because it's the smallest chunk of work in a much larger set of changes (which will take much longer to get out). This is useful on its own, and I'd like to get it into the Diffs codebase so the groupsource code team can start depending on it.

In future iterations, we'll also

  • centralize a way to get the identifier, including the logic that may require using different identifiers in different situations,
  • move Diff File specific code out of utils.js, which has become incredibly unwieldy and includes helpers for diff files, diff file lines, discussion notes, and more

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

Loading