Organizations: Organization chooser page (#10325)

* Initial work for an all-purporse URL scheme for organization-specific pages

* Split out handling of querystring

* Adds organization chooser step (>1 organization), use `-` as "unspecified" slug

* Do not handle additional kwargs, it's meaningless. Raise 404 and log a warning.

* Handle 'next_name' kwarg in tests, handle NoReverseMatch by raising 404

* Setup initial test cases

* Test case to ensure that the chooser list is empty when there's no organization membership

* Add further test assertions

* Update readthedocs/organizations/templates/organizations/organization_choose.html

Co-authored-by: Anthony <[email protected]>

* Revert leak from local dev env

* Organization queryset is already handled in list view

* Click=>Select + mark string for translation

* Remove <br>, use blocktrans

* Shorten heading

* Remove generic URL feature. Note: The removal is kept in a single commit. If we want to re-introduce the feature, this commit can be reverted.

* Overwrite get instead of dispatch

---------

Co-authored-by: Anthony <[email protected]>

Author: benjaoming

common

@@ -0,0 +1,51 @@
+{% extends "base.html" %}
+
+{% load i18n %}
+{% load gravatar %}
+{% load organizations %}
+
+{% block title %}{% trans "Choose an organization" %}{% endblock %}
+
+{% block organization-bar-organization %}active{% endblock %}
+
+{% block content %}
+  <div class="col-major organization-major">
+    <div class="module organizations organizations-list">
+      <div class="module-wrapper">
+
+        <h2>{% trans "Choose an organization" %}</h2>
+
+        <p>
+          {% blocktrans trimmed %}
+            You are a member of several organizations.
+            Select the organization that you want to use:
+          {% endblocktrans %}
+        </p>
+
+          {% if organization_list %}
+            <!-- BEGIN organization list -->
+            <div class="module-list">
+              <div class="module-list-wrapper">
+
+                <ul>
+                  {% for org in organization_list %}
+                    <li>
+                      <a href="{% url next_name slug=org.slug %}{% if next_querystring %}?{{ next_querystring }}{% endif %}">
+                        {% gravatar org.email 48 %}
+                        {{ org.name }}
+                      </a>
+                    </li>
+                  {% endfor %}
+                </ul>
+
+              </div>
+            </div>
+            <!-- END organization list -->
+          {% else %}
+            <p>{% trans "You aren't currently a member of any organizations." %}</p>
+          {% endif %}
+      </div>
+    </div>
+  </div>
+
+{% endblock %}

readthedocs/organizations/templates/organizations/organization_choose.html

@@ -19,11 +19,12 @@ def setUp(self):
         self.invite = get(TeamInvite, organization=self.organization, team=self.team)
         self.default_kwargs.update(
             {
-                'slug': self.organization.slug,
-                'team': self.team.slug,
-                'hash': self.invite.hash,
-                'owner': self.org_owner.pk,
-                'member': self.team_member.pk,
+                "slug": self.organization.slug,
+                "team": self.team.slug,
+                "hash": self.invite.hash,
+                "owner": self.org_owner.pk,
+                "member": self.team_member.pk,
+                "next_name": "organization_detail",
             }
         )
 
@@ -57,8 +58,8 @@ class AuthUserOrganizationsTest(OrganizationMixin, TestCase):
 
     response_data = {
         # Places where we 302 on success.
-        '/organizations/invite/{hash}/redeem/': {'status_code': 302},
-
+        "/organizations/choose/{next_name}/": {"status_code": 302},
+        "/organizations/invite/{hash}/redeem/": {"status_code": 302},
         # 405's where we should be POST'ing
         '/organizations/{slug}/owners/{owner}/delete/': {'status_code': 405},
         '/organizations/{slug}/teams/{team}/members/{member}/revoke/': {'status_code': 405},

readthedocs/organizations/tests/test_privacy_urls.py

@@ -376,3 +376,100 @@ def test_organization_signup(self):
             resp,
             reverse('organization_detail', kwargs={'slug': org.slug}),
         )
+
+
+@override_settings(
+    RTD_ALLOW_ORGANIZATIONS=True,
+    RTD_DEFAULT_FEATURES={
+        TYPE_AUDIT_LOGS: 90,
+    },
+)
+class OrganizationUnspecifiedChooser(TestCase):
+    def setUp(self):
+        self.owner = get(User, username="owner")
+        self.member = get(User, username="member")
+        self.project = get(Project, slug="project")
+        self.organization = get(
+            Organization,
+            owners=[self.owner],
+            projects=[self.project],
+        )
+        self.team = get(
+            Team,
+            organization=self.organization,
+            members=[self.member],
+        )
+        self.another_organization = get(
+            Organization,
+            owners=[self.owner],
+            projects=[self.project],
+        )
+        self.client.force_login(self.owner)
+
+    def test_choose_organization_edit(self):
+        """
+        Verify that the choose page works.
+        """
+        self.assertEqual(Organization.objects.count(), 2)
+        resp = self.client.get(
+            reverse("organization_choose", kwargs={"next_name": "organization_edit"})
+        )
+        self.assertEqual(resp.status_code, 200)
+        self.assertContains(
+            resp, reverse("organization_edit", kwargs={"slug": self.organization.slug})
+        )
+        self.assertContains(
+            resp,
+            reverse(
+                "organization_edit", kwargs={"slug": self.another_organization.slug}
+            ),
+        )
+
+
+@override_settings(
+    RTD_ALLOW_ORGANIZATIONS=True,
+    RTD_DEFAULT_FEATURES={
+        TYPE_AUDIT_LOGS: 90,
+    },
+)
+class OrganizationUnspecifiedSingleOrganizationRedirect(TestCase):
+    def setUp(self):
+        self.owner = get(User, username="owner")
+        self.member = get(User, username="member")
+        self.project = get(Project, slug="project")
+        self.organization = get(
+            Organization,
+            owners=[self.owner],
+            projects=[self.project],
+        )
+        self.client.force_login(self.owner)
+
+    def test_unspecified_slug_redirects_to_organization_edit(self):
+        self.assertEqual(Organization.objects.count(), 1)
+        resp = self.client.get(
+            reverse("organization_choose", kwargs={"next_name": "organization_edit"})
+        )
+        self.assertRedirects(
+            resp, reverse("organization_edit", kwargs={"slug": self.organization.slug})
+        )
+
+
+@override_settings(
+    RTD_ALLOW_ORGANIZATIONS=True,
+    RTD_DEFAULT_FEATURES={
+        TYPE_AUDIT_LOGS: 90,
+    },
+)
+class OrganizationUnspecifiedNoOrganizationRedirect(TestCase):
+    def setUp(self):
+        self.owner = get(User, username="owner")
+        self.member = get(User, username="member")
+        self.project = get(Project, slug="project")
+        self.client.force_login(self.owner)
+
+    def test_choose_organization_edit(self):
+        self.assertEqual(Organization.objects.count(), 0)
+        resp = self.client.get(
+            reverse("organization_choose", kwargs={"next_name": "organization_edit"})
+        )
+        self.assertContains(resp, "You aren't currently a member of any organizations")

readthedocs/organizations/tests/test_views.py

@@ -9,6 +9,11 @@
         views.ListOrganization.as_view(),
         name='organization_list',
     ),
