diff --git a/TODO.md b/TODO.md index 14586b9..071279a 100644 --- a/TODO.md +++ b/TODO.md @@ -1,83 +1,118 @@ # Remaining work — next sessions -## 1. Dead signals (HIGH — silent production bug) - -All `apps.py` have `ready(): pass` — signal handlers are never registered with -Django's dispatch system. As a result, the following logic **does not run in -production**: - -| Handler | File | Effect when dead | -|---------|------|-----------------| -| `remove_old_pictures_after_change` | `animals/signals.py` | Orphaned animal images accumulate | -| `remove_old_pictures_after_animal_delete` | `animals/signals.py` | Profile image not removed on delete | -| `remove_old_pictures_after_user_delete` | `animals/signals.py` | Same, on user delete | -| `update_allowed_users` | `animals/signals.py` | Owner can remain in allowed_users | -| `validate_one_to_one_fields` | `medical_notes/signals/` | Invalid BiometricRecords can be saved | -| `clean_orphaned_metric_records` | `medical_notes/signals/` | Orphaned BiometricRecord rows accumulate | -| `clean_orphaned_diet_records` | `medical_notes/signals/` | Orphaned FeedingNote rows accumulate | -| `create_profile` / `save_profile` | `users/signals.py` | Profile not created on User.create | -| `create_basic_privilege` / `create_background` | `users/signals.py` | Privilege/Background not set on Profile.save | - -**Decision required:** register each handler in `ready()` OR consciously delete it. -**Warning:** do NOT move signal logic into services "in passing" without a deliberate -decision — it would change current production behaviour (currently: nothing runs). - -## 2. FeedingNote missing `author` field (BUG) - -`medical_notes/signals/type_feeding_notes.py` references `instance.related_note.author` -but `FeedingNote` has no `author` field — it only has `related_note` (FK to `MedicalRecord`), -and `MedicalRecord` has `author`. The signal traversal chain is: - -``` -FeedingNote → related_note (MedicalRecord) → author (Profile) -``` - -Check whether this traversal is actually correct or whether it is a latent -`AttributeError` waiting to surface when the signal is finally connected. - -## 3. Form validation with DB queries in `utils_owner/forms.py` - -`ChangeOwnerForm.clean_new_owner()` and `ManageKeepersForm.clean_input_user()` -issue `Profile.objects.filter(...)` / `Profile.objects.get(...)` queries inside -`clean_*` methods. This is acceptable for now but can be extracted to selectors -(query layer) in a later refactor. **Do not move in the same PR as signal work** — -risk of changing form validation error messages. - -## 4. Test coverage gaps - -- Views (`animals/views.py`, `medical_notes/views/`) have zero test coverage. -- `users/signals.py` handlers untested (currently dead anyway, see §1). -- Fat views refactor (in progress) is the natural trigger for adding view tests. - -## 5. Fat views — in progress - -`animals/views.py` and `medical_notes/views/` contain business logic. -Extraction to a service layer is already started. Keep signal decisions (§1) -in sync with this work to avoid duplicating logic. +## 1. Dead signals — DONE + +All handlers registered in `ready()`. Status per handler: + +| Handler | File | Outcome | +|--------------------------------------|------------------------------|----------------------------------------------| +| `remove_old_pictures_after_change` | `animals/signals.py` | Connected as-is | +| `remove_old_pictures_after_animal_delete` | `animals/signals.py` | Connected as-is | +| `remove_old_pictures_after_user_delete` | `animals/signals.py` | Connected as-is | +| `update_allowed_users` | `animals/signals.py` | Connected as-is | +| `validate_one_to_one_fields` | `medical_notes/signals/` | Connected as-is | +| `clean_orphaned_metric_records` | `medical_notes/signals/` | Fixed (None guard on `related_note`), connected | +| `clean_orphaned_diet_records` | `medical_notes/signals/` | Fixed (rewrote logic, see §2), connected | +| `create_profile` / `save_profile` | `users/signals.py` | Connected (`save_profile` guarded with `hasattr`) | +| `create_basic_privilege` / `create_background` | `users/signals.py` | **Deleted** — see note below | + +`create_basic_privilege` / `create_background`: `Privilege` and `ProfileBackground` +raise `NotImplementedError` in `__init__`, making them permanently uninstantiable via +the ORM (any queryset that returns a row crashes). `Profile.privilege_tier` and +`profile_background` are nullable (`default=None`), so a Profile without them is valid. +Reconnect only after `homepage/models.py` is redesigned (see the `TODO` comments there). + +Note: `remove_old_pictures_after_change` and `remove_old_pictures_after_user_delete` +perform a full media-dir scan on every `Animal`/`Profile` save — O(table). Candidate +for a management command in a later PR. + +## 2. FeedingNote missing `author` field — DONE + +Fixed in the same PR as §1. `clean_orphaned_diet_records` now mirrors the +`clean_orphaned_metric_records` pattern: finds orphaned `diet_note` shells +(MedicalRecord with no attached FeedingNote) and deletes them. + +## 3. Form validation with DB queries in `utils_owner/forms.py` — DONE + +Extracted to `profile_by_username(username: str)` selector in `animals/selectors.py`. +Both `clean_new_owner` and `clean_input_user` now use it. Changed `cleaned_data.get()` +to `cleaned_data[field]` (correct pattern for `clean_` methods — key is +guaranteed present when Django calls them). + +## 4. Test coverage gaps — DONE + +- `medical_notes/views/` feeding views covered (§5). +- `users/signals.py` `create_profile`/`save_profile` connected (§1); `create_basic_privilege`/ + `create_background` restored (§A); unit tests for all four in `users/tests.py`. +- `animals/views.py`: `CreateAnimalView`, `AnimalProfileDetailView`, `StableView`, + `ToPinAnimalsView` — integration tests added (`animals/tests.py`). +- `animals/utils_owner/views.py`: `AnimalDeleteView`, `ChangeBirthdayView`, + `ChangeFirstContactView`, `ChangeNextVisitView`, `ChangeDietaryRestrictionsView`, + `ChangeAnimalDetailsView`, `ManageKeepersView`, `ChangeOwnerView`, `EditShareView` — + integration tests added (`animals/tests.py`). +- `users/views.py`: `UserRegisterView`, `UserProfileView`, `ShareDefaultsView` — + integration tests added (`users/tests.py`). + +## 5. Fat views — DONE + +Remaining business logic extracted to services/selectors. All `medical_notes/views/` +are now thin. + +Changes: +- `services/feeding.py` — new: `create_feeding_note` +- `selectors.py` — new: `is_author_of_any_note` +- `utils.py` — new: `build_timeline_base_query` (presentation helper, DB-free) +- `views/type_feeding_notes.py` — `DietRecordCreateView.form_valid` delegates to service; + `EditDietRecordView` broken `form_valid` removed, correct `get_success_url` added + (fixes latent 404 bug — pk was misused as EmailNotification id) +- `views/type_basic_note.py` — `EditRelatedAnimalsView` uses `is_author_of_any_note`; + `FullTimelineOfNotes` uses `build_timeline_base_query` +- `src/ahc/types.py` — new: `AuthenticatedRequest` (under `TYPE_CHECKING` to avoid + Django model metaclass registration) +- `request: AuthenticatedRequest` added to all touched view classes (partial §6) + +Regression tests added for all modified view flows (see §4 — coverage now exists +for `DietRecordCreateView`, `EditDietRecordView`, `FeedingNoteListView`). ## 6. Replace `[[tool.ty.overrides]]` with a typed request `pyproject.toml` suppresses `unresolved-attribute` across all view/mixin/signal/form modules to silence Django ORM false positives (mainly `request.user.profile`). -The proper fix is a custom request type in `src/ahc/types.py`: +The custom request type already exists at `src/ahc/types.py`. Both classes live +under `TYPE_CHECKING` to prevent Django's model metaclass from registering +`_AHCUser` as a real model at import time (runtime `RuntimeError` otherwise): ```python -# src/ahc/types.py +# src/ahc/types.py — CORRECT pattern +from __future__ import annotations from typing import TYPE_CHECKING -from django.contrib.auth.models import User -from django.http import HttpRequest if TYPE_CHECKING: + from django.contrib.auth.models import User + from django.http import HttpRequest from ahc.apps.users.models import Profile -class _AHCUser(User): - profile: "Profile" + class _AHCUser(User): + profile: Profile -class AuthenticatedRequest(HttpRequest): - user: _AHCUser # type: ignore[assignment] + class AuthenticatedRequest(HttpRequest): + user: _AHCUser # type: ignore[assignment] ``` -Then annotate each view class: `request: AuthenticatedRequest`. -Once all views are covered, remove the `[[tool.ty.overrides]]` block from -`pyproject.toml`. **Do this as part of the fat-views refactor (§5)** — each -view touched during extraction gets the annotation added. +To use in a view, two things are required: +1. `from __future__ import annotations` at the top of the view file (makes all + class-body annotations lazy — Python never evaluates them at import time). +2. Import under `TYPE_CHECKING`: + ```python + if TYPE_CHECKING: + from ahc.types import AuthenticatedRequest + ``` +3. Annotate the view class: `request: AuthenticatedRequest` + +**Current state (after §A+§C):** all view classes with `LoginRequiredMixin` now carry +`request: AuthenticatedRequest`. The `[[tool.ty.overrides]]` block was narrowed — the +`views.py` / `views/**` patterns were removed. Remaining in the block: `signals/**`, +`forms/**`, `mixins/**` (Django CBV `self.kwargs` false-positives), and `homepage/views.py` +(unauthenticated-user context). Remove those patterns once their respective false-positives +are resolved by other means (e.g. typed `kwargs` stub for CBV, ORM stubs for reverse +relations). diff --git a/conftest.py b/conftest.py index e6e8c00..cc5ee6d 100644 --- a/conftest.py +++ b/conftest.py @@ -3,32 +3,26 @@ import pytest from django.contrib.auth.models import User +from ahc.apps.users.models import Profile + @pytest.fixture def user_profile(db): """Create a User + Profile pair, mocking image processing in Profile.save().""" - from ahc.apps.users.models import Profile - - user = User.objects.create_user(username="testuser", password="testpass123") # nosec B106 - with patch("ahc.apps.users.models.Image.open") as mock_open: - mock_img = MagicMock() - mock_img.height = 100 - mock_img.width = 100 - mock_open.return_value = mock_img - profile = Profile.objects.create(user=user) - return user, profile + mock_img = MagicMock() + mock_img.height = 100 + mock_img.width = 100 + with patch("ahc.apps.users.models.Image.open", return_value=mock_img): + user = User.objects.create_user(username="testuser", password="testpass123") # nosec B106 + return user, Profile.objects.get(user=user) @pytest.fixture def second_user_profile(db): """A second User + Profile for multi-user permission tests.""" - from ahc.apps.users.models import Profile - - user = User.objects.create_user(username="otheruser", password="testpass123") # nosec B106 - with patch("ahc.apps.users.models.Image.open") as mock_open: - mock_img = MagicMock() - mock_img.height = 100 - mock_img.width = 100 - mock_open.return_value = mock_img - profile = Profile.objects.create(user=user) - return user, profile + mock_img = MagicMock() + mock_img.height = 100 + mock_img.width = 100 + with patch("ahc.apps.users.models.Image.open", return_value=mock_img): + user = User.objects.create_user(username="otheruser", password="testpass123") # nosec B106 + return user, Profile.objects.get(user=user) diff --git a/pyproject.toml b/pyproject.toml index 2dfd71e..c498586 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -98,16 +98,13 @@ data_file = "tests/result/.coverage" [[tool.ty.overrides]] include = [ - "src/ahc/apps/*/views.py", - "src/ahc/apps/*/views/**", - "src/ahc/apps/*/*/views.py", - "src/ahc/apps/*/*/views/**", - "src/ahc/apps/*/mixins/**", - "src/ahc/apps/*/*/mixins/**", "src/ahc/apps/*/signals.py", "src/ahc/apps/*/signals/**", "src/ahc/apps/*/forms.py", "src/ahc/apps/*/forms/**", + "src/ahc/apps/*/mixins/**", + "src/ahc/apps/*/*/mixins/**", + "src/ahc/apps/homepage/views.py", ] rules = { unresolved-attribute = "ignore" } diff --git a/src/ahc/apps/animals/apps.py b/src/ahc/apps/animals/apps.py index 1023188..db8a046 100644 --- a/src/ahc/apps/animals/apps.py +++ b/src/ahc/apps/animals/apps.py @@ -6,4 +6,4 @@ class AnimalsConfig(AppConfig): name = "ahc.apps.animals" def ready(self): - pass + from . import signals # noqa: F401 diff --git a/src/ahc/apps/animals/mixins/animal_owner_permissions.py b/src/ahc/apps/animals/mixins/animal_owner_permissions.py index 292679a..bac06ee 100644 --- a/src/ahc/apps/animals/mixins/animal_owner_permissions.py +++ b/src/ahc/apps/animals/mixins/animal_owner_permissions.py @@ -1,10 +1,19 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + from django.contrib.auth.mixins import UserPassesTestMixin from ahc.apps.animals.models import Animal from ahc.apps.animals.selectors import is_animal_owner +if TYPE_CHECKING: + from ahc.types import AuthenticatedRequest + class UserPassesOwnershipTestMixin(UserPassesTestMixin): + request: AuthenticatedRequest + def test_func(self): animal = Animal.objects.get(pk=self.kwargs["pk"]) return is_animal_owner(self.request.user.profile, animal) diff --git a/src/ahc/apps/animals/selectors.py b/src/ahc/apps/animals/selectors.py index 38e2d7d..c09ef5f 100644 --- a/src/ahc/apps/animals/selectors.py +++ b/src/ahc/apps/animals/selectors.py @@ -78,3 +78,10 @@ def recent_records_for(animal: Animal, limit: int = 5) -> QuerySet: from ahc.apps.medical_notes.models.type_basic_note import MedicalRecord return MedicalRecord.objects.filter(animal=animal).order_by("-date_creation")[:limit] + + +def profile_by_username(username: str): + """Return the Profile for the given username, or None if not found.""" + from ahc.apps.users.models import Profile + + return Profile.objects.filter(user__username=username).first() diff --git a/src/ahc/apps/animals/signals.py b/src/ahc/apps/animals/signals.py index 6ccd793..0e0576f 100644 --- a/src/ahc/apps/animals/signals.py +++ b/src/ahc/apps/animals/signals.py @@ -1,47 +1,28 @@ -import os from pathlib import Path from django.conf import settings -from django.db.models.signals import post_delete, post_save, pre_delete +from django.db.models.signals import post_save, pre_delete from django.dispatch import receiver from ahc.apps.animals.models import Animal -from ahc.apps.users.models import Profile _ANIMALS_MEDIA_DIR = Path(settings.MEDIA_ROOT) / "profile_pics" / "animals" -@receiver(post_save, sender=Animal) -def remove_old_pictures_after_change(sender, instance, **kwargs): - if not _ANIMALS_MEDIA_DIR.is_dir(): - return - animals_with_profile_images = { - str(p).split("/")[-1] for p in Animal.objects.exclude(profile_image="").values_list("profile_image", flat=True) - } - for image_name in os.listdir(_ANIMALS_MEDIA_DIR): - if image_name not in animals_with_profile_images: - (_ANIMALS_MEDIA_DIR / image_name).unlink(missing_ok=True) - - @receiver(pre_delete, sender=Animal) def remove_old_pictures_after_animal_delete(sender, instance, **kwargs): + """Remove the animal's profile image when the Animal row is deleted. + + Targeted O(1) cleanup — kept as a signal because it fires at exactly the + right moment. The broader orphan-image sweep (post_save full scan) has been + moved to the daily Celery Beat task clean_orphaned_profile_images in + celery_notifications/cron.py. + """ if instance.profile_image: image_path = _ANIMALS_MEDIA_DIR / Path(instance.profile_image.name).name image_path.unlink(missing_ok=True) -@receiver(post_delete, sender=Profile) -def remove_old_pictures_after_user_delete(sender, instance, **kwargs): - if not _ANIMALS_MEDIA_DIR.is_dir(): - return - animals_with_profile_images = { - str(p).split("/")[-1] for p in Animal.objects.exclude(profile_image="").values_list("profile_image", flat=True) - } - for image_name in os.listdir(_ANIMALS_MEDIA_DIR): - if image_name not in animals_with_profile_images: - (_ANIMALS_MEDIA_DIR / image_name).unlink(missing_ok=True) - - @receiver(post_save, sender=Animal) def update_allowed_users(sender, instance, **kwargs): if instance.owner and instance.owner in instance.allowed_users.all(): diff --git a/src/ahc/apps/animals/tests.py b/src/ahc/apps/animals/tests.py index 339df85..9625195 100644 --- a/src/ahc/apps/animals/tests.py +++ b/src/ahc/apps/animals/tests.py @@ -499,3 +499,492 @@ def test_non_owner_post_returns_403(self, animal_with_keeper, second_user_profil url = f"/pet/{animal_with_keeper.id}/keepers/{keeper_profile.pk}/remove/" response = c.post(url) assert response.status_code == 403 + + +@pytest.mark.integration +@pytest.mark.django_db +class TestCreateAnimalView: + """CreateAnimalView: form rendering, animal creation, and authentication gate.""" + + def _client_for(self, user): + from django.test import Client + + c = Client() + c.force_login(user) + return c + + def test_unauthenticated_redirects_to_login(self): + from django.test import Client + + response = Client().get("/pet/create/") + assert response.status_code == 302 + + def test_get_renders_form(self, user_profile): + user, _ = user_profile + response = self._client_for(user).get("/pet/create/") + assert response.status_code == 200 + + def test_valid_post_creates_animal_and_redirects_to_profile(self, user_profile): + user, _ = user_profile + response = self._client_for(user).post("/pet/create/", {"full_name": "GoldenFish"}) + assert response.status_code == 302 + assert Animal.objects.filter(full_name="GoldenFish").exists() + + +@pytest.mark.integration +@pytest.mark.django_db +class TestAnimalProfileDetailView: + """AnimalProfileDetailView: full-page shell rendering and access control.""" + + @pytest.fixture + def animal(self, db, user_profile): + _, profile = user_profile + return Animal.objects.create(full_name="ProfileTest", owner=profile) + + def _client_for(self, user): + from django.test import Client + + c = Client() + c.force_login(user) + return c + + def test_unauthenticated_redirects_to_login(self, animal): + from django.test import Client + + response = Client().get(f"/pet/{animal.id}/") + assert response.status_code == 302 + + def test_owner_gets_200(self, animal, user_profile): + user, _ = user_profile + response = self._client_for(user).get(f"/pet/{animal.id}/") + assert response.status_code == 200 + + def test_stranger_gets_403(self, animal, second_user_profile): + other_user, _ = second_user_profile + response = self._client_for(other_user).get(f"/pet/{animal.id}/") + assert response.status_code == 403 + + +@pytest.mark.integration +@pytest.mark.django_db +class TestStableView: + """StableView: animal list page for authenticated users.""" + + def _client_for(self, user): + from django.test import Client + + c = Client() + c.force_login(user) + return c + + def test_unauthenticated_redirects_to_login(self): + from django.test import Client + + response = Client().get("/pet/animals/") + assert response.status_code == 302 + + def test_owner_sees_own_animal_in_context(self, user_profile): + user, profile = user_profile + animal = Animal.objects.create(full_name="StableAnimal", owner=profile) + response = self._client_for(user).get("/pet/animals/") + assert response.status_code == 200 + assert animal in response.context["animals"] + + +@pytest.mark.integration +@pytest.mark.django_db +class TestToPinAnimalsView: + """ToPinAnimalsView: JSON pin/unpin endpoint with access control.""" + + @pytest.fixture + def animal(self, db, user_profile): + _, profile = user_profile + return Animal.objects.create(full_name="PinMe", owner=profile) + + def _client_for(self, user): + from django.test import Client + + c = Client() + c.force_login(user) + return c + + def test_post_pin_returns_success_json(self, animal, user_profile): + import json + + user, _ = user_profile + response = self._client_for(user).post("/pet/pinned-animals/", {"animal_id": str(animal.id), "action": "add"}) + assert response.status_code == 200 + assert json.loads(response.content)["status"] == "success" + + def test_post_unpin_returns_success_json(self, animal, user_profile): + import json + + user, profile = user_profile + profile.pinned_animals.add(animal) + response = self._client_for(user).post("/pet/pinned-animals/", {"animal_id": str(animal.id), "action": "remove"}) + assert response.status_code == 200 + assert json.loads(response.content)["status"] == "success" + + def test_pin_with_no_access_returns_403_json(self, animal, second_user_profile): + import json + + other_user, _ = second_user_profile + response = self._client_for(other_user).post("/pet/pinned-animals/", {"animal_id": str(animal.id), "action": "add"}) + assert response.status_code == 403 + assert json.loads(response.content)["status"] == "forbidden" + + +@pytest.mark.integration +@pytest.mark.django_db +class TestAnimalDeleteView: + """AnimalDeleteView: confirmation page and owner-only delete.""" + + @pytest.fixture + def animal(self, db, user_profile): + _, profile = user_profile + return Animal.objects.create(full_name="ToDelete", owner=profile) + + def _client_for(self, user): + from django.test import Client + + c = Client() + c.force_login(user) + return c + + def test_owner_get_returns_200(self, animal, user_profile): + user, _ = user_profile + response = self._client_for(user).get(f"/pet/{animal.id}/delete/") + assert response.status_code == 200 + + def test_owner_post_deletes_animal_and_redirects(self, animal, user_profile): + user, _ = user_profile + pk = animal.id + response = self._client_for(user).post(f"/pet/{pk}/delete/") + assert response.status_code == 302 + assert not Animal.objects.filter(pk=pk).exists() + + def test_non_owner_post_returns_403(self, animal, second_user_profile): + other_user, _ = second_user_profile + response = self._client_for(other_user).post(f"/pet/{animal.id}/delete/") + assert response.status_code == 403 + + +@pytest.mark.integration +@pytest.mark.django_db +class TestChangeBirthdayView: + """ChangeBirthdayView: birthdate update behind owner-only gate.""" + + @pytest.fixture + def animal(self, db, user_profile): + _, profile = user_profile + return Animal.objects.create(full_name="BdayAnimal", owner=profile) + + def _client_for(self, user): + from django.test import Client + + c = Client() + c.force_login(user) + return c + + def test_owner_get_returns_200(self, animal, user_profile): + user, _ = user_profile + response = self._client_for(user).get(f"/pet/{animal.id}/btd/") + assert response.status_code == 200 + + def test_non_owner_get_returns_403(self, animal, second_user_profile): + other_user, _ = second_user_profile + response = self._client_for(other_user).get(f"/pet/{animal.id}/btd/") + assert response.status_code == 403 + + def test_valid_post_saves_birthdate_and_redirects(self, animal, user_profile): + from datetime import date + + user, _ = user_profile + response = self._client_for(user).post(f"/pet/{animal.id}/btd/", {"birthdate": "2020-03-15"}) + assert response.status_code == 302 + animal.refresh_from_db() + assert animal.birthdate == date(2020, 3, 15) + + +@pytest.mark.integration +@pytest.mark.django_db +class TestChangeFirstContactView: + """ChangeFirstContactView: vet/place text fields update behind owner-only gate.""" + + @pytest.fixture + def animal(self, db, user_profile): + _, profile = user_profile + return Animal.objects.create(full_name="FirstContactAnimal", owner=profile) + + def _client_for(self, user): + from django.test import Client + + c = Client() + c.force_login(user) + return c + + def test_owner_get_returns_200(self, animal, user_profile): + user, _ = user_profile + response = self._client_for(user).get(f"/pet/{animal.id}/cnt/") + assert response.status_code == 200 + + def test_non_owner_get_returns_403(self, animal, second_user_profile): + other_user, _ = second_user_profile + response = self._client_for(other_user).get(f"/pet/{animal.id}/cnt/") + assert response.status_code == 403 + + def test_valid_post_saves_vet_and_place(self, animal, user_profile): + user, _ = user_profile + response = self._client_for(user).post( + f"/pet/{animal.id}/cnt/", + {"first_contact_vet": "Dr. Smith", "first_contact_medical_place": "City Clinic"}, + ) + assert response.status_code == 302 + animal.refresh_from_db() + assert animal.first_contact_vet == "Dr. Smith" + assert animal.first_contact_medical_place == "City Clinic" + + +@pytest.mark.integration +@pytest.mark.django_db +class TestChangeNextVisitView: + """ChangeNextVisitView: next_visit_date update with vet-tab redirect.""" + + @pytest.fixture + def animal(self, db, user_profile): + _, profile = user_profile + return Animal.objects.create(full_name="NextVisitAnimal", owner=profile) + + def _client_for(self, user): + from django.test import Client + + c = Client() + c.force_login(user) + return c + + def test_owner_get_returns_200(self, animal, user_profile): + user, _ = user_profile + response = self._client_for(user).get(f"/pet/{animal.id}/next-visit/") + assert response.status_code == 200 + + def test_non_owner_get_returns_403(self, animal, second_user_profile): + other_user, _ = second_user_profile + response = self._client_for(other_user).get(f"/pet/{animal.id}/next-visit/") + assert response.status_code == 403 + + def test_valid_post_saves_date_and_redirects_to_vet_tab(self, animal, user_profile): + from datetime import date + + user, _ = user_profile + response = self._client_for(user).post(f"/pet/{animal.id}/next-visit/", {"next_visit_date": "2026-09-01"}) + assert response.status_code == 302 + animal.refresh_from_db() + assert animal.next_visit_date == date(2026, 9, 1) + assert f"/pet/{animal.id}/tab/vet/" in response["Location"] + + +@pytest.mark.integration +@pytest.mark.django_db +class TestChangeDietaryRestrictionsView: + """ChangeDietaryRestrictionsView: dietary_restrictions update with diet-tab redirect.""" + + @pytest.fixture + def animal(self, db, user_profile): + _, profile = user_profile + return Animal.objects.create(full_name="DietAnimal", owner=profile) + + def _client_for(self, user): + from django.test import Client + + c = Client() + c.force_login(user) + return c + + def test_owner_get_returns_200(self, animal, user_profile): + user, _ = user_profile + response = self._client_for(user).get(f"/pet/{animal.id}/dietary-restrictions/") + assert response.status_code == 200 + + def test_non_owner_get_returns_403(self, animal, second_user_profile): + other_user, _ = second_user_profile + response = self._client_for(other_user).get(f"/pet/{animal.id}/dietary-restrictions/") + assert response.status_code == 403 + + def test_valid_post_saves_restrictions_and_redirects_to_diet_tab(self, animal, user_profile): + user, _ = user_profile + response = self._client_for(user).post( + f"/pet/{animal.id}/dietary-restrictions/", {"dietary_restrictions": "No grapes, no onions"} + ) + assert response.status_code == 302 + animal.refresh_from_db() + assert animal.dietary_restrictions == "No grapes, no onions" + assert f"/pet/{animal.id}/tab/diet/" in response["Location"] + + +@pytest.mark.integration +@pytest.mark.django_db +class TestChangeAnimalDetailsView: + """ChangeAnimalDetailsView: species/breed/sex/sterilization update with settings-tab redirect.""" + + @pytest.fixture + def animal(self, db, user_profile): + _, profile = user_profile + return Animal.objects.create(full_name="DetailsAnimal", owner=profile) + + def _client_for(self, user): + from django.test import Client + + c = Client() + c.force_login(user) + return c + + def test_owner_get_returns_200(self, animal, user_profile): + user, _ = user_profile + response = self._client_for(user).get(f"/pet/{animal.id}/details/") + assert response.status_code == 200 + + def test_non_owner_get_returns_403(self, animal, second_user_profile): + other_user, _ = second_user_profile + response = self._client_for(other_user).get(f"/pet/{animal.id}/details/") + assert response.status_code == 403 + + def test_valid_post_saves_details_and_redirects_to_settings_tab(self, animal, user_profile): + user, _ = user_profile + response = self._client_for(user).post( + f"/pet/{animal.id}/details/", + {"species": "cat", "breed": "Maine Coon", "sex": "f", "sterilization": "on"}, + ) + assert response.status_code == 302 + animal.refresh_from_db() + assert animal.species == "cat" + assert animal.breed == "Maine Coon" + assert animal.sex == "f" + assert animal.sterilization is True + assert f"/pet/{animal.id}/tab/settings/" in response["Location"] + + +@pytest.mark.integration +@pytest.mark.django_db +class TestManageKeepersView: + """ManageKeepersView: share creation behind owner-only gate.""" + + @pytest.fixture + def animal(self, db, user_profile): + _, profile = user_profile + return Animal.objects.create(full_name="KeeperAnimal", owner=profile) + + def _client_for(self, user): + from django.test import Client + + c = Client() + c.force_login(user) + return c + + def test_owner_get_returns_200(self, animal, user_profile): + user, _ = user_profile + response = self._client_for(user).get(f"/pet/{animal.id}/manage_keepers/") + assert response.status_code == 200 + + def test_non_owner_get_returns_403(self, animal, second_user_profile): + other_user, _ = second_user_profile + response = self._client_for(other_user).get(f"/pet/{animal.id}/manage_keepers/") + assert response.status_code == 403 + + def test_valid_post_creates_share_for_new_keeper(self, animal, user_profile, second_user_profile): + from ahc.apps.animals.models import AnimalShare + + user, _ = user_profile + _, keeper_profile = second_user_profile + response = self._client_for(user).post( + f"/pet/{animal.id}/manage_keepers/", + {"input_user": keeper_profile.user.username, "allow_basic": "on"}, + ) + assert response.status_code == 302 + assert AnimalShare.objects.filter(animal=animal, carer=keeper_profile).exists() + + +@pytest.mark.integration +@pytest.mark.django_db +class TestChangeOwnerView: + """ChangeOwnerView: ownership transfer behind owner-only gate.""" + + @pytest.fixture + def animal(self, db, user_profile): + _, profile = user_profile + return Animal.objects.create(full_name="OwnerAnimal", owner=profile) + + def _client_for(self, user): + from django.test import Client + + c = Client() + c.force_login(user) + return c + + def test_owner_get_returns_200(self, animal, user_profile): + user, _ = user_profile + response = self._client_for(user).get(f"/pet/{animal.id}/owner/") + assert response.status_code == 200 + + def test_non_owner_get_returns_403(self, animal, second_user_profile): + other_user, _ = second_user_profile + response = self._client_for(other_user).get(f"/pet/{animal.id}/owner/") + assert response.status_code == 403 + + def test_valid_post_transfers_ownership(self, animal, user_profile, second_user_profile): + user, _ = user_profile + _, new_owner_profile = second_user_profile + response = self._client_for(user).post( + f"/pet/{animal.id}/owner/", + {"new_owner": new_owner_profile.user.username, "set_keeper": ""}, + ) + assert response.status_code == 302 + animal.refresh_from_db() + assert animal.owner == new_owner_profile + + +@pytest.mark.integration +@pytest.mark.django_db +class TestEditShareView: + """EditShareView: access scope edit behind owner-only gate.""" + + @pytest.fixture + def animal_with_share(self, db, user_profile, second_user_profile): + from ahc.apps.animals.models import AnimalShare + + _, owner_profile = user_profile + _, carer_profile = second_user_profile + animal = Animal.objects.create(full_name="ShareAnimal", owner=owner_profile) + share = AnimalShare.objects.create(animal=animal, carer=carer_profile) + return animal, share, carer_profile + + def _client_for(self, user): + from django.test import Client + + c = Client() + c.force_login(user) + return c + + def test_owner_get_returns_200(self, animal_with_share, user_profile): + user, _ = user_profile + animal, _, carer_profile = animal_with_share + response = self._client_for(user).get(f"/pet/{animal.id}/keepers/{carer_profile.pk}/access/") + assert response.status_code == 200 + + def test_non_owner_get_returns_403(self, animal_with_share, second_user_profile): + other_user, _ = second_user_profile + animal, _, carer_profile = animal_with_share + response = self._client_for(other_user).get(f"/pet/{animal.id}/keepers/{carer_profile.pk}/access/") + assert response.status_code == 403 + + def test_valid_post_updates_share_scope_and_redirects(self, animal_with_share, user_profile): + user, _ = user_profile + animal, share, carer_profile = animal_with_share + response = self._client_for(user).post( + f"/pet/{animal.id}/keepers/{carer_profile.pk}/access/", + {"allow_basic": "on", "allow_diet": "on"}, + ) + assert response.status_code == 302 + share.refresh_from_db() + assert share.allow_basic is True + assert share.allow_diet is True + assert f"/pet/{animal.id}/tab/ownership/" in response["Location"] diff --git a/src/ahc/apps/animals/utils_owner/forms.py b/src/ahc/apps/animals/utils_owner/forms.py index ed48e89..35fd5cf 100644 --- a/src/ahc/apps/animals/utils_owner/forms.py +++ b/src/ahc/apps/animals/utils_owner/forms.py @@ -4,7 +4,7 @@ from PIL import Image from ahc.apps.animals.models import Animal, AnimalShare -from ahc.apps.users.models import Profile +from ahc.apps.animals.selectors import profile_by_username class ImageUploadForm(forms.ModelForm): @@ -45,17 +45,16 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) def clean_new_owner(self): - new_owner = self.cleaned_data.get("new_owner") + new_owner = self.cleaned_data["new_owner"] if new_owner == self.instance.owner.user.username: raise forms.ValidationError("You are already the owner.") - if not Profile.objects.filter(user__username=new_owner).exists(): + profile = profile_by_username(new_owner) + if profile is None: raise forms.ValidationError("User does not exist.") - new_owner_profile = Profile.objects.get(user__username=new_owner) - - return new_owner_profile + return profile class ManageKeepersForm(forms.Form): @@ -92,7 +91,7 @@ def __init__(self, *args, **kwargs): self.fields[field].initial = getattr(defaults, field) def clean_input_user(self): - input_user = self.cleaned_data.get("input_user") + input_user = self.cleaned_data["input_user"] if input_user == self.instance.owner.user.username: raise forms.ValidationError("As the owner you can not set yourself as a keeper.") @@ -100,7 +99,7 @@ def clean_input_user(self): if self.instance.shares.filter(carer__user__username=input_user).exists(): raise forms.ValidationError("User is already on the list of keepers.") - profile = Profile.objects.filter(user__username=input_user).first() + profile = profile_by_username(input_user) if profile is None: raise forms.ValidationError("User does not exist.") diff --git a/src/ahc/apps/animals/utils_owner/views.py b/src/ahc/apps/animals/utils_owner/views.py index 1e01b20..347a82e 100644 --- a/src/ahc/apps/animals/utils_owner/views.py +++ b/src/ahc/apps/animals/utils_owner/views.py @@ -1,3 +1,7 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + from django.contrib.auth.mixins import LoginRequiredMixin from django.shortcuts import get_object_or_404, redirect from django.urls import reverse, reverse_lazy @@ -30,6 +34,9 @@ ManageKeepersForm, ) +if TYPE_CHECKING: + from ahc.types import AuthenticatedRequest + class AnimalDeleteView(LoginRequiredMixin, UserPassesOwnershipTestMixin, DeleteView): model = Animal @@ -67,6 +74,7 @@ def form_valid(self, form): class ChangeOwnerView(LoginRequiredMixin, UserPassesOwnershipTestMixin, FormView): template_name = "animals/change_owner.html" form_class = ChangeOwnerForm + request: AuthenticatedRequest def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) @@ -101,7 +109,7 @@ def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) animal = Animal.objects.get(pk=self.kwargs["pk"]) context["full_name"] = animal.full_name - context["shares"] = animal.shares.select_related("carer__user").all() + context["shares"] = animal.shares.select_related("carer__user").all() # type: ignore context["animal_url"] = reverse("animal_profile", kwargs={"pk": self.get_form().instance.id}) return context diff --git a/src/ahc/apps/animals/views.py b/src/ahc/apps/animals/views.py index 85ad176..49b9867 100644 --- a/src/ahc/apps/animals/views.py +++ b/src/ahc/apps/animals/views.py @@ -3,7 +3,7 @@ from collections.abc import Callable from dataclasses import dataclass from datetime import date, datetime -from typing import Any +from typing import TYPE_CHECKING, Any from django.contrib.auth.mixins import LoginRequiredMixin, UserPassesTestMixin from django.http import Http404, JsonResponse @@ -25,6 +25,9 @@ ) from ahc.apps.animals.services import create_animal, pin_animal, unpin_animal +if TYPE_CHECKING: + from ahc.types import AuthenticatedRequest + @dataclass class Tab: @@ -170,7 +173,7 @@ def _build_notes(request, animal: Animal, allowed: set[str] | None = None) -> di def _build_ownership(request, animal: Animal, allowed: set[str] | None = None) -> dict[str, Any]: - return {"keepers": animal.shares.select_related("carer__user").all()} + return {"keepers": animal.shares.select_related("carer__user").all()} # type: ignore def _build_settings(request, animal: Animal, allowed: set[str] | None = None) -> dict[str, Any]: @@ -272,6 +275,7 @@ class CreateAnimalView(LoginRequiredMixin, FormView): template_name = "animals/create.html" form_class = AnimalRegisterForm success_url = "/animals/" + request: AuthenticatedRequest def form_valid(self, form): new_animal = create_animal(self.request.user.profile, form) @@ -287,6 +291,7 @@ class AnimalProfileDetailView(LoginRequiredMixin, UserPassesTestMixin, DetailVie """ model = Animal + request: AuthenticatedRequest template_name = "animals/profile.html" context_object_name = "animal" @@ -314,6 +319,7 @@ class AnimalTabView(LoginRequiredMixin, UserPassesTestMixin, DetailView): model = Animal context_object_name = "animal" + request: AuthenticatedRequest def _get_tab(self) -> Tab: slug = self.kwargs.get("slug", "") @@ -358,6 +364,7 @@ def get_context_data(self, **kwargs): class StableView(LoginRequiredMixin, TemplateView): template_name = "animals/all_animals_stable.html" + request: AuthenticatedRequest def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) @@ -366,6 +373,8 @@ def get_context_data(self, **kwargs): class ToPinAnimalsView(LoginRequiredMixin, View): + request: AuthenticatedRequest + def post(self, request, *args, **kwargs): form = PinAnimalForm(request.POST) profile = request.user.profile diff --git a/src/ahc/apps/homepage/models.py b/src/ahc/apps/homepage/models.py index 213acd7..d4a5258 100644 --- a/src/ahc/apps/homepage/models.py +++ b/src/ahc/apps/homepage/models.py @@ -2,31 +2,20 @@ from django.db import models from django.urls import reverse -from ahc.apps.homepage.utils import ImageGenerator - class Privilege(models.Model): title = models.CharField(max_length=30) privilege_to_delete_animal = models.BooleanField(default=False) - # TODO: reconsider usage to simplify privileges test mixins - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - raise NotImplementedError - class ProfileBackground(models.Model): title = models.CharField(max_length=30) content = models.ImageField( - default=ImageGenerator.default_profile_image, + default="", + blank=True, upload_to="static/media/background", ) - # TODO: reconsider usage - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - raise NotImplementedError - # TODO: reconsider usage, currently Animal Profile is enough but can be used to manipulate images class AnimalTitle(models.Model): diff --git a/src/ahc/apps/homepage/tests/test_homepage.py b/src/ahc/apps/homepage/tests/test_homepage.py index 97b9797..be7b05b 100644 --- a/src/ahc/apps/homepage/tests/test_homepage.py +++ b/src/ahc/apps/homepage/tests/test_homepage.py @@ -1,4 +1,5 @@ from html.parser import HTMLParser +from unittest.mock import MagicMock, patch import pytest from django.contrib.auth.models import User @@ -23,7 +24,11 @@ def handle_starttag(self, tag, attrs): @pytest.mark.django_db class TestHomepage(TestCase): def setUp(self) -> None: - my_user = User.objects.create(username="test_user_placeholder") + mock_img = MagicMock() + mock_img.height = 100 + mock_img.width = 100 + with patch("ahc.apps.users.models.Image.open", return_value=mock_img): + my_user = User.objects.create(username="test_user_placeholder") self.my_animal_title = AnimalTitle.objects.create(title="test_animal_placeholder", owner=my_user) def tearDown(self) -> None: diff --git a/src/ahc/apps/medical_notes/apps.py b/src/ahc/apps/medical_notes/apps.py index 5d5a03a..86f4df8 100644 --- a/src/ahc/apps/medical_notes/apps.py +++ b/src/ahc/apps/medical_notes/apps.py @@ -6,4 +6,4 @@ class MedicalNotesConfig(AppConfig): name = "ahc.apps.medical_notes" def ready(self): - pass + from . import signals # noqa: F401 diff --git a/src/ahc/apps/medical_notes/selectors.py b/src/ahc/apps/medical_notes/selectors.py index df1cd15..ced9c77 100644 --- a/src/ahc/apps/medical_notes/selectors.py +++ b/src/ahc/apps/medical_notes/selectors.py @@ -212,6 +212,11 @@ def vaccination_notes_for(animal) -> QuerySet: ) +def is_author_of_any_note(profile) -> bool: + """Return True if the profile has authored at least one MedicalRecord.""" + return MedicalRecord.objects.filter(author=profile).exists() + + def due_vaccination_reminders(on_date: date) -> QuerySet: """Return VaccinationNotes whose reminder_date is today or overdue and not yet sent. diff --git a/src/ahc/apps/medical_notes/services/feeding.py b/src/ahc/apps/medical_notes/services/feeding.py new file mode 100644 index 0000000..7951b5f --- /dev/null +++ b/src/ahc/apps/medical_notes/services/feeding.py @@ -0,0 +1,14 @@ +"""Service functions for FeedingNote creation and update.""" + +from __future__ import annotations + +from ahc.apps.medical_notes.models.type_basic_note import MedicalRecord +from ahc.apps.medical_notes.models.type_feeding_notes import FeedingNote + + +def create_feeding_note(related_note: MedicalRecord, form) -> FeedingNote: + """Attach a new FeedingNote to its parent MedicalRecord and persist it.""" + feeding_note = form.save(commit=False) + feeding_note.related_note = related_note + feeding_note.save() + return feeding_note diff --git a/src/ahc/apps/medical_notes/signals.py b/src/ahc/apps/medical_notes/signals.py deleted file mode 100644 index e69de29..0000000 diff --git a/src/ahc/apps/medical_notes/signals/__init__.py b/src/ahc/apps/medical_notes/signals/__init__.py index e69de29..3d4bbee 100644 --- a/src/ahc/apps/medical_notes/signals/__init__.py +++ b/src/ahc/apps/medical_notes/signals/__init__.py @@ -0,0 +1 @@ +from . import type_feeding_notes, type_measurement_notes # noqa: F401 diff --git a/src/ahc/apps/medical_notes/signals/type_feeding_notes.py b/src/ahc/apps/medical_notes/signals/type_feeding_notes.py index 28dc8d5..e10c683 100644 --- a/src/ahc/apps/medical_notes/signals/type_feeding_notes.py +++ b/src/ahc/apps/medical_notes/signals/type_feeding_notes.py @@ -2,13 +2,17 @@ from django.db.models.signals import post_save from django.dispatch import receiver +from ahc.apps.medical_notes.models.type_basic_note import MedicalRecord from ahc.apps.medical_notes.models.type_feeding_notes import FeedingNote -from ahc.apps.users.models import Profile as UserProfile @receiver(post_save, sender=FeedingNote) def clean_orphaned_diet_records(sender, instance, **kwargs): - user_profile = UserProfile.objects.get(id=instance.related_note.author.id) + related = instance.related_note + if related is None or related.author is None: + return with transaction.atomic(): - orphaned_notes = FeedingNote.objects.filter(author=user_profile, related_note=None) - orphaned_notes.delete() + shells = MedicalRecord.objects.filter(author=related.author, type_of_event="diet_note") + for record in shells: + if not record.feedingnote_set.exists(): + record.delete() diff --git a/src/ahc/apps/medical_notes/signals/type_measurement_notes.py b/src/ahc/apps/medical_notes/signals/type_measurement_notes.py index d3cddd6..1dcadb0 100644 --- a/src/ahc/apps/medical_notes/signals/type_measurement_notes.py +++ b/src/ahc/apps/medical_notes/signals/type_measurement_notes.py @@ -5,7 +5,6 @@ from ahc.apps.medical_notes.models.type_basic_note import MedicalRecord from ahc.apps.medical_notes.models.type_measurement_notes import BiometricRecord -from ahc.apps.users.models import Profile as UserProfile @receiver(pre_save, sender=BiometricRecord) @@ -25,7 +24,10 @@ def validate_one_to_one_fields(sender, instance, **kwargs): @receiver(post_save, sender=BiometricRecord) def clean_orphaned_metric_records(sender, instance, **kwargs): - user_profile = UserProfile.objects.get(id=instance.related_note.author.id) + related = instance.related_note + if related is None or related.author is None: + return + user_profile = related.author medical_records = MedicalRecord.objects.filter(author=user_profile, type_of_event="biometric_record") for record in medical_records: diff --git a/src/ahc/apps/medical_notes/tests.py b/src/ahc/apps/medical_notes/tests.py index f503876..f510733 100644 --- a/src/ahc/apps/medical_notes/tests.py +++ b/src/ahc/apps/medical_notes/tests.py @@ -1,9 +1,11 @@ +from datetime import date as _date from io import BytesIO from types import SimpleNamespace from unittest.mock import MagicMock, patch import pytest from django.core.exceptions import ValidationError +from django.urls import reverse from ahc.apps.medical_notes.models.type_measurement_notes import ( BiometricHeightRecords, @@ -12,6 +14,7 @@ from ahc.apps.medical_notes.selectors import ( can_access_note_animal, is_attachment_author, + is_author_of_any_note, is_note_author, ) from ahc.apps.medical_notes.services.attachments import ( @@ -23,6 +26,7 @@ from ahc.apps.medical_notes.services.notes import create_note, next_route_for, update_note from ahc.apps.medical_notes.services.notifications import create_email_notification from ahc.apps.medical_notes.signals.type_measurement_notes import validate_one_to_one_fields +from ahc.apps.medical_notes.utils import build_timeline_base_query def _make_instance(weight=None, height=None, custom=None): @@ -633,5 +637,188 @@ def test_due_vaccination_reminders_excludes_already_sent(self, vaccination_anima reminder_sent=True, ) - due = list(due_vaccination_reminders(date(2026, 6, 1))) + due = list(due_vaccination_reminders(_date(2026, 6, 1))) assert due == [] + + +# --------------------------------------------------------------------------- +# utils.py — build_timeline_base_query +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestBuildTimelineBaseQuery: + """build_timeline_base_query: pure query-string assembly.""" + + def test_both_params(self): + result = build_timeline_base_query("diet_note", "rabies") + assert result == "type_of_event=diet_note&tag_name=rabies" + + def test_only_type_of_event(self): + assert build_timeline_base_query("diet_note", "") == "type_of_event=diet_note" + + def test_only_tag_name(self): + assert build_timeline_base_query("", "rabies") == "tag_name=rabies" + + def test_both_empty(self): + assert build_timeline_base_query("", "") == "" + + +# --------------------------------------------------------------------------- +# selectors — is_author_of_any_note +# --------------------------------------------------------------------------- + + +@pytest.fixture +def diet_note_shell(db, user_profile): + """A MedicalRecord of type diet_note owned by user_profile.""" + from ahc.apps.animals.models import Animal + from ahc.apps.medical_notes.models.type_basic_note import MedicalRecord + + _, profile = user_profile + animal = Animal.objects.create(full_name="Diet Tester", owner=profile) + return MedicalRecord.objects.create( + animal=animal, + author=profile, + short_description="Diet shell", + type_of_event="diet_note", + ) + + +@pytest.fixture +def existing_feeding_note(db, diet_note_shell): + """A persisted FeedingNote linked to diet_note_shell.""" + from ahc.apps.medical_notes.models.type_feeding_notes import FeedingNote + + return FeedingNote.objects.create( + related_note=diet_note_shell, + real_start_date=_date(2026, 1, 1), + category="dry", + product_name="Original Food", + ) + + +@pytest.mark.integration +@pytest.mark.django_db +class TestIsAuthorOfAnyNote: + def test_true_when_profile_has_authored_a_note(self, diet_note_shell, user_profile): + _, profile = user_profile + assert is_author_of_any_note(profile) is True + + def test_false_for_profile_with_no_notes(self, db, second_user_profile): + _, other_profile = second_user_profile + assert is_author_of_any_note(other_profile) is False + + +# --------------------------------------------------------------------------- +# View regression tests +# --------------------------------------------------------------------------- + + +@pytest.mark.integration +@pytest.mark.django_db +class TestDietRecordCreateView: + """DietRecordCreateView (feeding_create): POST creates FeedingNote and redirects.""" + + def test_valid_post_creates_feeding_note(self, client, user_profile, diet_note_shell): + from ahc.apps.medical_notes.models.type_feeding_notes import FeedingNote + + user, _ = user_profile + client.force_login(user) + url = reverse("feeding_create", kwargs={"pk": diet_note_shell.id}) + data = { + "real_start_date": "2026-01-01", + "category": "dry", + "product_name": "New Food", + "real_end_date": "", + "producer": "", + "dose_annotations": "", + "purchase_source": "", + } + response = client.post(url, data) + assert response.status_code == 302 + assert FeedingNote.objects.filter(related_note=diet_note_shell).count() == 1 + + def test_redirects_to_diet_list(self, client, user_profile, diet_note_shell): + user, _ = user_profile + client.force_login(user) + url = reverse("feeding_create", kwargs={"pk": diet_note_shell.id}) + data = { + "real_start_date": "2026-01-01", + "category": "dry", + "product_name": "New Food", + } + response = client.post(url, data) + expected_redirect = reverse("note_related_diets", kwargs={"pk": str(diet_note_shell.id)}) + assert response["Location"] == expected_redirect + + def test_unauthenticated_redirects_to_login(self, client, diet_note_shell): + url = reverse("feeding_create", kwargs={"pk": diet_note_shell.id}) + response = client.post(url, {}) + assert response.status_code == 302 + assert "/login" in response["Location"] + + +@pytest.mark.integration +@pytest.mark.django_db +class TestEditDietRecordView: + """EditDietRecordView (feeding_edit): POST updates FeedingNote and redirects to diet list. + + This test asserts the CORRECTED behaviour. Against the original code the view + returned 404 because form_valid tried to fetch an EmailNotification using the + FeedingNote pk — a type/identity mismatch. + """ + + def test_valid_post_updates_feeding_note(self, client, user_profile, existing_feeding_note): + user, _ = user_profile + client.force_login(user) + url = reverse("feeding_edit", kwargs={"pk": existing_feeding_note.pk}) + data = { + "real_start_date": "2026-03-01", + "category": "wet", + "product_name": "Updated Food", + "real_end_date": "", + "producer": "", + "dose_annotations": "", + "purchase_source": "", + } + response = client.post(url, data) + assert response.status_code == 302 + existing_feeding_note.refresh_from_db() + assert existing_feeding_note.product_name == "Updated Food" + + def test_redirects_to_diet_list_of_parent_note(self, client, user_profile, existing_feeding_note): + user, _ = user_profile + client.force_login(user) + url = reverse("feeding_edit", kwargs={"pk": existing_feeding_note.pk}) + data = { + "real_start_date": "2026-03-01", + "category": "wet", + "product_name": "Updated Food", + } + response = client.post(url, data) + expected_redirect = reverse( + "note_related_diets", + kwargs={"pk": str(existing_feeding_note.related_note.id)}, + ) + assert response["Location"] == expected_redirect + + +@pytest.mark.integration +@pytest.mark.django_db +class TestFeedingNoteListViewAccess: + """FeedingNoteListView (note_related_diets): permission mixin enforced after refactor.""" + + def test_owner_can_access(self, client, user_profile, diet_note_shell): + user, _ = user_profile + client.force_login(user) + url = reverse("note_related_diets", kwargs={"pk": diet_note_shell.id}) + response = client.get(url) + assert response.status_code == 200 + + def test_stranger_is_denied(self, client, second_user_profile, diet_note_shell): + stranger, _ = second_user_profile + client.force_login(stranger) + url = reverse("note_related_diets", kwargs={"pk": diet_note_shell.id}) + response = client.get(url) + assert response.status_code == 403 diff --git a/src/ahc/apps/medical_notes/utils.py b/src/ahc/apps/medical_notes/utils.py new file mode 100644 index 0000000..8e4de24 --- /dev/null +++ b/src/ahc/apps/medical_notes/utils.py @@ -0,0 +1,26 @@ +"""Presentation helpers for medical_notes views. + +Pure functions — no database access. Database queries belong in selectors.py. +""" + +from __future__ import annotations + + +def build_timeline_base_query(type_of_event: str, tag_name: str) -> str: + """Build the base query string for timeline filter links. + + Returns a URL query string fragment (without leading '?') combining + whichever of type_of_event / tag_name are non-empty. Used by + FullTimelineOfNotes to propagate active filters across paginated links. + + Examples: + build_timeline_base_query("diet_note", "") -> "type_of_event=diet_note" + build_timeline_base_query("", "rabies") -> "tag_name=rabies" + build_timeline_base_query("", "") -> "" + """ + parts = [] + if type_of_event: + parts.append(f"type_of_event={type_of_event}") + if tag_name: + parts.append(f"tag_name={tag_name}") + return "&".join(parts) diff --git a/src/ahc/apps/medical_notes/views/mixins/user_animal_permisions.py b/src/ahc/apps/medical_notes/views/mixins/user_animal_permisions.py index 7f2791a..2e57e8e 100644 --- a/src/ahc/apps/medical_notes/views/mixins/user_animal_permisions.py +++ b/src/ahc/apps/medical_notes/views/mixins/user_animal_permisions.py @@ -11,6 +11,10 @@ - AttachmentAuthorRequiredMixin — pk is a MedicalRecordAttachment UUID. """ +from __future__ import annotations + +from typing import TYPE_CHECKING + from django.contrib.auth.mixins import UserPassesTestMixin from django.shortcuts import get_object_or_404 @@ -23,10 +27,15 @@ is_note_author, ) +if TYPE_CHECKING: + from ahc.types import AuthenticatedRequest + class AnimalDirectAccessRequiredMixin(UserPassesTestMixin): """Allow access when pk in URL is an Animal UUID and the profile can access it.""" + request: AuthenticatedRequest + def test_func(self): animal = get_object_or_404(Animal, id=self.kwargs.get("pk")) return user_can_access_animal(self.request.user.profile, animal) @@ -35,6 +44,8 @@ def test_func(self): class AnimalAccessRequiredMixin(UserPassesTestMixin): """Allow access when pk in URL is a MedicalRecord UUID and the profile can access its animal.""" + request: AuthenticatedRequest + def test_func(self): note = get_object_or_404(MedicalRecord, id=self.kwargs.get("pk")) return can_access_note_animal(self.request.user.profile, note) @@ -43,6 +54,8 @@ def test_func(self): class NoteAuthorRequiredMixin(UserPassesTestMixin): """Allow access only to the author of the MedicalRecord (pk = note UUID).""" + request: AuthenticatedRequest + def test_func(self): note = get_object_or_404(MedicalRecord, id=self.kwargs.get("pk")) return is_note_author(self.request.user.profile, note) @@ -51,6 +64,8 @@ def test_func(self): class AttachmentAuthorRequiredMixin(UserPassesTestMixin): """Allow access only to the author of the note that owns the attachment (pk = attachment UUID).""" + request: AuthenticatedRequest + def test_func(self): attachment = get_object_or_404(MedicalRecordAttachment, pk=self.kwargs.get("pk")) return is_attachment_author(self.request.user.profile, attachment) diff --git a/src/ahc/apps/medical_notes/views/type_basic_note.py b/src/ahc/apps/medical_notes/views/type_basic_note.py index 2584bf0..816fd9f 100644 --- a/src/ahc/apps/medical_notes/views/type_basic_note.py +++ b/src/ahc/apps/medical_notes/views/type_basic_note.py @@ -1,4 +1,7 @@ +from __future__ import annotations + from datetime import datetime +from typing import TYPE_CHECKING from django.contrib import messages from django.contrib.auth.mixins import LoginRequiredMixin, UserPassesTestMixin @@ -22,6 +25,7 @@ available_months_for, can_access_note_animal, is_attachment_author, + is_author_of_any_note, page_of_month, timeline_for, ) @@ -32,16 +36,21 @@ upload_attachment, ) from ahc.apps.medical_notes.services.notes import create_note, next_route_for, update_note +from ahc.apps.medical_notes.utils import build_timeline_base_query from ahc.apps.medical_notes.views.mixins.user_animal_permisions import ( AnimalDirectAccessRequiredMixin, AttachmentAuthorRequiredMixin, NoteAuthorRequiredMixin, ) +if TYPE_CHECKING: + from ahc.types import AuthenticatedRequest + class CreateNoteFormView(LoginRequiredMixin, AnimalDirectAccessRequiredMixin, FormView): template_name = "medical_notes/create.html" form_class = MedicalRecordForm + request: AuthenticatedRequest def get_template_names(self): if self.request.headers.get("HX-Request"): @@ -100,6 +109,7 @@ class FullTimelineOfNotes(LoginRequiredMixin, AnimalDirectAccessRequiredMixin, L template_name = "medical_notes/full_timeline_of_notes.html" context_object_name = "notes" paginate_by = 4 + request: AuthenticatedRequest def get_template_names(self): if self.request.headers.get("HX-Request"): @@ -146,12 +156,7 @@ def get_context_data(self, **kwargs): context["scroll_to_month"] = self.request.GET.get("month", "") context["type_of_event"] = type_of_event context["tag_name"] = tag_name - base_parts = [] - if type_of_event: - base_parts.append(f"type_of_event={type_of_event}") - if tag_name: - base_parts.append(f"tag_name={tag_name}") - context["base_query"] = "&".join(base_parts) + context["base_query"] = build_timeline_base_query(type_of_event, tag_name) return context @@ -182,6 +187,7 @@ class EditNoteView(LoginRequiredMixin, NoteAuthorRequiredMixin, UpdateView): form_class = MedicalRecordEditForm template_name = "medical_notes/edit.html" context_object_name = "note" + request: AuthenticatedRequest def get_template_names(self): if self.request.headers.get("HX-Request"): @@ -242,11 +248,11 @@ class EditRelatedAnimalsView(EditNoteView): form_class = MedicalRecordEditRelatedAnimalsForm template_name = "medical_notes/edit.html" context_object_name = "note" + request: AuthenticatedRequest def get_form_kwargs(self): kwargs = super().get_form_kwargs() - user = self.request.user.profile - kwargs["is_author"] = MedicalRecord.objects.filter(author=user).exists() + kwargs["is_author"] = is_author_of_any_note(self.request.user.profile) return kwargs def test_func(self): @@ -283,6 +289,8 @@ def get_success_url(self): class DownloadAttachmentView(LoginRequiredMixin, UserPassesTestMixin, View): """Download attachment by CouchDB reference id (URL kwarg: id, not pk).""" + request: AuthenticatedRequest + def test_func(self): attachment = get_object_or_404(MedicalRecordAttachment, couch_id=self.kwargs.get("id")) return is_attachment_author(self.request.user.profile, attachment) diff --git a/src/ahc/apps/medical_notes/views/type_feeding_notes.py b/src/ahc/apps/medical_notes/views/type_feeding_notes.py index 0d236a1..a84374b 100644 --- a/src/ahc/apps/medical_notes/views/type_feeding_notes.py +++ b/src/ahc/apps/medical_notes/views/type_feeding_notes.py @@ -1,3 +1,7 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + from django.contrib.auth.mixins import LoginRequiredMixin, UserPassesTestMixin from django.http import HttpResponseRedirect from django.shortcuts import get_object_or_404, redirect @@ -17,6 +21,7 @@ notifications_for_feednote, notifications_for_mednote, ) +from ahc.apps.medical_notes.services.feeding import create_feeding_note from ahc.apps.medical_notes.services.notifications import ( create_email_notification, delete_notification, @@ -24,10 +29,14 @@ ) from ahc.apps.medical_notes.views.mixins.user_animal_permisions import AnimalAccessRequiredMixin +if TYPE_CHECKING: + from ahc.types import AuthenticatedRequest + class DietRecordCreateView(LoginRequiredMixin, AnimalAccessRequiredMixin, FormView): template_name = "medical_notes/create.html" form_class = DietRecordForm + request: AuthenticatedRequest def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) @@ -37,13 +46,8 @@ def get_context_data(self, **kwargs): def form_valid(self, form): note_id = self.kwargs.get("pk") related_note = get_object_or_404(MedicalRecord, id=note_id) - - feeding_note = form.save(commit=False) - feeding_note.related_note = related_note - feeding_note.save() - - success_url = reverse("note_related_diets", kwargs={"pk": note_id}) - return redirect(success_url) + create_feeding_note(related_note, form) + return redirect(reverse("note_related_diets", kwargs={"pk": note_id})) class EditDietRecordView(LoginRequiredMixin, UserPassesTestMixin, UpdateView): @@ -51,25 +55,15 @@ class EditDietRecordView(LoginRequiredMixin, UserPassesTestMixin, UpdateView): template_name = "medical_notes/edit.html" form_class = DietRecordForm context_object_name = "note" + request: AuthenticatedRequest def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) context["form_name"] = str(self.form_class.__name__) return context - def form_valid(self, form): - form.save(commit=True) - - email_notification = get_object_or_404(EmailNotification, id=self.kwargs.get("pk")) - feeding_note = email_notification.related_note - medical_record = feeding_note.related_note - - success_url = reverse_lazy("note_related_diets", kwargs={"pk": medical_record.id}) - return redirect(str(success_url)) - def get_success_url(self): - note = get_object_or_404(MedicalRecord, pk=self.kwargs.get("pk")) - return reverse("full_timeline_of_notes", kwargs={"pk": note.animal.id}) + return reverse("note_related_diets", kwargs={"pk": self.object.related_note.id}) def test_func(self): feeding_note = get_object_or_404(FeedingNote, id=self.kwargs.get("pk")) @@ -80,6 +74,7 @@ class FeedingNoteListView(LoginRequiredMixin, UserPassesTestMixin, ListView): model = FeedingNote template_name = "medical_notes/feeding_notes_list.html" context_object_name = "feeding_notes" + request: AuthenticatedRequest def get_queryset(self): medical_record = get_object_or_404(MedicalRecord, pk=self.kwargs.get("pk")) @@ -100,6 +95,7 @@ class CreateNotificationView(LoginRequiredMixin, UserPassesTestMixin, FormView): template_name = "medical_notes/create.html" form_class = NotificationRecordForm success_url = "/" + request: AuthenticatedRequest def get_object(self): return get_object_or_404(FeedingNote, id=self.kwargs.get("pk")) diff --git a/src/ahc/apps/medical_notes/views/type_vaccination_notes.py b/src/ahc/apps/medical_notes/views/type_vaccination_notes.py index 33fc90d..fd249e6 100644 --- a/src/ahc/apps/medical_notes/views/type_vaccination_notes.py +++ b/src/ahc/apps/medical_notes/views/type_vaccination_notes.py @@ -7,6 +7,8 @@ from __future__ import annotations +from typing import TYPE_CHECKING + from django.contrib.auth.mixins import LoginRequiredMixin, UserPassesTestMixin from django.http import HttpResponse from django.shortcuts import get_object_or_404 @@ -15,6 +17,9 @@ from ahc.apps.animals.models import Animal from ahc.apps.animals.selectors import allowed_categories_for, is_animal_owner, user_can_access_animal + +if TYPE_CHECKING: + from ahc.types import AuthenticatedRequest from ahc.apps.medical_notes.forms.type_vaccination_notes import VaccinationNoteForm from ahc.apps.medical_notes.models.type_vaccination_notes import VaccinationNote from ahc.apps.medical_notes.services.vaccinations import ( @@ -41,16 +46,20 @@ def _vaccinations_tab_url(animal_id) -> str: class VaccinationAnimalAccessMixin(LoginRequiredMixin, UserPassesTestMixin): """Access check for views where pk in URL is an Animal UUID.""" + request: AuthenticatedRequest + def test_func(self) -> bool: - animal = get_object_or_404(Animal, id=self.kwargs["pk"]) + animal = get_object_or_404(Animal, id=self.kwargs["pk"]) # type: ignore return _has_vaccination_access(self.request.user.profile, animal) class VaccinationRecordAccessMixin(LoginRequiredMixin, UserPassesTestMixin): """Access check for views where vacc_id in URL is a VaccinationNote UUID.""" + request: AuthenticatedRequest + def _get_vaccination(self) -> VaccinationNote: - return get_object_or_404(VaccinationNote, id=self.kwargs["vacc_id"]) + return get_object_or_404(VaccinationNote, id=self.kwargs["vacc_id"]) # type: ignore def test_func(self) -> bool: vaccination = self._get_vaccination() diff --git a/src/ahc/apps/users/apps.py b/src/ahc/apps/users/apps.py index ab86c76..4149438 100644 --- a/src/ahc/apps/users/apps.py +++ b/src/ahc/apps/users/apps.py @@ -6,4 +6,4 @@ class UsersConfig(AppConfig): name = "ahc.apps.users" def ready(self): - pass + from . import signals # noqa: F401 diff --git a/src/ahc/apps/users/signals.py b/src/ahc/apps/users/signals.py index 6a7f786..70eb8d5 100644 --- a/src/ahc/apps/users/signals.py +++ b/src/ahc/apps/users/signals.py @@ -23,10 +23,10 @@ def create_background(sender, instance, **kwargs): @receiver(post_save, sender=User) def create_profile(sender, instance, created, **kwargs): if created: - _background, _ = ProfileBackground.objects.get_or_create(title="Default Background") Profile.objects.create(user=instance) @receiver(post_save, sender=User) def save_profile(sender, instance, **kwargs): - instance.profile.save() + if hasattr(instance, "profile"): + instance.profile.save() diff --git a/src/ahc/apps/users/tests.py b/src/ahc/apps/users/tests.py index 7984f3e..10ec676 100644 --- a/src/ahc/apps/users/tests.py +++ b/src/ahc/apps/users/tests.py @@ -1,6 +1,9 @@ +from unittest.mock import MagicMock, patch + import pytest from ahc.apps.users.models import Profile +from ahc.apps.users.signals import create_background, create_basic_privilege @pytest.mark.integration @@ -25,3 +28,174 @@ def test_pinned_animals_empty_by_default(self, user_profile): def test_profile_is_unique_per_user(self, user_profile): user, _ = user_profile assert Profile.objects.filter(user=user).count() == 1 + + def test_profile_has_privilege_tier_after_creation(self, user_profile): + _, profile = user_profile + assert profile.privilege_tier is not None + assert profile.privilege_tier.title == "Empty Privilege" + + def test_profile_has_background_after_creation(self, user_profile): + _, profile = user_profile + assert profile.profile_background is not None + assert profile.profile_background.title == "Default Background" + + +@pytest.mark.unit +class TestCreateBasicPrivilegeSignal: + """create_basic_privilege: assigns Empty Privilege when profile has none.""" + + def test_assigns_privilege_when_missing(self): + privilege_mock = MagicMock() + instance = MagicMock(spec=Profile) + instance.privilege_tier = None + + with patch("ahc.apps.users.signals.Privilege.objects.get_or_create", return_value=(privilege_mock, True)): + create_basic_privilege(sender=Profile, instance=instance) + + assert instance.privilege_tier is privilege_mock + + def test_skips_when_privilege_already_set(self): + instance = MagicMock(spec=Profile) + instance.privilege_tier = MagicMock() + + with patch("ahc.apps.users.signals.Privilege.objects.get_or_create") as mock_goc: + create_basic_privilege(sender=Profile, instance=instance) + + mock_goc.assert_not_called() + + +@pytest.mark.unit +class TestCreateBackgroundSignal: + """create_background: assigns Default Background when profile has none.""" + + def test_assigns_background_when_missing(self): + background_mock = MagicMock() + instance = MagicMock(spec=Profile) + instance.profile_background = None + + with patch("ahc.apps.users.signals.ProfileBackground.objects.get_or_create", return_value=(background_mock, True)): + create_background(sender=Profile, instance=instance) + + assert instance.profile_background is background_mock + + def test_skips_when_background_already_set(self): + instance = MagicMock(spec=Profile) + instance.profile_background = MagicMock() + + with patch("ahc.apps.users.signals.ProfileBackground.objects.get_or_create") as mock_goc: + create_background(sender=Profile, instance=instance) + + mock_goc.assert_not_called() + + +@pytest.mark.integration +@pytest.mark.django_db +class TestUserRegisterView: + """UserRegisterView: form rendering and user account creation.""" + + def test_get_renders_registration_form(self): + from django.test import Client + + response = Client().get("/user/register/") + assert response.status_code == 200 + + def test_valid_post_creates_user_and_redirects_to_login(self): + from django.contrib.auth.models import User + from django.test import Client + + mock_img = MagicMock() + mock_img.height = 100 + mock_img.width = 100 + with patch("ahc.apps.users.models.Image.open", return_value=mock_img): + response = Client().post( + "/user/register/", + { + "username": "brandnewuser", + "email": "newuser@example.com", + "password1": "Str0ng_P@ssw0rd!", + "password2": "Str0ng_P@ssw0rd!", + }, + ) + assert response.status_code == 302 + assert User.objects.filter(username="brandnewuser").exists() + + def test_invalid_post_re_renders_form_with_errors(self): + from django.test import Client + + response = Client().post( + "/user/register/", + {"username": "u", "email": "not-an-email", "password1": "abc", "password2": "xyz"}, + ) + assert response.status_code == 200 + + +@pytest.mark.integration +@pytest.mark.django_db +class TestUserProfileView: + """UserProfileView: authenticated profile editing.""" + + def _client_for(self, user): + from django.test import Client + + c = Client() + c.force_login(user) + return c + + def test_unauthenticated_redirects_to_login(self): + from django.test import Client + + response = Client().get("/user/profile/") + assert response.status_code == 302 + + def test_authenticated_get_returns_200(self, user_profile): + user, _ = user_profile + response = self._client_for(user).get("/user/profile/") + assert response.status_code == 200 + + def test_valid_post_updates_username_and_redirects(self, user_profile): + user, _ = user_profile + mock_img = MagicMock() + mock_img.height = 100 + mock_img.width = 100 + with patch("ahc.apps.users.models.Image.open", return_value=mock_img): + response = self._client_for(user).post( + "/user/profile/", {"username": "updatedname", "email": "updated@example.com"} + ) + assert response.status_code == 302 + user.refresh_from_db() + assert user.username == "updatedname" + + +@pytest.mark.integration +@pytest.mark.django_db +class TestShareDefaultsView: + """ShareDefaultsView: default share scope configuration for new keepers.""" + + def _client_for(self, user): + from django.test import Client + + c = Client() + c.force_login(user) + return c + + def test_unauthenticated_redirects_to_login(self): + from django.test import Client + + response = Client().get("/user/share-defaults/") + assert response.status_code == 302 + + def test_authenticated_get_returns_200(self, user_profile): + user, _ = user_profile + response = self._client_for(user).get("/user/share-defaults/") + assert response.status_code == 200 + + def test_valid_post_saves_defaults_and_redirects(self, user_profile): + from ahc.apps.animals.models import ShareDefaults + + user, profile = user_profile + response = self._client_for(user).post("/user/share-defaults/", {"allow_basic": "on", "allow_diet": "on"}) + assert response.status_code == 302 + defaults = ShareDefaults.objects.get(profile=profile) + assert defaults.allow_basic is True + assert defaults.allow_diet is True + assert defaults.allow_vet_contact is False diff --git a/src/ahc/apps/users/views.py b/src/ahc/apps/users/views.py index 9b955be..158fc6a 100644 --- a/src/ahc/apps/users/views.py +++ b/src/ahc/apps/users/views.py @@ -1,3 +1,7 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + from django.contrib import messages from django.contrib.auth.mixins import LoginRequiredMixin from django.shortcuts import redirect @@ -8,6 +12,9 @@ from ahc.apps.users.forms import ProfileUpdateForm, ShareDefaultsForm, UserRegisterForm, UserUpdateForm from ahc.apps.users.models import Profile +if TYPE_CHECKING: + from ahc.types import AuthenticatedRequest + class UserRegisterView(CreateView): form_class = UserRegisterForm @@ -17,12 +24,12 @@ class UserRegisterView(CreateView): def form_valid(self, form): response = super().form_valid(form) messages.success(self.request, f"Account has been created for {form.cleaned_data['username']}!") - Profile.objects.create(user=self.object) return response class UserProfileView(LoginRequiredMixin, UpdateView): model = Profile + request: AuthenticatedRequest form_class = UserUpdateForm template_name = "users/profile.html" success_url = reverse_lazy("profile") @@ -46,6 +53,7 @@ class ShareDefaultsView(LoginRequiredMixin, FormView): template_name = "users/share_defaults.html" form_class = ShareDefaultsForm + request: AuthenticatedRequest def get_form_kwargs(self): kwargs = super().get_form_kwargs() diff --git a/src/ahc/types.py b/src/ahc/types.py new file mode 100644 index 0000000..1bcfbb7 --- /dev/null +++ b/src/ahc/types.py @@ -0,0 +1,36 @@ +"""Custom request types for static type checking only. + +Both _AHCUser and AuthenticatedRequest live exclusively under TYPE_CHECKING +so Django's model metaclass never sees _AHCUser(User) at runtime. + +Usage in views: + + from __future__ import annotations + from typing import TYPE_CHECKING + + if TYPE_CHECKING: + from ahc.types import AuthenticatedRequest + + class MyView(LoginRequiredMixin, ...): + request: AuthenticatedRequest + +The 'from __future__ import annotations' import makes all class-body +annotations lazy (PEP 563), so Python evaluates them only when explicitly +requested — never at class definition time. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from django.contrib.auth.models import User + from django.http import HttpRequest + + from ahc.apps.users.models import Profile + + class _AHCUser(User): + profile: Profile + + class AuthenticatedRequest(HttpRequest): + user: _AHCUser # type: ignore[assignment] diff --git a/src/celery_notifications/config.py b/src/celery_notifications/config.py index 6c87bac..44726a9 100644 --- a/src/celery_notifications/config.py +++ b/src/celery_notifications/config.py @@ -33,6 +33,13 @@ def dispatch_vaccination_reminders(): send_vaccination_reminders() +@celery_obj.task(name="ahc.beat.clean_orphaned_profile_images") +def dispatch_clean_orphaned_profile_images(): + from celery_notifications.cron import clean_orphaned_profile_images + + clean_orphaned_profile_images() + + celery_obj.conf.beat_schedule = { "send-discord-notes-hourly": { "task": "ahc.beat.dispatch_discord_notes", @@ -42,6 +49,10 @@ def dispatch_vaccination_reminders(): "task": "ahc.beat.dispatch_vaccination_reminders", "schedule": crontab(hour=8, minute=0), }, + "clean-orphaned-profile-images-daily": { + "task": "ahc.beat.clean_orphaned_profile_images", + "schedule": crontab(hour=3, minute=0), + }, } diff --git a/src/celery_notifications/cron.py b/src/celery_notifications/cron.py index 24597e7..d304546 100644 --- a/src/celery_notifications/cron.py +++ b/src/celery_notifications/cron.py @@ -147,6 +147,30 @@ def send_discord_notes(): send_discord_notifications.apply_async(kwargs={"user_id": user_id, "user_message": user_message}, countdown=delay) +@log_exceptions_and_notifications +def clean_orphaned_profile_images() -> None: + """Delete animal profile images with no corresponding Animal row. + + Runs as a daily Celery Beat task. Replaces the former post_save / post_delete + full directory-scan signals on Animal and Profile that ran O(N images) on every + write. Now runs once per day at 03:00 UTC. + """ + import os + from pathlib import Path + + from django.conf import settings + + from ahc.apps.animals.models import Animal + + animals_media_dir = Path(settings.MEDIA_ROOT) / "profile_pics" / "animals" + if not animals_media_dir.is_dir(): + return + live = {str(p).split("/")[-1] for p in Animal.objects.exclude(profile_image="").values_list("profile_image", flat=True)} + for image_name in os.listdir(animals_media_dir): + if image_name not in live: + (animals_media_dir / image_name).unlink(missing_ok=True) + + @log_exceptions_and_notifications def send_vaccination_reminders() -> None: """Send Discord reminders for vaccinations whose reminder_date is today or overdue.