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..0071303 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,6 +25,7 @@ dependencies = [ "python-dateutil", "pyjwt", "defusedxml", + "aiohttp>=3", ] [dependency-groups] @@ -98,16 +99,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/forms.py b/src/ahc/apps/animals/forms.py index ee9e217..e5f5bad 100644 --- a/src/ahc/apps/animals/forms.py +++ b/src/ahc/apps/animals/forms.py @@ -24,7 +24,10 @@ def __init__(self, *args, **kwargs): def clean_full_name(self): full_name = self.cleaned_data.get("full_name") - if Animal.objects.filter(Q(full_name=full_name) & (Q(owner=self.user) | Q(allowed_users=self.user))).exists(): + # Only check living animals so a deceased pet's name can be reused for a new one. + if Animal.objects.filter( + Q(full_name=full_name) & (Q(owner=self.user) | Q(allowed_users=self.user)) & Q(date_of_death__isnull=True) + ).exists(): raise forms.ValidationError("An animal with that name is already in your care.") return full_name diff --git a/src/ahc/apps/animals/migrations/0006_animal_date_of_death_animal_memorial_note.py b/src/ahc/apps/animals/migrations/0006_animal_date_of_death_animal_memorial_note.py new file mode 100644 index 0000000..d144066 --- /dev/null +++ b/src/ahc/apps/animals/migrations/0006_animal_date_of_death_animal_memorial_note.py @@ -0,0 +1,22 @@ +# Generated by Django 6.0.5 on 2026-06-04 19:46 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("animals", "0005_add_vaccination_share_category"), + ] + + operations = [ + migrations.AddField( + model_name="animal", + name="date_of_death", + field=models.DateField(blank=True, default=None, null=True), + ), + migrations.AddField( + model_name="animal", + name="memorial_note", + field=models.CharField(blank=True, default=None, max_length=2500, null=True), + ), + ] 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/models.py b/src/ahc/apps/animals/models.py index f6f6552..8462e61 100644 --- a/src/ahc/apps/animals/models.py +++ b/src/ahc/apps/animals/models.py @@ -54,6 +54,14 @@ class Animal(models.Model): sex = models.CharField(max_length=1, choices=Sex.choices, default=None, blank=True, null=True) sterilization = models.BooleanField(default=False) + date_of_death = models.DateField(default=None, blank=True, null=True) + memorial_note = models.CharField(max_length=2500, default=None, blank=True, null=True) + + @property + def is_deceased(self) -> bool: + """Return True if a date of death has been recorded for this animal.""" + return self.date_of_death is not None + class AnimalShare(models.Model): """Through model for Animal.allowed_users — stores per-share access scope and expiry.""" diff --git a/src/ahc/apps/animals/selectors.py b/src/ahc/apps/animals/selectors.py index 38e2d7d..7ccc8cf 100644 --- a/src/ahc/apps/animals/selectors.py +++ b/src/ahc/apps/animals/selectors.py @@ -12,10 +12,15 @@ def _today() -> date: def animals_visible_to(profile) -> QuerySet[Animal]: - """Return all animals accessible to the given profile (owner or active keeper).""" + """Return all living animals accessible to the given profile (owner or active keeper). + + Deceased animals are excluded unconditionally — they are only accessible to the + owner via deceased_animals_for(). + """ today = _today() return ( - Animal.objects.filter( + Animal.objects.filter(date_of_death__isnull=True) + .filter( Q(owner=profile) | Q(shares__carer=profile, shares__valid_until__isnull=True) | Q(shares__carer=profile, shares__valid_until__gte=today) @@ -25,6 +30,14 @@ def animals_visible_to(profile) -> QuerySet[Animal]: ) +def deceased_animals_for(profile) -> QuerySet[Animal]: + """Return deceased animals owned by the profile, ordered by most recently deceased first. + + Carers are intentionally excluded: death withdraws management to the owner only. + """ + return Animal.objects.filter(owner=profile, date_of_death__isnull=False).order_by("-date_of_death") + + def active_share_for(profile, animal: Animal) -> AnimalShare | None: """Return the non-expired AnimalShare for this profile/animal pair, or None.""" today = _today() @@ -35,13 +48,41 @@ def active_share_for(profile, animal: Animal) -> AnimalShare | None: return share if share.is_active(today) else None -def user_can_access_animal(profile, animal: Animal) -> bool: - """Return True if the profile is the owner or holds an active (non-expired) share.""" +def user_can_view_animal(profile, animal: Animal) -> bool: + """Return True if the profile may view this animal (read-only access). + + The owner may always view — including deceased animals (read-only archive). + Carers may only view a living animal with an active, non-expired share. + """ if animal.owner == profile: return True + if animal.date_of_death is not None: + return False # death withdraws all non-owner access return active_share_for(profile, animal) is not None +def user_can_modify_animal(profile, animal: Animal) -> bool: + """Return True if the profile may write to this animal or its records. + + No writes are allowed on a deceased animal — not even by the owner. + The only permitted mutations on a deceased animal (editing the memorial note and + un-archiving) use is_animal_owner directly, bypassing this predicate by design. + """ + if animal.date_of_death is not None: + return False + if animal.owner == profile: + return True + return active_share_for(profile, animal) is not None + + +def user_can_access_animal(profile, animal: Animal) -> bool: + """Alias for user_can_view_animal kept for backward compatibility. + + Prefer user_can_view_animal (read contexts) or user_can_modify_animal (write contexts). + """ + return user_can_view_animal(profile, animal) + + def is_animal_owner(profile, animal: Animal) -> bool: """Return True if the profile is the owner of the animal.""" return animal.owner == profile @@ -51,11 +92,14 @@ def allowed_categories_for(profile, animal: Animal) -> set[str]: """Return the set of ShareCategory values the profile may see. Owners get all categories. Carers get only what their active share grants. + Carers always get an empty set on a deceased animal (death withdraws all carer access). An empty set means no data-category access (animal page itself still blocked - upstream by user_can_access_animal). + upstream by user_can_view_animal). """ if is_animal_owner(profile, animal): return {c.value for c in ShareCategory} + if animal.date_of_death is not None: + return set() share = active_share_for(profile, animal) if share is None: return set() @@ -68,6 +112,17 @@ def get_or_create_share_defaults(profile) -> ShareDefaults: return defaults +def animals_for_biometric_batch(profile) -> QuerySet[Animal]: + """Return all animals the profile may include in a batch biometric session. + + Currently mirrors animals_visible_to (owner or active share with any access), + matching the permission level of the single-record BiometricRecordCreateView. + TODO: narrow to allow_biometrics=True for carer shares once that flag is enforced + consistently across single-record creation too. + """ + return animals_visible_to(profile) + + def is_pinned(profile, animal: Animal) -> bool: """Return True if the animal is currently pinned by the given profile.""" return profile.pinned_animals.filter(pk=animal.pk).exists() @@ -78,3 +133,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/services.py b/src/ahc/apps/animals/services.py index 6269940..413ed0e 100644 --- a/src/ahc/apps/animals/services.py +++ b/src/ahc/apps/animals/services.py @@ -130,3 +130,32 @@ def set_animal_details( def remove_keeper(animal: Animal, keeper_id) -> None: """Remove a keeper from the animal's shares by Profile PK.""" AnimalShare.objects.filter(animal=animal, carer_id=keeper_id).delete() + + +def set_deceased(animal: Animal, date_of_death: date, memorial_note: str | None) -> None: + """Record an animal as deceased. + + AnimalShare rows are intentionally left intact (soft withdrawal). The deceased + gate in the selectors makes all shares inert while date_of_death is set, and + un-archiving with unset_deceased instantly restores the prior access configuration. + """ + animal.date_of_death = date_of_death + animal.memorial_note = memorial_note + animal.save() + + +def set_memorial_note(animal: Animal, memorial_note: str | None) -> None: + """Update the memorial note on a deceased animal without changing the death date.""" + animal.memorial_note = memorial_note + animal.save() + + +def unset_deceased(animal: Animal) -> None: + """Reverse an archiving action — the animal becomes living again. + + The memorial_note is preserved so that re-archiving retains historical context. + Existing AnimalShare rows are immediately effective again because the deceased gate + only checks date_of_death__isnull. + """ + animal.date_of_death = None + animal.save() 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/templates/animals/all_animals_stable.html b/src/ahc/apps/animals/templates/animals/all_animals_stable.html index ef61539..4fed3e1 100644 --- a/src/ahc/apps/animals/templates/animals/all_animals_stable.html +++ b/src/ahc/apps/animals/templates/animals/all_animals_stable.html @@ -21,6 +21,7 @@

All pets:

Operations:

Add animal + In memoriam archive
{% endif %} diff --git a/src/ahc/apps/animals/templates/animals/archive.html b/src/ahc/apps/animals/templates/animals/archive.html new file mode 100644 index 0000000..a207504 --- /dev/null +++ b/src/ahc/apps/animals/templates/animals/archive.html @@ -0,0 +1,29 @@ +{% extends 'homepage/base.html' %} +{% load static %} +{% block extra_css %} + +{% endblock %} +{% block content %} + + {% if user.is_authenticated %} + +
+