+    re_path(
+        r"^choose/(?P<next_name>[\w.-]+)/$",
+        views.ChooseOrganization.as_view(),
+        name="organization_choose",
+    ),
     re_path(
         r'^create/$',
         views.CreateOrganizationSignup.as_view(),

readthedocs/organizations/urls/private.py

@@ -133,11 +133,13 @@ class OrganizationView(CheckOrganizationsEnabled):
     """Mixin for an organization view that doesn't have nested components."""
 
     model = Organization
-    lookup_field = 'slug'
-    lookup_url_field = 'slug'
     form_class = OrganizationForm
     admin_only = True
 
+    # Only relevant when mixed into
+    lookup_field = 'slug'
+    lookup_url_field = 'slug'
+
     def get_queryset(self):
         if self.admin_only:
             return Organization.objects.for_admin_user(user=self.request.user)

readthedocs/organizations/views/base.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
 
@@ -69,6 +71,33 @@ def get_queryset(self):
         return Organization.objects.for_user(user=self.request.user)
 
 
+class ChooseOrganization(ListOrganization):
+    template_name = "organizations/organization_choose.html"
+
+    def get(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().get(request, *args, **kwargs)
+
+    def get_context_data(self, **kwargs):
+        c = super().get_context_data(**kwargs)
+        c["next_name"] = self.next_name
+        c["next_querystring"] = self.next_querystring
+        return c
+
+
 class EditOrganization(
         PrivateViewMixin,
         UpdateChangeReasonPostView,