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.

Sign up for GitHub

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

Jump to bottom

Notification: handle stable/latest aliases #132

Closed
humitos opened this issue Sep 18, 2023 · 1 comment · Fixed by #281
Closed

Notification: handle stable/latest aliases #132

humitos opened this issue Sep 18, 2023 · 1 comment · Fixed by #281
Assignees
@humitos
Labels
Accepted Accepted issue on our roadmap Improvement Minor improvement to code

Comments

@humitos
Copy link
Member

humitos commented Sep 18, 2023

Currently, our logic to show a notification is:

  • on latest show a warning about potentially unreleased features
  • on non-stable show a warning about maybe reading an outdated documentation

We need to handle the real branch/tag names these stable / latest versions refer to. Why is this? Because we want to show the latest notification on main too, for example. Also, we don't want to show the non-stable notification if the stable version points to v3.1 and the user is reading the v3.1.

The API should return a list of these alias, like:

{
  "latest": ["latest", "main"],
  "stable": ["stable", "v3.1],
}

and the JS code should check the current version against those.

Related #125
@humitos humitos added Improvement Minor improvement to code Accepted Accepted issue on our roadmap labels Sep 18, 2023
@github-project-automation github-project-automation bot moved this to Planned in 📍Roadmap Sep 18, 2023
@humitos humitos mentioned this issue Sep 18, 2023
Merged
@humitos humitos mentioned this issue Oct 10, 2023
Closed
@humitos humitos self-assigned this Oct 10, 2023
@humitos humitos moved this from Planned to In progress in 📍Roadmap Nov 22, 2023
humitos added a commit to readthedocs/readthedocs.org that referenced this issue Nov 22, 2023
@humitos
Addons: return addons.non_latest_version_warning.aliases field
970baf4 
This is useful to avoid showing notification on latest/stable aliases versions.

Related readthedocs/addons#132
@humitos humitos mentioned this issue Nov 22, 2023
Closed
@aanm aanm mentioned this issue Dec 11, 2023
Closed
@humitos humitos mentioned this issue Dec 12, 2023
Closed
@ericholscher ericholscher moved this from In progress to Planned in 📍Roadmap Jan 30, 2024
@qmonnet qmonnet mentioned this issue Feb 20, 2024
Closed
@humitos
Copy link
Member Author

humitos commented Mar 11, 2024

I implemented an initial version of this in readthedocs/readthedocs.org#10918 but it's kind of a quick solution for this specific problem. I thought a little more about this issue after working on #243 and I think we can solve this issues is a more general and generic way.

I see this issue related to the "version alias support". We currently don't support version aliases, but we may support them in the future somehow (readthedocs/readthedocs.org#5318). I don't think we have to move forward in that conversation right now, but since this issue is related, I found a better proposal for it.

We can add aliases field in the APIv3 response for Version resource. Currently, it will always be [] for most of the versions except for STABLE and LATEST since they "will point" to another real version.

With that field, together with adding versions.stable.* and versions.latest.* to the addons API we will be able to solve this problem by checking for the versions.*.aliases and comparing it with versions.current.slug.

Besides, this patter will be also helpful to solve #265 as well, since we will eventually be able to use versions.stable.urls.documentation too.

So, I'm saying that adding these "generic" fields will be useful to solve a larger amount of specific problems without requiring specific fields that are only useful for those particular issues.

I opened a PR on the backend with this idea in readthedocs/readthedocs.org#11205

@humitos humitos mentioned this issue Mar 11, 2024
Merged
@humitos humitos moved this from Planned to In progress in 📍Roadmap Mar 11, 2024
@humitos humitos moved this from In progress to Needs review in 📍Roadmap Mar 18, 2024
@humitos humitos mentioned this issue Apr 8, 2024
Merged
humitos added a commit that referenced this issue Apr 11, 2024
@humitos @agjohnson
API: simplify and standardize data usage (#281)
cb57a67 
Implements the changes in the API made in
readthedocs/readthedocs.org#11205. We need to
merge that PR and deploy addons together with the changes in this PR.

In this PR there are mainly no changes in the logic. The most important
thing to mention here is that we are standardizing the usage of the API
response and pinning to `v1` now (we were using `v0-alpha`). This means:

- we will be exposing the `v1` to users via the js custom event (see
#64)
- we are using standard/shared APIv3 serializers for resources for this
`/addons/` endpoint
- any breaking change we have to do in the future it will increase the
API response version


I'm not sure there are too much to review here, but I'd appreciate at
least a quick look at these changes. I'm happy to answer some questions
you may have here or on the underlying PR about the API response
changes.


### Related:
* Closes #132 
* Unblocks #265
* Unblocks theme integration (our theme and CPython's one)
* Requires readthedocs/readthedocs.org#11205

---------

Co-authored-by: Anthony <aj@ohess.org>
@github-project-automation github-project-automation bot moved this from Needs review to Done in 📍Roadmap Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@humitos humitos

Labels
Accepted Accepted issue on our roadmap Improvement Minor improvement to code
Projects
📍Roadmap
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

API: simplify and standardize data usage readthedocs/addons
1 participant
@humitos