In memoriam

+ {% if animals %} +
+ {% for animal in animals %} + {% include "partials/animal_card.html" %} + {% endfor %} +
+ {% else %} +

No archived animals.

+ {% endif %} +
+ +
+ Back to all pets +
+ + {% endif %} + +{% endblock %} diff --git a/src/ahc/apps/animals/templates/animals/change_deceased.html b/src/ahc/apps/animals/templates/animals/change_deceased.html new file mode 100644 index 0000000..3f6ea98 --- /dev/null +++ b/src/ahc/apps/animals/templates/animals/change_deceased.html @@ -0,0 +1,16 @@ +{% extends "homepage/base.html" %} +{% load static %} + +{% block content %} +
+

Archive animal as deceased

+

Recording a date of death will archive this animal. Carers will lose access immediately. You can restore the animal at any time from the profile page.

+
+ {% csrf_token %} + {% include "partials/form_fields.html" %} + +
+
+ Back to Settings +
+{% endblock %} diff --git a/src/ahc/apps/animals/templates/animals/change_memorial_note.html b/src/ahc/apps/animals/templates/animals/change_memorial_note.html new file mode 100644 index 0000000..1d2da1b --- /dev/null +++ b/src/ahc/apps/animals/templates/animals/change_memorial_note.html @@ -0,0 +1,15 @@ +{% extends "homepage/base.html" %} +{% load static %} + +{% block content %} +
+

Edit memorial note

+
+ {% csrf_token %} + {% include "partials/form_fields.html" %} + +
+
+ Back to profile +
+{% endblock %} diff --git a/src/ahc/apps/animals/templates/animals/profile.html b/src/ahc/apps/animals/templates/animals/profile.html index 6482dd2..8d17595 100644 --- a/src/ahc/apps/animals/templates/animals/profile.html +++ b/src/ahc/apps/animals/templates/animals/profile.html @@ -38,12 +38,37 @@ {% block content %}
+ {% if is_deceased %} +
+
+

In memoriam — {{ animal.date_of_death }}

+ {% if animal.memorial_note %}

{{ animal.memorial_note }}

{% endif %} +
+ {% if is_owner %} +
+ Edit memorial note +
+ {% csrf_token %} + +
+
+ {% endif %} +
+ {% endif %} +
+ {% if is_deceased %} +
+ Animal's profile picture +
+ {% else %} Animal's profile picture + {% endif %}

{{ animal.full_name }}

