From f87c36984286216eddf2343a29ccdaa94b0e8e51 Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Thu, 4 Jan 2024 21:42:54 +0200 Subject: [PATCH 01/31] Add HomeUserRelation --- homes/migrations/0007_homeuserrelation.py | 30 +++++++++++++++++++++++ homes/models.py | 26 ++++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 homes/migrations/0007_homeuserrelation.py diff --git a/homes/migrations/0007_homeuserrelation.py b/homes/migrations/0007_homeuserrelation.py new file mode 100644 index 0000000..9c77354 --- /dev/null +++ b/homes/migrations/0007_homeuserrelation.py @@ -0,0 +1,30 @@ +# Generated by Django 5.0 on 2024-01-04 19:24 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('homes', '0006_alter_home_home_group'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='HomeUserRelation', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('home', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='home_user_relations', to='homes.home')), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='home_user_relations', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'verbose_name': 'home user relation', + 'verbose_name_plural': 'home user relations', + 'db_table': 'home_user_relation', + 'unique_together': {('user', 'home')}, + }, + ), + ] diff --git a/homes/models.py b/homes/models.py index db0a32a..0f1dfa9 100644 --- a/homes/models.py +++ b/homes/models.py @@ -1,5 +1,6 @@ import datetime from typing import TYPE_CHECKING +from django.contrib.auth import get_user_model from django.db.models import Count, Q, QuerySet from django.utils import timezone from datetime import timedelta @@ -16,6 +17,9 @@ from residents.models import Resident +user_model = get_user_model() + + def _generate_date_range(days_ago: int) -> list[datetime.date]: """Generates a list of dates starting from today and going back a specified number of days. @@ -199,6 +203,28 @@ def _structure_resident_data( } +class HomeUserRelation(models.Model): + user = models.ForeignKey( + to=user_model, + on_delete=models.CASCADE, + related_name="home_user_relations", + ) + home = models.ForeignKey( + to="homes.Home", + on_delete=models.CASCADE, + related_name="home_user_relations", + ) + + def __str__(self) -> str: + return f"{self.user} - {self.home}" + + class Meta: + db_table = "home_user_relation" + verbose_name = _("home user relation") + verbose_name_plural = _("home user relations") + unique_together = ("user", "home") + + class Home(models.Model): name = models.CharField(max_length=25) # add a foreign key relationship to HomeGroup From e288e86a9e73940db8a96c441d3e4df530a04af1 Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Thu, 4 Jan 2024 21:43:10 +0200 Subject: [PATCH 02/31] Add home.members --- homes/models.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/homes/models.py b/homes/models.py index 0f1dfa9..2acd53a 100644 --- a/homes/models.py +++ b/homes/models.py @@ -11,6 +11,7 @@ import pandas as pd from shortuuid.django_fields import ShortUUIDField + from core.constants import WEEK_DAYS, WEEKLY_ACTIVITY_RANGES if TYPE_CHECKING: @@ -252,6 +253,11 @@ def __str__(self) -> str: def get_absolute_url(self): return reverse("home-detail-view", kwargs={"url_uuid": self.url_uuid}) + @property + def members(self) -> QuerySet[user_model]: + """Returns a QuerySet of all members of this home.""" + return user_model.objects.filter(home_user_relations__home=self) + @property def current_residents(self) -> models.QuerySet["Resident"]: """Returns a QuerySet of all current residents for this home.""" From 1908e8b8d05073c51c858cade5eed6f4b1d782bd Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Thu, 4 Jan 2024 21:43:45 +0200 Subject: [PATCH 03/31] Register HomeUserRelation --- homes/admin.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/homes/admin.py b/homes/admin.py index 28ad5b2..638b7cd 100644 --- a/homes/admin.py +++ b/homes/admin.py @@ -1,6 +1,6 @@ from django.contrib import admin -from .models import Home, HomeGroup +from .models import Home, HomeGroup, HomeUserRelation @admin.register(Home) @@ -8,6 +8,12 @@ class HomeAdmin(admin.ModelAdmin): pass +# register the HomeUserRelation model +@admin.register(HomeUserRelation) +class HomeUserRelationAdmin(admin.ModelAdmin): + pass + + # register the HomeGroup model @admin.register(HomeGroup) class HomeGroupAdmin(admin.ModelAdmin): From cd00d43c114caf142732102ee858a1ff2fa0a5e6 Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Thu, 4 Jan 2024 21:46:12 +0200 Subject: [PATCH 04/31] Add User.homes and get_full_name --- accounts/models.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/accounts/models.py b/accounts/models.py index b756e6d..0f5813b 100644 --- a/accounts/models.py +++ b/accounts/models.py @@ -1,3 +1,4 @@ +from django.db.models import QuerySet from django.contrib.auth.models import AbstractUser from django.utils.translation import gettext_lazy as _ @@ -10,3 +11,12 @@ class Meta: def __str__(self): return self.username + + def get_full_name(self) -> str: + return self.first_name + " " + self.last_name + + @property + def homes(self) -> QuerySet["homes.Home"]: + from homes.models import Home + + return Home.objects.filter(homeuserrelation__user=self) From 3561629e721a99115d35baa0a586e5489513d862 Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Thu, 4 Jan 2024 22:15:23 +0200 Subject: [PATCH 05/31] Fix User.homes --- accounts/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/accounts/models.py b/accounts/models.py index 0f5813b..325b485 100644 --- a/accounts/models.py +++ b/accounts/models.py @@ -19,4 +19,4 @@ def get_full_name(self) -> str: def homes(self) -> QuerySet["homes.Home"]: from homes.models import Home - return Home.objects.filter(homeuserrelation__user=self) + return Home.objects.filter(home_user_relations__user=self) From 0ea1e6123b0e06290aca24546ba0968ddabc4da9 Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Thu, 4 Jan 2024 22:16:03 +0200 Subject: [PATCH 06/31] Add homes with and without groups --- homes/templates/homes/home_group_list.html | 8 +++-- homes/views.py | 34 ++++++++++++++++++---- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/homes/templates/homes/home_group_list.html b/homes/templates/homes/home_group_list.html index 9da4cc5..b56a2de 100644 --- a/homes/templates/homes/home_group_list.html +++ b/homes/templates/homes/home_group_list.html @@ -7,14 +7,16 @@

