Feature/55 order of items on sidebar incorrect

README.md

@@ -28,6 +28,12 @@ Other collections, such as [the ones in the AAOF Legacy Collection](https://www.
 - `bfd9000_web/`: Django application for managing and viewing the BFD9000 data.
   To run the application, follow the instructions in `bfd9000_web/README.md`.
 
+## Docker Notes
+
+When you run the app with Docker you will need a one-time volume ownership fix so the app's non-root user can write media files.
+
+See `docker.md` for the exact commands and troubleshooting steps.
+
 ## Django Bootstrap
 
 The archive app provides a convenience management command to initialize a local development database:

bfd9000_web/BFD9000/urls.py

@@ -17,15 +17,24 @@
 
 from django.contrib import admin
 from django.contrib.auth import views as auth_views
+from django.contrib.auth import logout
+from django.http import HttpRequest, HttpResponse
+from django.shortcuts import redirect
 from django.urls import path, include
 from django.conf import settings
 from django.conf.urls.static import static
 from drf_spectacular.views import SpectacularAPIView, SpectacularRedocView, SpectacularSwaggerView
 
+
+def logout_view(request: HttpRequest) -> HttpResponse:
+    """Log out on any request method and redirect to login."""
+    logout(request)
+    return redirect('login')
+
 urlpatterns = [
     path('admin/', admin.site.urls),
     path('login/', auth_views.LoginView.as_view(template_name='archive/login.html'), name='login'),
-    path('logout/', auth_views.LogoutView.as_view(next_page='login'), name='logout'),
+    path('logout/', logout_view, name='logout'),
     path('', include('archive.urls')),
     # OpenAPI Schema
     path('api/schema/', SpectacularAPIView.as_view(), name='schema'),

bfd9000_web/archive/management/commands/setup_curator_group.py

@@ -0,0 +1,40 @@
+from django.contrib.auth.models import Group, Permission
+from django.contrib.contenttypes.models import ContentType
+from django.core.management.base import BaseCommand
+
+from archive.models import Encounter, Record, Subject
+
+class Command(BaseCommand):
+    help = (
+        "Create or update the Curator group with required add/change permissions. "
+        "Safe to rerun: removes delete_* and auth.* permissions on each run, "
+        "but does not remove other manually granted permissions. "
+        "See docs/permissions.md for the full policy."
+    )
+
+    def handle(self, *args, **options):
+        group, _ = Group.objects.get_or_create(name="Curator")
+        subject_ct = ContentType.objects.get_for_model(Subject)
+        encounter_ct = ContentType.objects.get_for_model(Encounter)
+        record_ct = ContentType.objects.get_for_model(Record)
+
+        subject_add = Permission.objects.get(codename="add_subject", content_type=subject_ct)
+        subject_change = Permission.objects.get(codename="change_subject", content_type=subject_ct)
+        encounter_add = Permission.objects.get(codename="add_encounter", content_type=encounter_ct)
+        encounter_change = Permission.objects.get(codename="change_encounter", content_type=encounter_ct)
+
+        required_perms = [subject_add, subject_change, encounter_add, encounter_change]
+        for perm in required_perms:
+            group.permissions.add(perm)
+
+        forbidden_perms = Permission.objects.filter(
+            content_type__in=[subject_ct, encounter_ct, record_ct],
+            codename__startswith="delete_",
+        )
+        auth_management_perms = Permission.objects.filter(content_type__app_label="auth")
+        forbidden_perms = forbidden_perms | auth_management_perms
+
+        for perm in forbidden_perms:
+            group.permissions.remove(perm)
+
+        self.stdout.write(self.style.SUCCESS("Curator group permissions updated."))

bfd9000_web/archive/permissions.py

@@ -0,0 +1,52 @@
+# pyright: reportIncompatibleMethodOverride=false
+
+from typing import Any
+
+from django.db.models import Model
+from rest_framework.permissions import SAFE_METHODS, BasePermission
+from rest_framework.request import Request
+
+
+class CuratorOrSuperuserEditPermission(BasePermission):
+    """Require model add/change perms for writes and auth for reads."""
+
+    def has_permission(self, request: Request, view: Any):  # pyright: ignore[reportIncompatibleMethodOverride]
+        if not request.user or not request.user.is_authenticated:
+            return False
+        if request.user.is_superuser:
+            return True
+        if request.method in SAFE_METHODS:
+            return True
+        if request.method == "DELETE":
+            return False
+
+        qs = getattr(view, "queryset", None)
+        if qs is None and hasattr(view, "get_queryset"):
+            try:
+                qs = view.get_queryset()
+            except Exception:
+                qs = None
+        model: type[Model] | None = getattr(qs, "model", None)
+        if model is None:
+            return False
+
+        app_label = model._meta.app_label
+        model_name = model._meta.model_name
+        if model_name is None:
+            return False
+        if request.method == "POST":
+            return request.user.has_perm(f"{app_label}.add_{model_name}")
+        if request.method in {"PUT", "PATCH"}:
+            return request.user.has_perm(f"{app_label}.change_{model_name}")
+        return False
+
+
+class RecordPermission(BasePermission):
+    """Allow authenticated users to read/create/update records."""
+
+    def has_permission(self, request: Request, view: Any):  # pyright: ignore[reportIncompatibleMethodOverride]
+        if not request.user or not request.user.is_authenticated:
+            return False
+        if request.method == "DELETE":
+            return bool(request.user.is_superuser)
+        return True

bfd9000_web/archive/templates/archive/base.html

@@ -5,7 +5,7 @@
     <meta name="viewport" content="width=device-width, initial-scale=1.0">
     <title>{% block title %}BFD9000{% endblock %}</title>
     {% load static %}
-    <link rel="icon" href="{% static 'favicon.ico' %}" type="image/x-icon">
+    <link rel="icon" href="{% static 'favicon.svg' %}" type="image/svg+xml">
     <link href="{% static 'css/output.css' %}" rel="stylesheet" type="text/css" />
     <style>
         ::selection {

bfd9000_web/archive/templates/archive/base_index.html

@@ -6,21 +6,24 @@
 <div class="card shadow-sm bg-base-100 h-full grow">
     <div class="p-4 flex space-between align-center border-b-2 border-primary">
         <div class="flex grow items-end gap-2">
-            <h1 class="text-3xl font-bold leading-none">BFD9000</h1>
+            <a href="{% url 'archive:index' %}" class="text-3xl font-bold leading-none">BFD9000</a>
             <span class="font-medium text-base-content/70" style="align-self:flex-end;font-size:0.65rem;line-height:1;">v{{ app_version|default:"nover" }}</span>
         </div>
         <span class="my-auto text-sm text-nowrap mr-4">Logged in as: <strong>{{ user.username }}</strong></span>
-        <a href="{% url 'logout' %}" class="btn btn-sm grow-0">Logout</a>
+        <form method="post" action="{% url 'logout' %}" class="m-0">
+            {% csrf_token %}
+            <button type="submit" class="btn btn-sm grow-0">Logout</button>
+        </form>
     </div>
 
     <div class="flex-[1_1_0] flex">
         <ul class="menu bg-base-300 w-36">
             <li>
-                <a href="{% url 'archive:index' %}" data-nav="home">
+                <a href="{% url 'archive:subjects' %}" data-nav="subjects">
                     <svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor" class="size-6">
-                        <path stroke-linecap="round" stroke-linejoin="round" d="m2.25 12 8.954-8.955c.44-.439 1.152-.439 1.591 0L21.75 12M4.5 9.75v10.125c0 .621.504 1.125 1.125 1.125H9.75v-4.875c0-.621.504-1.125 1.125-1.125h2.25c.621 0 1.125.504 1.125 1.125V21h4.125c.621 0 1.125-.504 1.125-1.125V9.75M8.25 21h8.25" />
+                      <path stroke-linecap="round" stroke-linejoin="round" d="M15.75 6a3.75 3.75 0 1 1-7.5 0 3.75 3.75 0 0 1 7.5 0ZM4.501 20.118a7.5 7.5 0 0 1 14.998 0A17.933 17.933 0 0 1 12 21.75c-2.676 0-5.216-.584-7.499-1.632Z" />
                     </svg>
-                    Home
+                    Subjects
                 </a>
             </li>
             <li>
@@ -39,22 +42,6 @@ <h1 class="text-3xl font-bold leading-none">BFD9000</h1>
                     Records
                 </a>
             </li>
-            <li>
-                <a href="{% url 'archive:subjects' %}" data-nav="subjects">
-                    <svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor" class="size-6">
-                      <path stroke-linecap="round" stroke-linejoin="round" d="M15.75 6a3.75 3.75 0 1 1-7.5 0 3.75 3.75 0 0 1 7.5 0ZM4.501 20.118a7.5 7.5 0 0 1 14.998 0A17.933 17.933 0 0 1 12 21.75c-2.676 0-5.216-.584-7.499-1.632Z" />
-                    </svg>
-                    Subjects
-                </a>
-            </li>
-            <li>
-                <a href="{% url 'archive:scan' %}" data-nav="scan">
-                    <svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor" class="size-6">
-                      <path stroke-linecap="round" stroke-linejoin="round" d="M6.429 9.75 2.25 12l4.179 2.25m0-4.5 5.571 3 5.571-3m-11.142 0L2.25 7.5 12 2.25l9.75 5.25-4.179 2.25m0 0L21.75 12l-4.179 2.25m0 0 4.179 2.25L12 21.75 2.25 16.5l4.179-2.25m11.142 0-5.571 3-5.571-3" />
-                    </svg>
-                    Scan
-                </a>
-            </li>
             <script>
                 // Highlight active navigation item based on current path, after stripping script_name prefix
                 document.addEventListener('DOMContentLoaded', () => {
@@ -79,6 +66,8 @@ <h1 class="text-3xl font-bold leading-none">BFD9000</h1>
                     }
                 });
             </script>
+<!-- Scan nav item intentionally omitted: scanning is accessible via
+     the "Scan New Record" button on the Records view (/records/create/). -->
         </ul>
         <div class="flex-[1_1_0] flex flex-col overflow-y-auto">
             {% block content %}{% endblock %}

bfd9000_web/archive/templates/archive/encounters.html

@@ -46,7 +46,9 @@
         </table>
     </div>
     <div class="bg-base-300 p-2 flex items-center gap-3">
+        {% if perms.archive.add_encounter %}
         <a href="{% url 'archive:encounter_create' %}" class="btn btn-primary rounded-full">New Encounter</a>
+        {% endif %}
         <div class="grow"></div>
         <span class="text-sm text-base-content/60">
             <span id="encounter-count-badge"></span>

bfd9000_web/archive/templates/archive/records.html

@@ -47,6 +47,7 @@
         </table>
     </div>
     <div class="bg-base-300 p-2 flex items-center gap-3">
+        <a href="{% url 'archive:record_create' %}" class="btn btn-primary rounded-full">Add Record</a>
         <div class="grow"></div>
         <span class="text-sm text-base-content/60">
             <span id="record-count-badge"></span>

bfd9000_web/archive/templates/archive/subjects.html

@@ -46,11 +46,12 @@
         </table>
     </div>
     <div class="bg-base-300 p-2 flex items-center gap-3">
+        {% if perms.archive.add_subject %}
         <a
             href="{% url 'archive:subject_create' %}"
             class="btn btn-primary rounded-full"
-            >New Subject</a
-        >
+            >New Subject</a>
+        {% endif %}
         <div class="grow"></div>
         <span class="text-sm text-base-content/60">
             <span id="subject-count-badge"></span>

bfd9000_web/archive/tests/test_auth_views.py

@@ -0,0 +1,27 @@
+"""Tests for project auth endpoints."""
+
+from django.contrib.auth.models import User
+from django.test import TestCase
+from django.urls import reverse
+
+
+class AuthViewTests(TestCase):
+    """Validate login/logout view behavior."""
+
+    def test_logout_get_redirects_without_error(self):
+        user = User.objects.create_user(username="testuser", password="testpassword")
+        self.client.force_login(user)
+
+        response = self.client.get(reverse("logout"))
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.url, reverse("login"))
+        self.assertNotIn('_auth_user_id', self.client.session)
+
+    def test_logout_post_logs_user_out(self):
+        user = User.objects.create_user(username="testuser", password="testpassword")
+        self.client.force_login(user)
+
+        response = self.client.post(reverse("logout"))
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.url, reverse("login"))
+        self.assertNotIn("_auth_user_id", self.client.session)

