Merge pull request #6864 from readthedocs/remove-private-resolver

ericholscher · web-flow · commit 675b1a067eda · 2020-04-07T06:55:59.000-07:00
Remove private argument from resolver
diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py
@@ -77,7 +77,6 @@
     GITLAB_URL,
     MEDIA_TYPES,
     PRIVACY_CHOICES,
-    PRIVATE,
 )
 from readthedocs.projects.models import APIProject, Project
 from readthedocs.projects.version_handling import determine_stable_version
@@ -307,11 +306,9 @@ def get_absolute_url(self):
                     'version_slug': self.slug,
                 },
             )
-        private = self.privacy_level == PRIVATE
         external = self.type == EXTERNAL
         return self.project.get_docs_url(
             version_slug=self.slug,
-            private=private,
             external=external,
         )
 
@@ -364,12 +361,10 @@ def supports_wipe(self):
         return not self.type == EXTERNAL
 
     def get_subdomain_url(self):
-        private = self.privacy_level == PRIVATE
         external = self.type == EXTERNAL
         return self.project.get_docs_url(
             version_slug=self.slug,
             lang_slug=self.project.language,
-            private=private,
             external=external,
         )
 
diff --git a/readthedocs/core/resolver.py b/readthedocs/core/resolver.py
@@ -6,7 +6,6 @@
 from django.conf import settings
 
 from readthedocs.core.utils.extend import SettingsOverrideObject
-from readthedocs.projects.constants import PRIVATE
 from readthedocs.builds.constants import EXTERNAL
 
 log = logging.getLogger(__name__)
@@ -59,7 +58,6 @@ def base_resolve_path(
             filename,
             version_slug=None,
             language=None,
-            private=False,
             single_version=None,
             subproject_slug=None,
             subdomain=None,
@@ -68,7 +66,6 @@ def base_resolve_path(
         """Resolve a with nothing smart, just filling in the blanks."""
         # Only support `/docs/project' URLs outside our normal environment. Normally
         # the path should always have a subdomain or CNAME domain
-        # pylint: disable=unused-argument
         if subdomain or cname or (self._use_subdomain()):
             url = '/'
         else:
@@ -100,16 +97,12 @@ def resolve_path(
             single_version=None,
             subdomain=None,
             cname=None,
-            private=None,
     ):
         """Resolve a URL with a subset of fields defined."""
         cname = cname or project.get_canonical_custom_domain()
         version_slug = version_slug or project.get_default_version()
         language = language or project.language
 
-        if private is None:
-            private, _ = self._get_private_and_external(project, version_slug)
-
         filename = self._fix_filename(project, filename)
 
         current_project = project
@@ -144,12 +137,10 @@ def resolve_path(
             single_version=single_version,
             subproject_slug=subproject_slug,
             cname=cname,
-            private=private,
             subdomain=subdomain,
         )
 
-    def resolve_domain(self, project, private=None):
-        # pylint: disable=unused-argument
+    def resolve_domain(self, project):
         canonical_project = self._get_canonical_project(project)
         domain = canonical_project.get_canonical_custom_domain()
         if domain:
@@ -162,14 +153,14 @@ def resolve_domain(self, project, private=None):
 
     def resolve(
             self, project, require_https=False, filename='', query_params='',
-            private=None, external=None, **kwargs
+            external=None, **kwargs
     ):
         version_slug = kwargs.get('version_slug')
 
-        if private is None or external is None:
-            if version_slug is None:
-                version_slug = project.get_default_version()
-            private, external = self._get_private_and_external(project, version_slug)
+        if version_slug is None:
+            version_slug = project.get_default_version()
+        if external is None:
+            external = self._is_external(project, version_slug)
 
         canonical_project = self._get_canonical_project(project)
         custom_domain = canonical_project.get_canonical_custom_domain()
@@ -200,7 +191,7 @@ def resolve(
         protocol = 'https' if use_https_protocol else 'http'
 
         path = self.resolve_path(
-            project, filename=filename, private=private, **kwargs
+            project, filename=filename, **kwargs
         )
         return urlunparse((protocol, domain, path, '', query_params, ''))
 
@@ -247,16 +238,14 @@ def _get_project_subdomain(self, project):
         subdomain_slug = project.slug.replace('_', '-')
         return '{}.{}'.format(subdomain_slug, settings.PUBLIC_DOMAIN)
 
-    def _get_private_and_external(self, project, version_slug):
-        from readthedocs.builds.models import Version
-        try:
-            version = project.versions.get(slug=version_slug)
-            private = version.privacy_level == PRIVATE
-            external = version.type == EXTERNAL
-        except Version.DoesNotExist:
-            private = settings.DEFAULT_PRIVACY_LEVEL == PRIVATE
-            external = False
-        return private, external
+    def _is_external(self, project, version_slug):
+        type_ = (
+            project.versions
+            .values_list('type', flat=True)
+            .filter(slug=version_slug)
+            .first()
+        )
+        return type_ == EXTERNAL
 
     def _fix_filename(self, project, filename):
         """
diff --git a/readthedocs/core/views/serve.py b/readthedocs/core/views/serve.py
@@ -451,7 +451,6 @@ def changefreqs_generator():
                     href = project.get_docs_url(
                         version_slug=version.slug,
                         lang_slug=translation.language,
-                        private=False,
                     )
                     element['languages'].append({
                         'hreflang': hreflang_formatter(translation.language),
diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py
@@ -486,7 +486,7 @@ def delete(self, *args, **kwargs):  # pylint: disable=arguments-differ
     def get_absolute_url(self):
         return reverse('projects_detail', args=[self.slug])
 
-    def get_docs_url(self, version_slug=None, lang_slug=None, private=None, external=False):
+    def get_docs_url(self, version_slug=None, lang_slug=None, external=False):
         """
         Return a URL for the docs.
 
@@ -496,7 +496,6 @@ def get_docs_url(self, version_slug=None, lang_slug=None, private=None, external
             project=self,
             version_slug=version_slug,
             language=lang_slug,
-            private=private,
             external=external,
         )
 
diff --git a/readthedocs/proxito/tests/test_full.py b/readthedocs/proxito/tests/test_full.py
@@ -441,7 +441,6 @@ def test_sitemap_xml(self):
                 self.project.get_docs_url(
                     version_slug=version.slug,
                     lang_slug=self.project.language,
-                    private=False,
                 ),
             )
 
@@ -451,7 +450,6 @@ def test_sitemap_xml(self):
             self.project.get_docs_url(
                 version_slug=private_version.slug,
                 lang_slug=self.project.language,
-                private=True,
             ),
         )
         # The `translation` project doesn't have a version named `not-translated-version`
@@ -463,7 +461,6 @@ def test_sitemap_xml(self):
             self.project.get_docs_url(
                 version_slug=not_translated_public_version.slug,
                 lang_slug=translation.language,
-                private=False,
             ),
         )
         # hreflang should use hyphen instead of underscore
