diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index b51ed12053e..e37e4ffd73c 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -1,6 +1,5 @@ """Project forms.""" -import datetime import json from random import choice from re import fullmatch @@ -8,14 +7,12 @@ import dns.name import dns.resolver -import pytz from allauth.socialaccount.models import SocialAccount from django import forms from django.conf import settings from django.contrib.auth.models import User from django.db.models import Q from django.urls import reverse -from django.utils import timezone from django.utils.translation import gettext_lazy as _ from readthedocs.builds.constants import INTERNAL @@ -689,22 +686,11 @@ class Meta: def __init__(self, *args, **kwargs): self.project = kwargs.pop("project", None) - - tzinfo = pytz.timezone("America/Los_Angeles") - addons_enabled_by_default = timezone.now() > datetime.datetime( - 2024, 10, 7, 0, 0, 0, tzinfo=tzinfo - ) - - addons, created = AddonsConfig.objects.get_or_create(project=self.project) - if created: - addons.enabled = addons_enabled_by_default - addons.save() - - kwargs["instance"] = addons + kwargs["instance"] = self.project.addons super().__init__(*args, **kwargs) # Keep the ability to disable addons completely on Read the Docs for Business - if not settings.RTD_ALLOW_ORGANIZATIONS and addons_enabled_by_default: + if not settings.RTD_ALLOW_ORGANIZATIONS: self.fields["enabled"].disabled = True def clean(self): diff --git a/readthedocs/projects/migrations/0139_addons_filetreediff_field.py b/readthedocs/projects/migrations/0139_addons_filetreediff_field.py new file mode 100644 index 00000000000..4fb8925ebe5 --- /dev/null +++ b/readthedocs/projects/migrations/0139_addons_filetreediff_field.py @@ -0,0 +1,37 @@ +# Generated by Django 4.2.16 on 2024-11-25 11:57 + +from django.db import migrations, models +from django_safemigrate import Safe + + +def forwards_func(apps, schema_editor): + """ + Enable FileTreeDiff on those projects with the feature flag enabled. + """ + Feature = apps.get_model("projects", "Feature") + feature = Feature.objects.filter(feature_id="generate_manifest_for_file_tree_diff").first() + if feature: + for project in feature.projects.all().iterator(): + project.addons.filetreediff_enabled = True + project.addons.save() + +class Migration(migrations.Migration): + safe = Safe.before_deploy + + dependencies = [ + ('projects', '0138_remove_old_fields'), + ] + + operations = [ + migrations.AddField( + model_name='addonsconfig', + name='filetreediff_enabled', + field=models.BooleanField(blank=True, default=False, null=True), + ), + migrations.AddField( + model_name='historicaladdonsconfig', + name='filetreediff_enabled', + field=models.BooleanField(blank=True, default=False, null=True), + ), + migrations.RunPython(forwards_func), + ] diff --git a/readthedocs/projects/migrations/0140_addons_options_base_version.py b/readthedocs/projects/migrations/0140_addons_options_base_version.py new file mode 100644 index 00000000000..adfc81d28cb --- /dev/null +++ b/readthedocs/projects/migrations/0140_addons_options_base_version.py @@ -0,0 +1,27 @@ +# Generated by Django 4.2.16 on 2024-11-25 12:15 + +from django.db import migrations, models +import django.db.models.deletion +from django_safemigrate import Safe + + +class Migration(migrations.Migration): + safe = Safe.before_deploy + + dependencies = [ + ('builds', '0059_add_version_date_index'), + ('projects', '0139_addons_filetreediff_field'), + ] + + operations = [ + migrations.AddField( + model_name='addonsconfig', + name='options_base_version', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='builds.version', verbose_name='Base version to compare against (eg. DocDiff, File Tree Diff)'), + ), + migrations.AddField( + model_name='historicaladdonsconfig', + name='options_base_version', + field=models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='builds.version', verbose_name='Base version to compare against (eg. DocDiff, File Tree Diff)'), + ), + ] diff --git a/readthedocs/projects/migrations/0141_create_addonsconfig.py b/readthedocs/projects/migrations/0141_create_addonsconfig.py new file mode 100644 index 00000000000..947f3bafe34 --- /dev/null +++ b/readthedocs/projects/migrations/0141_create_addonsconfig.py @@ -0,0 +1,29 @@ +# Generated by Django 4.2.16 on 2024-11-26 09:04 + +from django.db import migrations +from django_safemigrate import Safe + + +def forwards_func(apps, schema_editor): + """ + Create ``AddonsConfig`` for those projects without it. + + We were creating these models when the user accesses to the dashboard or the API itself. + Now, we are creating them on a ``post_save`` Django Signal. + """ + AddonsConfig = apps.get_model("projects", "AddonsConfig") + Project = apps.get_model("projects", "Project") + for project in Project.objects.filter(addons__isnull=True).iterator(): + AddonsConfig.objects.get_or_create(project=project) + + +class Migration(migrations.Migration): + safe = Safe.before_deploy + + dependencies = [ + ('projects', '0140_addons_options_base_version'), + ] + + operations = [ + migrations.RunPython(forwards_func), + ] diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index b163282473e..c62e5149a6e 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -171,6 +171,13 @@ class AddonsConfig(TimeStampedModel): # https://github.com/readthedocs/addons/pull/415 options_load_when_embedded = models.BooleanField(default=False) + options_base_version = models.ForeignKey( + "builds.Version", + verbose_name=_("Base version to compare against (eg. DocDiff, File Tree Diff)"), + null=True, + on_delete=models.SET_NULL, + ) + # Analytics # NOTE: we keep analytics disabled by default to save resources. @@ -185,6 +192,9 @@ class AddonsConfig(TimeStampedModel): # EthicalAds ethicalads_enabled = models.BooleanField(default=True) + # File Tree Diff + filetreediff_enabled = models.BooleanField(default=False, null=True, blank=True) + # Flyout flyout_enabled = models.BooleanField(default=True) flyout_sorting = models.CharField( @@ -1914,7 +1924,6 @@ def add_features(sender, **kwargs): RESOLVE_PROJECT_FROM_HEADER = "resolve_project_from_header" USE_PROXIED_APIS_WITH_PREFIX = "use_proxied_apis_with_prefix" ALLOW_VERSION_WARNING_BANNER = "allow_version_warning_banner" - GENERATE_MANIFEST_FOR_FILE_TREE_DIFF = "generate_manifest_for_file_tree_diff" # Versions sync related features SKIP_SYNC_TAGS = "skip_sync_tags" @@ -1975,10 +1984,6 @@ def add_features(sender, **kwargs): ALLOW_VERSION_WARNING_BANNER, _("Dashboard: Allow project to use the version warning banner."), ), - ( - GENERATE_MANIFEST_FOR_FILE_TREE_DIFF, - _("Build: Generate a file manifest for file tree diff."), - ), # Versions sync related features ( SKIP_SYNC_BRANCHES, diff --git a/readthedocs/projects/signals.py b/readthedocs/projects/signals.py index 1fb0db17d3c..4309270a7ce 100644 --- a/readthedocs/projects/signals.py +++ b/readthedocs/projects/signals.py @@ -6,9 +6,7 @@ from django.db.models.signals import post_save from django.dispatch import receiver -from readthedocs.builds.models import Version -from readthedocs.projects.constants import MKDOCS -from readthedocs.projects.models import AddonsConfig +from readthedocs.projects.models import AddonsConfig, Project log = structlog.get_logger(__name__) @@ -24,29 +22,7 @@ files_changed = django.dispatch.Signal() -@receiver(post_save, sender=Version) -def enable_addons_on_new_mkdocs_projects(instance, *args, **kwargs): - """ - Enable Addons on MkDocs projects. - - We removed all the `mkdocs.yml` manipulation that set the theme to `readthedocs` if - undefined and injects JS and CSS files to show the old flyout. - - Now, we are enabling addons by default on MkDocs projects to move forward - with the idea of removing the magic executed on behalves the users and - promote addons more. - - Reference https://github.com/readthedocs/addons/issues/72#issuecomment-1926647293 - """ - version = instance - project = instance.project - - if version.documentation_type == MKDOCS: - config, created = AddonsConfig.objects.get_or_create(project=project) - if created: - log.info( - "Creating AddonsConfig automatically for MkDocs project.", - project_slug=project.slug, - ) - config.enabled = True - config.save() +@receiver(post_save, sender=Project) +def create_addons_on_new_projects(instance, *args, **kwargs): + """Create ``AddonsConfig`` on new projects.""" + AddonsConfig.objects.get_or_create(project=instance) diff --git a/readthedocs/projects/tasks/search.py b/readthedocs/projects/tasks/search.py index 74cbb3adfa8..9c09398aeac 100644 --- a/readthedocs/projects/tasks/search.py +++ b/readthedocs/projects/tasks/search.py @@ -6,7 +6,7 @@ from readthedocs.builds.models import Build, Version from readthedocs.filetreediff import write_manifest from readthedocs.filetreediff.dataclasses import FileTreeDiffFile, FileTreeDiffManifest -from readthedocs.projects.models import Feature, HTMLFile, Project +from readthedocs.projects.models import HTMLFile, Project from readthedocs.projects.signals import files_changed from readthedocs.search.documents import PageDocument from readthedocs.search.utils import index_objects, remove_indexed_files @@ -166,10 +166,14 @@ def _get_indexers(*, version: Version, build: Build, search_index_name=None): # File tree diff is under a feature flag for now, # and we only allow to compare PR previews against the latest version. - has_feature = version.project.has_feature( - Feature.GENERATE_MANIFEST_FOR_FILE_TREE_DIFF + base_version = ( + version.project.addons.options_base_version.slug + if version.project.addons.options_base_version + else LATEST + ) + create_manifest = version.project.addons.filetreediff_enabled and ( + version.is_external or version.slug == base_version ) - create_manifest = has_feature and (version.is_external or version.slug == LATEST) if create_manifest: file_manifest_indexer = FileManifestIndexer( version=version, diff --git a/readthedocs/projects/tests/test_version_handling.py b/readthedocs/projects/tests/test_version_handling.py index 0f70ef16376..5be3c129d96 100644 --- a/readthedocs/projects/tests/test_version_handling.py +++ b/readthedocs/projects/tests/test_version_handling.py @@ -2,7 +2,7 @@ import pytest from readthedocs.builds.models import Build, Version -from readthedocs.projects.models import AddonsConfig, Project +from readthedocs.projects.models import Project from readthedocs.projects.version_handling import ( sort_versions_calver, sort_versions_custom_pattern, @@ -18,7 +18,6 @@ def setup(self, requests_mock): self.requests_mock = requests_mock self.project = fixture.get(Project, slug="project") - self.addons = fixture.get(AddonsConfig, project=self.project) self.version = self.project.versions.get(slug="latest") self.build = fixture.get( Build, diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index 9efa0db13af..16a43710ed1 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -74,7 +74,6 @@ WebHookForm, ) from readthedocs.projects.models import ( - AddonsConfig, Domain, EmailHook, EnvironmentVariable, @@ -442,9 +441,6 @@ def done(self, form_list, **kwargs): # attributes directly from other forms project = basics_form.save() - # Create an AddonsConfig object for this project. - AddonsConfig.objects.get_or_create(project=project) - self.finish_import_project(self.request, project) return HttpResponseRedirect( diff --git a/readthedocs/proxito/tests/responses/v1.json b/readthedocs/proxito/tests/responses/v1.json index 8375ae823e1..ccfb8e2696d 100644 --- a/readthedocs/proxito/tests/responses/v1.json +++ b/readthedocs/proxito/tests/responses/v1.json @@ -149,9 +149,7 @@ "doc_diff": { "enabled": true, "base_url": "https://project.dev.readthedocs.io/en/latest/index.html", - "inject_styles": true, - "base_host": "", - "base_page": "" + "inject_styles": true }, "filetreediff": { "enabled": false diff --git a/readthedocs/proxito/tests/test_headers.py b/readthedocs/proxito/tests/test_headers.py index f8ace044030..8a4c199721b 100644 --- a/readthedocs/proxito/tests/test_headers.py +++ b/readthedocs/proxito/tests/test_headers.py @@ -12,7 +12,7 @@ from readthedocs.builds.models import Version from readthedocs.organizations.models import Organization from readthedocs.projects.constants import PRIVATE, PUBLIC -from readthedocs.projects.models import AddonsConfig, Domain, HTTPHeader +from readthedocs.projects.models import Domain, HTTPHeader from .base import BaseDocServing @@ -200,8 +200,6 @@ def test_user_domain_headers(self): self.assertEqual(r[http_header_secure], http_header_value) def test_force_addons_header(self): - fixture.get(AddonsConfig, project=self.project, enabled=True) - r = self.client.get( "/en/latest/", secure=True, headers={"host": "project.dev.readthedocs.io"} ) diff --git a/readthedocs/proxito/tests/test_hosting.py b/readthedocs/proxito/tests/test_hosting.py index 1fc367ec10f..a990fe2a984 100644 --- a/readthedocs/proxito/tests/test_hosting.py +++ b/readthedocs/proxito/tests/test_hosting.py @@ -24,7 +24,7 @@ PUBLIC, SINGLE_VERSION_WITHOUT_TRANSLATIONS, ) -from readthedocs.projects.models import AddonsConfig, Domain, Feature, Project +from readthedocs.projects.models import Domain, Project @override_settings( @@ -138,22 +138,18 @@ def test_get_config_unsupported_version(self): assert r.json() == self._get_response_dict("v3") 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.notifications_enabled = False - addons.notifications_show_on_latest = False - addons.notifications_show_on_non_stable = False - addons.notifications_show_on_external = False - addons.save() + self.project.addons.analytics_enabled = False + self.project.addons.doc_diff_enabled = False + self.project.addons.external_version_warning_enabled = False + self.project.addons.ethicalads_enabled = False + self.project.addons.flyout_enabled = False + self.project.addons.hotkeys_enabled = False + self.project.addons.search_enabled = False + self.project.addons.notifications_enabled = False + self.project.addons.notifications_show_on_latest = False + self.project.addons.notifications_show_on_non_stable = False + self.project.addons.notifications_show_on_external = False + self.project.addons.save() r = self.client.get( reverse("proxito_readthedocs_docs_addons"), @@ -723,13 +719,8 @@ def test_custom_domain_url(self): ) def test_linkpreviews(self): - addons = fixture.get( - AddonsConfig, - project=self.project, - ) - - addons.linkpreviews_enabled = True - addons.save() + self.project.addons.linkpreviews_enabled = True + self.project.addons.save() r = self.client.get( reverse("proxito_readthedocs_docs_addons"), @@ -787,7 +778,7 @@ def test_number_of_queries_project_version_slug(self): active=True, ) - with self.assertNumQueries(24): + with self.assertNumQueries(20): r = self.client.get( reverse("proxito_readthedocs_docs_addons"), { @@ -816,7 +807,7 @@ def test_number_of_queries_url(self): active=True, ) - with self.assertNumQueries(26): + with self.assertNumQueries(22): r = self.client.get( reverse("proxito_readthedocs_docs_addons"), { @@ -852,7 +843,7 @@ def test_number_of_queries_url_subproject(self): active=True, ) - with self.assertNumQueries(35): + with self.assertNumQueries(31): r = self.client.get( reverse("proxito_readthedocs_docs_addons"), { @@ -878,7 +869,7 @@ def test_number_of_queries_url_translations(self): language=language, ) - with self.assertNumQueries(62): + with self.assertNumQueries(58): r = self.client.get( reverse("proxito_readthedocs_docs_addons"), { @@ -895,11 +886,8 @@ def test_number_of_queries_url_translations(self): @mock.patch("readthedocs.filetreediff.get_manifest") def test_file_tree_diff(self, get_manifest): - get( - Feature, - projects=[self.project], - feature_id=Feature.GENERATE_MANIFEST_FOR_FILE_TREE_DIFF, - ) + self.project.addons.filetreediff_enabled = True + self.project.addons.save() pr_version = get( Version, project=self.project, @@ -953,7 +941,7 @@ def test_file_tree_diff(self, get_manifest): ], ), ] - with self.assertNumQueries(30): + with self.assertNumQueries(25): r = self.client.get( reverse("proxito_readthedocs_docs_addons"), { diff --git a/readthedocs/proxito/views/hosting.py b/readthedocs/proxito/views/hosting.py index 4315de518d4..933f359fbec 100644 --- a/readthedocs/proxito/views/hosting.py +++ b/readthedocs/proxito/views/hosting.py @@ -30,7 +30,7 @@ ADDONS_FLYOUT_SORTING_PYTHON_PACKAGING, ADDONS_FLYOUT_SORTING_SEMVER_READTHEDOCS_COMPATIBLE, ) -from readthedocs.projects.models import AddonsConfig, Feature, Project +from readthedocs.projects.models import Project from readthedocs.projects.version_handling import ( comparable_version, sort_versions_calver, @@ -338,10 +338,6 @@ def _v1(self, project, version, build, filename, url, request): sorted_versions_active_built_not_hidden = Version.objects.none() user = request.user - # Automatically create an AddonsConfig with the default values for - # projects that don't have one already - AddonsConfig.objects.get_or_create(project=project) - versions_active_built_not_hidden = ( self._get_versions(request, project) .select_related("project") @@ -574,6 +570,11 @@ def _v1(self, project, version, build, filename, url, request): # If we don't know the filename, we cannot return the data required by DocDiff to work. # In that case, we just don't include the `doc_diff` object in the response. if url: + base_version_slug = ( + project.addons.options_base_version.slug + if project.addons.options_base_version + else LATEST + ) data["addons"].update( { "doc_diff": { @@ -581,20 +582,13 @@ def _v1(self, project, version, build, filename, url, request): # "http://test-builds-local.devthedocs.org/en/latest/index.html" "base_url": resolver.resolve( project=project, - # NOTE: we are using LATEST version to compare against to for now. - # Ideally, this should be configurable by the user. - version_slug=LATEST, + version_slug=base_version_slug, language=project.language, filename=filename, ) if filename else None, "inject_styles": True, - # NOTE: `base_host` and `base_page` are not required, since - # we are constructing the `base_url` in the backend instead - # of the frontend, as the doc-diff extension does. - "base_host": "", - "base_page": "", }, } ) @@ -637,16 +631,18 @@ def _get_filetreediff_response(self, *, request, project, version, resolver): if not version.is_external: return None - if not project.has_feature(Feature.GENERATE_MANIFEST_FOR_FILE_TREE_DIFF): + if not project.addons.filetreediff_enabled: return None - latest_version = project.get_latest_version() - if not latest_version or not self._has_permission( - request=request, version=latest_version + base_version = ( + project.addons.options_base_version or project.get_latest_version() + ) + if not base_version or not self._has_permission( + request=request, version=base_version ): return None - diff = get_diff(version_a=version, version_b=latest_version) + diff = get_diff(version_a=version, version_b=base_version) if not diff: return None @@ -666,7 +662,7 @@ def _get_filetreediff_response(self, *, request, project, version, resolver): "base": resolver.resolve_version( project=project, filename=filename, - version=latest_version, + version=base_version, ), }, } @@ -684,7 +680,7 @@ def _get_filetreediff_response(self, *, request, project, version, resolver): "base": resolver.resolve_version( project=project, filename=filename, - version=latest_version, + version=base_version, ), }, } @@ -702,7 +698,7 @@ def _get_filetreediff_response(self, *, request, project, version, resolver): "base": resolver.resolve_version( project=project, filename=filename, - version=latest_version, + version=base_version, ), }, } diff --git a/readthedocs/rtd_tests/tests/test_imported_file.py b/readthedocs/rtd_tests/tests/test_imported_file.py index 9795780a13d..50af94abc74 100644 --- a/readthedocs/rtd_tests/tests/test_imported_file.py +++ b/readthedocs/rtd_tests/tests/test_imported_file.py @@ -11,7 +11,7 @@ from readthedocs.builds.constants import BUILD_STATE_FINISHED, EXTERNAL, LATEST from readthedocs.builds.models import Build, Version from readthedocs.filetreediff.dataclasses import FileTreeDiffFile, FileTreeDiffManifest -from readthedocs.projects.models import Feature, HTMLFile, ImportedFile, Project +from readthedocs.projects.models import HTMLFile, ImportedFile, Project from readthedocs.projects.tasks.search import index_build from readthedocs.search.documents import PageDocument @@ -352,14 +352,11 @@ def test_update_content(self): def test_create_file_tree_manifest(self, write_manifest): assert self.version.slug == LATEST index_build(self.build.pk) - # Feature flag is not enabled. + # File Tree Diff is not enabled by default write_manifest.assert_not_called() - get( - Feature, - feature_id=Feature.GENERATE_MANIFEST_FOR_FILE_TREE_DIFF, - projects=[self.project], - ) + self.project.addons.filetreediff_enabled = True + self.project.addons.save() index_build(self.build.pk) manifest = FileTreeDiffManifest( build_id=self.build.pk, @@ -401,5 +398,6 @@ def test_create_file_tree_manifest(self, write_manifest): new_version.save() with override_settings(DOCROOT=self.test_dir): self._copy_storage_dir(new_version) + write_manifest.reset_mock() index_build(self.build.pk) write_manifest.assert_called_once_with(new_version, manifest)