Skip to content

Refactor error handling and some more in the visualization

What does this MR do?

This MR touches a feature that is behind two feature flag

  • ci_pipeline_editor_page
  • ci_config_visualization_tab

It removes a pattern that was introduced in the CI config visualization where we had errors and warnings. Instead, we now put them all as errors, but we set them using immediate watchers. Why is this required?

The graph itself doesn't know anything about how it acquire its data. It simply expects a prop to being passed down. That data is obtained usually through a graphQL query, and therefore it could fail to be fetched, or be empty (new Ci config file would always be empty!). So we have checks in place when the component mounts and updates to make check sure that:

1- The data isn't empty, in which case we show an info message and won't mount to graph itself to prevent additional errors from being raised

2- The CI config is invalid, which would mean that we can't render the graph

The component mounts when the user click on the tab. to visualize it, and then stay mounted even if the user changes tab. Screen_Shot_2021-01-04_at_10.28.01_AM

This mean that we need to watch for immediate (to ensure that this is applied when the component first mounts) and when it updates, which is the normal watcher behaviour. This clean the code because:

  • Warnings and Errors separation was arbitrary and it would not evolve naturally over time
  • It added a lot of logic to merge errors and warnings together to avoid needing to have 2 alert component in the DOM
  • Watchers are the perfect usecase here for it can look for change in props and make sure our errors are reported as soon as the change occurs.

Screenshots (strongly suggested)

Nothing changes visually, but here is a screenshot of each error you can see. Note that there currently is a visual regression on alert in gitlab-ui, so the formatting is off:

Screen_Shot_2021-01-05_at_12.08.43_PM Screen_Shot_2021-01-05_at_12.10.23_PM Screen_Shot_2021-01-05_at_11.54.11_AM

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

To test this, you can ensure that the proper errors are shown.

  1. Enable :ci_pipeline_editor_page in your rails console
  2. Enable :ci_config_visualization_tab in your rails console
  3. Navigate to CI / CD -> Editor
  4. Click on "Visualize" tab (if you have a valid CI configuration, it will render the graph)
  5. To trigger an error, the ones you can tests easily are empty data and invalid config.
  6. To have an empty data banner, you need to load a project with a CI config file created that is empty, and then navigate
  7. To have an Invalid CI config file, well... be creative 😄 (Anything that isnt a valid file should trigger this)

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 #294330 (closed)

Edited by Frédéric Caplette

Merge request reports

Loading