@@ -476,7 +473,6 @@ def test_sitemap_xml(self):
             self.project.get_docs_url(
                 version_slug=external_version.slug,
                 lang_slug=self.project.language,
-                private=True,
             ),
         )
 
@@ -486,7 +482,6 @@ def test_sitemap_xml(self):
             self.project.get_docs_url(
                 version_slug=stable_version.slug,
                 lang_slug=self.project.language,
-                private=False,
             ),)
         self.assertEqual(response.context['versions'][0]['priority'], 1)
         self.assertEqual(response.context['versions'][0]['changefreq'], 'weekly')
@@ -497,7 +492,6 @@ def test_sitemap_xml(self):
             self.project.get_docs_url(
                 version_slug='latest',
                 lang_slug=self.project.language,
-                private=False,
             ),)
         self.assertEqual(response.context['versions'][1]['priority'], 0.9)
         self.assertEqual(response.context['versions'][1]['changefreq'], 'daily')
diff --git a/readthedocs/proxito/views/serve.py b/readthedocs/proxito/views/serve.py
@@ -461,7 +461,6 @@ def changefreqs_generator():
                         href = project.get_docs_url(
                             version_slug=version.slug,
                             lang_slug=translation.language,
-                            private=False,
                         )
                         element['languages'].append({
                             'hreflang': hreflang_formatter(translation.language),
diff --git a/readthedocs/rtd_tests/tests/test_doc_serving.py b/readthedocs/rtd_tests/tests/test_doc_serving.py
@@ -284,7 +284,6 @@ def test_sitemap_xml(self):
                 self.public.get_docs_url(
                     version_slug=version.slug,
                     lang_slug=self.public.language,
-                    private=False,
                 ),
             )
 
@@ -294,7 +293,6 @@ def test_sitemap_xml(self):
             self.public.get_docs_url(
                 version_slug=private_version.slug,
                 lang_slug=self.public.language,
-                private=True,
             ),
         )
         # The `translation` project doesn't have a version named `not-translated-version`
@@ -306,7 +304,6 @@ def test_sitemap_xml(self):
             self.public.get_docs_url(
                 version_slug=not_translated_public_version.slug,
                 lang_slug=translation.language,
-                private=False,
             ),
         )
         # hreflang should use hyphen instead of underscore
@@ -319,7 +316,6 @@ def test_sitemap_xml(self):
             self.public.get_docs_url(
                 version_slug=external_version.slug,
                 lang_slug=self.public.language,
-                private=True,
             ),
         )
 
@@ -329,7 +325,6 @@ def test_sitemap_xml(self):
             self.public.get_docs_url(
                 version_slug=stable_version.slug,
                 lang_slug=self.public.language,
-                private=False,
             ),)
         self.assertEqual(response.context['versions'][0]['priority'], 1)
         self.assertEqual(response.context['versions'][0]['changefreq'], 'weekly')
@@ -340,7 +335,6 @@ def test_sitemap_xml(self):
             self.public.get_docs_url(
                 version_slug='latest',
                 lang_slug=self.public.language,
-                private=False,
             ),)
         self.assertEqual(response.context['versions'][1]['priority'], 0.9)
         self.assertEqual(response.context['versions'][1]['changefreq'], 'daily')
diff --git a/readthedocs/rtd_tests/tests/test_resolver.py b/readthedocs/rtd_tests/tests/test_resolver.py
@@ -463,13 +463,6 @@ def test_domain_public(self):
         with override_settings(USE_SUBDOMAIN=True):
             url = resolve_domain(project=self.translation)
             self.assertEqual(url, 'pip.public.readthedocs.org')
-        # Private overrides domain
-        with override_settings(USE_SUBDOMAIN=False):
-            url = resolve_domain(project=self.translation, private=True)
-            self.assertEqual(url, 'readthedocs.org')
-        with override_settings(USE_SUBDOMAIN=True):
-            url = resolve_domain(project=self.translation, private=True)
-            self.assertEqual(url, 'pip.public.readthedocs.org')
 
     @override_settings(
         PRODUCTION_DOMAIN='readthedocs.org',
@@ -584,25 +577,28 @@ def test_resolver_subproject_alias(self):
 
     @override_settings(PRODUCTION_DOMAIN='readthedocs.org')
     def test_resolver_private_project(self):
+        self.pip.privacy_level = PRIVATE
+        self.pip.save()
         with override_settings(USE_SUBDOMAIN=False):
-            url = resolve(project=self.pip, private=True)
+            url = resolve(project=self.pip)
             self.assertEqual(url, 'http://readthedocs.org/docs/pip/en/latest/')
         with override_settings(USE_SUBDOMAIN=True):
-            url = resolve(project=self.pip, private=True)
+            url = resolve(project=self.pip)
             self.assertEqual(url, 'http://pip.readthedocs.org/en/latest/')
 
     @override_settings(PRODUCTION_DOMAIN='readthedocs.org')
     def test_resolver_private_project_override(self):
         self.pip.privacy_level = PRIVATE
+        self.pip.save()
         with override_settings(USE_SUBDOMAIN=False):
             url = resolve(project=self.pip)
             self.assertEqual(url, 'http://readthedocs.org/docs/pip/en/latest/')
-            url = resolve(project=self.pip, private=False)
+            url = resolve(project=self.pip)
             self.assertEqual(url, 'http://readthedocs.org/docs/pip/en/latest/')
         with override_settings(USE_SUBDOMAIN=True):
             url = resolve(project=self.pip)
             self.assertEqual(url, 'http://pip.readthedocs.org/en/latest/')
-            url = resolve(project=self.pip, private=False)
+            url = resolve(project=self.pip)
             self.assertEqual(url, 'http://pip.readthedocs.org/en/latest/')
 
     @override_settings(PRODUCTION_DOMAIN='readthedocs.org')
@@ -613,12 +609,12 @@ def test_resolver_private_version_override(self):
         with override_settings(USE_SUBDOMAIN=False):
             url = resolve(project=self.pip)
             self.assertEqual(url, 'http://readthedocs.org/docs/pip/en/latest/')
-            url = resolve(project=self.pip, private=False)
+            url = resolve(project=self.pip)
             self.assertEqual(url, 'http://readthedocs.org/docs/pip/en/latest/')
         with override_settings(USE_SUBDOMAIN=True):
             url = resolve(project=self.pip)
             self.assertEqual(url, 'http://pip.readthedocs.org/en/latest/')
-            url = resolve(project=self.pip, private=False)
+            url = resolve(project=self.pip)
             self.assertEqual(url, 'http://pip.readthedocs.org/en/latest/')
 
     @override_settings(
@@ -627,16 +623,16 @@ def test_resolver_private_version_override(self):
     )
     def test_resolver_public_domain_overrides(self):
         with override_settings(USE_SUBDOMAIN=False):
-            url = resolve(project=self.pip, private=True)
+            url = resolve(project=self.pip)
             self.assertEqual(url, 'http://readthedocs.org/docs/pip/en/latest/')
-            url = resolve(project=self.pip, private=False)
+            url = resolve(project=self.pip)
             self.assertEqual(url, 'http://readthedocs.org/docs/pip/en/latest/')
         with override_settings(USE_SUBDOMAIN=True):
-            url = resolve(project=self.pip, private=True)
+            url = resolve(project=self.pip)
             self.assertEqual(
                 url, 'http://pip.public.readthedocs.org/en/latest/',
             )
-            url = resolve(project=self.pip, private=False)
+            url = resolve(project=self.pip)
             self.assertEqual(
                 url, 'http://pip.public.readthedocs.org/en/latest/',
             )
@@ -649,14 +645,14 @@ def test_resolver_public_domain_overrides(self):
             canonical=True,
         )
         with override_settings(USE_SUBDOMAIN=True):
-            url = resolve(project=self.pip, private=True)
+            url = resolve(project=self.pip)
             self.assertEqual(url, 'http://docs.foobar.com/en/latest/')
-            url = resolve(project=self.pip, private=False)
+            url = resolve(project=self.pip)
             self.assertEqual(url, 'http://docs.foobar.com/en/latest/')
         with override_settings(USE_SUBDOMAIN=False):
-            url = resolve(project=self.pip, private=True)
+            url = resolve(project=self.pip)
             self.assertEqual(url, 'http://docs.foobar.com/en/latest/')
-            url = resolve(project=self.pip, private=False)
+            url = resolve(project=self.pip)
             self.assertEqual(url, 'http://docs.foobar.com/en/latest/')
 
     @override_settings(
@@ -666,14 +662,14 @@ def test_resolver_public_domain_overrides(self):
     )
     def test_resolver_domain_https(self):
         with override_settings(PUBLIC_DOMAIN_USES_HTTPS=True):
-            url = resolve(project=self.pip, private=True)
+            url = resolve(project=self.pip)
             self.assertEqual(url, 'https://pip.readthedocs.io/en/latest/')
 
-            url = resolve(project=self.pip, private=False)
+            url = resolve(project=self.pip)
             self.assertEqual(url, 'https://pip.readthedocs.io/en/latest/')
 
         with override_settings(PUBLIC_DOMAIN_USES_HTTPS=False):
-            url = resolve(project=self.pip, private=True)
+            url = resolve(project=self.pip)
             self.assertEqual(url, 'http://pip.readthedocs.io/en/latest/')
 
 

Original file line number	Diff line number	Diff line change
`@@ -451,7 +451,6 @@ def changefreqs_generator():`
`451`	`451`	`href = project.get_docs_url(`
`452`	`452`	`version_slug=version.slug,`
`453`	`453`	`lang_slug=translation.language,`
`454`		`- private=False,`
`455`	`454`	`)`
`456`	`455`	`element['languages'].append({`
`457`	`456`	`'hreflang': hreflang_formatter(translation.language),`
Original file line number	Diff line number	Diff line change
`@@ -461,7 +461,6 @@ def changefreqs_generator():`
`461`	`461`	`href = project.get_docs_url(`
`462`	`462`	`version_slug=version.slug,`
`463`	`463`	`lang_slug=translation.language,`
`464`		`- private=False,`
`465`	`464`	`)`
`466`	`465`	`element['languages'].append({`
`467`	`466`	`'hreflang': hreflang_formatter(translation.language),`