diff --git a/src/ahc/apps/animals/templates/animals/tabs/_settings.html b/src/ahc/apps/animals/templates/animals/tabs/_settings.html index ee0031e..9f71c60 100644 --- a/src/ahc/apps/animals/templates/animals/tabs/_settings.html +++ b/src/ahc/apps/animals/templates/animals/tabs/_settings.html @@ -23,7 +23,10 @@
Records
Danger zone
- Remove this animal from the files +
+ Archive animal as deceased + Remove this animal from the files +
diff --git a/src/ahc/apps/animals/tests.py b/src/ahc/apps/animals/tests.py index 339df85..5b25c81 100644 --- a/src/ahc/apps/animals/tests.py +++ b/src/ahc/apps/animals/tests.py @@ -6,10 +6,13 @@ from ahc.apps.animals.models import Animal from ahc.apps.animals.selectors import ( animals_visible_to, + deceased_animals_for, is_animal_owner, is_pinned, recent_records_for, user_can_access_animal, + user_can_modify_animal, + user_can_view_animal, ) from ahc.apps.animals.services import ( add_keeper, @@ -18,9 +21,12 @@ process_profile_image, remove_keeper, set_birthday, + set_deceased, set_first_contact, + set_memorial_note, transfer_ownership, unpin_animal, + unset_deceased, ) from ahc.apps.animals.signals import update_allowed_users @@ -112,6 +118,7 @@ def test_keeper_can_access(self): profile = MagicMock() animal = MagicMock() animal.owner = MagicMock() + animal.date_of_death = None # living animal — carer access applies with patch("ahc.apps.animals.selectors.active_share_for", return_value=MagicMock()): assert user_can_access_animal(profile, animal) is True @@ -119,6 +126,7 @@ def test_stranger_cannot_access(self): profile = MagicMock() animal = MagicMock() animal.owner = MagicMock() + animal.date_of_death = None # living animal — no share with patch("ahc.apps.animals.selectors.active_share_for", return_value=None): assert user_can_access_animal(profile, animal) is False @@ -499,3 +507,773 @@ 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"] + + +@pytest.mark.unit +class TestIsDeceasedProperty: + """Animal.is_deceased: reflects date_of_death presence.""" + + def test_false_when_no_date_of_death(self): + animal = Animal(full_name="Live") + assert animal.is_deceased is False + + def test_true_when_date_of_death_set(self): + animal = Animal(full_name="Gone", date_of_death=date(2024, 1, 1)) + assert animal.is_deceased is True + + +@pytest.mark.unit +class TestDeceasedPredicates: + """user_can_view_animal / user_can_modify_animal behaviour on deceased animals.""" + + def _make_animal(self, owner, deceased=False): + animal = MagicMock(spec=Animal) + animal.owner = owner + animal.date_of_death = date(2024, 1, 1) if deceased else None + return animal + + def test_owner_can_view_living_animal(self): + profile = MagicMock() + animal = self._make_animal(owner=profile, deceased=False) + assert user_can_view_animal(profile, animal) is True + + def test_owner_can_view_deceased_animal(self): + profile = MagicMock() + animal = self._make_animal(owner=profile, deceased=True) + assert user_can_view_animal(profile, animal) is True + + def test_owner_cannot_modify_deceased_animal(self): + profile = MagicMock() + animal = self._make_animal(owner=profile, deceased=True) + assert user_can_modify_animal(profile, animal) is False + + def test_owner_can_modify_living_animal(self): + profile = MagicMock() + animal = self._make_animal(owner=profile, deceased=False) + assert user_can_modify_animal(profile, animal) is True + + def test_carer_cannot_view_deceased_animal(self): + profile = MagicMock() + owner = MagicMock() + animal = self._make_animal(owner=owner, deceased=True) + assert user_can_view_animal(profile, animal) is False + + def test_carer_cannot_modify_deceased_animal(self): + profile = MagicMock() + owner = MagicMock() + animal = self._make_animal(owner=owner, deceased=True) + assert user_can_modify_animal(profile, animal) is False + + +@pytest.mark.integration +@pytest.mark.django_db +class TestDeceasedSelectors: + """animals_visible_to / deceased_animals_for on deceased animals.""" + + @pytest.fixture + def deceased_animal(self, db, user_profile): + _, profile = user_profile + return Animal.objects.create(full_name="Passed", owner=profile, date_of_death=date(2024, 3, 15)) + + def test_animals_visible_to_excludes_deceased_for_owner(self, deceased_animal, user_profile): + _, profile = user_profile + assert deceased_animal not in animals_visible_to(profile) + + def test_animals_visible_to_excludes_deceased_for_carer(self, deceased_animal, second_user_profile, user_profile): + from ahc.apps.animals.models import AnimalShare + + _, carer_profile = second_user_profile + AnimalShare.objects.create(animal=deceased_animal, carer=carer_profile) + assert deceased_animal not in animals_visible_to(carer_profile) + + def test_deceased_animals_for_returns_owners_deceased(self, deceased_animal, user_profile): + _, profile = user_profile + assert deceased_animal in deceased_animals_for(profile) + + def test_deceased_animals_for_excludes_carers_deceased(self, deceased_animal, second_user_profile): + _, other_profile = second_user_profile + assert deceased_animal not in deceased_animals_for(other_profile) + + def test_deceased_animals_for_excludes_living(self, animal, user_profile): + _, profile = user_profile + assert animal not in deceased_animals_for(profile) + + +@pytest.mark.integration +@pytest.mark.django_db +class TestDeceasedServices: + """set_deceased / set_memorial_note / unset_deceased services.""" + + def test_set_deceased_records_date_and_note(self, animal): + set_deceased(animal, date_of_death=date(2024, 5, 1), memorial_note="Rest in peace") + animal.refresh_from_db() + assert animal.date_of_death == date(2024, 5, 1) + assert animal.memorial_note == "Rest in peace" + assert animal.is_deceased is True + + def test_set_deceased_does_not_delete_shares(self, animal, second_user_profile): + from ahc.apps.animals.models import AnimalShare + + _, carer_profile = second_user_profile + share = AnimalShare.objects.create(animal=animal, carer=carer_profile) + set_deceased(animal, date_of_death=date(2024, 5, 1), memorial_note=None) + assert AnimalShare.objects.filter(pk=share.pk).exists() + + def test_unset_deceased_clears_date_and_restores_visibility(self, animal, user_profile, second_user_profile): + from ahc.apps.animals.models import AnimalShare + + _, carer_profile = second_user_profile + AnimalShare.objects.create(animal=animal, carer=carer_profile) + set_deceased(animal, date_of_death=date(2024, 5, 1), memorial_note="Farewell") + assert animal not in animals_visible_to(carer_profile) + + unset_deceased(animal) + animal.refresh_from_db() + assert animal.date_of_death is None + assert animal.memorial_note == "Farewell" # preserved after un-archive + assert animal in animals_visible_to(carer_profile) + + def test_set_memorial_note_updates_note_only(self, animal): + set_deceased(animal, date_of_death=date(2024, 5, 1), memorial_note="Original") + set_memorial_note(animal, memorial_note="Updated") + animal.refresh_from_db() + assert animal.memorial_note == "Updated" + assert animal.date_of_death == date(2024, 5, 1) + + +@pytest.mark.integration +@pytest.mark.django_db +class TestMarkDeceasedView: + """MarkDeceasedView: owner can archive; carer cannot.""" + + 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}/deceased/") + assert response.status_code == 200 + + def test_carer_get_returns_403(self, animal, second_user_profile): + other_user, _ = second_user_profile + response = self._client_for(other_user).get(f"/pet/{animal.id}/deceased/") + assert response.status_code == 403 + + def test_owner_post_archives_and_redirects(self, animal, user_profile): + user, _ = user_profile + response = self._client_for(user).post( + f"/pet/{animal.id}/deceased/", + {"date_of_death": "2024-04-01", "memorial_note": "Goodbye"}, + ) + assert response.status_code == 302 + animal.refresh_from_db() + assert animal.date_of_death == date(2024, 4, 1) + assert animal.memorial_note == "Goodbye" + + def test_future_date_rejected(self, animal, user_profile): + user, _ = user_profile + response = self._client_for(user).post( + f"/pet/{animal.id}/deceased/", + {"date_of_death": "2099-12-31"}, + ) + assert response.status_code == 200 # form re-render with error + + +@pytest.mark.integration +@pytest.mark.django_db +class TestUnarchiveAnimalView: + """UnarchiveAnimalView: owner can un-archive; carer cannot.""" + + @pytest.fixture + def deceased_animal(self, db, user_profile): + _, profile = user_profile + return Animal.objects.create(full_name="Passed", owner=profile, date_of_death=date(2024, 3, 15)) + + def _client_for(self, user): + from django.test import Client + + c = Client() + c.force_login(user) + return c + + def test_owner_post_restores_animal(self, deceased_animal, user_profile): + user, _ = user_profile + response = self._client_for(user).post(f"/pet/{deceased_animal.id}/unarchive/") + assert response.status_code == 302 + deceased_animal.refresh_from_db() + assert deceased_animal.date_of_death is None + + def test_carer_post_returns_403(self, deceased_animal, second_user_profile): + other_user, _ = second_user_profile + response = self._client_for(other_user).post(f"/pet/{deceased_animal.id}/unarchive/") + assert response.status_code == 403 + + +@pytest.mark.integration +@pytest.mark.django_db +class TestAnimalProfileViewDeceased: + """AnimalProfileDetailView / AnimalTabView gate on deceased animals.""" + + @pytest.fixture + def deceased_animal(self, db, user_profile): + _, profile = user_profile + return Animal.objects.create(full_name="Passed", owner=profile, date_of_death=date(2024, 3, 15)) + + def _client_for(self, user): + from django.test import Client + + c = Client() + c.force_login(user) + return c + + def test_owner_can_view_deceased_profile(self, deceased_animal, user_profile): + user, _ = user_profile + response = self._client_for(user).get(f"/pet/{deceased_animal.id}/") + assert response.status_code == 200 + + def test_carer_blocked_on_deceased_profile(self, deceased_animal, second_user_profile, user_profile): + from ahc.apps.animals.models import AnimalShare + + other_user, carer_profile = second_user_profile + AnimalShare.objects.create(animal=deceased_animal, carer=carer_profile) + response = self._client_for(other_user).get(f"/pet/{deceased_animal.id}/") + assert response.status_code == 403 + + def test_owner_blocked_from_settings_tab_on_deceased(self, deceased_animal, user_profile): + user, _ = user_profile + response = self._client_for(user).get(f"/pet/{deceased_animal.id}/tab/settings/") + assert response.status_code == 403 + + +@pytest.mark.integration +@pytest.mark.django_db +class TestStableAndArchiveViews: + """StableView excludes deceased; ArchiveView shows only owner's deceased.""" + + @pytest.fixture + def deceased_animal(self, db, user_profile): + _, profile = user_profile + return Animal.objects.create(full_name="Passed", owner=profile, date_of_death=date(2024, 3, 15)) + + def _client_for(self, user): + from django.test import Client + + c = Client() + c.force_login(user) + return c + + def test_stable_view_excludes_deceased(self, animal, deceased_animal, user_profile): + user, _ = user_profile + response = self._client_for(user).get("/pet/animals/") + assert response.status_code == 200 + assert animal in response.context["animals"] + assert deceased_animal not in response.context["animals"] + + def test_archive_view_includes_deceased(self, deceased_animal, user_profile): + user, _ = user_profile + response = self._client_for(user).get("/pet/archive/") + assert response.status_code == 200 + assert deceased_animal in response.context["animals"] + + def test_archive_view_excludes_living(self, animal, user_profile): + user, _ = user_profile + response = self._client_for(user).get("/pet/archive/") + assert animal not in response.context["animals"] + + def test_archive_view_excludes_other_owners_deceased(self, deceased_animal, second_user_profile): + other_user, _ = second_user_profile + response = self._client_for(other_user).get("/pet/archive/") + assert deceased_animal not in response.context["animals"] diff --git a/src/ahc/apps/animals/urls.py b/src/ahc/apps/animals/urls.py index c9fb55b..9a8cbdc 100644 --- a/src/ahc/apps/animals/urls.py +++ b/src/ahc/apps/animals/urls.py @@ -22,6 +22,10 @@ path("/details/", animal_owner_views.ChangeAnimalDetailsView.as_view(), name="animal_details"), path("/keepers//remove/", animal_owner_views.RemoveKeeperView.as_view(), name="remove_keeper"), path("/keepers//access/", animal_owner_views.EditShareView.as_view(), name="edit_share"), + path("/deceased/", animal_owner_views.MarkDeceasedView.as_view(), name="animal_deceased"), + path("/memorial/", animal_owner_views.EditMemorialNoteView.as_view(), name="animal_memorial"), + path("/unarchive/", animal_owner_views.UnarchiveAnimalView.as_view(), name="animal_unarchive"), path("animals/", animal_views.StableView.as_view(), name="animals_stable"), + path("archive/", animal_views.ArchiveView.as_view(), name="animals_archive"), path("pinned-animals/", animal_views.ToPinAnimalsView.as_view(), name="pinned_animals"), ] diff --git a/src/ahc/apps/animals/utils_owner/forms.py b/src/ahc/apps/animals/utils_owner/forms.py index ed48e89..818cb79 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.") @@ -177,3 +176,30 @@ class ChangeAnimalDetailsForm(forms.ModelForm): class Meta: model = Animal fields = ["species", "breed", "sex", "sterilization"] + + +class MarkDeceasedForm(forms.ModelForm): + class Meta: + model = Animal + fields = ["date_of_death", "memorial_note"] + widgets = { + "date_of_death": forms.DateInput(attrs={"type": "date"}), + "memorial_note": forms.Textarea(attrs={"rows": 6, "cols": 2}), + } + + def clean_date_of_death(self): + dod = self.cleaned_data.get("date_of_death") + if dod is None: + raise forms.ValidationError("Date of death is required to archive an animal.") + if dod > date.today(): + raise forms.ValidationError("Date of death cannot be set in the future.") + return dod + + +class EditMemorialNoteForm(forms.ModelForm): + class Meta: + model = Animal + fields = ["memorial_note"] + widgets = { + "memorial_note": forms.Textarea(attrs={"rows": 8, "cols": 2}), + } diff --git a/src/ahc/apps/animals/utils_owner/views.py b/src/ahc/apps/animals/utils_owner/views.py index 1e01b20..735a905 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 @@ -12,10 +16,13 @@ remove_keeper, set_animal_details, set_birthday, + set_deceased, set_dietary_restrictions, set_first_contact, + set_memorial_note, set_next_visit, transfer_ownership, + unset_deceased, update_share, ) from ahc.apps.animals.utils_owner.forms import ( @@ -25,11 +32,16 @@ ChangeFirstContactForm, ChangeNextVisitForm, ChangeOwnerForm, + EditMemorialNoteForm, EditShareForm, ImageUploadForm, ManageKeepersForm, + MarkDeceasedForm, ) +if TYPE_CHECKING: + from ahc.types import AuthenticatedRequest + class AnimalDeleteView(LoginRequiredMixin, UserPassesOwnershipTestMixin, DeleteView): model = Animal @@ -67,6 +79,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 +114,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 @@ -281,3 +294,73 @@ def form_valid(self, form): } update_share(form.instance, scope=scope, valid_until=cd.get("valid_until")) return redirect(reverse("animal_tab", kwargs={"pk": self.kwargs["pk"], "slug": "ownership"})) + + +class MarkDeceasedView(LoginRequiredMixin, UserPassesOwnershipTestMixin, FormView): + """Record the animal as deceased and store an optional memorial note. + + Uses UserPassesOwnershipTestMixin (which checks is_animal_owner directly) so this + view works even when the animal is already marked deceased, allowing owners to + correct the date or update the memorial note. + """ + + form_class = MarkDeceasedForm + template_name = "animals/change_deceased.html" + + def get_form_kwargs(self): + kwargs = super().get_form_kwargs() + kwargs["instance"] = get_object_or_404(Animal, pk=self.kwargs["pk"]) + return kwargs + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["animal_id"] = self.kwargs["pk"] + return context + + def form_valid(self, form): + set_deceased( + get_object_or_404(Animal, pk=self.kwargs["pk"]), + date_of_death=form.cleaned_data["date_of_death"], + memorial_note=form.cleaned_data["memorial_note"], + ) + return redirect(reverse("animal_profile", kwargs={"pk": self.kwargs["pk"]})) + + +class EditMemorialNoteView(LoginRequiredMixin, UserPassesOwnershipTestMixin, FormView): + """Edit the memorial note on a deceased animal (owner-only). + + Intentionally bypasses user_can_modify_animal so the owner may update the + memorial note while the animal remains in the archived/deceased state. + """ + + form_class = EditMemorialNoteForm + template_name = "animals/change_memorial_note.html" + + def get_form_kwargs(self): + kwargs = super().get_form_kwargs() + kwargs["instance"] = get_object_or_404(Animal, pk=self.kwargs["pk"]) + return kwargs + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["animal_id"] = self.kwargs["pk"] + return context + + def form_valid(self, form): + set_memorial_note( + get_object_or_404(Animal, pk=self.kwargs["pk"]), + memorial_note=form.cleaned_data["memorial_note"], + ) + return redirect(reverse("animal_profile", kwargs={"pk": self.kwargs["pk"]})) + + +class UnarchiveAnimalView(LoginRequiredMixin, UserPassesOwnershipTestMixin, View): + """Reverse an archiving action — the animal becomes living again (owner-only, POST). + + Uses UserPassesOwnershipTestMixin so this view works on a deceased animal. + Bypasses user_can_modify_animal by design: this is the intentional reversal path. + """ + + def post(self, request, pk, *args, **kwargs): + unset_deceased(get_object_or_404(Animal, pk=pk)) + return redirect(reverse("animal_profile", kwargs={"pk": pk})) diff --git a/src/ahc/apps/animals/views.py b/src/ahc/apps/animals/views.py index 85ad176..93d4056 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 @@ -19,12 +19,16 @@ from ahc.apps.animals.selectors import ( allowed_categories_for, animals_visible_to, + deceased_animals_for, is_animal_owner, is_pinned, - user_can_access_animal, + user_can_view_animal, ) 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 +174,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]: @@ -251,10 +255,13 @@ def _base_profile_context(request, animal: Animal) -> dict[str, Any]: profile = request.user.profile owner = is_animal_owner(profile, animal) allowed = allowed_categories_for(profile, animal) + deceased = animal.is_deceased def _tab_visible(tab: Tab) -> bool: + # Owner-only mutation tabs (settings, ownership) are hidden for deceased animals — + # the animal is read-only; the owner's only permitted actions are on the profile page. if tab.owner_only: - return owner + return owner and not deceased if not tab.categories: return True return owner or bool(tab.categories & allowed) @@ -262,6 +269,7 @@ def _tab_visible(tab: Tab) -> bool: return { "now": timezone.now().date(), "is_owner": owner, + "is_deceased": deceased, "is_pinned": is_pinned(profile, animal), "allowed_categories": allowed, "tabs": [t for t in TABS_LIST if _tab_visible(t)], @@ -272,6 +280,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 +296,7 @@ class AnimalProfileDetailView(LoginRequiredMixin, UserPassesTestMixin, DetailVie """ model = Animal + request: AuthenticatedRequest template_name = "animals/profile.html" context_object_name = "animal" @@ -301,7 +311,7 @@ def get_context_data(self, **kwargs): def test_func(self): animal = self.get_object() - return user_can_access_animal(self.request.user.profile, animal) + return user_can_view_animal(self.request.user.profile, animal) class AnimalTabView(LoginRequiredMixin, UserPassesTestMixin, DetailView): @@ -314,6 +324,7 @@ class AnimalTabView(LoginRequiredMixin, UserPassesTestMixin, DetailView): model = Animal context_object_name = "animal" + request: AuthenticatedRequest def _get_tab(self) -> Tab: slug = self.kwargs.get("slug", "") @@ -325,13 +336,15 @@ def _get_tab(self) -> Tab: def test_func(self): animal = self.get_object() profile = self.request.user.profile - if not user_can_access_animal(profile, animal): + if not user_can_view_animal(profile, animal): return False tab = TAB_REGISTRY.get(self.kwargs.get("slug", "")) if tab is None: return True + # Owner-only tabs (settings, ownership) are blocked on deceased animals to enforce + # the read-only archive mode. The owner's only write actions are on the profile page. if tab.owner_only: - return is_animal_owner(profile, animal) + return is_animal_owner(profile, animal) and not animal.is_deceased if tab.categories and not is_animal_owner(profile, animal): allowed = allowed_categories_for(profile, animal) return bool(tab.categories & allowed) @@ -358,6 +371,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) @@ -365,7 +379,25 @@ def get_context_data(self, **kwargs): return context +class ArchiveView(LoginRequiredMixin, TemplateView): + """Owner-only read-only archive of deceased animals. + + Only animals owned by the current user are shown — carers are intentionally + excluded because death withdraws management to the owner only. + """ + + template_name = "animals/archive.html" + request: AuthenticatedRequest + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["animals"] = deceased_animals_for(self.request.user.profile) + return context + + 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/migrations/0005_alter_profilebackground_content.py b/src/ahc/apps/homepage/migrations/0005_alter_profilebackground_content.py new file mode 100644 index 0000000..9d69f22 --- /dev/null +++ b/src/ahc/apps/homepage/migrations/0005_alter_profilebackground_content.py @@ -0,0 +1,17 @@ +# Generated by Django 6.0.5 on 2026-06-04 19:46 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("homepage", "0004_delete_cronjob"), + ] + + operations = [ + migrations.AlterField( + model_name="profilebackground", + name="content", + field=models.ImageField(blank=True, default="", upload_to="static/media/background"), + ), + ] 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/templates/homepage/homepage.html b/src/ahc/apps/homepage/templates/homepage/homepage.html index e675408..1df47ca 100644 --- a/src/ahc/apps/homepage/templates/homepage/homepage.html +++ b/src/ahc/apps/homepage/templates/homepage/homepage.html @@ -16,6 +16,7 @@

