From 3ae42393a646a594b2e70145a69023dfc2363a21 Mon Sep 17 00:00:00 2001 From: Daniele Procida Date: Mon, 20 Apr 2026 05:05:01 +0300 Subject: [PATCH 1/8] Updated psycopg2 --- dashboard/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dashboard/requirements.txt b/dashboard/requirements.txt index ebc23e5..1ccb8b6 100644 --- a/dashboard/requirements.txt +++ b/dashboard/requirements.txt @@ -2,7 +2,7 @@ Django==5.2.13 PyYAML==6.0.2 docutils==0.21.2 whitenoise==6.9.0 -psycopg2-binary==2.9.10 +psycopg2-binary==2.9.11 tzdata==2025.1 django-browser-reload==1.18.0 django-tinymce==4.1.0 From bfc38a171b79d802786a9eedb3eca6da753815cf Mon Sep 17 00:00:00 2001 From: Daniele Procida Date: Mon, 20 Apr 2026 05:47:48 +0300 Subject: [PATCH 2/8] Added a testing shim for OIDC. This allows tests in environments that haven't loaded OIDC to continue running. See https://docs.pytest.org/en/latest/reference/fixtures.html#conftest-py-sharing-fixtures-across-multiple-files for notes on shared fixtures. --- dashboard/conftest.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 dashboard/conftest.py diff --git a/dashboard/conftest.py b/dashboard/conftest.py new file mode 100644 index 0000000..67d462a --- /dev/null +++ b/dashboard/conftest.py @@ -0,0 +1,28 @@ +import sys +import types + +from django.http import HttpResponse + + +# Creat a "fake" mozilla_django_oidc.views so that tests will run, +# even if mozilla_django_oidc is not available at import time for +# tests. + +if "mozilla_django_oidc" not in sys.modules: + oidc_module = types.ModuleType("mozilla_django_oidc") + oidc_views_module = types.ModuleType("mozilla_django_oidc.views") + + class _DummyOIDCView: + @classmethod + def as_view(cls): + def _view(request, *args, **kwargs): + return HttpResponse("") + + return _view + + oidc_views_module.OIDCAuthenticationRequestView = _DummyOIDCView + oidc_views_module.OIDCAuthenticationCallbackView = _DummyOIDCView + oidc_views_module.OIDCLogoutView = _DummyOIDCView + oidc_module.views = oidc_views_module + sys.modules["mozilla_django_oidc"] = oidc_module + sys.modules["mozilla_django_oidc.views"] = oidc_views_module \ No newline at end of file From 3a8a10d0949647981555bbfcc8b879fc96290f6d Mon Sep 17 00:00:00 2001 From: Daniele Procida Date: Mon, 20 Apr 2026 06:26:24 +0300 Subject: [PATCH 3/8] Added a series of tests for view functions Updated: test_toggle_condition_url_patterns (Simplified, and now only checks the reversed route, rather than the string) New checks for user permissions: * test_action_toggle_commitment_denies_user_without_permission * test_action_toggle_condition_denies_user_without_permission * test_action_select_reason_denies_user_without_permission New checks for HTTP methods: * test_action_toggle_commitment_rejects_non_put_method * test_action_toggle_condition_rejects_non_put_method * test_action_select_reason_rejects_non_put_method New checks for expected results: new: test_action_toggle_commitment_allows_authorized_put_and_updates_commitment new: test_action_toggle_condition_allows_authorized_put_and_updates_status new: test_action_select_reason_allows_authorized_put_and_sets_reason I used Copilot to help create these tests. --- dashboard/projects/test_views.py | 255 ++++++++++++++++++++++++++++++- 1 file changed, 251 insertions(+), 4 deletions(-) diff --git a/dashboard/projects/test_views.py b/dashboard/projects/test_views.py index b70c423..9c3c873 100644 --- a/dashboard/projects/test_views.py +++ b/dashboard/projects/test_views.py @@ -1,10 +1,257 @@ import pytest +from urllib.parse import parse_qs, urlparse from django.urls import reverse +from django.contrib.auth.models import Permission, User + +from framework.models import Condition, Level, Objective, ObjectiveGroup, Reason, WorkCycle +from projects.models import Commitment, Project, ProjectObjective, ProjectObjectiveCondition def test_toggle_condition_url_patterns(): - assert reverse( + url = reverse("projects:action_toggle_condition", args=[1]) + assert url == "/action_toggle_condition/1" + + +@pytest.fixture +def user_without_permissions(client): + user = User.objects.create_user(username="no_perm", password="password") + client.login(username="no_perm", password="password") + return user + + +@pytest.fixture +def user_can_change_commitment(client): + user = User.objects.create_user(username="change_commitment", password="password") + permission = Permission.objects.get( + codename="change_commitment", + content_type__app_label="projects", + ) + user.user_permissions.add(permission) + client.login(username="change_commitment", password="password") + return user + + +@pytest.fixture +def user_can_change_projectobjectivecondition(client): + user = User.objects.create_user( + username="change_projectobjectivecondition", password="password" + ) + permission = Permission.objects.get( + codename="change_projectobjectivecondition", + content_type__app_label="projects", + ) + user.user_permissions.add(permission) + client.login(username="change_projectobjectivecondition", password="password") + return user + + +@pytest.fixture +def user_can_change_projectobjective(client): + user = User.objects.create_user( + username="change_projectobjective", password="password" + ) + permission = Permission.objects.get( + codename="change_projectobjective", + content_type__app_label="projects", + ) + user.user_permissions.add(permission) + client.login(username="change_projectobjective", password="password") + return user + + +@pytest.fixture +def objective_group(): + return ObjectiveGroup.objects.create(name="group") + + +@pytest.fixture +def objective(objective_group): + return Objective.objects.create(name="objective", group=objective_group, weight=1) + + +@pytest.fixture +def level(): + return Level.objects.create(name="level", value=1) + + +@pytest.fixture +def work_cycle(): + return WorkCycle.objects.create(name="wc", timestamp="2026-01-01", is_current=True) + + +@pytest.fixture +def project(objective, level, work_cycle): + return Project.objects.create(name="project") + + +@pytest.fixture +def condition(objective, level): + return Condition.objects.create(name="condition", objective=objective, level=level) + + +@pytest.fixture +def project_objective(project, objective): + return ProjectObjective.objects.get(project=project, objective=objective) + + +@pytest.fixture +def project_objective_condition(project, objective, condition): + return ProjectObjectiveCondition.objects.get( + project=project, + objective=objective, + condition=condition, + ) + + +@pytest.fixture +def commitment(project, objective, level, work_cycle): + return Commitment.objects.get( + project=project, + objective=objective, + level=level, + work_cycle=work_cycle, + ) + + +@pytest.fixture +def reason(): + return Reason.objects.create(name="not-started", value=1) + + +@pytest.mark.django_db +def test_action_toggle_commitment_denies_user_without_permission( + client, user_without_permissions, commitment +): + url = reverse("projects:action_toggle_commitment", args=[commitment.id]) + response = client.put(url) + + assert response.status_code == 302 + expected_redirect = f"{reverse('login')}?next={url}" + assert response.url == expected_redirect + + +@pytest.mark.django_db +def test_action_toggle_condition_denies_user_without_permission( + client, user_without_permissions, project_objective_condition +): + url = ( + reverse( + "projects:action_toggle_condition", + args=[project_objective_condition.id], + ) + + "?status=&target=done" + ) + response = client.put(url) + + assert response.status_code == 302 + parsed = urlparse(response.url) + assert parsed.path == reverse("login") + assert parse_qs(parsed.query)["next"][0] == url + + +@pytest.mark.django_db +def test_action_select_reason_denies_user_without_permission( + client, user_without_permissions, project_objective, reason +): + url = reverse("projects:action_select_reason", args=[project_objective.id]) + response = client.generic( + "PUT", + url, + data=f"ifnotstarted={reason.id}", + content_type="application/x-www-form-urlencoded", + ) + + assert response.status_code == 302 + expected_redirect = f"{reverse('login')}?next={url}" + assert response.url == expected_redirect + + +@pytest.mark.django_db +def test_action_toggle_commitment_rejects_non_put_method( + client, user_can_change_commitment, commitment +): + url = reverse("projects:action_toggle_commitment", args=[commitment.id]) + response = client.get(url) + + assert response.status_code == 405 + + +@pytest.mark.django_db +def test_action_toggle_commitment_allows_authorized_put_and_updates_commitment( + client, user_can_change_commitment, commitment +): + assert commitment.committed is False + + url = reverse("projects:action_toggle_commitment", args=[commitment.id]) + response = client.put(url) + + commitment.refresh_from_db() + assert response.status_code == 200 + assert commitment.committed is True + assert response["HX-Trigger-After-Swap"] == "updateCommitment" + + +@pytest.mark.django_db +def test_action_toggle_condition_rejects_non_put_method( + client, user_can_change_projectobjectivecondition, project_objective_condition +): + url = ( + reverse( + "projects:action_toggle_condition", + args=[project_objective_condition.id], + ) + + "?status=&target=done" + ) + response = client.get(url) + + assert response.status_code == 405 + + +@pytest.mark.django_db +def test_action_toggle_condition_allows_authorized_put_and_updates_status( + client, user_can_change_projectobjectivecondition, project_objective_condition +): + assert project_objective_condition.status == "" + + url = ( + reverse( "projects:action_toggle_condition", - args=[1], - query={"status": "DO", "target": "done"} - ) == "/action_toggle_condition/1?status=DO&target=done" + args=[project_objective_condition.id], + ) + + "?status=&target=done" + ) + response = client.put(url) + + project_objective_condition.refresh_from_db() + assert response.status_code == 200 + assert project_objective_condition.status == "DO" + assert "HX-Trigger-After-Swap" in response + + +@pytest.mark.django_db +def test_action_select_reason_rejects_non_put_method( + client, user_can_change_projectobjective, project_objective +): + url = reverse("projects:action_select_reason", args=[project_objective.id]) + response = client.get(url) + + assert response.status_code == 405 + + +@pytest.mark.django_db +def test_action_select_reason_allows_authorized_put_and_sets_reason( + client, user_can_change_projectobjective, project_objective, reason +): + assert project_objective.unstarted_reason is None + + url = reverse("projects:action_select_reason", args=[project_objective.id]) + response = client.generic( + "PUT", + url, + data=f"ifnotstarted={reason.id}", + content_type="application/x-www-form-urlencoded", + ) + + project_objective.refresh_from_db() + assert response.status_code == 200 + assert project_objective.unstarted_reason_id == reason.id From 77815810c6c4790be974256c0d10dfbde4e2f4f1 Mon Sep 17 00:00:00 2001 From: Daniele Procida Date: Mon, 20 Apr 2026 07:01:19 +0300 Subject: [PATCH 4/8] Added a permissions constraint for project_basic_form_save This fixes an auth issue. It was possible to POST a change to a project, without even being authenticated. The only safeguard was the user interface, that didn't offer make it possible. This adds @permission_required("projects.change_project") to the view, and some tests. * added a fixture for a user with permission to change a project * added a test: do we reject changes from unauthenticated users? * added a test: do we reject changes from unauthorised users? * added a test: do we allow and save changes from users with the right permissions? I used Copilot to uncover the issue and help create the tests. --- dashboard/projects/test_views.py | 84 ++++++++++++++++++++++++++++++++ dashboard/projects/views.py | 1 + 2 files changed, 85 insertions(+) diff --git a/dashboard/projects/test_views.py b/dashboard/projects/test_views.py index 9c3c873..468ae65 100644 --- a/dashboard/projects/test_views.py +++ b/dashboard/projects/test_views.py @@ -59,6 +59,18 @@ def user_can_change_projectobjective(client): return user +@pytest.fixture +def user_can_change_project(client): + user = User.objects.create_user(username="change_project", password="password") + permission = Permission.objects.get( + codename="change_project", + content_type__app_label="projects", + ) + user.user_permissions.add(permission) + client.login(username="change_project", password="password") + return user + + @pytest.fixture def objective_group(): return ObjectiveGroup.objects.create(name="group") @@ -118,6 +130,78 @@ def reason(): return Reason.objects.create(name="not-started", value=1) +@pytest.mark.django_db +def test_project_basic_form_save_denies_unauthenticated_user(client, project): + original_owner = project.owner + url = reverse("projects:project_basic_form_save", args=[project.id]) + response = client.post( + url, + data={ + "name": project.name, + "url": project.url, + "group": "", + "owner": "changed owner", + "driver": project.driver or "", + "agreement_status": "", + "last_review": "", + "last_review_status": "", + }, + ) + + project.refresh_from_db() + assert response.status_code == 302 + assert response.url == f"{reverse('login')}?next={url}" + assert project.owner == original_owner + + +@pytest.mark.django_db +def test_project_basic_form_save_denies_user_without_permission( + client, user_without_permissions, project +): + original_owner = project.owner + url = reverse("projects:project_basic_form_save", args=[project.id]) + response = client.post( + url, + data={ + "name": project.name, + "url": project.url, + "group": "", + "owner": "changed owner", + "driver": project.driver or "", + "agreement_status": "", + "last_review": "", + "last_review_status": "", + }, + ) + + project.refresh_from_db() + assert response.status_code == 302 + assert response.url == f"{reverse('login')}?next={url}" + assert project.owner == original_owner + + +@pytest.mark.django_db +def test_project_basic_form_save_allows_user_with_permission(client, user_can_change_project, project): + url = reverse("projects:project_basic_form_save", args=[project.id]) + response = client.post( + url, + data={ + "name": project.name, + "url": project.url, + "group": "", + "owner": "changed owner", + "driver": project.driver or "", + "agreement_status": "", + "last_review": "", + "last_review_status": "", + }, + ) + + project.refresh_from_db() + assert response.status_code == 200 + assert project.owner == "changed owner" + + @pytest.mark.django_db def test_action_toggle_commitment_denies_user_without_permission( client, user_without_permissions, commitment diff --git a/dashboard/projects/views.py b/dashboard/projects/views.py index 4e88c26..73e7a89 100644 --- a/dashboard/projects/views.py +++ b/dashboard/projects/views.py @@ -234,6 +234,7 @@ def action_select_reason(request, projectobjective_id): # form methods +@permission_required("projects.change_project") @require_http_methods(["POST"]) def project_basic_form_save(request, project_id): instance = Project.objects.get(id=project_id) From 58114d598ff9c1c82c25a5b5f89e243f4a17bf5b Mon Sep 17 00:00:00 2001 From: Daniele Procida Date: Mon, 20 Apr 2026 08:53:44 +0300 Subject: [PATCH 5/8] Added a failing test for cascading behaviour If a new Level and Condition are added, these do not cascade to relationships with Commitments. --- dashboard/framework/test_models.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/dashboard/framework/test_models.py b/dashboard/framework/test_models.py index 0647fff..677470e 100644 --- a/dashboard/framework/test_models.py +++ b/dashboard/framework/test_models.py @@ -153,3 +153,29 @@ def test_new_objective_means_new_commitments( assert project.commitment_set.count() == 2 assert work_cycle.commitment_set.count() == 2 + + +@pytest.mark.django_db +def test_new_condition_with_new_level_backfills_commitment( + project, objective, condition, work_cycle +): + # A new condition at a new level should create a matching commitment + # for existing rows. + + assert Commitment.objects.filter( + project=project, objective=objective, work_cycle=work_cycle + ).count() == 1 + + new_level = Level.objects.create(name="test_level_2", value=2) + Condition.objects.create( + name="test_condition_2", objective=objective, level=new_level + ) + + # Expected behaviour: creating a new condition/level backfills + # commitments. + assert Commitment.objects.filter( + project=project, + objective=objective, + work_cycle=work_cycle, + level=new_level, + ).exists() From 4e3bf46c21f368c71daa881322548b7ceb09a55a Mon Sep 17 00:00:00 2001 From: Daniele Procida Date: Mon, 20 Apr 2026 14:30:19 +0200 Subject: [PATCH 6/8] Added Commitments creation to Condition.save() Fixes a failing test for Commitments that don't get created properly. --- dashboard/framework/models.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dashboard/framework/models.py b/dashboard/framework/models.py index f2741cc..8b9a0b1 100644 --- a/dashboard/framework/models.py +++ b/dashboard/framework/models.py @@ -166,6 +166,7 @@ def save(self, *args, **kwargs): from projects.models import ( ProjectObjective, ProjectObjectiveCondition, + Commitment ) # avoids circular import super().save(*args, **kwargs) @@ -178,6 +179,13 @@ def save(self, *args, **kwargs): objective=projectobjective.objective, condition=self, ) + for work_cycle in WorkCycle.objects.all(): + Commitment.objects.get_or_create( + work_cycle=work_cycle, + project=projectobjective.project, + objective=projectobjective.objective, + level=self.level, + ) class Meta: ordering = ["objective__name", "level__value"] From 2493d53680a08782244ecd501b084b59c0f4d481 Mon Sep 17 00:00:00 2001 From: David Wilding Date: Sun, 26 Apr 2026 09:12:55 +0800 Subject: [PATCH 7/8] remove unused/duplicate imports --- dashboard/projects/test_views.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/dashboard/projects/test_views.py b/dashboard/projects/test_views.py index 75f5d12..371ebd1 100644 --- a/dashboard/projects/test_views.py +++ b/dashboard/projects/test_views.py @@ -3,10 +3,6 @@ from django.test import override_settings from django.urls import reverse -from django.contrib.auth.models import Permission, User - -from framework.models import Condition, Level, Objective, ObjectiveGroup, Reason, WorkCycle -from projects.models import Commitment, Project, ProjectObjective, ProjectObjectiveCondition from framework.models import ( Condition, From 4f5b12af19ad4c93940f421667fd400582f28a2e Mon Sep 17 00:00:00 2001 From: David Wilding Date: Sun, 26 Apr 2026 09:13:51 +0800 Subject: [PATCH 8/8] format testing code --- dashboard/framework/test_models.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/dashboard/framework/test_models.py b/dashboard/framework/test_models.py index 677470e..f79d878 100644 --- a/dashboard/framework/test_models.py +++ b/dashboard/framework/test_models.py @@ -162,9 +162,12 @@ def test_new_condition_with_new_level_backfills_commitment( # A new condition at a new level should create a matching commitment # for existing rows. - assert Commitment.objects.filter( - project=project, objective=objective, work_cycle=work_cycle - ).count() == 1 + assert ( + Commitment.objects.filter( + project=project, objective=objective, work_cycle=work_cycle + ).count() + == 1 + ) new_level = Level.objects.create(name="test_level_2", value=2) Condition.objects.create(