Addons: make default root CSS selector a shared option (#11767)

Add a "global" shared CSS root selector that is used all across our features: link previews, docdiff, search, embed API, etc. Also, put the "documentation tool name" as a top level config as well. With that information we can define better defaults for CSS selectors and other stuffs in the future if needed. Example: if documentation tool is set as Sphinx by the user, we will be able to use `[role=main] a.internal` for the Link Preview CSS selector. Related readthedocs/addons#433
readthedocs · Nov 21, 2024 · e4d23c3 · e4d23c3
1 parent e0f17cc
commit e4d23c3
Show file tree

Hide file tree

Showing 7 changed files with 88 additions and 60 deletions.
diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py
@@ -653,19 +653,16 @@ class Meta:
         fields = (
             "enabled",
             "project",
+            "options_root_selector",
             "analytics_enabled",
             "doc_diff_enabled",
-            "doc_diff_root_selector",
             "flyout_enabled",
             "flyout_sorting",
             "flyout_sorting_latest_stable_at_beginning",
             "flyout_sorting_custom_pattern",
             "hotkeys_enabled",
             "search_enabled",
             "linkpreviews_enabled",
-            "linkpreviews_root_selector",
-            "linkpreviews_doctool_name",
-            "linkpreviews_doctool_version",
             "notifications_enabled",
             "notifications_show_on_latest",
             "notifications_show_on_non_stable",
@@ -681,16 +678,12 @@ class Meta:
             ),
             "notifications_show_on_latest": _("Show a notification on latest version"),
             "linkpreviews_enabled": _("Enabled"),
-            "linkpreviews_root_selector": _("Root selector"),
-            "linkpreviews_doctool_name": _("Documentation tool name"),
-            "linkpreviews_doctool_version": _("Documentation tool version"),
+            "options_root_selector": _("CSS main content selector"),
         }
