Skip to content

Notifications: respect project.versioning_scheme for links #265

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

Merged
merged 11 commits into from
Apr 16, 2024

Conversation

humitos
Copy link
Member

@humitos humitos commented Mar 5, 2024

This is a workaround for now since the backend API is not returning this URLs due to https://github.com/readthedocs/readthedocs-ops/issues/1323. However, we cannot fix that quickly.

I'm re-implementing a small version of the resolver for this by generating the URL based on the project.versioning_scheme for now.

We should come back to this once the API returns the URLs we need.

Closes #243

@humitos humitos requested a review from a team as a code owner March 5, 2024 12:16
@humitos humitos requested a review from agjohnson March 5, 2024 12:16
This is a workaround for now since the backend API is not returning this URLs
due to readthedocs/readthedocs-ops#1323. However, we
cannot fix that quickly.

I'm re-implementing a small version of the resolver for this by generating the
URL based on the `project.versioning_scheme` for now.

We should come back to this once the API returns the URLs we need.

Closes #243
@humitos humitos force-pushed the humitos/notifications-link-fix branch from bb36156 to 5af6764 Compare March 5, 2024 12:16
@humitos
Copy link
Member Author

humitos commented Mar 18, 2024

I updated this PR to use the URLs returned from the backend. It requires:

@agjohnson agjohnson requested a review from stsewd March 21, 2024 17:23
@humitos humitos added the Status: blocked Issue is blocked on another issue label Mar 26, 2024
humitos added a commit that referenced this pull request Apr 11, 2024
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 <[email protected]>
@humitos
Copy link
Member Author

humitos commented Apr 16, 2024

The API backend changes were merged, this PR is unblocked now and needs a small updates to make the tests pass and other small bits.

@humitos humitos removed the Status: blocked Issue is blocked on another issue label Apr 16, 2024
@humitos humitos merged commit 22e0877 into main Apr 16, 2024
@humitos humitos deleted the humitos/notifications-link-fix branch April 16, 2024 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Notifications: stable version link in banner doesn't respect versioning_scheme
3 participants