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: prepare Proxito and dashboard to enable them by default #11513

Merged
merged 9 commits into from
Sep 23, 2024

Conversation

humitos
Copy link
Member

@humitos humitos commented Aug 5, 2024

Prepare Proxito's code and dashboard to enable addons by default starting on October 7th, as planned:
https://about.readthedocs.com/blog/2024/07/addons-by-default/

  • Removes the "Enabled" checkbox from the dashboard. All the addons will be enabled by default. Users will be able to disable each of them one by one, tho
  • Always inject the HTTP header that tells Cloudflare to inject our JavaScript 1

Note this code is temporary and can be deployed before reaching that particular date. The idea is to remove this code once this behavior becomes the default.

Related #11474

Footnotes

  1. we will eventually remove this header and always inject our files.

Prepare Proxito's code and dashboard to enable addons by default starting on
October 7th, as planned:
https://about.readthedocs.com/blog/2024/07/addons-by-default/

Note this code is temporary and can be deployed _before_ reaching that
particular date. The idea is to remove this code once this behavior becomes the
default.

Related #11474
@humitos humitos requested a review from ericholscher August 5, 2024 14:48
@ericholscher
Copy link
Member

Makes sense. I do wonder if we keep the ability to disable Addons if someone wants, and then we don't inject them at all? I can see some folks wanting that, especially on Business.

@humitos
Copy link
Member Author

humitos commented Aug 6, 2024

I do wonder if we keep the ability to disable Addons if someone wants, and then we don't inject them at all? I can see some folks wanting that, especially on Business.

This is a good point. I would probably only allow this on Business. I think we don't want OSS project without addons.

Currently, they can disable all the addons one by one, but we will still inject our JS and run our code -- even if we don't show any of them. I understand there would be specific cases where the customer doesn't want that, right?

@ericholscher
Copy link
Member

Yea, it seems useful to at least have the option to not inject anything, since that might be useful in a few different contexts.

humitos added 4 commits August 7, 2024 11:35
This reverts commit 235514a.
Keep the ability to let commercial users to disable Addons completely. This
means that our Cloudflare working won't even inject them.
@humitos
Copy link
Member Author

humitos commented Aug 12, 2024

I updated this PR to keep the ability to disable addons on "for Business".

We will require to create AddonsConfig object for all the projects on Oct 7th. Something like should be enough, I guess:

for project_slug in Project.objects.values_list("slug", flat=True):
  AddonsConfig.objects.get_or_create(project__slug=project_slug)

@humitos humitos marked this pull request as ready for review August 12, 2024 13:38
@humitos humitos requested a review from a team as a code owner August 12, 2024 13:38
Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.. I wonder about having the AddonsConfig.objects.get_or_create(project=project) code run somewhere in the codebase on that day automatically? Putting it in the Addons middleware is probably too heavy, but could be an option so we don't need to remember to run this on the CLI manually?

@humitos
Copy link
Member Author

humitos commented Aug 13, 2024

Putting it in the Addons middleware is probably too heavy,

He, this was my first option as well; but I revert it because of the same reason.

but could be an option so we don't need to remember to run this on the CLI manually?

I can write a Celery task to be run on that specific date automatically.

@humitos
Copy link
Member Author

humitos commented Aug 13, 2024

I can write a Celery task to be run on that specific date automatically.

Hrm... it seems the crontab doesn't accept day, month, year... tho.

@humitos
Copy link
Member Author

humitos commented Aug 13, 2024

So, on October 7th we need to do the following things in both platforms (also local development):

  1. Mark Default all past projects to True on DISABLE_SPHINX_MANIPULATION feature flag.
  2. Run the following snippet to create AddonsConfig for all the projects:
    for project_slug in Project.objects.values_list("slug", flat=True).order_by("pk").iterator():
       AddonsConfig.objects.get_or_create(project__slug=project_slug)
  3. Enable addons on all the project that disable it (community only)
    if not settings.RTD_ALLOW_ORGANIZATIONS:
        for addons in AddonsConfig.objects.filter(enabled=False):
            addons.enabled = True
            addons.save()

@humitos
Copy link
Member Author

humitos commented Aug 13, 2024

For now... I added a reminder in Slack at least 🙃

@humitos
Copy link
Member Author

humitos commented Sep 23, 2024

I re-reviewed this PR to check that everything was OK, and we are ready to merge it 👍🏼

@humitos humitos merged commit c3a0268 into main Sep 23, 2024
8 checks passed
@humitos humitos deleted the humitos/enable-addons-by-default branch September 23, 2024 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants