Skip to content

Commit

Permalink
Addons: update form to show all the options (#11031)
Browse files Browse the repository at this point in the history 
* Addons: update form to show all the options

Add all the configuration options to the `AddonsConfigForm` so the user is able
to enable/disable each of the addons independently.

Closes readthedocs/ext-theme#211

* Addons: create an `AddonsConfig` for projects automatically

* Addons: remove features related to them

* Create `AddonsConfig` object automatically

Create an `AddonsConfig` object when going to the form view to edit its
preferences. By default, disable all the addons for now.

* Remove the Crispy layout from Python code

* Update tests

* Typo

* Missing save the object

* Make form label more explicit
  • Loading branch information
@humitos
humitos authored Jan 22, 2024
1 parent 4b0f214 commit 3cfc29b
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 128 deletions.
33 changes: 26 additions & 7 deletions readthedocs/projects/forms.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -479,17 +479,36 @@ class AddonsConfigForm(forms.ModelForm):

class Meta:
model = AddonsConfig
fields = ("enabled", "project")
fields = (
"enabled",
"project",
"analytics_enabled",
"doc_diff_enabled",
"external_version_warning_enabled",
"flyout_enabled",
"hotkeys_enabled",
"search_enabled",
"stable_latest_version_warning_enabled",
)
labels = {
"enabled": _("Enable Addons"),
"external_version_warning_enabled": _(
"Show a notification on builds from pull requests"
),
"stable_latest_version_warning_enabled": _(
"Show a notification on non-stable and latest versions"
),
}

def __init__(self, *args, **kwargs):
self.project = kwargs.pop("project", None)
kwargs["instance"] = getattr(self.project, "addons", None)
super().__init__(*args, **kwargs)
addons, created = AddonsConfig.objects.get_or_create(project=self.project)
if created:
addons.enabled = False
addons.save()

try:
self.fields["enabled"].initial = self.project.addons.enabled
except AddonsConfig.DoesNotExist:
self.fields["enabled"].initial = False
kwargs["instance"] = addons
super().__init__(*args, **kwargs)

def clean_project(self):
return self.project
Expand Down
58 changes: 0 additions & 58 deletions readthedocs/projects/models.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1976,25 +1976,6 @@ def add_features(sender, **kwargs):
# Build related features
SCALE_IN_PROTECTION = "scale_in_prtection"

# Addons related features
HOSTING_INTEGRATIONS = "hosting_integrations"
# NOTE: this is mainly temporal while we are rolling these features out.
# The idea here is to have more control over particular projects and do some testing.
# All these features will be enabled by default to all projects,
# and we can disable them if we want to
ADDONS_ANALYTICS_DISABLED = "addons_analytics_disabled"
ADDONS_DOC_DIFF_DISABLED = "addons_doc_diff_disabled"
ADDONS_ETHICALADS_DISABLED = "addons_ethicalads_disabled"
ADDONS_EXTERNAL_VERSION_WARNING_DISABLED = (
"addons_external_version_warning_disabled"
)
ADDONS_FLYOUT_DISABLED = "addons_flyout_disabled"
ADDONS_NON_LATEST_VERSION_WARNING_DISABLED = (
"addons_non_latest_version_warning_disabled"
)
ADDONS_SEARCH_DISABLED = "addons_search_disabled"
ADDONS_HOTKEYS_DISABLED = "addons_hotkeys_disabled"

FEATURES = (
(
MKDOCS_THEME_RTD,
Expand Down Expand Up @@ -2088,45 +2069,6 @@ def add_features(sender, **kwargs):
SCALE_IN_PROTECTION,
_("Build: Set scale-in protection before/after building."),
),
# Addons related features.
(
HOSTING_INTEGRATIONS,
_(
"Proxito: Inject 'readthedocs-addons.js' as <script> HTML tag in responses."
),
),
(
ADDONS_ANALYTICS_DISABLED,
_("Addons: Disable Analytics."),
),
(
ADDONS_DOC_DIFF_DISABLED,
_("Addons: Disable Doc Diff."),
),
(
ADDONS_ETHICALADS_DISABLED,
_("Addons: Disable EthicalAds."),
),
(
ADDONS_EXTERNAL_VERSION_WARNING_DISABLED,
_("Addons: Disable External version warning."),
),
(
ADDONS_FLYOUT_DISABLED,
_("Addons: Disable Flyout."),
),
(
ADDONS_NON_LATEST_VERSION_WARNING_DISABLED,
_("Addons: Disable Non latest version warning."),
),
(
ADDONS_SEARCH_DISABLED,
_("Addons: Disable Search."),
),
(
ADDONS_HOTKEYS_DISABLED,
_("Addons: Disable Hotkeys."),
),
)

FEATURES = sorted(FEATURES, key=lambda l: l[1])
Expand Down
58 changes: 18 additions & 40 deletions readthedocs/proxito/tests/test_hosting.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
PUBLIC,
SINGLE_VERSION_WITHOUT_TRANSLATIONS,
)
from readthedocs.projects.models import Domain, Feature, Project
from readthedocs.projects.models import AddonsConfig, Domain, Project