Homes

{% translate "Homes showing the percent of residents by activity level" %}

- {% for home_group in home_groups %} + + + {% for home_group in home_groups_with_homes %}
- {{ home_group }} + {{ home_group.group_name }}
    - {% for home in home_group.homes.all %} + {% for home in home_group.homes %}
  • diff --git a/homes/views.py b/homes/views.py index 8f54e4b..630b701 100644 --- a/homes/views.py +++ b/homes/views.py @@ -2,7 +2,7 @@ from django.shortcuts import get_object_or_404 from django.views.generic.detail import DetailView -from django.views.generic.list import ListView +from django.views.generic.base import TemplateView from .charts import ( prepare_activity_counts_by_resident_and_activity_type_chart, @@ -14,18 +14,40 @@ prepare_work_by_type_chart, ) -from .models import Home, HomeGroup +from .models import Home -class HomeGroupListView(ListView): - model = HomeGroup - context_object_name = "home_groups" +class HomeGroupListView(TemplateView): template_name = "homes/home_group_list.html" def get_context_data(self, **kwargs: Any) -> dict[str, Any]: context = super().get_context_data(**kwargs) - context["homes_without_group"] = Home.objects.filter(home_group__isnull=True) + context["homes_without_group"] = self.request.user.homes.filter( + home_group__isnull=True, + ) + + context["homes_with_group"] = self.request.user.homes.filter( + home_group__isnull=False, + ) + + # group homes with group by group name + home_groups_with_homes = {} + + for home in context["homes_with_group"]: + if home.home_group.name not in home_groups_with_homes: + home_groups_with_homes[home.home_group.name] = [] + + home_groups_with_homes[home.home_group.name].append(home) + + # Restructure home_groups_with_homes to a list of tuples + # to make it easier to iterate over in the template + home_groups_with_homes = [ + {"group_name": name, "homes": homes} + for name, homes in home_groups_with_homes.items() + ] + + context["home_groups_with_homes"] = home_groups_with_homes return context From e7f92a4aec73cbac982005930bdd7a38ff68f0ee Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Thu, 4 Jan 2024 22:22:27 +0200 Subject: [PATCH 07/31] Add i18n --- homes/templates/homes/home_group_list.html | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/homes/templates/homes/home_group_list.html b/homes/templates/homes/home_group_list.html index b56a2de..7a725ff 100644 --- a/homes/templates/homes/home_group_list.html +++ b/homes/templates/homes/home_group_list.html @@ -3,12 +3,10 @@ {% load i18n %} {% block content %} -

    Homes

    +

    {% translate "Homes" %}

    {% translate "Homes showing the percent of residents by activity level" %}

    - - {% for home_group in home_groups_with_homes %}
    @@ -36,7 +34,6 @@

    Homes

    {% endif %}
    -
  • {% endfor %}
@@ -52,6 +49,7 @@

Homes

