Skip to content

Use an event hub to coordinate all possible ways to expand all diff files

Thomas Randolph requested to merge defect/collapsed-diff-files/expand-all-banner into master

What does this MR do?

For #273475 (closed).

The current flow for expanding files (and expanding all files) is something like this:

  1. In some component, trigger a Vuex action
  2. In state, update the property on the file(s)
  3. In the file component, react to the property changing
  4. As a reaction, do all the behavior for loading a diff (with lots of exceptions)

This MR modifies this flow to "cut out the middle man." The new flow is like this:

  1. In some component, emit an event to the shared Diffs event hub
  2. In the Diff File component, listen for events telling it to expand
  3. Do all the behavior for loading a diff (with lots of exceptions)

The biggest immediate benefit is that there's only a single way to toggle the collapsed state of a Diff File, and it's in the diff file, rather than being spread around the codebase. This resolves some bugs around deviant expansion that isn't implemented the same way.

The next largest benefit is that we're not publishing a store update to every single store consumer when the diff files need to be expanded; we're only listening for that event in the components that know how to handle it (the diff files themselves).

Screenshots (strongly suggested)

N/A, file expansion works the same way, it's just being triggered differently.

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

Merge request reports