@override_settings(
Expand Down Expand Up @@ -128,42 +128,20 @@ def test_get_config_unsupported_version(self):
assert r.status_code == 400
assert r.json() == self._get_response_dict("v2")

def test_disabled_addons_via_feature_flags(self):
fixture.get(
Feature,
projects=[self.project],
feature_id=Feature.ADDONS_ANALYTICS_DISABLED,
)
fixture.get(
Feature,
projects=[self.project],
feature_id=Feature.ADDONS_EXTERNAL_VERSION_WARNING_DISABLED,
)
fixture.get(
Feature,
projects=[self.project],
feature_id=Feature.ADDONS_NON_LATEST_VERSION_WARNING_DISABLED,
)
fixture.get(
Feature,
projects=[self.project],
feature_id=Feature.ADDONS_DOC_DIFF_DISABLED,
)
fixture.get(
Feature,
projects=[self.project],
feature_id=Feature.ADDONS_FLYOUT_DISABLED,
)
fixture.get(
Feature,
projects=[self.project],
feature_id=Feature.ADDONS_SEARCH_DISABLED,
)
fixture.get(
Feature,
projects=[self.project],
feature_id=Feature.ADDONS_HOTKEYS_DISABLED,
def test_disabled_addons_via_addons_config(self):
addons = fixture.get(
AddonsConfig,
project=self.project,
)
addons.analytics_enabled = False
addons.doc_diff_enabled = False
addons.external_version_warning_enabled = False
addons.ethicalads_enabled = False
addons.flyout_enabled = False
addons.hotkeys_enabled = False
addons.search_enabled = False
addons.stable_latest_version_warning_enabled = False
addons.save()

r = self.client.get(
reverse("proxito_readthedocs_docs_addons"),
Expand Down Expand Up @@ -678,7 +656,7 @@ def test_number_of_queries_project_version_slug(self):
active=True,
)

with self.assertNumQueries(17):
with self.assertNumQueries(20):
r = self.client.get(
reverse("proxito_readthedocs_docs_addons"),
{
Expand Down Expand Up @@ -707,7 +685,7 @@ def test_number_of_queries_url(self):
active=True,
)

with self.assertNumQueries(17):
with self.assertNumQueries(20):
r = self.client.get(
reverse("proxito_readthedocs_docs_addons"),
{
Expand Down Expand Up @@ -743,7 +721,7 @@ def test_number_of_queries_url_subproject(self):
active=True,
)

with self.assertNumQueries(21):
with self.assertNumQueries(24):
r = self.client.get(
reverse("proxito_readthedocs_docs_addons"),
{
Expand All @@ -769,7 +747,7 @@ def test_number_of_queries_url_translations(self):
language=language,
)

with self.assertNumQueries(21):
with self.assertNumQueries(24):
r = self.client.get(
reverse("proxito_readthedocs_docs_addons"),
{
Expand Down
35 changes: 12 additions & 23 deletions readthedocs/proxito/views/hosting.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
from readthedocs.core.resolver import Resolver
from readthedocs.core.unresolver import UnresolverError, unresolver
from readthedocs.core.utils.extend import SettingsOverrideObject
from readthedocs.projects.models import Feature, Project
from readthedocs.projects.models import AddonsConfig, Project

log = structlog.get_logger(__name__) # noqa

Expand Down Expand Up @@ -280,15 +280,9 @@ def _v0(self, project, version, build, filename, url, user):
# en (original), es, ru
project_translations = itertools.chain([main_project], project_translations)

# Make one DB query here and then check on Python code
# TODO: make usage of ``Project.addons.<name>_enabled`` to decide if enabled
#
# NOTE: using ``feature_id__startswith="addons_"`` to make the query faster.
# It went down from 20ms to 1ms since it does not have to check the
# `Project.pub_date` against all the features.
project_features = project.features.filter(
feature_id__startswith="addons_"
).values_list("feature_id", flat=True)
# Automatically create an AddonsConfig with the default values for
# projects that don't have one already
AddonsConfig.objects.get_or_create(project=project)

data = {
"api_version": "0",
Expand Down Expand Up @@ -320,24 +314,21 @@ def _v0(self, project, version, build, filename, url, user):
# serializer than the keys ``project``, ``version`` and ``build`` from the top level.
"addons": {
"analytics": {
"enabled": Feature.ADDONS_ANALYTICS_DISABLED
not in project_features,
"enabled": project.addons.analytics_enabled,
# TODO: consider adding this field into the ProjectSerializer itself.
# NOTE: it seems we are removing this feature,
# so we may not need the ``code`` attribute here
# https://github.com/readthedocs/readthedocs.org/issues/9530
"code": project.analytics_code,
},
"external_version_warning": {
"enabled": Feature.ADDONS_EXTERNAL_VERSION_WARNING_DISABLED
not in project_features,
"enabled": project.addons.external_version_warning_enabled,
# NOTE: I think we are moving away from these selectors
# since we are doing floating noticications now.
# "query_selector": "[role=main]",
},
"non_latest_version_warning": {
"enabled": Feature.ADDONS_NON_LATEST_VERSION_WARNING_DISABLED
not in project_features,
"enabled": project.addons.stable_latest_version_warning_enabled,
# NOTE: I think we are moving away from these selectors
# since we are doing floating noticications now.
# "query_selector": "[role=main]",
Expand All @@ -346,7 +337,7 @@ def _v0(self, project, version, build, filename, url, user):
),
},
"flyout": {
"enabled": Feature.ADDONS_FLYOUT_DISABLED not in project_features,
"enabled": project.addons.flyout_enabled,
"translations": [
{
# TODO: name this field "display_name"
Expand Down Expand Up @@ -398,7 +389,7 @@ def _v0(self, project, version, build, filename, url, user):
# },
},
"search": {
"enabled": Feature.ADDONS_SEARCH_DISABLED not in project_features,
"enabled": project.addons.search_enabled,
"project": project.slug,
"version": version.slug if version else None,
"api_endpoint": "/_/api/v3/search/",
Expand All @@ -416,7 +407,7 @@ def _v0(self, project, version, build, filename, url, user):
else None,
},
"hotkeys": {
"enabled": Feature.ADDONS_HOTKEYS_DISABLED not in project_features,
"enabled": project.addons.hotkeys_enabled,
"doc_diff": {
"enabled": True,
"trigger": "KeyD", # Could be something like "Ctrl + D"
Expand All @@ -437,8 +428,7 @@ def _v0(self, project, version, build, filename, url, user):
data["addons"].update(
{
"doc_diff": {
"enabled": Feature.ADDONS_DOC_DIFF_DISABLED
not in project_features,
"enabled": project.addons.doc_diff_enabled,
# "http://test-builds-local.devthedocs.org/en/latest/index.html"
"base_url": resolver.resolve(
project=project,
Expand Down Expand Up @@ -478,8 +468,7 @@ def _v0(self, project, version, build, filename, url, user):
data["addons"].update(
{
"ethicalads": {
"enabled": Feature.ADDONS_ETHICALADS_DISABLED
not in project_features,
"enabled": project.addons.ethicalads_enabled,
# NOTE: this endpoint is not authenticated, the user checks are done over an annonymous user for now
#
# NOTE: it requires ``settings.USE_PROMOS=True`` to return ``ad_free=false`` here
Expand Down

0 comments on commit 3cfc29b

Please sign in to comment.