Operations:

Manage all animals Add animal + Batch measurements

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/homepage/views.py b/src/ahc/apps/homepage/views.py index e100c62..de0c7f1 100644 --- a/src/ahc/apps/homepage/views.py +++ b/src/ahc/apps/homepage/views.py @@ -1,7 +1,6 @@ -from django.db.models import Q from django.views.generic import TemplateView -from ahc.apps.animals.models import Animal +from ahc.apps.animals.selectors import animals_visible_to from ahc.apps.users.models import Profile as UserProfile @@ -17,17 +16,12 @@ def get_context_data(self, **kwargs): return context user_query = UserProfile.objects.get(user=user) + profile = user.profile if user_query.allow_recennt_animals_list: - pinned_animals_query = user_query.pinned_animals.all() - - context["pinned_animals"] = pinned_animals_query - - if user_query.allow_recennt_animals_list: - recent_created_animals_query = Animal.objects.filter( - Q(owner=self.request.user.profile) | Q(allowed_users=self.request.user.profile) - ).order_by("-creation_date")[:3] - - context["recent_animals"] = recent_created_animals_query + # Deceased animals are excluded: pinned_animals may contain a now-deceased + # animal so we filter explicitly; animals_visible_to already excludes deceased. + context["pinned_animals"] = user_query.pinned_animals.filter(date_of_death__isnull=True) + context["recent_animals"] = animals_visible_to(profile).order_by("-creation_date")[:3] return context 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/forms/biometric_batch.py b/src/ahc/apps/medical_notes/forms/biometric_batch.py new file mode 100644 index 0000000..87ee7d3 --- /dev/null +++ b/src/ahc/apps/medical_notes/forms/biometric_batch.py @@ -0,0 +1,74 @@ +"""Forms for the batch biometric measurement entry screen.""" + +from __future__ import annotations + +from django import forms +from django.forms import formset_factory + +from ahc.apps.medical_notes.forms.type_measurement_notes import BiometricRecordForm + + +class BiometricBatchSessionForm(forms.Form): + """Controls the measurement type and unit for an entire batch session. + + A single session covers one record_type (weight / height / custom) applied + to all animals on the screen. The optional `unit` field overrides the model + default for weight ("g") and height ("mm"). Custom measurements require both + `custom_name` and `custom_unit`. + """ + + record_type = forms.ChoiceField(choices=BiometricRecordForm.RECORD_CHOICES, label="Measurement type") + unit = forms.CharField( + max_length=12, + required=False, + label="Unit", + help_text="Leave blank to use the model default (g for weight, mm for height).", + ) + custom_name = forms.CharField( + max_length=30, + required=False, + label="Measurement name", + help_text="Required when record type is Custom.", + ) + custom_unit = forms.CharField( + max_length=12, + required=False, + label="Custom unit", + help_text="Required when record type is Custom.", + ) + + def clean(self): + cleaned = super().clean() + if cleaned.get("record_type") == "custom": + if not cleaned.get("custom_name"): + self.add_error("custom_name", "Measurement name is required for custom records.") + if not cleaned.get("custom_unit"): + self.add_error("custom_unit", "Custom unit is required for custom records.") + return cleaned + + +class BiometricBatchRowForm(forms.Form): + """A single animal row in the batch measurement table. + + The `animal_id` hidden field carries the animal UUID from GET to POST so the + view can look it up in the allowed set without trusting URL order. When + `include` is True, `value` must be provided. + """ + + include = forms.BooleanField(required=False, label="Include") + animal_id = forms.UUIDField(widget=forms.HiddenInput) + value = forms.DecimalField( + max_digits=8, + decimal_places=3, + required=False, + label="Value", + ) + + def clean(self): + cleaned = super().clean() + if cleaned.get("include") and cleaned.get("value") is None: + self.add_error("value", "A value is required for checked animals.") + return cleaned + + +BiometricBatchFormSet = formset_factory(BiometricBatchRowForm, extra=0) diff --git a/src/ahc/apps/medical_notes/forms/type_basic_note.py b/src/ahc/apps/medical_notes/forms/type_basic_note.py index 867c092..b4680a2 100644 --- a/src/ahc/apps/medical_notes/forms/type_basic_note.py +++ b/src/ahc/apps/medical_notes/forms/type_basic_note.py @@ -1,5 +1,6 @@ from django import forms +from ahc.apps.animals.selectors import animals_visible_to from ahc.apps.medical_notes.models.type_basic_note import MedicalRecord, MedicalRecordAttachment @@ -50,13 +51,22 @@ class Meta: def __init__(self, *args, **kwargs): animal_choices = kwargs.pop("animal_choices", None) + profile = kwargs.pop("profile", None) + exclude_id = kwargs.pop("exclude_id", None) type_of_event_param = kwargs.pop("type_of_event_param", None) super().__init__(*args, **kwargs) - # self.Meta.fields.append('TYPES_OF_EVENTS') if animal_choices: self.fields["additional_animals"].widget.choices = animal_choices + # Restrict the validation queryset so a posted UUID of a deceased or inaccessible + # animal fails form validation, not just widget display. + if profile is not None: + qs = animals_visible_to(profile) + if exclude_id is not None: + qs = qs.exclude(id=exclude_id) + self.fields["additional_animals"].queryset = qs + if type_of_event_param in set(event[0] for event in self.TYPES_OF_EVENTS): self.fields["type_of_event"].initial = type_of_event_param else: @@ -105,12 +115,20 @@ def __init__(self, *args, **kwargs): kwargs.pop("animal") animal_choices = kwargs.pop("animal_choices", None) is_author = kwargs.pop("is_author", None) + profile = kwargs.pop("profile", None) super().__init__(*args, **kwargs) if animal_choices: self.fields["animal"].widget.choices = animal_choices self.fields["additional_animals"].widget.choices = animal_choices + # Restrict validation querysets so a posted UUID of a deceased or inaccessible + # animal fails form validation, not just widget display. + if profile is not None: + qs = animals_visible_to(profile) + self.fields["animal"].queryset = qs + self.fields["additional_animals"].queryset = qs + if not is_author: del self.fields["animal"] diff --git a/src/ahc/apps/medical_notes/selectors.py b/src/ahc/apps/medical_notes/selectors.py index df1cd15..9dd3efb 100644 --- a/src/ahc/apps/medical_notes/selectors.py +++ b/src/ahc/apps/medical_notes/selectors.py @@ -13,7 +13,7 @@ from django.db.models import QuerySet from django.utils import timezone -from ahc.apps.animals.selectors import animals_visible_to, user_can_access_animal +from ahc.apps.animals.selectors import animals_visible_to, user_can_modify_animal from ahc.apps.medical_notes.models.type_basic_note import MedicalRecord, MedicalRecordAttachment @@ -194,8 +194,12 @@ def is_attachment_author(profile, attachment: MedicalRecordAttachment) -> bool: def can_access_note_animal(profile, note: MedicalRecord) -> bool: - """Return True if the profile is owner or keeper of the animal linked to the note.""" - return user_can_access_animal(profile, note.animal) + """Return True if the profile may write to the animal linked to the note. + + Uses user_can_modify_animal so that deceased animals are always blocked — neither + the owner nor any carer may add or edit notes on a deceased animal. + """ + return user_can_modify_animal(profile, note.animal) def vaccination_notes_for(animal) -> QuerySet: @@ -212,6 +216,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. @@ -221,6 +230,7 @@ def due_vaccination_reminders(on_date: date) -> QuerySet: return ( VaccinationNote.objects.filter(reminder_date__lte=on_date, reminder_sent=False) + .filter(related_note__animal__date_of_death__isnull=True) .select_related("related_note__animal__owner__user") .exclude(reminder_date=None) ) diff --git a/src/ahc/apps/medical_notes/services/biometrics.py b/src/ahc/apps/medical_notes/services/biometrics.py index 0b03652..80d90a0 100644 --- a/src/ahc/apps/medical_notes/services/biometrics.py +++ b/src/ahc/apps/medical_notes/services/biometrics.py @@ -2,6 +2,9 @@ from __future__ import annotations +from django.db import transaction + +from ahc.apps.medical_notes.models.type_basic_note import MedicalRecord from ahc.apps.medical_notes.models.type_measurement_notes import ( BiometricCustomRecords, BiometricHeightRecords, @@ -48,3 +51,45 @@ def create_biometric_record(animal, related_note, record_type: str, data: dict) related_note=related_note, custom_biometric_record=sub_record, ) + + +def create_batch_biometric_records( + author_profile, + record_type: str, + rows: list[tuple], + allowed_ids: set, +) -> int: + """Create a MedicalRecord + BiometricRecord pair for each (animal, data_dict) row. + + Rows whose animal.id is absent from allowed_ids are silently skipped — this is a + defence-in-depth fence; the caller (view) should already have filtered rows to the + permitted set. + + Pairs are created one after the other inside a single transaction. This ordering is + intentional: the clean_orphaned_metric_records post_save signal on BiometricRecord + deletes any biometric_record MedicalRecords by the same author that have no attached + BiometricRecord at the moment of each save. Creating all notes before all biometries + would cause the first biometry save to delete the still-empty sibling notes. Creating + each (note, biometry) pair in sequence avoids that. + + Returns the number of pairs created. + """ + count = 0 + with transaction.atomic(): + for animal, data_dict in rows: + if animal.id not in allowed_ids: + continue + note = MedicalRecord.objects.create( + animal=animal, + author=author_profile, + type_of_event="biometric_record", + short_description=f"Measurement: {record_type}", + ) + create_biometric_record( + animal=animal, + related_note=note, + record_type=record_type, + data=data_dict, + ) + count += 1 + return count 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/templates/medical_notes/biometric_batch.html b/src/ahc/apps/medical_notes/templates/medical_notes/biometric_batch.html new file mode 100644 index 0000000..eaaa799 --- /dev/null +++ b/src/ahc/apps/medical_notes/templates/medical_notes/biometric_batch.html @@ -0,0 +1,63 @@ +{% extends "homepage/base.html" %} +{% load static %} +{% block extra_js %} + +{% endblock %} +{% block content %} +
+