{{ home }}
+ {% if home.resident_counts_by_activity_level_chart_data %}
From 65ba47230185e03c970c9787acba4ee4361d7d5e Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Thu, 4 Jan 2024 22:43:19 +0200 Subject: [PATCH 08/31] Add condition for superuser --- homes/views.py | 57 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 19 deletions(-) diff --git a/homes/views.py b/homes/views.py index 630b701..20f0493 100644 --- a/homes/views.py +++ b/homes/views.py @@ -17,35 +17,54 @@ from .models import Home +def regroup_homes_by_home_group(homes): + # group homes with group by group name + home_groups_with_homes = {} + + for home in homes: + if home.home_group.name not in home_groups_with_homes: + home_groups_with_homes[home.home_group.name] = [] + + home_groups_with_homes[home.home_group.name].append(home) + + # Restructure home_groups_with_homes to a list of tuples + # to make it easier to iterate over in the template + home_groups_with_homes = [ + {"group_name": name, "homes": homes} + for name, homes in home_groups_with_homes.items() + ] + + return home_groups_with_homes + + class HomeGroupListView(TemplateView): template_name = "homes/home_group_list.html" def get_context_data(self, **kwargs: Any) -> dict[str, Any]: context = super().get_context_data(**kwargs) - context["homes_without_group"] = self.request.user.homes.filter( - home_group__isnull=True, - ) + user = self.request.user - context["homes_with_group"] = self.request.user.homes.filter( - home_group__isnull=False, - ) + if user.is_superuser: + context["homes_without_group"] = Home.objects.filter( + home_group__isnull=True, + ) - # group homes with group by group name - home_groups_with_homes = {} + context["homes_with_group"] = Home.objects.filter( + home_group__isnull=False, + ) + else: + context["homes_without_group"] = self.request.user.homes.filter( + home_group__isnull=True, + ) - for home in context["homes_with_group"]: - if home.home_group.name not in home_groups_with_homes: - home_groups_with_homes[home.home_group.name] = [] + context["homes_with_group"] = self.request.user.homes.filter( + home_group__isnull=False, + ) - home_groups_with_homes[home.home_group.name].append(home) - - # Restructure home_groups_with_homes to a list of tuples - # to make it easier to iterate over in the template - home_groups_with_homes = [ - {"group_name": name, "homes": homes} - for name, homes in home_groups_with_homes.items() - ] + home_groups_with_homes = regroup_homes_by_home_group( + context["homes_with_group"], + ) context["home_groups_with_homes"] = home_groups_with_homes From bfbf0fffe682c941e2957da722dd687174b460af Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Thu, 4 Jan 2024 23:45:01 +0200 Subject: [PATCH 09/31] Add HomeGroupFactory --- homes/factories.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/homes/factories.py b/homes/factories.py index ef73ca0..c297657 100644 --- a/homes/factories.py +++ b/homes/factories.py @@ -1,10 +1,29 @@ import factory from factory import Sequence +from .models import Home, HomeGroup, HomeUserRelation + class HomeFactory(factory.django.DjangoModelFactory): class Meta: - model = "homes.Home" + model = Home django_get_or_create = ("name",) name: str = Sequence(lambda n: f"Home {n}") + + +class HomeGroupFactory(factory.django.DjangoModelFactory): + class Meta: + model = HomeGroup + django_get_or_create = ("name",) + + name: str = Sequence(lambda n: f"Home Group {n}") + + +class HomeUserRelationFactory(factory.django.DjangoModelFactory): + class Meta: + model = HomeUserRelation + django_get_or_create = ("home", "user") + + home: Home = factory.SubFactory(HomeFactory) + user: Home = factory.SubFactory(HomeFactory) From 5f7b034c96dce0f7088aaf63e25ebd66e082881e Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Thu, 4 Jan 2024 23:45:22 +0200 Subject: [PATCH 10/31] Initial HomeGroupListViewTest --- homes/tests.py | 201 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 198 insertions(+), 3 deletions(-) diff --git a/homes/tests.py b/homes/tests.py index 3bcc15f..35cfdd0 100644 --- a/homes/tests.py +++ b/homes/tests.py @@ -1,8 +1,11 @@ from datetime import date, timedelta from io import StringIO from django.core.management import call_command -from django.test import TestCase + from django.core.management.base import CommandError +from django.contrib.auth import get_user_model +from django.test import TestCase +from django.urls import reverse from django.utils import timezone from core.constants import WEEKLY_ACTIVITY_RANGES @@ -10,10 +13,12 @@ from metrics.models import ResidentActivity from residents.models import Residency, Resident -from .factories import HomeFactory -from .models import Home +from .factories import HomeFactory, HomeGroupFactory, HomeUserRelationFactory +from .models import Home, HomeGroup, HomeUserRelation from residents.factories import ResidentFactory, ResidencyFactory +User = get_user_model() + class HomeModelTests(TestCase): def setUp(self): @@ -321,3 +326,193 @@ def test_current_residents_with_recent_activity_metadata(self): [date.today() - timedelta(days=i) for i in range(7)], ) self.assertTrue(day_data["was_active"]) # Assuming activity every day + + +class HomeGroupListViewTest(TestCase): + def setUp(self): + self.url = reverse("home-list-view") + + # Setup test users + self.user = User.objects.create_user( + username="testuser", + password="password", + ) + self.superuser = User.objects.create_superuser( + username="admin", + password="admin", + ) + + # Setup test home groups using factories + self.home_group_with_home = HomeGroupFactory(name="Group with home") + self.home_group_without_homes = HomeGroupFactory(name="Group without homes") + + # Setup test homes using factories + self.home_with_group = HomeFactory( + name="Home 1", + home_group=self.home_group_with_home, + ) + self.home_without_group = HomeFactory( + name="Home 3", + ) + self.home_without_user = HomeFactory( + name="Home 4", + ) + + # associate user with homes + self.home_user_relation_1 = HomeUserRelationFactory( + home=self.home_with_group, + user=self.user, + ) + self.home_user_relation_2 = HomeUserRelationFactory( + home=self.home_without_group, + user=self.user, + ) + + def test_mock_data(self): + # Check the total count of Homes, HomeGroups, and HomeUserRelations + self.assertEqual(Home.objects.count(), 3) + self.assertEqual(HomeGroup.objects.count(), 2) + self.assertEqual(HomeUserRelation.objects.count(), 2) + + # Assert that homes are correctly associated with their home groups + self.assertEqual(self.home_with_group.home_group, self.home_group_with_home) + self.assertIsNone(self.home_without_group.home_group) + self.assertIsNone(self.home_without_user.home_group) + + # Check if the HomeGroup with no homes actually has no homes associated + self.assertFalse(self.home_group_without_homes.homes.exists()) + + # Assert user belongs to the correct number of homes + self.assertEqual(self.user.homes.count(), 2) + + # Check if the specific homes are associated with the user + self.assertIn(self.home_with_group, self.user.homes.all()) + self.assertIn(self.home_without_group, self.user.homes.all()) + self.assertNotIn(self.home_without_user, self.user.homes.all()) + + # Assert superuser has no homes associated + self.assertEqual(self.superuser.homes.count(), 0) + + def test_context_data_for_regular_user(self): + # Log in as the regular user + self.client.login( + username="testuser", + password="password", + ) + + # Get the response from the HomeGroupListView + response = self.client.get(self.url) + + # Check that the response status code is 200 + self.assertEqual(response.status_code, 200) + + # Extract the context data + context = response.context + + # Check that homes without a group are correctly in the context + homes_without_group = context["homes_without_group"] + self.assertIn(self.home_without_group, homes_without_group) + self.assertNotIn(self.home_without_user, homes_without_group) + + # Check that homes with a group are correctly in the context + homes_with_group = context["homes_with_group"] + self.assertIn(self.home_with_group, homes_with_group) + self.assertNotIn(self.home_without_user, homes_with_group) + + # Ensure that the home_groups_with_homes context is correctly formatted + home_groups_with_homes = context["home_groups_with_homes"] + self.assertTrue( + any( + group["group_name"] == self.home_group_with_home.name + for group in home_groups_with_homes + ), + ) + self.assertTrue( + all( + self.home_with_group in group["homes"] + for group in home_groups_with_homes + if group["group_name"] == self.home_group_with_home.name + ), + ) + + def test_context_data_for_superuser(self): + # Log in as the superuser + self.client.login(username="admin", password="admin") + + # Get the response from the HomeGroupListView + response = self.client.get(self.url) + + # Check that the response status code is 200 + self.assertEqual(response.status_code, 200) + + # Extract the context data + context = response.context + + # Check that all homes are in the context data, regardless of home group + all_homes = Home.objects.all() + homes_without_group = context["homes_without_group"] + homes_with_group = context["homes_with_group"] + + for home in all_homes: + if home.home_group is None: + self.assertIn(home, homes_without_group) + else: + self.assertIn(home, homes_with_group) + + # Ensure that the home_groups_with_homes context is correctly formatted + home_groups_with_homes = context["home_groups_with_homes"] + self.assertTrue( + any( + group["group_name"] == self.home_group_with_home.name + for group in home_groups_with_homes + ), + ) + self.assertFalse( + any( + group["group_name"] == self.home_group_without_homes.name + for group in home_groups_with_homes + ), + ) + + # Check that the homes in the context are correctly associated with the home groups + # Find the group corresponding to home_group_with_home + group_with_home = next( + ( + group + for group in home_groups_with_homes + if group["group_name"] == self.home_group_with_home.name + ), + None, + ) + + # Assert that the group is found + self.assertIsNotNone( + group_with_home, + "The expected home group was not found in the context.", + ) + + # Assert that home_with_group is in the found group + self.assertIn( + self.home_with_group, + group_with_home["homes"], + "Home with group was not found in the expected group.", + ) + + # home group without homes should not be in the context + self.assertFalse( + any( + group["group_name"] == self.home_group_without_homes.name + for group in home_groups_with_homes + ), + ) + + # homes without groups should correctly list the homes + self.assertIn( + self.home_without_group, + homes_without_group, + ) + + def test_home_group_list_view_uses_correct_template(self): + self.client.login(username="testuser", password="password") + response = self.client.get(self.url) + self.assertTemplateUsed(response, "homes/home_group_list.html") From 859cb53d69809e142cd4b3838aca8f1a44f44e87 Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Thu, 4 Jan 2024 23:55:08 +0200 Subject: [PATCH 11/31] Update test cases for new setUp data --- homes/tests.py | 228 +++++++++++++++++++++++++++++-------------------- 1 file changed, 136 insertions(+), 92 deletions(-) diff --git a/homes/tests.py b/homes/tests.py index 35cfdd0..cb02e3c 100644 --- a/homes/tests.py +++ b/homes/tests.py @@ -343,39 +343,75 @@ def setUp(self): ) # Setup test home groups using factories - self.home_group_with_home = HomeGroupFactory(name="Group with home") + self.home_group_with_multiple_homes = HomeGroupFactory( + name="Group with multiple homes", + ) + self.home_group_with_single_home = HomeGroupFactory( + name="Group with single home", + ) self.home_group_without_homes = HomeGroupFactory(name="Group without homes") # Setup test homes using factories - self.home_with_group = HomeFactory( - name="Home 1", - home_group=self.home_group_with_home, + # Homes in home_group_with_multiple_homes + self.home_in_group_with_multiple_homes1 = HomeFactory( + name="Home in multi-home group 1", + home_group=self.home_group_with_multiple_homes, ) + self.home_in_group_with_multiple_homes2 = HomeFactory( + name="Home in multi-home group 2", + home_group=self.home_group_with_multiple_homes, + ) + + # Single home in home_group_with_single_home + self.home_in_group_with_single_home = HomeFactory( + name="Home in single-home group", + home_group=self.home_group_with_single_home, + ) + + # Home without any group self.home_without_group = HomeFactory( - name="Home 3", + name="Home without group", ) + + # Home not associated with any user self.home_without_user = HomeFactory( - name="Home 4", + name="Home without user", ) - # associate user with homes - self.home_user_relation_1 = HomeUserRelationFactory( - home=self.home_with_group, + # Associate user with homes + HomeUserRelationFactory( + home=self.home_in_group_with_multiple_homes1, user=self.user, ) - self.home_user_relation_2 = HomeUserRelationFactory( - home=self.home_without_group, + HomeUserRelationFactory( + home=self.home_in_group_with_multiple_homes2, user=self.user, ) + HomeUserRelationFactory( + home=self.home_in_group_with_single_home, + user=self.user, + ) + HomeUserRelationFactory(home=self.home_without_group, user=self.user) def test_mock_data(self): - # Check the total count of Homes, HomeGroups, and HomeUserRelations - self.assertEqual(Home.objects.count(), 3) - self.assertEqual(HomeGroup.objects.count(), 2) - self.assertEqual(HomeUserRelation.objects.count(), 2) + # Check the total count of Homes and HomeGroups + self.assertEqual(Home.objects.count(), 5) # Updated count + self.assertEqual(HomeGroup.objects.count(), 3) # Updated count + self.assertEqual(HomeUserRelation.objects.count(), 4) # Updated count # Assert that homes are correctly associated with their home groups - self.assertEqual(self.home_with_group.home_group, self.home_group_with_home) + self.assertEqual( + self.home_in_group_with_multiple_homes1.home_group, + self.home_group_with_multiple_homes, + ) + self.assertEqual( + self.home_in_group_with_multiple_homes2.home_group, + self.home_group_with_multiple_homes, + ) + self.assertEqual( + self.home_in_group_with_single_home.home_group, + self.home_group_with_single_home, + ) self.assertIsNone(self.home_without_group.home_group) self.assertIsNone(self.home_without_user.home_group) @@ -383,57 +419,73 @@ def test_mock_data(self): self.assertFalse(self.home_group_without_homes.homes.exists()) # Assert user belongs to the correct number of homes - self.assertEqual(self.user.homes.count(), 2) + self.assertEqual(self.user.homes.count(), 4) # Updated count # Check if the specific homes are associated with the user - self.assertIn(self.home_with_group, self.user.homes.all()) + self.assertIn(self.home_in_group_with_multiple_homes1, self.user.homes.all()) + self.assertIn(self.home_in_group_with_multiple_homes2, self.user.homes.all()) + self.assertIn(self.home_in_group_with_single_home, self.user.homes.all()) self.assertIn(self.home_without_group, self.user.homes.all()) self.assertNotIn(self.home_without_user, self.user.homes.all()) # Assert superuser has no homes associated self.assertEqual(self.superuser.homes.count(), 0) - def test_context_data_for_regular_user(self): - # Log in as the regular user - self.client.login( - username="testuser", - password="password", - ) + def test_context_data_for_regular_user(self): + # Log in as the regular user + self.client.login(username="testuser", password="password") + + # Get the response from the HomeGroupListView + response = self.client.get(self.url) - # Get the response from the HomeGroupListView - response = self.client.get(self.url) + # Check that the response status code is 200 + self.assertEqual(response.status_code, 200) - # Check that the response status code is 200 - self.assertEqual(response.status_code, 200) + # Extract the context data + context = response.context - # Extract the context data - context = response.context + # Check that homes without a group are correctly in the context + homes_without_group = context["homes_without_group"] + self.assertIn(self.home_without_group, homes_without_group) + self.assertNotIn(self.home_without_user, homes_without_group) - # Check that homes without a group are correctly in the context - homes_without_group = context["homes_without_group"] - self.assertIn(self.home_without_group, homes_without_group) - self.assertNotIn(self.home_without_user, homes_without_group) + # Check that homes with a group are correctly in the context + homes_with_group = context["homes_with_group"] + self.assertIn(self.home_in_group_with_multiple_homes1, homes_with_group) + self.assertIn(self.home_in_group_with_multiple_homes2, homes_with_group) + self.assertIn(self.home_in_group_with_single_home, homes_with_group) + self.assertNotIn(self.home_without_user, homes_with_group) - # Check that homes with a group are correctly in the context - homes_with_group = context["homes_with_group"] - self.assertIn(self.home_with_group, homes_with_group) - self.assertNotIn(self.home_without_user, homes_with_group) + # Ensure that the home_groups_with_homes context is correctly formatted + home_groups_with_homes = context["home_groups_with_homes"] - # Ensure that the home_groups_with_homes context is correctly formatted - home_groups_with_homes = context["home_groups_with_homes"] - self.assertTrue( - any( - group["group_name"] == self.home_group_with_home.name - for group in home_groups_with_homes - ), + # Check for the presence of each group and their corresponding homes + for group in [ + self.home_group_with_multiple_homes, + self.home_group_with_single_home, + ]: + group_in_context = next( + (g for g in home_groups_with_homes if g["group_name"] == group.name), + None, ) - self.assertTrue( - all( - self.home_with_group in group["homes"] - for group in home_groups_with_homes - if group["group_name"] == self.home_group_with_home.name - ), + self.assertIsNotNone( + group_in_context, + f"Group '{group.name}' not found in context.", ) + if group == self.home_group_with_multiple_homes: + self.assertIn( + self.home_in_group_with_multiple_homes1, + group_in_context["homes"], + ) + self.assertIn( + self.home_in_group_with_multiple_homes2, + group_in_context["homes"], + ) + elif group == self.home_group_with_single_home: + self.assertIn( + self.home_in_group_with_single_home, + group_in_context["homes"], + ) def test_context_data_for_superuser(self): # Log in as the superuser @@ -461,44 +513,38 @@ def test_context_data_for_superuser(self): # Ensure that the home_groups_with_homes context is correctly formatted home_groups_with_homes = context["home_groups_with_homes"] - self.assertTrue( - any( - group["group_name"] == self.home_group_with_home.name - for group in home_groups_with_homes - ), - ) - self.assertFalse( - any( - group["group_name"] == self.home_group_without_homes.name - for group in home_groups_with_homes - ), - ) - # Check that the homes in the context are correctly associated with the home groups - # Find the group corresponding to home_group_with_home - group_with_home = next( - ( - group - for group in home_groups_with_homes - if group["group_name"] == self.home_group_with_home.name - ), - None, - ) - - # Assert that the group is found - self.assertIsNotNone( - group_with_home, - "The expected home group was not found in the context.", - ) - - # Assert that home_with_group is in the found group - self.assertIn( - self.home_with_group, - group_with_home["homes"], - "Home with group was not found in the expected group.", - ) + # Check for the presence of each group and their corresponding homes + for group in [ + self.home_group_with_multiple_homes, + self.home_group_with_single_home, + ]: + group_in_context = next( + (g for g in home_groups_with_homes if g["group_name"] == group.name), + None, + ) + self.assertIsNotNone( + group_in_context, + f"Group '{group.name}' not found in context.", + ) - # home group without homes should not be in the context + # Validate the homes within each group + if group == self.home_group_with_multiple_homes: + self.assertIn( + self.home_in_group_with_multiple_homes1, + group_in_context["homes"], + ) + self.assertIn( + self.home_in_group_with_multiple_homes2, + group_in_context["homes"], + ) + elif group == self.home_group_with_single_home: + self.assertIn( + self.home_in_group_with_single_home, + group_in_context["homes"], + ) + + # Validate the handling of the group without homes self.assertFalse( any( group["group_name"] == self.home_group_without_homes.name @@ -506,11 +552,9 @@ def test_context_data_for_superuser(self): ), ) - # homes without groups should correctly list the homes - self.assertIn( - self.home_without_group, - homes_without_group, - ) + # Validate homes without groups + self.assertIn(self.home_without_group, homes_without_group) + self.assertIn(self.home_without_user, homes_without_group) def test_home_group_list_view_uses_correct_template(self): self.client.login(username="testuser", password="password") From b8b2ceeb7061e00115c0adf61132c7ce6526b21d Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Fri, 5 Jan 2024 00:00:31 +0200 Subject: [PATCH 12/31] Return early for unauthenticated user --- homes/views.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/homes/views.py b/homes/views.py index 20f0493..e73a628 100644 --- a/homes/views.py +++ b/homes/views.py @@ -45,6 +45,9 @@ def get_context_data(self, **kwargs: Any) -> dict[str, Any]: user = self.request.user + if not user.is_authenticated: + return context + if user.is_superuser: context["homes_without_group"] = Home.objects.filter( home_group__isnull=True, From 0cc241820b086205c19fe309aebedc908fc9b506 Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Fri, 5 Jan 2024 19:45:26 +0200 Subject: [PATCH 13/31] Initial 403 template --- templates/403.html | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 templates/403.html diff --git a/templates/403.html b/templates/403.html new file mode 100644 index 0000000..8b2d6a7 --- /dev/null +++ b/templates/403.html @@ -0,0 +1,15 @@ +{% extends "base.html" %} + +{% load i18n %} + +{% block title %} + {% translate "Forbidden" %} +{% endblock %} + +{% block content %} +

