From b55eba8ed206d148c180b2b7e84402c5cae8e1b6 Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Mon, 14 Oct 2024 12:18:08 -0700 Subject: [PATCH 1/6] Enable subproject filters for projects with subprojects This at least shows the UI element if it makes sense to show. Currently there's no way without knowing the query syntax to get subprojects, which has caused confusion for a few of our users and broke existing behavior. Refs https://github.com/readthedocs/readthedocs.org/pull/11638 --- readthedocs/proxito/views/hosting.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/readthedocs/proxito/views/hosting.py b/readthedocs/proxito/views/hosting.py index b74f9c4ab71..e6a6d018ab9 100644 --- a/readthedocs/proxito/views/hosting.py +++ b/readthedocs/proxito/views/hosting.py @@ -489,12 +489,12 @@ def _v1(self, project, version, build, filename, url, request): # to the user's query. # It uses "Search query sintax": # https://docs.readthedocs.io/en/stable/server-side-search/syntax.html - # [ - # "Include subprojects", - # f"subprojects:{project.slug}/{version.slug}", - # ], + [ + "Include subprojects", + f"subprojects:{project.slug}/{version.slug}", + ], ] - if version + if project.subprojects.exists() else [], "default_filter": f"project:{project.slug}/{version.slug}" if version From 9f59f7838957f1ebd2aa338d1a9053cd48f2cd9b Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Mon, 14 Oct 2024 12:22:02 -0700 Subject: [PATCH 2/6] Also show on subprojects --- readthedocs/proxito/views/hosting.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/readthedocs/proxito/views/hosting.py b/readthedocs/proxito/views/hosting.py index e6a6d018ab9..1e901809aeb 100644 --- a/readthedocs/proxito/views/hosting.py +++ b/readthedocs/proxito/views/hosting.py @@ -494,7 +494,8 @@ def _v1(self, project, version, build, filename, url, request): f"subprojects:{project.slug}/{version.slug}", ], ] - if project.subprojects.exists() + # Show the subprojects filter on the parent project and subproject + if project.subprojects.exists() or project.superprojects.exists() else [], "default_filter": f"project:{project.slug}/{version.slug}" if version From 66222a75b0e0841748831f8745dbfb36fc0e1cbd Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Mon, 14 Oct 2024 14:02:33 -0700 Subject: [PATCH 3/6] Update to support subprojects having the right filter --- readthedocs/proxito/views/hosting.py | 30 ++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/readthedocs/proxito/views/hosting.py b/readthedocs/proxito/views/hosting.py index 1e901809aeb..d1bbc22543b 100644 --- a/readthedocs/proxito/views/hosting.py +++ b/readthedocs/proxito/views/hosting.py @@ -489,14 +489,11 @@ def _v1(self, project, version, build, filename, url, request): # to the user's query. # It uses "Search query sintax": # https://docs.readthedocs.io/en/stable/server-side-search/syntax.html - [ - "Include subprojects", - f"subprojects:{project.slug}/{version.slug}", - ], - ] - # Show the subprojects filter on the parent project and subproject - if project.subprojects.exists() or project.superprojects.exists() - else [], + # [ + # "Include subprojects", + # f"subprojects:{project.slug}/{version.slug}", + # ], + ], "default_filter": f"project:{project.slug}/{version.slug}" if version else None, @@ -515,6 +512,23 @@ def _v1(self, project, version, build, filename, url, request): }, } + # Show the subprojects filter on the parent project and subproject + if project.subprojects.exists(): + data["addons"]["search"]["filters"].append( + [ + "Include subprojects", + f"subprojects:{project.slug}/{version.slug}", + ] + ) + if project.superprojects.exists(): + superproject = project.superprojects.first() + data["addons"]["search"]["filters"].append( + [ + "Include subprojects", + f"subprojects:{superproject.slug}/{version.slug}", + ] + ) + # DocDiff depends on `url=` GET attribute. # This attribute allows us to know the exact filename where the request was made. # If we don't know the filename, we cannot return the data required by DocDiff to work. From 9c1dfe43d9afdf164dc18658ec4379310c2220b0 Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Mon, 14 Oct 2024 14:38:20 -0700 Subject: [PATCH 4/6] Fix tests --- readthedocs/proxito/tests/test_hosting.py | 8 ++++---- readthedocs/proxito/views/hosting.py | 4 +++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/readthedocs/proxito/tests/test_hosting.py b/readthedocs/proxito/tests/test_hosting.py index fdc19e0eefd..bcda4a0383f 100644 --- a/readthedocs/proxito/tests/test_hosting.py +++ b/readthedocs/proxito/tests/test_hosting.py @@ -736,7 +736,7 @@ def test_number_of_queries_project_version_slug(self): active=True, ) - with self.assertNumQueries(22): + with self.assertNumQueries(24): r = self.client.get( reverse("proxito_readthedocs_docs_addons"), { @@ -765,7 +765,7 @@ def test_number_of_queries_url(self): active=True, ) - with self.assertNumQueries(24): + with self.assertNumQueries(26): r = self.client.get( reverse("proxito_readthedocs_docs_addons"), { @@ -801,7 +801,7 @@ def test_number_of_queries_url_subproject(self): active=True, ) - with self.assertNumQueries(31): + with self.assertNumQueries(35): r = self.client.get( reverse("proxito_readthedocs_docs_addons"), { @@ -827,7 +827,7 @@ def test_number_of_queries_url_translations(self): language=language, ) - with self.assertNumQueries(60): + with self.assertNumQueries(62): r = self.client.get( reverse("proxito_readthedocs_docs_addons"), { diff --git a/readthedocs/proxito/views/hosting.py b/readthedocs/proxito/views/hosting.py index d1bbc22543b..0104b4a9a36 100644 --- a/readthedocs/proxito/views/hosting.py +++ b/readthedocs/proxito/views/hosting.py @@ -513,6 +513,8 @@ def _v1(self, project, version, build, filename, url, request): } # Show the subprojects filter on the parent project and subproject + # TODO: Remove these queries and try to find a way to get this data + # from the resolver, which has already done these queries. if project.subprojects.exists(): data["addons"]["search"]["filters"].append( [ @@ -521,7 +523,7 @@ def _v1(self, project, version, build, filename, url, request): ] ) if project.superprojects.exists(): - superproject = project.superprojects.first() + superproject = project.superprojects.first().parent data["addons"]["search"]["filters"].append( [ "Include subprojects", From dff06b73fb920032e04a668793ba4596aa85d7b9 Mon Sep 17 00:00:00 2001 From: Eric Holscher <25510+ericholscher@users.noreply.github.com> Date: Tue, 15 Oct 2024 07:06:17 -0700 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: Manuel Kaufmann --- readthedocs/proxito/views/hosting.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/readthedocs/proxito/views/hosting.py b/readthedocs/proxito/views/hosting.py index 0104b4a9a36..2fb75ebb667 100644 --- a/readthedocs/proxito/views/hosting.py +++ b/readthedocs/proxito/views/hosting.py @@ -515,6 +515,8 @@ def _v1(self, project, version, build, filename, url, request): # Show the subprojects filter on the parent project and subproject # TODO: Remove these queries and try to find a way to get this data # from the resolver, which has already done these queries. + # TODO: Replace this fixed filters with the work proposed in + # https://github.com/readthedocs/addons/issues/22 if project.subprojects.exists(): data["addons"]["search"]["filters"].append( [ From 68d86bcb7988242ba271ee935044ca6f3f8beb9e Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Tue, 15 Oct 2024 07:09:04 -0700 Subject: [PATCH 6/6] Check for version --- readthedocs/proxito/views/hosting.py | 39 ++++++++++++++-------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/readthedocs/proxito/views/hosting.py b/readthedocs/proxito/views/hosting.py index 2fb75ebb667..564ee05f928 100644 --- a/readthedocs/proxito/views/hosting.py +++ b/readthedocs/proxito/views/hosting.py @@ -513,25 +513,26 @@ def _v1(self, project, version, build, filename, url, request): } # Show the subprojects filter on the parent project and subproject - # TODO: Remove these queries and try to find a way to get this data - # from the resolver, which has already done these queries. - # TODO: Replace this fixed filters with the work proposed in - # https://github.com/readthedocs/addons/issues/22 - if project.subprojects.exists(): - data["addons"]["search"]["filters"].append( - [ - "Include subprojects", - f"subprojects:{project.slug}/{version.slug}", - ] - ) - if project.superprojects.exists(): - superproject = project.superprojects.first().parent - data["addons"]["search"]["filters"].append( - [ - "Include subprojects", - f"subprojects:{superproject.slug}/{version.slug}", - ] - ) + if version: + # TODO: Remove these queries and try to find a way to get this data + # from the resolver, which has already done these queries. + # TODO: Replace this fixed filters with the work proposed in + # https://github.com/readthedocs/addons/issues/22 + if project.subprojects.exists(): + data["addons"]["search"]["filters"].append( + [ + "Include subprojects", + f"subprojects:{project.slug}/{version.slug}", + ] + ) + if project.superprojects.exists(): + superproject = project.superprojects.first().parent + data["addons"]["search"]["filters"].append( + [ + "Include subprojects", + f"subprojects:{superproject.slug}/{version.slug}", + ] + ) # DocDiff depends on `url=` GET attribute. # This attribute allows us to know the exact filename where the request was made.