+
         widgets = {
-            "doc_diff_root_selector": forms.TextInput(
-                attrs={"placeholder": AddonsConfig.DOC_DIFF_DEFAULT_ROOT_SELECTOR}
-            ),
-            "linkpreviews_root_selector": forms.TextInput(
-                attrs={"placeholder": AddonsConfig.LINKPREVIEWS_DEFAULT_ROOT_SELECTOR}
+            "options_root_selector": forms.TextInput(
+                attrs={"placeholder": "[role=main]"}
             ),
         }
 

diff --git a/readthedocs/projects/migrations/0137_use_generic_root_selector.py b/readthedocs/projects/migrations/0137_use_generic_root_selector.py
@@ -0,0 +1,25 @@
+# Generated by Django 4.2.16 on 2024-11-20 12:35
+
+from django.db import migrations, models
+from django_safemigrate import Safe
+
+
+class Migration(migrations.Migration):
+    safe = Safe.before_deploy
+
+    dependencies = [
+        ('projects', '0136_addons_customscript_notnull'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='addonsconfig',
+            name='options_root_selector',
+            field=models.CharField(blank=True, help_text='CSS selector for the main content of the page. Leave it blank for auto-detect.', max_length=128, null=True),
+        ),
+        migrations.AddField(
+            model_name='historicaladdonsconfig',
+            name='options_root_selector',
+            field=models.CharField(blank=True, help_text='CSS selector for the main content of the page. Leave it blank for auto-detect.', max_length=128, null=True),
+        ),
+    ]
diff --git a/readthedocs/projects/migrations/0138_remove_old_fields.py b/readthedocs/projects/migrations/0138_remove_old_fields.py
@@ -0,0 +1,47 @@
+# Generated by Django 4.2.16 on 2024-11-20 12:35
+
+from django.db import migrations
+from django_safemigrate import Safe
+
+
+class Migration(migrations.Migration):
+    safe = Safe.after_deploy
+
+    dependencies = [
+        ('projects', '0137_use_generic_root_selector'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='addonsconfig',
+            name='doc_diff_root_selector',
+        ),
+        migrations.RemoveField(
+            model_name='addonsconfig',
+            name='linkpreviews_doctool_name',
+        ),
+        migrations.RemoveField(
+            model_name='addonsconfig',
+            name='linkpreviews_doctool_version',
+        ),
+        migrations.RemoveField(
+            model_name='addonsconfig',
+            name='linkpreviews_root_selector',
+        ),
+        migrations.RemoveField(
+            model_name='historicaladdonsconfig',
+            name='doc_diff_root_selector',
+        ),
+        migrations.RemoveField(
+            model_name='historicaladdonsconfig',
+            name='linkpreviews_doctool_name',
+        ),
+        migrations.RemoveField(
+            model_name='historicaladdonsconfig',
+            name='linkpreviews_doctool_version',
+        ),
+        migrations.RemoveField(
+            model_name='historicaladdonsconfig',
+            name='linkpreviews_root_selector',
+        ),
+    ]
diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py
@@ -144,13 +144,6 @@ class AddonsConfig(TimeStampedModel):
     Everything is enabled by default.
     """
 
-    DOC_DIFF_DEFAULT_ROOT_SELECTOR = "[role=main]"
-    LINKPREVIEWS_DEFAULT_ROOT_SELECTOR = "[role=main] a.internal"
-    LINKPREVIEWS_DOCTOOL_NAME_CHOICES = (
-        ("sphinx", "Sphinx"),
-        ("other", "Other"),
-    )
-
     # Model history
     history = ExtraHistoricalRecords()
 
@@ -167,6 +160,13 @@ class AddonsConfig(TimeStampedModel):
         help_text="Enable/Disable all the addons on this project",
     )
 
+    options_root_selector = models.CharField(
+        null=True,
+        blank=True,
+        max_length=128,
+        help_text="CSS selector for the main content of the page. Leave it blank for auto-detect.",
+    )
+
     # Whether or not load addons library when the requested page is embedded (e.g. inside an iframe)
     # https://github.com/readthedocs/addons/pull/415
     options_load_when_embedded = models.BooleanField(default=False)
@@ -181,12 +181,6 @@ class AddonsConfig(TimeStampedModel):
     doc_diff_enabled = models.BooleanField(default=True)
     doc_diff_show_additions = models.BooleanField(default=True)
     doc_diff_show_deletions = models.BooleanField(default=True)
-    doc_diff_root_selector = models.CharField(
-        null=True,
-        blank=True,
-        max_length=128,
-        help_text="CSS selector for the main content of the page",
-    )
 
     # EthicalAds
     ethicalads_enabled = models.BooleanField(default=True)
@@ -242,18 +236,6 @@ class AddonsConfig(TimeStampedModel):
 
     # Link Previews
     linkpreviews_enabled = models.BooleanField(default=False)
-    linkpreviews_root_selector = models.CharField(null=True, blank=True, max_length=128)
-    linkpreviews_doctool_name = models.CharField(
-        choices=LINKPREVIEWS_DOCTOOL_NAME_CHOICES,
-        null=True,
-        blank=True,
-        max_length=128,
-    )
-    linkpreviews_doctool_version = models.CharField(
-        null=True,
-        blank=True,
-        max_length=128,
-    )
 
 
 class AddonSearchFilter(TimeStampedModel):

diff --git a/readthedocs/proxito/tests/responses/v1.json b/readthedocs/proxito/tests/responses/v1.json
@@ -122,7 +122,8 @@
   },
   "addons": {
     "options": {
-      "load_when_embedded": false
+      "load_when_embedded": false,
+      "root_selector": null
     },
     "analytics": {
       "enabled": false,
@@ -148,7 +149,6 @@
     "doc_diff": {
       "enabled": true,
       "base_url": "https://project.dev.readthedocs.io/en/latest/index.html",
-      "root_selector": null,
       "inject_styles": true,
       "base_host": "",
       "base_page": ""
@@ -169,12 +169,7 @@
       "src": null
     },
     "linkpreviews": {
-      "enabled": false,
-      "root_selector": "[role=main] a.internal",
-      "doctool": {
-        "name": null,
-        "version": null
-      }
+      "enabled": false
     }
   }
 }
diff --git a/readthedocs/proxito/tests/test_hosting.py b/readthedocs/proxito/tests/test_hosting.py
@@ -729,9 +729,6 @@ def test_linkpreviews(self):
         )
 
         addons.linkpreviews_enabled = True
-        addons.linkpreviews_root_selector = "[role=main] a"
-        addons.linkpreviews_doctool_name = "sphinx"
-        addons.linkpreviews_doctool_version = "8.0.1"
         addons.save()
 
         r = self.client.get(
@@ -748,11 +745,6 @@ def test_linkpreviews(self):
         )
         expected = {
             "enabled": True,
-            "root_selector": "[role=main] a",
-            "doctool": {
-                "name": "sphinx",
-                "version": "8.0.1",
-            },
         }
 
         assert r.status_code == 200

diff --git a/readthedocs/proxito/views/hosting.py b/readthedocs/proxito/views/hosting.py
@@ -456,6 +456,7 @@ def _v1(self, project, version, build, filename, url, request):
             "addons": {
                 "options": {
                     "load_when_embedded": project.addons.options_load_when_embedded,
+                    "root_selector": project.addons.options_root_selector,
                 },
                 "analytics": {
                     "enabled": project.addons.analytics_enabled,
@@ -519,12 +520,6 @@ def _v1(self, project, version, build, filename, url, request):
                 },
                 "linkpreviews": {
                     "enabled": project.addons.linkpreviews_enabled,
-                    "root_selector": project.addons.linkpreviews_root_selector
-                    or project.addons.LINKPREVIEWS_DEFAULT_ROOT_SELECTOR,
-                    "doctool": {
-                        "name": project.addons.linkpreviews_doctool_name,
-                        "version": project.addons.linkpreviews_doctool_version,
-                    },
                 },
                 "hotkeys": {
                     "enabled": project.addons.hotkeys_enabled,
@@ -594,7 +589,6 @@ def _v1(self, project, version, build, filename, url, request):
                         )
                         if filename
                         else None,
-                        "root_selector": project.addons.doc_diff_root_selector,
                         "inject_styles": True,
                         # NOTE: `base_host` and `base_page` are not required, since
                         # we are constructing the `base_url` in the backend instead