+ {% translate "Forbidden" %} +

+ +

{% translate "You do not have permission to access this page." %}

+{% endblock %} From 8609e37053d55680eb944575ae6b3752714925d8 Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Fri, 5 Jan 2024 19:45:42 +0200 Subject: [PATCH 14/31] Add allowed hosts --- core/settings.py | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/core/settings.py b/core/settings.py index cb7219a..c7b3a6b 100644 --- a/core/settings.py +++ b/core/settings.py @@ -29,10 +29,22 @@ SECRET_KEY = "django-insecure-+24wlkd-xp!1)z)9#2=3gk+fhv-r9mo4*(kcfc=drz2=68m^-r" # SECURITY WARNING: don't run with debug turned on in production! -DEBUG = True - -ALLOWED_HOSTS = env.list("DJANGO_ALLOWED_HOSTS", default=[]) -CSRF_TRUSTED_ORIGINS = env.list("DJANGO_CSRF_TRUSTED_ORIGINS", default=[]) +DEBUG = env.bool( + "DJANGO_DEBUG", + default=True, +) + +ALLOWED_HOSTS = env.list( + "DJANGO_ALLOWED_HOSTS", + default=[ + "localhost", + "127.0.0.1", + ], +) +CSRF_TRUSTED_ORIGINS = env.list( + "DJANGO_CSRF_TRUSTED_ORIGINS", + default=[], +) INSTALLED_APPS = [ "django.contrib.admin", From 7d767acf3ff4da28a364e7a784e138281f3cd732 Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Fri, 5 Jan 2024 19:46:12 +0200 Subject: [PATCH 15/31] Add authentication on home view --- homes/models.py | 8 ++++++++ homes/views.py | 18 ++++++++++++++---- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/homes/models.py b/homes/models.py index 2acd53a..2faf29d 100644 --- a/homes/models.py +++ b/homes/models.py @@ -258,6 +258,14 @@ def members(self) -> QuerySet[user_model]: """Returns a QuerySet of all members of this home.""" return user_model.objects.filter(home_user_relations__home=self) + def has_access(self, user: user_model) -> bool: + """Returns True if the user has access to this home. + + - Superusers have access to all homes. + - Members of the home have access to the home. + """ + return user.is_superuser or user in self.members.all() + @property def current_residents(self) -> models.QuerySet["Resident"]: """Returns a QuerySet of all current residents for this home.""" diff --git a/homes/views.py b/homes/views.py index e73a628..0a650f0 100644 --- a/homes/views.py +++ b/homes/views.py @@ -1,4 +1,7 @@ from typing import Any + +from django.contrib.auth.mixins import LoginRequiredMixin +from django.core.exceptions import PermissionDenied from django.shortcuts import get_object_or_404 from django.views.generic.detail import DetailView @@ -37,7 +40,7 @@ def regroup_homes_by_home_group(homes): return home_groups_with_homes -class HomeGroupListView(TemplateView): +class HomeGroupListView(LoginRequiredMixin, TemplateView): template_name = "homes/home_group_list.html" def get_context_data(self, **kwargs: Any) -> dict[str, Any]: @@ -74,7 +77,10 @@ def get_context_data(self, **kwargs: Any) -> dict[str, Any]: return context -class HomeDetailView(DetailView): +# user should be logged in + + +class HomeDetailView(LoginRequiredMixin, DetailView): model = Home context_object_name = "home" @@ -89,11 +95,15 @@ def get_object(self, queryset=None): url_uuid=url_uuid, ) # Filter the queryset based on url_uuid - obj = get_object_or_404( + home = get_object_or_404( queryset, ) # Get the object or return a 404 error if not found - return obj + # ensure the user has access to the home + if not home.has_access(user=self.request.user): + raise PermissionDenied + + return home def prepare_activity_charts(self, context): """Prepare activity charts and add them to the template context.""" From a2ef4757a3cf316cf3fcf3d2a062296265893252 Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Fri, 5 Jan 2024 20:06:12 +0200 Subject: [PATCH 16/31] Add HomeDetailViewTests --- homes/tests.py | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/homes/tests.py b/homes/tests.py index cb02e3c..165aacd 100644 --- a/homes/tests.py +++ b/homes/tests.py @@ -1,4 +1,5 @@ from datetime import date, timedelta +from http import HTTPStatus from io import StringIO from django.core.management import call_command @@ -560,3 +561,52 @@ def test_home_group_list_view_uses_correct_template(self): self.client.login(username="testuser", password="password") response = self.client.get(self.url) self.assertTemplateUsed(response, "homes/home_group_list.html") + + +class HomeDetailViewTests(TestCase): + def setUp(self): + # Create users + self.regular_user = User.objects.create_user( + username="regular", + password="test", + ) + self.super_user = User.objects.create_superuser( + username="super", + password="test", + ) + self.member_user = User.objects.create_user(username="member", password="test") + + # Create a home + self.home = HomeFactory() + + self.url = reverse("home-detail-view", kwargs={"url_uuid": self.home.url_uuid}) + + # Create a relation where member_user is a member of home + HomeUserRelationFactory(home=self.home, user=self.member_user) + + def test_access_denied_non_member(self): + self.client.login(username="regular", password="test") + + response = self.client.get(self.url) + self.assertEqual( + response.status_code, + HTTPStatus.FORBIDDEN, + ) + + def test_access_granted_member(self): + self.client.login(username="member", password="test") + + response = self.client.get(self.url) + self.assertEqual( + response.status_code, + HTTPStatus.OK, + ) + + def test_access_granted_superuser(self): + self.client.login(username="super", password="test") + + response = self.client.get(self.url) + self.assertEqual( + response.status_code, + HTTPStatus.OK, + ) From ae67028b535c827a9a19a96e8210d41f44b16ccf Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Fri, 5 Jan 2024 20:35:51 +0200 Subject: [PATCH 17/31] Pass user into form; refactor form code --- activities/views.py | 14 +++++++-- metrics/forms.py | 69 ++++++++++++++++++++++++++++++++++----------- 2 files changed, 63 insertions(+), 20 deletions(-) diff --git a/activities/views.py b/activities/views.py index 817b5f4..46cceb1 100644 --- a/activities/views.py +++ b/activities/views.py @@ -1,15 +1,16 @@ import uuid +from django.db import transaction +from django.contrib.auth.mixins import LoginRequiredMixin from django.urls import reverse_lazy from django.views.generic import ListView, FormView -from django.db import transaction from metrics.models import ResidentActivity from residents.models import Residency, Resident from metrics.forms import ResidentActivityForm -class ResidentActivityListView(ListView): +class ResidentActivityListView(LoginRequiredMixin, ListView): template_name = "activities/resident_activity_list.html" queryset = ResidentActivity.objects.all() context_object_name = "activities" @@ -17,11 +18,18 @@ class ResidentActivityListView(ListView): ordering = ["-activity_date"] -class ResidentActivityFormView(FormView): +class ResidentActivityFormView(LoginRequiredMixin, FormView): template_name = "activities/resident_activity_form.html" form_class = ResidentActivityForm success_url = reverse_lazy("activity-list-view") + def get_form_kwargs(self): + kwargs = super().get_form_kwargs() + + kwargs["user"] = self.request.user + + return kwargs + @transaction.atomic def post(self, request, *args, **kwargs): """Override the post method to add the resident activity in the same diff --git a/metrics/forms.py b/metrics/forms.py index c787a03..a86724b 100644 --- a/metrics/forms.py +++ b/metrics/forms.py @@ -1,8 +1,11 @@ # import the forms module from django from django import forms +from django.db.models import QuerySet from residents.models import Residency from .models import ResidentActivity +from django.contrib.auth import get_user_model +user_model = get_user_model() activity_type_choices = [ (choice[0], choice[1]) for choice in ResidentActivity.ActivityTypeChoices.choices @@ -12,38 +15,68 @@ ] -def get_resident_choices(): - # Fetch Residency objects with related 'home' and 'resident' in a single query - residencies = Residency.objects.filter(move_out__isnull=True).select_related( - "home", - "resident", - ) +def group_residents_by_home( + residencies: QuerySet[Residency], +) -> dict[str, list[tuple[int, str]]]: + """Group residents by home. + + Args: + residencies (QuerySet): A QuerySet of Residency objects with related 'home' and 'resident'. + Returns: + dict: A dictionary with home names as keys and a list of (resident_id, resident_name) tuples as values. + """ # Initialize a dictionary to group residents by home - resident_by_home = {} + residents_by_home = {} for residency in residencies: home_name = residency.home.name resident_name = residency.resident.full_name # Assuming full_name is a method - if home_name not in resident_by_home: - resident_by_home[home_name] = [] + if home_name not in residents_by_home: + residents_by_home[home_name] = [] - resident_by_home[home_name].append((residency.resident.id, resident_name)) + residents_by_home[home_name].append((residency.resident.id, resident_name)) - # Sort residents within each home + # Sort residents by name within each home resident_name_col_index = 1 - for home in resident_by_home: - resident_by_home[home].sort( + for home in residents_by_home: + residents_by_home[home].sort( key=lambda x: x[resident_name_col_index], - ) # Sort by resident name + ) + + return residents_by_home + + +def prepare_resident_choices(residencies: QuerySet[Residency]) -> list[tuple[int, str]]: + """Prepare a list of resident choices for a form. + + The list is sorted by home name and then by resident name. + + Args: + residencies (QuerySet): A QuerySet of Residency objects with related 'home' and 'resident'. + """ + residents_by_home = group_residents_by_home(residencies) # Sort the homes and convert the dictionary to the desired list format home_name_col_index = 0 resident_choices = sorted( - resident_by_home.items(), + residents_by_home.items(), key=lambda x: x[home_name_col_index], ) # Sort by home name + + return resident_choices + + +def get_resident_choices(user=None): + # Fetch Residency objects with related 'home' and 'resident' in a single query + residencies = Residency.objects.filter(move_out__isnull=True).select_related( + "home", + "resident", + ) + + resident_choices = prepare_resident_choices(residencies) + return resident_choices @@ -59,7 +92,9 @@ class ResidentActivityForm(forms.Form): activity_minutes = forms.IntegerField() caregiver_role = forms.ChoiceField(choices=caregiver_role_choices) - def __init__(self, *args, **kwargs): + def __init__(self, user: user_model, *args, **kwargs): super().__init__(*args, **kwargs) - self.fields["residents"].choices = get_resident_choices() + user = kwargs.pop("user", None) + + self.fields["residents"].choices = get_resident_choices(user=user) From ae7dc67befa6b40e58a05d2ca286f4497070144d Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Fri, 5 Jan 2024 20:46:24 +0200 Subject: [PATCH 18/31] Filter home choices by user status --- activities/views.py | 6 ++++++ metrics/forms.py | 21 +++++++++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/activities/views.py b/activities/views.py index 46cceb1..b3feada 100644 --- a/activities/views.py +++ b/activities/views.py @@ -24,6 +24,12 @@ class ResidentActivityFormView(LoginRequiredMixin, FormView): success_url = reverse_lazy("activity-list-view") def get_form_kwargs(self): + """Override the get_form_kwargs method to pass the user to the form. + + This will allow the form to filter the residents by the user's + homes or the superuser to filter by all homes. + """ + kwargs = super().get_form_kwargs() kwargs["user"] = self.request.user diff --git a/metrics/forms.py b/metrics/forms.py index a86724b..0696d05 100644 --- a/metrics/forms.py +++ b/metrics/forms.py @@ -70,12 +70,20 @@ def prepare_resident_choices(residencies: QuerySet[Residency]) -> list[tuple[int def get_resident_choices(user=None): # Fetch Residency objects with related 'home' and 'resident' in a single query - residencies = Residency.objects.filter(move_out__isnull=True).select_related( + if user.is_superuser: + residencies = Residency.objects.filter(move_out__isnull=True) + else: + residencies = Residency.objects.filter( + home__in=user.homes.all(), + move_out__isnull=True, + ) + + residencies.select_related( "home", "resident", ) - resident_choices = prepare_resident_choices(residencies) + resident_choices = prepare_resident_choices(residencies=residencies) return resident_choices @@ -92,9 +100,14 @@ class ResidentActivityForm(forms.Form): activity_minutes = forms.IntegerField() caregiver_role = forms.ChoiceField(choices=caregiver_role_choices) - def __init__(self, user: user_model, *args, **kwargs): - super().__init__(*args, **kwargs) + def __init__(self, *args, **kwargs): + """Initialize the form. + Include the request user in the form kwargs if available so it + can be used to filter the resident choices. + """ user = kwargs.pop("user", None) + super().__init__(*args, **kwargs) + self.fields["residents"].choices = get_resident_choices(user=user) From 8f62456dccea5d416096be3400aecf93bc1bb4ed Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Fri, 5 Jan 2024 21:00:54 +0200 Subject: [PATCH 19/31] Fix failing tests --- metrics/tests.py | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/metrics/tests.py b/metrics/tests.py index 9059c24..22c0f9a 100644 --- a/metrics/tests.py +++ b/metrics/tests.py @@ -2,18 +2,45 @@ from django.test import TestCase from .models import ResidentActivity -from homes.factories import HomeFactory +from homes.factories import HomeFactory, HomeUserRelationFactory from residents.factories import ResidentFactory, ResidencyFactory from datetime import date from django.urls import reverse +from django.contrib.auth import get_user_model + +user_model = get_user_model() class ResidentActivityFormViewTestCase(TestCase): def setUp(self): + # Create a user + self.general_user = user_model.objects.create_user( + username="gerneraluser", + email="general@tzo.com", + password="testpassword", + ) + self.home_user = user_model.objects.create_user( + username="testuser", + email="test@email.com", + password="testpassword", + ) + self.superuser = user_model.objects.create_superuser( + username="superuser", + email="superuser@test.com", + password="superuserpassword", + ) + # Create test data using factories self.home1 = HomeFactory(name="Home 1") + + # Add the user to the home + HomeUserRelationFactory(home=self.home1, user=self.home_user) + + # Create two residents self.resident1 = ResidentFactory(first_name="Alice") self.resident2 = ResidentFactory(first_name="Bob") + + # Create a residency for each resident self.residency1 = ResidencyFactory( home=self.home1, resident=self.resident1, @@ -41,6 +68,9 @@ def test_resident_activity_form_view_create_multiple_resident_activity(self): "caregiver_role": ResidentActivity.CaregiverRoleChoices.NURSE, } + # log in superuser + self.client.force_login(self.superuser) + # Make POST request response = self.client.post( reverse("resident-activity-form-view"), @@ -91,6 +121,9 @@ def test_activity_rollback_on_residency_exception(self): "caregiver_role": ResidentActivity.CaregiverRoleChoices.NURSE, } + # log in superuser + self.client.force_login(self.superuser) + # Make POST request response = self.client.post( reverse("resident-activity-form-view"), From c7e0ad2977ab592dd24889aa3a857d8ad2d7d412 Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Fri, 5 Jan 2024 21:06:56 +0200 Subject: [PATCH 20/31] Add User.can_add_activity --- accounts/models.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/accounts/models.py b/accounts/models.py index 325b485..cbde945 100644 --- a/accounts/models.py +++ b/accounts/models.py @@ -20,3 +20,12 @@ def homes(self) -> QuerySet["homes.Home"]: from homes.models import Home return Home.objects.filter(home_user_relations__user=self) + + @property + def can_add_activity(self) -> bool: + """Return True if the user can add an activity. + + A user can add an activity if they are a superuser or if they + are associated with at least one home. + """ + return self.is_superuser or self.homes.exists() From 18957823e3b623486895c84fa9f9c26d8de09dc5 Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Fri, 5 Jan 2024 21:11:47 +0200 Subject: [PATCH 21/31] Hide Add Activity button for users who can't add activity --- templates/navigation.html | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/templates/navigation.html b/templates/navigation.html index a6d75e7..9004d3b 100644 --- a/templates/navigation.html +++ b/templates/navigation.html @@ -6,7 +6,7 @@ GeriLife Caregiving -