Architecture proposal: Refactor of `pipeline_editor_app` component

Description of the problem

pipeline_editor_app is big already and it will keep growing as we add features. This component is currently in charge of:

  • Fetching the data for the editor, visualization and linter
  • Commit logic to save changes
  • Handle success and error banners for the section and the editor
  • Show multiple loading icons for each tabs and its own content

This make the component big, hard to reason with and test. It will also create a lot more merge conflicts in the section since everything will be pushed into this component as the section grows.

Goals

This refactor should leave us with smaller components that are easy to test and have a better separation of concerns.

Architecture proposal

We should start by breaking the component in 3:

  • The topmost logic that fetches the data should be its own component and we can keep the name pipeline_editor_app
  • Under this component, we would have a new component pipeline_editor_home (I am up for suggestion on this name)
  • We would break out the commit logic and make it its own component, pipeline_editor_commit

Pipeline editor app

This component would fetch the graphql queries in pipeline_editor_app (blob_content and ci_config.graphql) and render errors that are related to the network. If there is an error fetching the data, we should ensure that this is a state wide error and that each individual tab does not need to handle this state.

Question Should we disable the tabs while it loads and only enable them if there is no error? Or perhaps we could show both notification on top of another, which would convey the right message. For example, the visualization state would be: (failed to load => viz data is empty).

The data this component fetches could be passed down as prop, but the middle component pipeline_editor_home would only pass it down as well seems sub-optimal. The 2 options here would be:

  • Create local graphQL queries to access the cache from the children components
  • Use provide/inject to pass down the data, and we can have the children subscribe when they need to.

Question Does provide inject has the same problem where we would pass the data directly through the DOM? And follow-up, does graphQL local queries sound like a good idea for child component to fetch the data they need?

Pipeline editor home

This would serve as the middle(wo)man between the fetched data and the components we want instantiated, a wrapper in a sense. So the idea would be that this component simply adds each of the main component in a tab (editor, viz, and lint).

It would be agnostics to errors and any business logic. We will be forced to have it contain some room for an alert because of the commit form where the form is at the bottom, but we would want the error to render at the top. So its child will emit an event if it needs to display an error at the top (generic pattern we can reuse elsewhere)

So in essence, it should simply render the components. If there are additional logic to handle, let's create them as their own new "parent" component. For example, pipeline_editor_commit will take all the logic to commit and instantiate the form (see below)

Pipeline editor commit

pipeline_editor_commit will take all the logic to commit and instantiate the form. It will receive the editor content (through either inject or graphQL query) and when you click on submit, it take care of the business logic and the error message that can happen. If there is a commit error, we emit an event that will be caught by the wrapper and display the appropriate error. The child will pass all the data for the error so that the parent stays agnostics as to what kind of error it can receives.

Data story

Because a picture is worth a thousand words:

Screen_Shot_2021-01-20_at_1.09.31_PM

Edited by Frédéric Caplette