Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consolidate build detail view notification patterns #9279

Closed
agjohnson opened this issue May 31, 2022 · 2 comments
Closed

Consolidate build detail view notification patterns #9279

agjohnson opened this issue May 31, 2022 · 2 comments
Labels
Improvement Minor improvement to code Needed: design decision A core team decision is required

Comments

@agjohnson
Copy link
Contributor

We haven't yet been able to prioritize replacing old build detail template logic with something maintainable, and need to consolidate build detail notifications/warnings/suggestions/errors to reign in this UI for future work on templates. We got hung up on a conversation about build errors/warnings specifically (#3399), but haven't discussed how to handle suggestions and some warning blocks.

Most these blocks are template logic right now. This is problematic both because template logic is a bad place to maintain application logic, but also because the data is abstracted heavily at this point.

For instance, a build that has a .readthedocs.yaml that fails to parse during build will result in a Build(config=None) and we show the user a suggestion to "add a .readthedocs.yaml file" instead. This is because we interpret this data on build view differently than we do in the build task.

As a most minimal step, these blocks should emit from application logic not template logic. This at least puts the code where it's most likely going to be maintained. It also sets us up to move the constructs to build-time code/model constructs later. Processing everything once, at build time, does a better job of solving the data abstraction issue though.

In my mind:

  • We immediately are moving template logic notifications into application view logic instead
    • Template blocks become classes that we can easily add to a build detail content
    • Template logic reduces to {% if suggestion %}{{ suggestion }}{% endif %}
    • Build detail page can only show one suggestion at a time
    • Build detail page can show multiple warnings
    • Build detail page can only show one error message
  • We eventually emit everything from the build process instead. We mostly move this logic. But we aren't blocked on refactoring all of our build error/warning messaging up front.
  • If the notification is not build specific, it should be a project-level site notification instead.
@humitos
Copy link
Member

humitos commented Oct 31, 2023

I'm considering this in the proposal I'm writing. Builds will have different types of notifications attached to. These notification will be iterated from the front-end and shown to the user.

@humitos
Copy link
Member

humitos commented Jan 11, 2024

Most of the work required here was implemented in #10890. The usage of the APIv3 to display the notifications is being discussed in the ext-theme repository at readthedocs/ext-theme#259

@humitos humitos closed this as completed Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Minor improvement to code Needed: design decision A core team decision is required
Projects
Archived in project
Development

No branches or pull requests

2 participants