bfd9000_web/archive/tests/test_encounters.py

@@ -210,7 +210,7 @@ def test_update_encounter(self):
         self.assertEqual(response.data['actual_period_start'], '2020-02-01')
 
     def test_delete_encounter(self):
-        """Should delete encounter"""
+        """Non-superuser should not delete encounter."""
         # Create encounter via API
         url = reverse('archive:subject-encounters-list', kwargs={'subject_pk': self.subject.id})
         create_response = self.client.post(url, {
@@ -221,11 +221,44 @@ def test_delete_encounter(self):
 
         url = reverse('archive:encounter-detail', kwargs={'pk': encounter_id})
         response = self.client.delete(url)
-        self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+        # Verify encounter still exists
+        self.assertTrue(Encounter.objects.filter(pk=encounter_id).exists())
+
+    def test_superuser_can_delete_encounter(self):
+        """Superuser should be able to delete encounter."""
+        url = reverse('archive:subject-encounters-list', kwargs={'subject_pk': self.subject.id})
+        create_response = self.client.post(url, {
+            "actual_period_start": "2020-01-01",
+            "procedure_code": self.procedure.id
+        }, format='json')
+        encounter_id = create_response.data['id']
 
-        # Verify deletion
+        superuser = User.objects.create_superuser(
+            username="admin",
+            password="adminpass",
+            email="admin@example.com",
+        )
+        self.client.force_authenticate(user=superuser)
+        detail_url = reverse('archive:encounter-detail', kwargs={'pk': encounter_id})
+        response = self.client.delete(detail_url)
+        self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
         self.assertFalse(Encounter.objects.filter(pk=encounter_id).exists())
 
+    def test_regular_user_without_encounter_perms_cannot_create_encounter(self):
+        """Regular authenticated user should not create encounter."""
+        regular_user = User.objects.create_user(username='regular', password='testpassword')
+        self.client.force_authenticate(user=regular_user)
+
+        url = reverse('archive:subject-encounters-list', kwargs={'subject_pk': self.subject.id})
+        data = {
+            "actual_period_start": "2020-01-01",
+            "procedure_code": self.procedure.id
+        }
+        response = self.client.post(url, data, format='json')
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
     def test_encounter_age_calculation(self):
         """Should automatically calculate age_at_encounter from subject birth_date"""
         url = reverse('archive:subject-encounters-list', kwargs={'subject_pk': self.subject.id})

bfd9000_web/archive/tests/test_permissions_ui.py

@@ -0,0 +1,45 @@
+"""UI permission tests for subject and encounter create buttons."""
+
+from django.contrib.auth.models import Permission, User
+from django.test import override_settings
+from django.urls import reverse
+
+from .base import CleanupTestCase
+
+
+@override_settings(
+    STORAGES={
+        "default": {"BACKEND": "django.core.files.storage.FileSystemStorage"},
+        "staticfiles": {"BACKEND": "django.contrib.staticfiles.storage.StaticFilesStorage"},
+    }
+)
+class PermissionUiTests(CleanupTestCase):
+    """Verify template button visibility follows Django permissions."""
+
+    def test_regular_user_does_not_see_create_buttons(self):
+        user = User.objects.create_user(username="regular", password="testpassword")
+        self.client.force_login(user)
+
+        subjects_response = self.client.get(reverse("archive:subjects"))
+        encounters_response = self.client.get(reverse("archive:encounters"))
+
+        self.assertEqual(subjects_response.status_code, 200)
+        self.assertEqual(encounters_response.status_code, 200)
+        self.assertNotContains(subjects_response, "New Subject")
+        self.assertNotContains(encounters_response, "New Encounter")
+
+    def test_curator_like_user_sees_create_buttons(self):
+        user = User.objects.create_user(username="curator", password="testpassword")
+        add_subject = Permission.objects.get(codename="add_subject")
+        add_encounter = Permission.objects.get(codename="add_encounter")
+        user.user_permissions.add(add_subject, add_encounter)
+
+        self.client.force_login(user)
+
+        subjects_response = self.client.get(reverse("archive:subjects"))
+        encounters_response = self.client.get(reverse("archive:encounters"))
+
+        self.assertEqual(subjects_response.status_code, 200)
+        self.assertEqual(encounters_response.status_code, 200)
+        self.assertContains(subjects_response, "New Subject")
+        self.assertContains(encounters_response, "New Encounter")