Merge pull request #29 from dimagi/sk/nav

various tweaks
dimagi · Jul 25, 2023 · 28a5ab9 · 28a5ab9
2 parents 368b242 + 712884e
commit 28a5ab9
Show file tree

Hide file tree

Showing 23 changed files with 239 additions and 33 deletions.
diff --git a/README.md b/README.md
@@ -16,6 +16,10 @@ need to edit some settings.
     # install requirements
     $ pip install -r requirements-dev.txt
 
+    # install git hooks
+    $ pre-commit install
+    $ pre-commit run -a
+
     # create env file and edit the settings as needed (or export settings directly)
     $ cp .env_template .env
 
@@ -29,6 +33,7 @@ need to edit some settings.
     $ inv build-js [-w]
 
     # run Django
+    $ ./manage.py migrate
     $ ./manage.py runserver
 
 ## Basic Commands
@@ -45,6 +50,10 @@ Some useful command are available via the `tasks.py` file:
 
       $ python manage.py createsuperuser
 
+- To promote a user to superuser, use this command:
+
+      $ python manage.py promote_user_to_superuser <email>
+
 For convenience, you can keep your normal user logged in on Chrome and your superuser logged in on Firefox (or similar), so that you can see how the site behaves for both kinds of users.
 
 ### Test coverage

diff --git a/commcare_connect/conftest.py b/commcare_connect/conftest.py
@@ -1,14 +1,29 @@
 import pytest
 
-from commcare_connect.users.models import User
-from commcare_connect.users.tests.factories import UserFactory
+from commcare_connect.users.models import Organization, User
+from commcare_connect.users.tests.factories import OrgWithUsersFactory, UserFactory
 
 
 @pytest.fixture(autouse=True)
 def media_storage(settings, tmpdir):
     settings.MEDIA_ROOT = tmpdir.strpath
 
 
+@pytest.fixture
+def organization(db) -> Organization:
+    return OrgWithUsersFactory()
+
+
 @pytest.fixture
 def user(db) -> User:
     return UserFactory()
+
+
+@pytest.fixture
+def org_user_member(organization) -> User:
+    return organization.memberships.filter(role="member").first().user
+
+
+@pytest.fixture
+def org_user_admin(organization) -> User:
+    return organization.memberships.filter(role="admin").first().user
diff --git a/commcare_connect/opportunity/views.py b/commcare_connect/opportunity/views.py
@@ -9,7 +9,7 @@
 
 class OrganizationUserMixin(LoginRequiredMixin, UserPassesTestMixin):
     def test_func(self):
-        return self.request.user.organizations.filter(organization__slug=self.kwargs.get("org_slug")).exists()
+        return self.request.org_membership is not None
 
 
 class OpportunityList(OrganizationUserMixin, ListView):
@@ -44,9 +44,6 @@ def get_form_kwargs(self):
         kwargs["org_slug"] = self.kwargs.get("org_slug")
         return kwargs
 
-    def test_func(self):
-        return self.request.user.organizations.filter(organization__slug=self.kwargs.get("org_slug")).exists()
-
 
 class OpportunityEdit(OrganizationUserMixin, UpdateView):
     model = Opportunity

diff --git a/commcare_connect/templates/base.html b/commcare_connect/templates/base.html
@@ -1,4 +1,5 @@
-{% load static i18n %}<!DOCTYPE html>
+{% load static i18n %}
+{% load active_link %}<!DOCTYPE html>
 {% get_current_language as LANGUAGE_CODE %}
 <html lang="{{ LANGUAGE_CODE }}">
   <head>
@@ -38,30 +39,33 @@
 
           <div class="collapse navbar-collapse" id="navbarSupportedContent">
             <ul class="navbar-nav mr-auto">
-              <li class="nav-item active">
-                <a class="nav-link" href="{% url 'home' %}">Home <span class="visually-hidden">(current)</span></a>
+              <li class="nav-item">
+                <a class="nav-link {% active_link "home" %}" href="{% url 'home' %}">Home <span class="visually-hidden">(current)</span></a>
               </li>
               <li class="nav-item">
-                <a class="nav-link" href="{% url 'about' %}">About</a>
+                <a class="nav-link {% active_link "about" %}" href="{% url 'about' %}">About</a>
               </li>
               {% if request.user.is_authenticated %}
                 <li class="nav-item">
-                  <a class="nav-link" href="{% url 'users:detail' request.user.pk %}">{% translate "My Profile" %}</a>
+                  <a class="nav-link {% active_link "list || create || edit" namespace='opportunity' %}" href="{% url 'opportunity:list' request.org.slug %}">{% translate "Opportunities" %}</a>
+                </li>
+                <li class="nav-item">
+                  <a class="nav-link {% active_link "users:detail" %}" href="{% url 'users:detail' request.user.pk %}">{% translate "My Profile" %}</a>
                 </li>
                 <li class="nav-item">
                   {# URL provided by django-allauth/account/urls.py #}
-                  <a class="nav-link" href="{% url 'account_logout' %}">{% translate "Sign Out" %}</a>
+                  <a class="nav-link {% active_link "account_logout" %}" href="{% url 'account_logout' %}">{% translate "Sign Out" %}</a>
                 </li>
               {% else %}
                 {% if ACCOUNT_ALLOW_REGISTRATION %}
               <li class="nav-item">
                 {# URL provided by django-allauth/account/urls.py #}
-                <a id="sign-up-link" class="nav-link" href="{% url 'account_signup' %}">{% translate "Sign Up" %}</a>
+                <a id="sign-up-link" class="nav-link {% active_link "account_signup" %}" href="{% url 'account_signup' %}">{% translate "Sign Up" %}</a>
               </li>
               {% endif %}
                 <li class="nav-item">
                   {# URL provided by django-allauth/account/urls.py #}
-                  <a id="log-in-link" class="nav-link" href="{% url 'account_login' %}">{% translate "Sign In" %}</a>
+                  <a id="log-in-link" class="nav-link {% active_link "account_login" %}" href="{% url 'account_login' %}">{% translate "Sign In" %}</a>
                 </li>
               {% endif %}
             </ul>

diff --git a/commcare_connect/templates/opportunity/opportunity_list.html b/commcare_connect/templates/opportunity/opportunity_list.html
@@ -30,12 +30,13 @@
         </tr>
         {% empty %}
         <tr>
-          <td>No articles yet.</td>
+          <td colspan="3">No opportunities yet.</td>
         </tr>
       {% endfor %}
       </tbody>
     </table>
 
+    {% if page_obj.num_pages > 1 %}
     <nav aria-label="Page navigation">
       <ul class="pagination justify-content-center">
         {% if page_obj.has_previous %}
@@ -80,5 +81,6 @@
         {% endif %}
       </ul>
     </nav>
+    {% endif %}
   </div>
 {% endblock content %}
diff --git a/commcare_connect/users/admin.py b/commcare_connect/users/admin.py
@@ -44,14 +44,15 @@ class UserAdmin(auth_admin.UserAdmin):
     )
 
 
+class UserOrganizationMembershipInline(admin.TabularInline):
+    list_display = ["organization", "user", "role"]
+    model = UserOrganizationMembership
+
+
 @admin.register(Organization)
 class OrganizationAdmin(admin.ModelAdmin):
     form = OrganizationCreationForm
     list_display = ["name", "created_by"]
     search_fields = ["name"]
     ordering = ["name"]
-
-
-@admin.register(UserOrganizationMembership)
-class UserOrganizationMembershipAdmin(admin.ModelAdmin):
-    list_display = ["organization", "user", "role"]
+    inlines = [UserOrganizationMembershipInline]
diff --git a/commcare_connect/users/helpers.py b/commcare_connect/users/helpers.py
@@ -0,0 +1,16 @@
+from commcare_connect.users.models import Organization
+
+
+def get_organization_for_request(request, view_kwargs):
+    if not request.user.is_authenticated:
+        return
+
+    org_slug = view_kwargs.get("org_slug", None)
+    if org_slug:
+        try:
+            return Organization.objects.get(slug=org_slug, memberships__user=request.user)
+        except Organization.DoesNotExist:
+            return None
+
+    membership = request.user.memberships.select_related("organization").first()
+    return membership.organization if membership else None
diff --git a/commcare_connect/users/management/__init__.py b/commcare_connect/users/management/__init__.py
diff --git a/commcare_connect/users/management/commands/__init__.py b/commcare_connect/users/management/commands/__init__.py
diff --git a/commcare_connect/users/management/commands/promote_user_to_superuser.py b/commcare_connect/users/management/commands/promote_user_to_superuser.py
@@ -0,0 +1,20 @@
+from django.core.management.base import BaseCommand, CommandError
+
+from commcare_connect.users.models import User
+
+
+class Command(BaseCommand):
+    help = "Promotes the given user to a superuser and provides admin access."
+
+    def add_arguments(self, parser):
+        parser.add_argument("email", type=str)
+
+    def handle(self, email, **options):
+        try:
+            user = User.objects.get(email=email)
+        except User.DoesNotExist:
+            raise CommandError(f"No user with email {email} found!")
+        user.is_superuser = True
+        user.is_staff = True
+        user.save()
+        print(f"{email} successfully promoted to superuser and can now access the admin site")
diff --git a/commcare_connect/users/middleware.py b/commcare_connect/users/middleware.py
@@ -0,0 +1,31 @@
+from django.utils.deprecation import MiddlewareMixin
+from django.utils.functional import SimpleLazyObject
+
+from .helpers import get_organization_for_request
+from .models import UserOrganizationMembership as Membership
+
+
+def _get_organization(request, view_kwargs):
+    if not hasattr(request, "_cached_org"):
+        team = get_organization_for_request(request, view_kwargs)
+        request._cached_org = team
+    return request._cached_org
+
+
+def _get_org_membership(request):
+    if not hasattr(request, "_cached_org_membership"):
+        org = request.org
+        membership = None
+        if org:
+            try:
+                membership = Membership.objects.get(organization=org, user=request.user) if org else None
+            except Membership.DoesNotExist:
+                pass
+        request._cached_org_membership = membership
+    return request._cached_org_membership
+
+
+class OrganizationMiddleware(MiddlewareMixin):
+    def process_view(self, request, view_func, view_args, view_kwargs):
+        request.org = SimpleLazyObject(lambda: _get_organization(request, view_kwargs))
+        request.org_membership = SimpleLazyObject(lambda: _get_org_membership(request))
diff --git a/...e_connect/users/migrations/0003_alter_userorganizationmembership_organization_and_more.py b/...e_connect/users/migrations/0003_alter_userorganizationmembership_organization_and_more.py
@@ -0,0 +1,28 @@
+# Generated by Django 4.2.1 on 2023-07-25 09:24
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("users", "0002_organization_userorganizationmembership"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="userorganizationmembership",
+            name="organization",
+            field=models.ForeignKey(
+                on_delete=django.db.models.deletion.CASCADE, related_name="memberships", to="users.organization"
+            ),
+        ),
+        migrations.AlterField(
+            model_name="userorganizationmembership",
+            name="user",
+            field=models.ForeignKey(
+                on_delete=django.db.models.deletion.DO_NOTHING, related_name="memberships", to=settings.AUTH_USER_MODEL
+            ),
+        ),
+    ]
diff --git a/commcare_connect/users/models.py b/commcare_connect/users/models.py
@@ -43,7 +43,7 @@ class Organization(BaseModel):
     def save(self, *args, **kwargs):
         if not self.id:
             self.slug = slugify_uniquely(self.name, self.__class__)
-        super().save(*args, *kwargs)
+        super().save(*args, **kwargs)
 
     def __str__(self):
         return self.slug
@@ -57,13 +57,11 @@ class Role(models.TextChoices):
     organization = models.ForeignKey(
         Organization,
         on_delete=models.CASCADE,
-        related_name="users",
-        related_query_name="user",
+        related_name="memberships",
     )
     user = models.ForeignKey(
         User,
         on_delete=models.DO_NOTHING,
-        related_name="organizations",
-        related_query_name="organization",
+        related_name="memberships",
     )
     role = models.CharField(max_length=20, choices=Role.choices, default=Role.MEMBER)
diff --git a/commcare_connect/users/tests/factories.py b/commcare_connect/users/tests/factories.py
@@ -2,9 +2,11 @@
 from typing import Any
 
 from django.contrib.auth import get_user_model
-from factory import Faker, post_generation
+from factory import Faker, RelatedFactory, SubFactory, post_generation
 from factory.django import DjangoModelFactory
 
+from commcare_connect.users.models import UserOrganizationMembership
+
 
 class UserFactory(DjangoModelFactory):
     email = Faker("email")
@@ -29,3 +31,24 @@ def password(self, create: bool, extracted: Sequence[Any], **kwargs):
     class Meta:
         model = get_user_model()
         django_get_or_create = ["email"]
+
+
+class OrganizationFactory(DjangoModelFactory):
+    name = Faker("company")
+
+    class Meta:
+        model = "users.Organization"
+
+
+class MembershipFactory(DjangoModelFactory):
+    class Meta:
+        model = UserOrganizationMembership
+
+    user = SubFactory(UserFactory)
+    organization = SubFactory(OrganizationFactory)
+    role = "admin"
+
+
+class OrgWithUsersFactory(OrganizationFactory):
+    admin = RelatedFactory(MembershipFactory, "organization", role="admin")
+    member = RelatedFactory(MembershipFactory, "organization", role="member")
diff --git a/commcare_connect/users/tests/test_urls.py b/commcare_connect/users/tests/test_urls.py
@@ -9,10 +9,10 @@ def test_detail(user: User):
 
 
 def test_update():
-    assert reverse("users:update") == "/users/~update/"
-    assert resolve("/users/~update/").view_name == "users:update"
+    assert reverse("users:update") == "/users/update/"
+    assert resolve("/users/update/").view_name == "users:update"
 
 
 def test_redirect():
-    assert reverse("users:redirect") == "/users/~redirect/"
-    assert resolve("/users/~redirect/").view_name == "users:redirect"
+    assert reverse("users:redirect") == "/users/redirect/"
+    assert resolve("/users/redirect/").view_name == "users:redirect"
diff --git a/commcare_connect/users/tests/test_views.py b/commcare_connect/users/tests/test_views.py
@@ -9,7 +9,7 @@
 from django.urls import reverse
 
 from commcare_connect.users.forms import UserAdminChangeForm
-from commcare_connect.users.models import User
+from commcare_connect.users.models import Organization, User
 from commcare_connect.users.tests.factories import UserFactory
 from commcare_connect.users.views import UserRedirectView, UserUpdateView, user_detail_view
 
@@ -71,10 +71,22 @@ def test_get_redirect_url(self, user: User, rf: RequestFactory):
         view = UserRedirectView()
         request = rf.get("/fake-url")
         request.user = user
+        request.org = None
 
         view.request = request
         assert view.get_redirect_url() == f"/users/{user.pk}/"
 
+    def test_get_redirect_url_for_org_user(
+        self, organization: Organization, org_user_member: User, rf: RequestFactory
+    ):
+        view = UserRedirectView()
+        request = rf.get("/fake-url")
+        request.user = org_user_member
+        request.org = organization
+
+        view.request = request
+        assert view.get_redirect_url() == f"/a/{organization.slug}/opportunity/"
+
 
 class TestUserDetailView:
     def test_authenticated(self, user: User, rf: RequestFactory):

diff --git a/commcare_connect/users/urls.py b/commcare_connect/users/urls.py
@@ -4,7 +4,7 @@
 
 app_name = "users"
 urlpatterns = [
-    path("~redirect/", view=user_redirect_view, name="redirect"),
-    path("~update/", view=user_update_view, name="update"),
+    path("redirect/", view=user_redirect_view, name="redirect"),
+    path("update/", view=user_update_view, name="update"),
     path("<int:pk>/", view=user_detail_view, name="detail"),
 ]