Skip to content

Merge feature branch for Pipeline Editor app refactor

Frédéric Caplette requested to merge 292930-breakdown-pipeline_editor_app-into-smaller-components into master

What does this MR do?

It merges two already reviewed MR into master. The idea is to have this feature branch, now completed, become a single commit in our git history for the refactor. The rationale not to keep 2 commits (one for the components and one for tests) is that if we ever need to revert, the first commit cannot be picked on its own as it would break the code base severely. For that reason I prefer one lengthy commit that contains everything for the refactor in our history.

To review this, you can make sure everything still works locally. This also preserve the new functionality that was created by another engineer during the time of this refactor where changing your config file will now trigger a warning if you have not saved them and try to navigate away (added a video to show it still works after the conflicts were resolved). To review this MR, you can simply check the second commit in this branch, which resolves the conflicts. Everything else is exactly the same as what has already been reviewed.

NOTE there are changes to structure.db in the second commit, but they are basically "uncommiting" whatI changed by accident. If you look at the entire changes , structure.db is not changed

This refactor has more context in the architectural discussion that preceded this change. The gist of it as as follow:

  • Breakdown the pipeline_editor_app component into smaller components that represent part of the new section.
  • The component structure is as follow:
pipeline_editor_app 
  -> pipeline_editor_home
       -> pipeline_editor_header
       -> pipeline_editor_tabs
         -> text_editor
         -> pipeline_graph
         -> ci_lint
       -> commit_section
  • Use provide/inject for all static values at the top level
  • Have graphQL with apollo fetch the top level data, then pass it down using props
  • Add events where needed for children component to emit their changes upward

We also update all the tests files. Both parts have been reviewed indivually

Part
1: Merge the code change in feature branch
2: Merge test suites changes in feature branch
3 Merge feature branch into master (You are here)

No changelogs required since there are no user facing changes

Screenshots (strongly suggested)

No visual changes, but here a video of everything still working

authoring

Here is the navigation block working

Navigation block

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

Related to #292930 (closed)

Edited by Frédéric Caplette

Merge request reports