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

Addons: allow users to opt-in into the new addons #212

Closed
humitos opened this issue Sep 7, 2023 · 6 comments · Fixed by #213
Closed

Addons: allow users to opt-in into the new addons #212

humitos opened this issue Sep 7, 2023 · 6 comments · Fixed by #213
Assignees
Labels
Feature New feature

Comments

@humitos
Copy link
Member

humitos commented Sep 7, 2023

We have deployed the new addons for projects using build.commands and it has been working fine. We haven't received any complain and I've already saw users jumping into build.commands because they have the flyout there as well (see readthedocs/readthedocs.org#10706)

Also, we've found a way to "migrate old projects/builds using the old integrations to the new one" dynamically using a Cloudflare Worker (see https://github.com/readthedocs/readthedocs-ops/pull/1402).

However, we can't do this migration automatically for all the projects because it won't look exactly the same. We will be moving the flyout from the bottom left being integrated into the navbar, to the bottom right being fixed in a floating position. I created an issue in the theme to make this integration to look exactly the same at readthedocs/sphinx_rtd_theme#1523 -- but it needs a non-trivial amount of work for that to define the exact addons API, work on JS/CSS, release a new version, wait for people to migrate to this version, etc.

We can follow a simpler path here: "allow users to opt-in into the new addons". When opt-in we can communicate these differences (look&feel changes and extra features) and they can disable if they don't like it. This will give users the features we already have in the new addons immediately to them, it will allow us to gradually roll this out and fix issues as we go and also give us feedback about that integration into the theme should be.

Related:

@agjohnson
Copy link
Contributor

This would be an easy UI to get started on. The template work will be minimal, we just need the Form and View instances. After that, templates would add a Project -> Admin -> Addons UI, basically just showing the initial Form instance.

There is definitely a long term plan here and a short term plan. I talked about this in another issue that I am not finding now.

In the short term our Form would be a simple form with "Enable addons beta", or maybe even "Enable flyout beta". But we wouldn't reuse this Form long term, so it wouldn't be a good candidate to build on top of.

Long term, the goal would be to have a Project -> Admin -> Addons -> Flyout sub-admin page, with separate Form class for each addon configuration. I'd want to probably try with using a tabbed interface and multiple form instances first, as otherwise we need to create a lot of redundant views/templates/etc.

So, I'd say it's 100% okay to start with a basic form for "Enable this beta", understanding we won't build on top of this.

But, also, if we feel we already want per-addon configuration this may not be too much more work, so could be a place to start as well.

@humitos humitos added the Feature New feature label Sep 12, 2023
@agjohnson
Copy link
Contributor

For assignment, I'm happy to help with the ext-theme changes, but they will also be very small to start, and mostly just duplicating existing templates. @humitos would you like to start with the form/view/url additions here first? I've assigned us both for now, but it might be easy enough to add all at once too.

@humitos
Copy link
Member Author

humitos commented Sep 13, 2023

I see two different approaches here:

  1. add a Project.force_addons_enabled (bool) with a simple project's form that only exposes that field and template that shows it
  2. add a Projects.addons (foreign key) to a new model AddonsConfig that contains all the configs for each addon. Then we create one Form per addon

humitos added a commit to readthedocs/readthedocs.org that referenced this issue Sep 13, 2023
I created the URL, models, view and form to prepare ourselves for the beta
addons. Currently, if the `Project.addons` fields is not null we add a HTTP
header to tell Cloudflare to enforce the new beta addons on this project.

In the future, we can use each of the `ProjectAddonsConfig` field to let the
user decide which of the addons they want to enable/disable and change their
configurations.

* Related: readthedocs/ext-theme#212
humitos added a commit that referenced this issue Sep 13, 2023
Show a small form to the user where they can enable/disable the new beta addons
on their project.

* Closes #212
* Requires readthedocs/readthedocs.org#10733
@humitos
Copy link
Member Author

humitos commented Sep 13, 2023

I went ahead and opened readthedocs/readthedocs.org#10733 and #213

@agjohnson
Copy link
Contributor

add a Project.force_addons_enabled (bool) with a simple project's form that only exposes that field and template that shows it

This sounds like what we are wanting immediately, for this issue -- just the beta opt in/out.

add a Projects.addons (foreign key) to a new model AddonsConfig that contains all the configs for each addon. Then we create one Form per addon

And this sounds like the continuation of this work that we'll want for #211. Or we can continue thinking how we'll lay this out in code more, I don't have strong opinions on the modeling structure.

Does that match what you're thinking too?

@humitos
Copy link
Member Author

humitos commented Sep 14, 2023

Yeah, but I went ahead already and implemented both of them: the immediate requirement (enable/disable beta addons) and the underlying modeling needed to allow users customize the addons --which is currently not exposed to users yet.

humitos added a commit to readthedocs/readthedocs.org that referenced this issue Sep 18, 2023
* Addons: allow users to opt-in into the beta addons

I created the URL, models, view and form to prepare ourselves for the beta
addons. Currently, if the `Project.addons` fields is not null we add a HTTP
header to tell Cloudflare to enforce the new beta addons on this project.

In the future, we can use each of the `ProjectAddonsConfig` field to let the
user decide which of the addons they want to enable/disable and change their
configurations.

* Related: readthedocs/ext-theme#212

* Changes from feedback

Use a `AddonsConfig.enabled` field to decide whether or not all the addons are
enabled.

* Docstring to document the HTTP headers to communicate with CF

* Addons: update modelling and queryset

* NGINX: pass the `X-RTD-Force-Addons` HTTP header

* Declare the URL only when ext-theme is enabled
humitos added a commit that referenced this issue Sep 18, 2023
* Addons: allow users to opt-in into the beta addons

Show a small form to the user where they can enable/disable the new beta addons
on their project.

* Closes #212
* Requires readthedocs/readthedocs.org#10733

* Add beta label on the menu item

* Upate the notification and ask for feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature
Projects
Status: Done
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants