Remove generic URL feature 😿️

Author: benjaoming

readthedocs/organizations/tests/test_views.py

@@ -406,16 +406,6 @@ def setUp(self):
         )
         self.client.force_login(self.owner)
 
-    def test_members_list_redirects_to_organization_choose(self):
-        self.assertEqual(Organization.objects.count(), 2)
-        resp = self.client.get(reverse("organization_members", kwargs={"slug": "-"}))
-        self.assertRedirects(
-            resp,
-            reverse(
-                "organization_choose", kwargs={"next_name": "organization_members"}
-            ),
-        )
-
     def test_choose_organization_edit(self):
         """
         Verify that the choose page works.
@@ -435,17 +425,6 @@ def test_choose_organization_edit(self):
             ),
         )
 
-    def test_no_redirect_organization_team_detail(self):
-        """
-        Verify that organization views w extra URL kwargs just return 404 if unspecified slug.
-        """
-        resp = self.client.get(
-            reverse(
-                "organization_team_detail", kwargs={"slug": "-", "team": self.team.pk}
-            )
-        )
-        self.assertEqual(resp.status_code, 404)
-
 
 @override_settings(
     RTD_ALLOW_ORGANIZATIONS=True,
@@ -467,7 +446,9 @@ def setUp(self):
 
     def test_unspecified_slug_redirects_to_organization_edit(self):
         self.assertEqual(Organization.objects.count(), 1)
-        resp = self.client.get(reverse("organization_edit", kwargs={"slug": "-"}))
+        resp = self.client.get(
+            reverse("organization_choose", kwargs={"next_name": "organization_edit"})
+        )
         self.assertRedirects(
             resp, reverse("organization_edit", kwargs={"slug": self.organization.slug})
         )
@@ -486,16 +467,6 @@ def setUp(self):
         self.project = get(Project, slug="project")
         self.client.force_login(self.owner)
 
-    def test_unspecified_slug_redirects_to_organization_edit(self):
-        self.assertEqual(Organization.objects.count(), 0)
-        resp = self.client.get(reverse("organization_members", kwargs={"slug": "-"}))
-        self.assertRedirects(
-            resp,
-            reverse(
-                "organization_choose", kwargs={"next_name": "organization_members"}
-            ),
-        )
-
     def test_choose_organization_edit(self):
         self.assertEqual(Organization.objects.count(), 0)
         resp = self.client.get(

readthedocs/organizations/views/base.py

@@ -1,7 +1,9 @@
 """Base classes for organization views."""
 from functools import lru_cache
 
+from django.conf import settings
 from django.contrib.messages.views import SuccessMessageMixin
+from django.http import Http404
 from django.shortcuts import get_object_or_404
 from django.urls import reverse_lazy
 
@@ -18,14 +20,27 @@
     Team,
     TeamMember,
 )
-from readthedocs.organizations.views.decorators import (
-    redirect_if_organization_unspecified,
-    redirect_if_organizations_disabled,
-)
+
+
+class CheckOrganizationsEnabled:
+
+    """
+    Return 404 if organizations aren't enabled.
+
+    All organization views should inherit this class.
+    This is mainly for our tests to work,
+    adding the organization urls conditionally on readthedocs/urls.py
+    doesn't work as the file is evaluated only once, not per-test case.
+    """
+
+    def dispatch(self, *args, **kwargs):
+        if not settings.RTD_ALLOW_ORGANIZATIONS:
+            raise Http404
+        return super().dispatch(*args, **kwargs)
 
 
 # Mixins
-class OrganizationMixin:
+class OrganizationMixin(CheckOrganizationsEnabled):
 
     """
     Mixin class that provides organization sublevel objects.
@@ -43,11 +58,6 @@ class OrganizationMixin:
     org_url_field = 'slug'
     admin_only = True
 
-    @redirect_if_organizations_disabled
-    @redirect_if_organization_unspecified
-    def dispatch(self, *args, **kwargs):
-        return super().dispatch(*args, **kwargs)
-
     def get_queryset(self):
         """Return queryset that returns organizations for user."""
         return self.get_organization_queryset()
@@ -118,7 +128,7 @@ def get_form(self, data=None, files=None, **kwargs):
 
 
 # Base views
-class OrganizationView:
+class OrganizationView(CheckOrganizationsEnabled):
 
     """Mixin for an organization view that doesn't have nested components."""
 
@@ -130,11 +140,6 @@ class OrganizationView:
     lookup_field = 'slug'
     lookup_url_field = 'slug'
 
-    @redirect_if_organizations_disabled
-    @redirect_if_organization_unspecified
-    def dispatch(self, *args, **kwargs):
-        return super().dispatch(*args, **kwargs)
-
     def get_queryset(self):
         if self.admin_only:
             return Organization.objects.for_admin_user(user=self.request.user)

readthedocs/organizations/views/decorators.py

@@ -4,8 +4,10 @@
 from django.conf import settings
 from django.contrib import messages
 from django.http import HttpResponseBadRequest
-from django.urls import reverse_lazy
+from django.shortcuts import redirect
+from django.urls import reverse, reverse_lazy
 from django.utils import timezone
+from django.utils.http import urlencode
 from django.utils.translation import gettext_lazy as _
 from vanilla import CreateView, DeleteView, FormView, ListView, UpdateView
 
@@ -72,10 +74,27 @@ def get_queryset(self):
 class ChooseOrganization(ListOrganization):
     template_name = "organizations/organization_choose.html"
 
+    def dispatch(self, request, *args, **kwargs):
+
+        self.next_name = self.kwargs["next_name"]
+        self.next_querystring = self.request.GET.get("next_querystring")
+
+        # Check if user has exactly 1 organization and automatically redirect in this case
+        organizations = self.get_queryset()
+        if organizations.count() == 1:
+            redirect_url = reverse(
+                self.next_name, kwargs={"slug": organizations[0].slug}
+            )
+            if self.next_querystring:
+                redirect_url += "?" + urlencode(self.next_querystring)
+            return redirect(redirect_url)
+
+        return super().dispatch(request, *args, **kwargs)
+
     def get_context_data(self, **kwargs):
         c = super().get_context_data(**kwargs)
-        c["next_name"] = self.kwargs["next_name"]
-        c["next_querystring"] = self.request.GET.get("next_querystring")
+        c["next_name"] = self.next_name
+        c["next_querystring"] = self.next_querystring
         return c
 
 

readthedocs/organizations/views/private.py

@@ -8,27 +8,21 @@
 
 from readthedocs.organizations.models import Team
 from readthedocs.organizations.views.base import (
+    CheckOrganizationsEnabled,
     OrganizationMixin,
     OrganizationTeamMemberView,
     OrganizationTeamView,
     OrganizationView,
 )
-from readthedocs.organizations.views.decorators import (
-    redirect_if_organizations_disabled,
-)
 from readthedocs.projects.models import Project
 
 log = structlog.get_logger(__name__)
 
 
-class OrganizationTemplateView(TemplateView):
+class OrganizationTemplateView(CheckOrganizationsEnabled, TemplateView):
 
     """Wrapper around `TemplateView` to check if organizations are enabled."""
 
-    @redirect_if_organizations_disabled
-    def dispatch(self, *args, **kwargs):
-        return super().dispatch(*args, **kwargs)
-
 
 # Organization
 
@@ -98,14 +92,10 @@ def get_context_data(self, **kwargs):
         return context
 
 
-class RedirectRedeemTeamInvitation(GenericView):
+class RedirectRedeemTeamInvitation(CheckOrganizationsEnabled, GenericView):
 
     """Redirect invitation links to the new view."""
 
-    @redirect_if_organizations_disabled
-    def dispatch(self, *args, **kwargs):
-        return super().dispatch(*args, **kwargs)
-
     # pylint: disable=unused-argument
     def get(self, request, *args, **kwargs):
         return HttpResponseRedirect(