Batch measurement entry

+

Select a measurement type, check each animal you measured, enter the value, then save all at once.

+ +
+ {% csrf_token %} + +
+ Session settings + {% include "partials/form_fields.html" with form=session_form %} +
+ + {{ formset.management_form }} + + {% if rows %} + + + + + + + + + + {% for row_form, animal in rows %} + + + + + + {% endfor %} + +
IncludeAnimalValue
{{ row_form.include }} + {{ row_form.animal_id }} + {{ animal.full_name }} + + {{ row_form.value }} + {% for error in row_form.value.errors %} + {{ error }} + {% endfor %} + {% for error in row_form.non_field_errors %} + {{ error }} + {% endfor %} +
+ {% else %} +

No animals available. Add an animal first, or ask an owner to share one with you.

+ {% endif %} + + +
+ +
+ + Back to homepage + +
+{% endblock %} diff --git a/src/ahc/apps/medical_notes/tests.py b/src/ahc/apps/medical_notes/tests.py index f503876..91a8ae1 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): @@ -194,10 +198,10 @@ def test_is_attachment_author_false_for_other_profile(self): attachment.medical_record.author = MagicMock() assert is_attachment_author(MagicMock(), attachment) is False - def test_can_access_note_animal_delegates_to_animals_selector(self): + def test_can_access_note_animal_delegates_to_modify_predicate(self): profile = MagicMock() note = MagicMock() - with patch("ahc.apps.medical_notes.selectors.user_can_access_animal", return_value=True) as mock_selector: + with patch("ahc.apps.medical_notes.selectors.user_can_modify_animal", return_value=True) as mock_selector: result = can_access_note_animal(profile, note) mock_selector.assert_called_once_with(profile, note.animal) @@ -206,7 +210,7 @@ def test_can_access_note_animal_delegates_to_animals_selector(self): def test_can_access_note_animal_returns_false_when_denied(self): profile = MagicMock() note = MagicMock() - with patch("ahc.apps.medical_notes.selectors.user_can_access_animal", return_value=False): + with patch("ahc.apps.medical_notes.selectors.user_can_modify_animal", return_value=False): assert can_access_note_animal(profile, note) is False @@ -633,5 +637,544 @@ 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 + + +# --------------------------------------------------------------------------- +# Batch biometric entry — forms +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestBiometricBatchRowForm: + """BiometricBatchRowForm: include+value cross-field validation.""" + + def test_checked_row_without_value_is_invalid(self): + import uuid + + from ahc.apps.medical_notes.forms.biometric_batch import BiometricBatchRowForm + + form = BiometricBatchRowForm(data={"include": True, "animal_id": str(uuid.uuid4()), "value": ""}) + assert not form.is_valid() + assert "value" in form.errors + + def test_unchecked_row_without_value_is_valid(self): + import uuid + + from ahc.apps.medical_notes.forms.biometric_batch import BiometricBatchRowForm + + form = BiometricBatchRowForm(data={"include": False, "animal_id": str(uuid.uuid4()), "value": ""}) + assert form.is_valid() + + def test_checked_row_with_value_is_valid(self): + import uuid + + from ahc.apps.medical_notes.forms.biometric_batch import BiometricBatchRowForm + + form = BiometricBatchRowForm(data={"include": True, "animal_id": str(uuid.uuid4()), "value": "12.500"}) + assert form.is_valid() + + +@pytest.mark.unit +class TestBiometricBatchSessionForm: + """BiometricBatchSessionForm: custom type requires name and unit.""" + + def test_custom_type_without_name_and_unit_is_invalid(self): + from ahc.apps.medical_notes.forms.biometric_batch import BiometricBatchSessionForm + + form = BiometricBatchSessionForm(data={"record_type": "custom", "unit": "", "custom_name": "", "custom_unit": ""}) + assert not form.is_valid() + assert "custom_name" in form.errors + assert "custom_unit" in form.errors + + def test_weight_type_without_unit_is_valid(self): + from ahc.apps.medical_notes.forms.biometric_batch import BiometricBatchSessionForm + + form = BiometricBatchSessionForm(data={"record_type": "weight", "unit": "", "custom_name": "", "custom_unit": ""}) + assert form.is_valid() + + +# --------------------------------------------------------------------------- +# Batch biometric entry — service +# --------------------------------------------------------------------------- + + +@pytest.fixture +def batch_animals(db, user_profile): + """Three animals owned by user_profile for batch service tests.""" + from ahc.apps.animals.models import Animal + + _, profile = user_profile + a1 = Animal.objects.create(full_name="Alpha", owner=profile) + a2 = Animal.objects.create(full_name="Beta", owner=profile) + a3 = Animal.objects.create(full_name="Gamma", owner=profile) + return (a1, a2, a3), profile + + +@pytest.mark.integration +@pytest.mark.django_db +class TestCreateBatchBiometricRecordsService: + """create_batch_biometric_records: creates N pairs, enforces allowed_ids, no signal orphans.""" + + def test_creates_expected_number_of_pairs(self, batch_animals): + from decimal import Decimal + + 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.medical_notes.services.biometrics import create_batch_biometric_records + + (a1, a2, _), profile = batch_animals + rows = [ + (a1, {"weight": Decimal("4.5"), "weight_unit_to_present": "kg"}), + (a2, {"weight": Decimal("8.0"), "weight_unit_to_present": "kg"}), + ] + n = create_batch_biometric_records(profile, "weight", rows, allowed_ids={a1.id, a2.id}) + + assert n == 2 + assert MedicalRecord.objects.filter(type_of_event="biometric_record", author=profile).count() == 2 + assert BiometricRecord.objects.filter(animal__in=[a1, a2]).count() == 2 + + def test_skips_animal_not_in_allowed_ids(self, batch_animals): + import uuid + from decimal import Decimal + + from ahc.apps.medical_notes.models.type_measurement_notes import BiometricRecord + from ahc.apps.medical_notes.services.biometrics import create_batch_biometric_records + + (a1, _, _), profile = batch_animals + outsider_id = uuid.uuid4() + rows = [ + (a1, {"weight": Decimal("5.0"), "weight_unit_to_present": "kg"}), + ] + n = create_batch_biometric_records(profile, "weight", rows, allowed_ids={outsider_id}) + + assert n == 0 + assert BiometricRecord.objects.count() == 0 + + def test_no_orphaned_notes_after_batch(self, batch_animals): + """Regression: the clean_orphaned_metric_records signal must not delete sibling notes. + + This test fails if the service creates all MedicalRecord rows before any + BiometricRecord — the first BiometricRecord save would then wipe the still-empty + sibling notes. Correct sequential (note, biometry) pairing prevents this. + """ + from decimal import Decimal + + 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.medical_notes.services.biometrics import create_batch_biometric_records + + (a1, a2, a3), profile = batch_animals + rows = [ + (a1, {"weight": Decimal("3.0"), "weight_unit_to_present": "g"}), + (a2, {"weight": Decimal("6.0"), "weight_unit_to_present": "g"}), + (a3, {"weight": Decimal("9.0"), "weight_unit_to_present": "g"}), + ] + create_batch_biometric_records(profile, "weight", rows, allowed_ids={a1.id, a2.id, a3.id}) + + note_count = MedicalRecord.objects.filter(type_of_event="biometric_record", author=profile).count() + biometric_count = BiometricRecord.objects.filter(animal__in=[a1, a2, a3]).count() + assert note_count == 3, f"Expected 3 notes, got {note_count} (signal deleted orphans)" + assert biometric_count == 3 + + +# --------------------------------------------------------------------------- +# Batch biometric entry — view integration +# --------------------------------------------------------------------------- + + +@pytest.fixture +def two_owned_animals(db, user_profile): + """Two animals owned by user_profile for view tests.""" + from ahc.apps.animals.models import Animal + + _, profile = user_profile + a1 = Animal.objects.create(full_name="Dog", owner=profile) + a2 = Animal.objects.create(full_name="Cat", owner=profile) + return (a1, a2), profile + + +@pytest.mark.integration +@pytest.mark.django_db +class TestBiometricBatchCreateView: + """BiometricBatchCreateView: GET renders formset rows, POST creates pairs, stranger blocked.""" + + def test_get_renders_one_row_per_animal(self, client, user_profile, two_owned_animals): + user, _ = user_profile + client.force_login(user) + response = client.get(reverse("biometric_batch")) + + assert response.status_code == 200 + content = response.content.decode() + assert "Dog" in content + assert "Cat" in content + + def test_post_creates_pairs_for_checked_rows_only(self, client, user_profile, two_owned_animals): + from ahc.apps.medical_notes.models.type_basic_note import MedicalRecord + from ahc.apps.medical_notes.models.type_measurement_notes import BiometricRecord + + user, _ = user_profile + (a1, a2), _ = two_owned_animals + client.force_login(user) + + data = { + "record_type": "weight", + "unit": "kg", + "custom_name": "", + "custom_unit": "", + "form-TOTAL_FORMS": "2", + "form-INITIAL_FORMS": "0", + "form-MIN_NUM_FORMS": "0", + "form-MAX_NUM_FORMS": "1000", + "form-0-include": "on", + "form-0-animal_id": str(a1.id), + "form-0-value": "12.5", + "form-1-include": "", + "form-1-animal_id": str(a2.id), + "form-1-value": "", + } + response = client.post(reverse("biometric_batch"), data) + + assert response.status_code == 302 + assert MedicalRecord.objects.filter(type_of_event="biometric_record").count() == 1 + assert BiometricRecord.objects.filter(animal=a1).count() == 1 + assert BiometricRecord.objects.filter(animal=a2).count() == 0 + + def test_post_ignores_animal_outside_allowed_set(self, client, user_profile, second_user_profile, two_owned_animals): + """A row carrying a stranger's animal_id must produce no records.""" + from ahc.apps.medical_notes.models.type_measurement_notes import BiometricRecord + + user, _ = user_profile + _, other_profile = second_user_profile + from ahc.apps.animals.models import Animal + + stranger_animal = Animal.objects.create(full_name="Stranger", owner=other_profile) + client.force_login(user) + + data = { + "record_type": "weight", + "unit": "kg", + "custom_name": "", + "custom_unit": "", + "form-TOTAL_FORMS": "1", + "form-INITIAL_FORMS": "0", + "form-MIN_NUM_FORMS": "0", + "form-MAX_NUM_FORMS": "1000", + "form-0-include": "on", + "form-0-animal_id": str(stranger_animal.id), + "form-0-value": "5.0", + } + response = client.post(reverse("biometric_batch"), data) + + assert response.status_code == 302 + assert BiometricRecord.objects.count() == 0 + + def test_unauthenticated_redirects_to_login(self, client): + response = client.get(reverse("biometric_batch")) + assert response.status_code == 302 + assert "/login" in response["Location"] + + +@pytest.mark.integration +@pytest.mark.django_db +class TestDeceasedAnimalWriteBlocking: + """Verify all medical-notes write paths block deceased animals.""" + + @pytest.fixture + def setup(self, db, user_profile, second_user_profile): + from ahc.apps.animals.models import Animal, AnimalShare + from ahc.apps.medical_notes.models.type_basic_note import MedicalRecord + + _, owner_profile = user_profile + owner_user, _ = user_profile + _, carer_profile = second_user_profile + carer_user, _ = second_user_profile + + living = Animal.objects.create(full_name="Living", owner=owner_profile) + deceased = Animal.objects.create(full_name="Passed", owner=owner_profile, date_of_death=_date(2024, 1, 1)) + AnimalShare.objects.create(animal=deceased, carer=carer_profile) + AnimalShare.objects.create(animal=living, carer=carer_profile) + + note_on_living = MedicalRecord.objects.create( + animal=living, author=owner_profile, short_description="live note", type_of_event="fast_note" + ) + return { + "owner_user": owner_user, + "carer_user": carer_user, + "owner_profile": owner_profile, + "carer_profile": carer_profile, + "living": living, + "deceased": deceased, + "note_on_living": note_on_living, + } + + def _client_for(self, user): + from django.test import Client + + c = Client() + c.force_login(user) + return c + + def test_owner_cannot_create_note_on_deceased(self, setup): + s = setup + # medical_notes URLs are mounted under /note/ (see ahc/urls.py) + response = self._client_for(s["owner_user"]).post( + f"/note/{s['deceased'].id}/create/", + {"type_of_event": "fast_note", "short_description": "new note"}, + ) + assert response.status_code == 403 + + def test_carer_cannot_create_note_on_deceased(self, setup): + s = setup + response = self._client_for(s["carer_user"]).post( + f"/note/{s['deceased'].id}/create/", + {"type_of_event": "fast_note", "short_description": "carer note"}, + ) + assert response.status_code == 403 + + def test_deceased_animal_not_in_batch_allowed_set(self, setup): + """Deceased animal must never appear in the formset offered by BiometricBatchCreateView.""" + s = setup + response = self._client_for(s["owner_user"]).get(reverse("biometric_batch")) + assert response.status_code == 200 + # BiometricBatchCreateView._build_context stores animals inside 'rows' as (form, animal) tuples + offered_ids = {str(animal.id) for _, animal in response.context["rows"]} + assert str(s["deceased"].id) not in offered_ids + assert str(s["living"].id) in offered_ids + + def test_form_queryset_rejects_deceased_uuid_in_additional_animals(self, setup): + """MedicalRecordForm.additional_animals queryset must reject a deceased animal UUID.""" + from ahc.apps.medical_notes.forms.type_basic_note import MedicalRecordForm + + s = setup + form = MedicalRecordForm( + data={ + "type_of_event": "fast_note", + "short_description": "test", + "additional_animals": [str(s["deceased"].id)], + }, + profile=s["owner_profile"], + exclude_id=s["living"].id, + ) + assert not form.is_valid() + assert "additional_animals" in form.errors + + def test_can_access_note_animal_returns_false_for_deceased(self, setup): + """can_access_note_animal must block even the owner on a deceased animal's note.""" + from ahc.apps.medical_notes.models.type_basic_note import MedicalRecord + from ahc.apps.medical_notes.selectors import can_access_note_animal + + s = setup + deceased_note = MedicalRecord.objects.create( + animal=s["deceased"], author=s["owner_profile"], short_description="old note", type_of_event="fast_note" + ) + assert can_access_note_animal(s["owner_profile"], deceased_note) is False + assert can_access_note_animal(s["carer_profile"], deceased_note) is False + + def test_due_vaccination_reminders_excludes_deceased(self, setup): + """Vaccination reminders must not fire for deceased animals.""" + from ahc.apps.medical_notes.models.type_basic_note import MedicalRecord + from ahc.apps.medical_notes.models.type_vaccination_notes import VaccinationNote + from ahc.apps.medical_notes.selectors import due_vaccination_reminders + + s = setup + living_note = MedicalRecord.objects.create( + animal=s["living"], author=s["owner_profile"], short_description="vacc base", type_of_event="vaccination_note" + ) + deceased_note = MedicalRecord.objects.create( + animal=s["deceased"], + author=s["owner_profile"], + short_description="deceased vacc", + type_of_event="vaccination_note", + ) + today = _date(2025, 1, 1) + living_vacc = VaccinationNote.objects.create(related_note=living_note, reminder_date=today, reminder_sent=False) + deceased_vacc = VaccinationNote.objects.create(related_note=deceased_note, reminder_date=today, reminder_sent=False) + reminders = list(due_vaccination_reminders(today)) + reminder_ids = {v.pk for v in reminders} + assert living_vacc.pk in reminder_ids + assert deceased_vacc.pk not in reminder_ids diff --git a/src/ahc/apps/medical_notes/urls.py b/src/ahc/apps/medical_notes/urls.py index 25b8744..08e39f4 100644 --- a/src/ahc/apps/medical_notes/urls.py +++ b/src/ahc/apps/medical_notes/urls.py @@ -23,6 +23,7 @@ measurement_views.BiometricRecordCreateView.as_view(), name="biometric_create", ), + path("biometric/batch/", measurement_views.BiometricBatchCreateView.as_view(), name="biometric_batch"), path("/attachment_edit/", notes_views.EditMedicalRecordAttachmentDescription.as_view(), name="attachment_edit"), path("/attachment_delete/", notes_views.DeleteMedicalRecordAttachment.as_view(), name="attachment_delete"), path( 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..1f5716c 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 @@ -3,19 +3,29 @@ Each mixin implements test_func by delegating to a selector from ahc.apps.medical_notes.selectors, keeping views free of inline permission logic. -Two access-level patterns exist: -- AnimalDirectAccessRequiredMixin — pk in URL is an Animal UUID directly. -- AnimalAccessRequiredMixin — pk in URL is a MedicalRecord UUID; access is - checked on the linked animal. +Four access-level patterns exist: +- AnimalDirectViewMixin — pk in URL is an Animal UUID; grants read-only access + (owner always; carer only if living animal + active share). +- AnimalDirectModifyMixin — pk in URL is an Animal UUID; grants write access + (blocked entirely on deceased animals, even for the owner). +- AnimalAccessRequiredMixin — pk in URL is a MedicalRecord UUID; write access + checked on the linked animal via can_access_note_animal. - NoteAuthorRequiredMixin — pk is a MedicalRecord UUID; author-only access. - AttachmentAuthorRequiredMixin — pk is a MedicalRecordAttachment UUID. + +AnimalDirectAccessRequiredMixin is kept as a backward-compatible alias for +AnimalDirectViewMixin; prefer the explicit names in new code. """ +from __future__ import annotations + +from typing import TYPE_CHECKING + from django.contrib.auth.mixins import UserPassesTestMixin from django.shortcuts import get_object_or_404 from ahc.apps.animals.models import Animal -from ahc.apps.animals.selectors import user_can_access_animal +from ahc.apps.animals.selectors import user_can_modify_animal, user_can_view_animal from ahc.apps.medical_notes.models.type_basic_note import MedicalRecord, MedicalRecordAttachment from ahc.apps.medical_notes.selectors import ( can_access_note_animal, @@ -23,17 +33,45 @@ is_note_author, ) +if TYPE_CHECKING: + from ahc.types import AuthenticatedRequest + + +class AnimalDirectViewMixin(UserPassesTestMixin): + """Allow read access when pk in URL is an Animal UUID. + + The owner may always view (including deceased animals in read-only mode). + Carers are blocked on deceased animals. + """ + + request: AuthenticatedRequest + + def test_func(self): + animal = get_object_or_404(Animal, id=self.kwargs.get("pk")) + return user_can_view_animal(self.request.user.profile, animal) + + +class AnimalDirectModifyMixin(UserPassesTestMixin): + """Allow write access when pk in URL is an Animal UUID. -class AnimalDirectAccessRequiredMixin(UserPassesTestMixin): - """Allow access when pk in URL is an Animal UUID and the profile can access it.""" + Deceased animals are blocked for everyone — including the owner. + """ + + 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) + return user_can_modify_animal(self.request.user.profile, animal) + + +# Backward-compatible alias — existing views that only need read access continue to work. +AnimalDirectAccessRequiredMixin = AnimalDirectViewMixin class AnimalAccessRequiredMixin(UserPassesTestMixin): - """Allow access when pk in URL is a MedicalRecord UUID and the profile can access its animal.""" + """Allow write access when pk in URL is a MedicalRecord UUID and the profile may write to its animal.""" + + request: AuthenticatedRequest def test_func(self): note = get_object_or_404(MedicalRecord, id=self.kwargs.get("pk")) @@ -43,6 +81,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 +91,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..dfa6f15 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,22 @@ 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, + AnimalDirectModifyMixin, AttachmentAuthorRequiredMixin, NoteAuthorRequiredMixin, ) +if TYPE_CHECKING: + from ahc.types import AuthenticatedRequest + -class CreateNoteFormView(LoginRequiredMixin, AnimalDirectAccessRequiredMixin, FormView): +class CreateNoteFormView(LoginRequiredMixin, AnimalDirectModifyMixin, FormView): template_name = "medical_notes/create.html" form_class = MedicalRecordForm + request: AuthenticatedRequest def get_template_names(self): if self.request.headers.get("HX-Request"): @@ -50,7 +60,11 @@ def get_template_names(self): def get_form_kwargs(self): kwargs = super().get_form_kwargs() - kwargs["animal_choices"] = animal_choices_for(self.request.user.profile, exclude_id=self.kwargs.get("pk")) + profile = self.request.user.profile + primary_id = self.kwargs.get("pk") + kwargs["animal_choices"] = animal_choices_for(profile, exclude_id=primary_id) + kwargs["profile"] = profile + kwargs["exclude_id"] = primary_id kwargs["type_of_event_param"] = self.request.GET.get("type_of_event") return kwargs @@ -100,6 +114,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 +161,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 +192,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"): @@ -196,7 +207,9 @@ def get_context_data(self, **kwargs): def get_form_kwargs(self): kwargs = super().get_form_kwargs() - kwargs["animal_choices"] = animal_choices_for(self.request.user.profile) + profile = self.request.user.profile + kwargs["animal_choices"] = animal_choices_for(profile) + kwargs["profile"] = profile note = get_object_or_404(MedicalRecord, pk=self.kwargs.get("pk")) kwargs["animal"] = get_object_or_404(AnimalProfile, id=note.animal.id) @@ -242,11 +255,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 +296,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_measurement_notes.py b/src/ahc/apps/medical_notes/views/type_measurement_notes.py index beb2f22..ea664e1 100644 --- a/src/ahc/apps/medical_notes/views/type_measurement_notes.py +++ b/src/ahc/apps/medical_notes/views/type_measurement_notes.py @@ -1,16 +1,27 @@ +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 get_object_or_404, redirect +from django.shortcuts import get_object_or_404, redirect, render from django.urls import reverse +from django.views import View from django.views.generic.edit import FormView from ahc.apps.animals.models import Animal as AnimalProfile +from ahc.apps.animals.selectors import animals_for_biometric_batch +from ahc.apps.medical_notes.forms.biometric_batch import BiometricBatchFormSet, BiometricBatchSessionForm from ahc.apps.medical_notes.forms.type_measurement_notes import BiometricRecordForm from ahc.apps.medical_notes.models.type_basic_note import MedicalRecord -from ahc.apps.medical_notes.services.biometrics import create_biometric_record -from ahc.apps.medical_notes.views.mixins.user_animal_permisions import AnimalDirectAccessRequiredMixin +from ahc.apps.medical_notes.services.biometrics import create_batch_biometric_records, create_biometric_record +from ahc.apps.medical_notes.views.mixins.user_animal_permisions import AnimalDirectModifyMixin +if TYPE_CHECKING: + from ahc.types import AuthenticatedRequest -class BiometricRecordCreateView(LoginRequiredMixin, AnimalDirectAccessRequiredMixin, FormView): + +class BiometricRecordCreateView(LoginRequiredMixin, AnimalDirectModifyMixin, FormView): template_name = "medical_notes/create.html" form_class = BiometricRecordForm @@ -34,3 +45,68 @@ def form_valid(self, form): ) return redirect(reverse("animal_profile", kwargs={"pk": animal_id})) + + +class BiometricBatchCreateView(LoginRequiredMixin, View): + """Full-page form for entering one measurement type for multiple animals at once. + + A single POST creates one MedicalRecord + BiometricRecord pair per checked row. + The session form controls the record type and unit; the formset has one row per + animal accessible to the current user. + """ + + template_name = "medical_notes/biometric_batch.html" + request: AuthenticatedRequest + + def _build_context(self, session_form, formset, animals): + return { + "session_form": session_form, + "formset": formset, + "rows": list(zip(formset.forms, animals, strict=False)), + } + + def get(self, request, *args, **kwargs): + animals = list(animals_for_biometric_batch(request.user.profile)) + session_form = BiometricBatchSessionForm() + formset = BiometricBatchFormSet(initial=[{"animal_id": a.id} for a in animals]) + return render(request, self.template_name, self._build_context(session_form, formset, animals)) + + def post(self, request, *args, **kwargs): + animals = list(animals_for_biometric_batch(request.user.profile)) + session_form = BiometricBatchSessionForm(request.POST) + formset = BiometricBatchFormSet(request.POST) + + if not (session_form.is_valid() and formset.is_valid()): + return render(request, self.template_name, self._build_context(session_form, formset, animals)) + + profile = request.user.profile + allowed = {a.id: a for a in animals} + record_type = session_form.cleaned_data["record_type"] + unit = session_form.cleaned_data.get("unit") or "" + custom_name = session_form.cleaned_data.get("custom_name", "") + custom_unit = session_form.cleaned_data.get("custom_unit", "") + + rows = [] + for form in formset.forms: + if not form.cleaned_data.get("include"): + continue + animal_id = form.cleaned_data["animal_id"] + if animal_id not in allowed: + continue + value = form.cleaned_data["value"] + if record_type == "weight": + data_dict = {"weight": value, "weight_unit_to_present": unit or "g"} + elif record_type == "height": + data_dict = {"height": value, "height_unit_to_present": unit or "mm"} + else: + data_dict = {"custom_name": custom_name, "custom_value": str(value), "custom_unit": custom_unit} + rows.append((allowed[animal_id], data_dict)) + + n = create_batch_biometric_records( + author_profile=profile, + record_type=record_type, + rows=rows, + allowed_ids=set(allowed.keys()), + ) + messages.success(request, f"Saved {n} measurement(s).") + return redirect(reverse("biometric_batch")) 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..c0b93a6 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 @@ -14,7 +16,10 @@ from django.views import View from ahc.apps.animals.models import Animal -from ahc.apps.animals.selectors import allowed_categories_for, is_animal_owner, user_can_access_animal +from ahc.apps.animals.selectors import allowed_categories_for, is_animal_owner, user_can_modify_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 ( @@ -25,8 +30,12 @@ def _has_vaccination_access(profile, animal: Animal) -> bool: - """Return True if profile may read/write vaccinations on this animal.""" - if not user_can_access_animal(profile, animal): + """Return True if profile may write (add/edit/delete) vaccinations on this animal. + + Uses user_can_modify_animal so deceased animals are blocked for everyone, + including the owner. + """ + if not user_can_modify_animal(profile, animal): return False if is_animal_owner(profile, animal): return True @@ -41,16 +50,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..9b6cc41 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": "test-fixture-password-1", + "password2": "test-fixture-password-1", + }, + ) + 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. diff --git a/static/js/biometric_batch.js b/static/js/biometric_batch.js new file mode 100644 index 0000000..bff2efb --- /dev/null +++ b/static/js/biometric_batch.js @@ -0,0 +1,24 @@ +document.addEventListener('DOMContentLoaded', function () { + var recordTypeField = document.getElementById('id_record_type'); + if (!recordTypeField) return; + + var unitRow = document.getElementById('field_unit'); + var customNameRow = document.getElementById('field_custom_name'); + var customUnitRow = document.getElementById('field_custom_unit'); + + function toggleSessionFields() { + var type = recordTypeField.value; + if (type === 'custom') { + if (unitRow) unitRow.style.display = 'none'; + if (customNameRow) customNameRow.style.display = 'block'; + if (customUnitRow) customUnitRow.style.display = 'block'; + } else { + if (unitRow) unitRow.style.display = 'block'; + if (customNameRow) customNameRow.style.display = 'none'; + if (customUnitRow) customUnitRow.style.display = 'none'; + } + } + + toggleSessionFields(); + recordTypeField.addEventListener('change', toggleSessionFields); +}); diff --git a/uv.lock b/uv.lock index 0646ed7..a1e0af3 100644 --- a/uv.lock +++ b/uv.lock @@ -91,6 +91,7 @@ name = "animals-healthcare-application" version = "0.1.0" source = { virtual = "." } dependencies = [ + { name = "aiohttp" }, { name = "celery" }, { name = "cffi" }, { name = "cryptography" }, @@ -130,6 +131,7 @@ dev = [ [package.metadata] requires-dist = [ + { name = "aiohttp", specifier = ">=3" }, { name = "celery", specifier = ">=5.4" }, { name = "cffi", specifier = ">=1.17" }, { name = "cryptography", specifier = ">=43" },