From 727dbb1b5f3f20b6cb3804b0bd077df6f8fb35c9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 26 Feb 2026 20:34:42 +0000 Subject: [PATCH 1/2] List flow rule exceptions on gradebook-single with deactivate support Co-authored-by: inducer <352067+inducer@users.noreply.github.com> --- course/grades.py | 12 ++ course/templates/course/gradebook-single.html | 60 +++++++ course/views.py | 33 ++++ relate/urls.py | 8 + tests/test_grades/test_grades.py | 147 ++++++++++++++++++ 5 files changed, 260 insertions(+) diff --git a/course/grades.py b/course/grades.py index c8b1d0f9b..51bf1c855 100644 --- a/course/grades.py +++ b/course/grades.py @@ -55,6 +55,7 @@ from course.flow import adjust_flow_session_page_data from course.models import ( FlowPageVisit, + FlowRuleException, FlowSession, GradeChange, GradeStateMachine, @@ -1045,6 +1046,16 @@ def view_single_grade(pctx: CoursePageContext, participation_id: str, # }}} + flow_rule_exceptions: list[FlowRuleException] | None = None + if show_privileged_info and opportunity.flow_id: + flow_rule_exceptions = list( + FlowRuleException.objects + .filter( + participation=participation, + flow_id=opportunity.flow_id) + .order_by("creation_time") + .select_related("creator")) + return render_course_page(pctx, "course/gradebook-single.html", { "opportunity": opportunity, "avg_grade_percentage": avg_grade_percentage, @@ -1061,6 +1072,7 @@ def view_single_grade(pctx: CoursePageContext, participation_id: str, or PPerm.end_flow_session or PPerm.regrade_flow_session or PPerm.recalculate_flow_session_grade), + "flow_rule_exceptions": flow_rule_exceptions, }) # }}} diff --git a/course/templates/course/gradebook-single.html b/course/templates/course/gradebook-single.html index 95186675b..b3cc41940 100644 --- a/course/templates/course/gradebook-single.html +++ b/course/templates/course/gradebook-single.html @@ -344,6 +344,66 @@

{% trans "Flow sessions" %}

{% endif %} + + {% if flow_rule_exceptions != None %} +

{% trans "Flow rule exceptions" %}

+ {% if not flow_rule_exceptions %} +

{% trans "(no exceptions)" %}

+ {% else %} + + + + + + + + + + {% if pperm.grant_exception %} + + {% endif %} + + + + {% for exc in flow_rule_exceptions %} + + + + + + + + {% if pperm.grant_exception %} + + {% endif %} + + {% endfor %} + +
{% trans "Kind" %}{% trans "Created" %}{% trans "Creator" %}{% trans "Expiration" %}{% trans "Comment" %}{% trans "Active" %}{% trans "Actions" %}
+ {% if not exc.active %}{% endif %} + {{ exc.get_kind_display }} + {% if not exc.active %}{% endif %} + {{ exc.creation_time }}{% if exc.creator %}{{ exc.creator.username }}{% else %}{% trans "(none)" %}{% endif %}{% if exc.expiration %}{{ exc.expiration }}{% else %}{% trans "(none)" %}{% endif %}{% if exc.comment %}{{ exc.comment }}{% endif %} + {% if exc.active %} + {% trans "Yes" %} + {% else %} + {% trans "No" %} + {% endif %} + + {% if exc.active %} +
+ {% csrf_token %} + +
+ {% endif %} +
+ {% endif %} + {% endif %} {# }}} #} {% endblock %} diff --git a/course/views.py b/course/views.py index b9a54c211..6484e81e3 100644 --- a/course/views.py +++ b/course/views.py @@ -1356,6 +1356,39 @@ def transfer_attr(name: str): # }}} +# {{{ deactivate flow rule exception + +@http_dec.require_POST +@course_view +@transaction.atomic +def deactivate_flow_exception(pctx: CoursePageContext, + exception_id: str, opportunity_id: str) -> http.HttpResponse: + + if not pctx.has_permission(PPerm.grant_exception): + raise PermissionDenied(_("may not grant exceptions")) + + exception = get_object_or_404(FlowRuleException, id=int(exception_id)) + + if exception.participation.course != pctx.course: + raise SuspiciousOperation(_("exception does not belong to this course")) + + if not exception.active: + messages.add_message(pctx.request, messages.INFO, + _("Exception was already deactivated.")) + else: + exception.active = False + exception.save() + messages.add_message(pctx.request, messages.SUCCESS, + _("Exception deactivated.")) + + return redirect("relate-view_single_grade", + pctx.course.identifier, + exception.participation.id, + opportunity_id) + +# }}} + + # {{{ ssh keypair @login_required diff --git a/relate/urls.py b/relate/urls.py index 390285e28..c95ead80b 100644 --- a/relate/urls.py +++ b/relate/urls.py @@ -501,6 +501,14 @@ "/$", course.views.grant_exception_stage_3, name="relate-grant_exception_stage_3"), + re_path(r"^course" + "/" + COURSE_ID_REGEX + + "/deactivate-exception" + "/(?P[0-9]+)" + "/(?P[0-9]+)" + "/$", + course.views.deactivate_flow_exception, + name="relate-deactivate_flow_exception"), # }}} diff --git a/tests/test_grades/test_grades.py b/tests/test_grades/test_grades.py index aab612162..83e485d55 100644 --- a/tests/test_grades/test_grades.py +++ b/tests/test_grades/test_grades.py @@ -1684,6 +1684,153 @@ def test_filter_out_pre_public_grade_changes(self): self.assertEqual(len(resp_gchanges), 2) +class ViewSingleGradeFlowRuleExceptionsTest(GradesTestMixin, TestCase): + # Tests for flow_rule_exceptions context in view_single_grade + + def test_flow_rule_exceptions_not_in_context_for_no_flow_id(self): + gopp = factories.GradingOpportunityFactory( + course=self.course, identifier="no_flow_id", flow_id=None) + resp = self.get_view_single_grade(self.student_participation, gopp) + self.assertEqual(resp.status_code, 200) + self.assertIsNone(resp.context["flow_rule_exceptions"]) + + def test_flow_rule_exceptions_empty(self): + resp = self.get_view_single_grade( + self.student_participation, self.gopp) + self.assertEqual(resp.status_code, 200) + exceptions = resp.context["flow_rule_exceptions"] + self.assertIsNotNone(exceptions) + self.assertEqual(len(exceptions), 0) + + def test_flow_rule_exceptions_shown(self): + exc = factories.FlowRuleExceptionFactory( + participation=self.student_participation, + flow_id=self.flow_id, + kind=constants.FlowRuleKind.access, + active=True) + resp = self.get_view_single_grade( + self.student_participation, self.gopp) + self.assertEqual(resp.status_code, 200) + exceptions = resp.context["flow_rule_exceptions"] + self.assertIsNotNone(exceptions) + self.assertEqual(len(exceptions), 1) + self.assertEqual(exceptions[0].pk, exc.pk) + + def test_flow_rule_exceptions_includes_inactive(self): + factories.FlowRuleExceptionFactory( + participation=self.student_participation, + flow_id=self.flow_id, + kind=constants.FlowRuleKind.access, + active=False) + resp = self.get_view_single_grade( + self.student_participation, self.gopp) + self.assertEqual(resp.status_code, 200) + exceptions = resp.context["flow_rule_exceptions"] + self.assertIsNotNone(exceptions) + self.assertEqual(len(exceptions), 1) + + def test_flow_rule_exceptions_not_shown_to_student(self): + factories.FlowRuleExceptionFactory( + participation=self.student_participation, + flow_id=self.flow_id, + kind=constants.FlowRuleKind.access, + active=True) + with self.temporarily_switch_to_user(self.student_participation.user): + resp = self.get_view_single_grade( + self.student_participation, self.gopp, + force_login_instructor=False) + self.assertEqual(resp.status_code, 200) + self.assertIsNone(resp.context["flow_rule_exceptions"]) + + +class DeactivateFlowExceptionTest(GradesTestMixin, TestCase): + # Tests for views.deactivate_flow_exception + + def get_deactivate_url(self, exception, opportunity, course_identifier=None): + course_identifier = ( + course_identifier or self.get_default_course_identifier()) + return reverse("relate-deactivate_flow_exception", kwargs={ + "course_identifier": course_identifier, + "exception_id": exception.pk, + "opportunity_id": opportunity.pk, + }) + + def post_deactivate(self, exception, opportunity, course_identifier=None, + force_login_instructor=True): + url = self.get_deactivate_url(exception, opportunity, course_identifier) + if force_login_instructor: + switch_to = self.get_default_instructor_user( + course_identifier or self.get_default_course_identifier()) + else: + switch_to = None + with self.temporarily_switch_to_user(switch_to): + return self.client.post(url) + + def test_deactivate_success(self): + exc = factories.FlowRuleExceptionFactory( + participation=self.student_participation, + flow_id=self.flow_id, + kind=constants.FlowRuleKind.access, + active=True) + resp = self.post_deactivate(exc, self.gopp) + self.assertEqual(resp.status_code, 302) + exc.refresh_from_db() + self.assertFalse(exc.active) + self.assertAddMessageCalledWith("Exception deactivated.") + + def test_deactivate_already_inactive(self): + exc = factories.FlowRuleExceptionFactory( + participation=self.student_participation, + flow_id=self.flow_id, + kind=constants.FlowRuleKind.access, + active=False) + resp = self.post_deactivate(exc, self.gopp) + self.assertEqual(resp.status_code, 302) + exc.refresh_from_db() + self.assertFalse(exc.active) + self.assertAddMessageCalledWith("Exception was already deactivated.") + + def test_deactivate_no_permission(self): + exc = factories.FlowRuleExceptionFactory( + participation=self.student_participation, + flow_id=self.flow_id, + kind=constants.FlowRuleKind.access, + active=True) + with self.temporarily_switch_to_user(self.student_participation.user): + resp = self.client.post(self.get_deactivate_url(exc, self.gopp)) + self.assertEqual(resp.status_code, 403) + exc.refresh_from_db() + self.assertTrue(exc.active) + + def test_deactivate_get_not_allowed(self): + exc = factories.FlowRuleExceptionFactory( + participation=self.student_participation, + flow_id=self.flow_id, + kind=constants.FlowRuleKind.access, + active=True) + url = self.get_deactivate_url(exc, self.gopp) + with self.temporarily_switch_to_user( + self.get_default_instructor_user()): + resp = self.client.get(url) + self.assertEqual(resp.status_code, 405) + exc.refresh_from_db() + self.assertTrue(exc.active) + + def test_deactivate_wrong_course(self): + another_course = factories.CourseFactory(identifier="another-course") + another_participation = factories.ParticipationFactory( + course=another_course) + exc = factories.FlowRuleExceptionFactory( + participation=another_participation, + flow_id=self.flow_id, + kind=constants.FlowRuleKind.access, + active=True) + resp = self.post_deactivate(exc, self.gopp) + self.assertEqual(resp.status_code, 400) + exc.refresh_from_db() + self.assertTrue(exc.active) + + class EditGradingOpportunityTest(GradesTestMixin, TestCase): # test grades.edit_grading_opportunity From 41bd6215e8971ac22ef6a5165d2569e376646ddc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 27 Feb 2026 20:23:10 +0000 Subject: [PATCH 2/2] Fix test: pass course_identifier to get_default_instructor_user Co-authored-by: inducer <352067+inducer@users.noreply.github.com> --- tests/test_grades/test_grades.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_grades/test_grades.py b/tests/test_grades/test_grades.py index 83e485d55..b758a7dcb 100644 --- a/tests/test_grades/test_grades.py +++ b/tests/test_grades/test_grades.py @@ -1810,7 +1810,8 @@ def test_deactivate_get_not_allowed(self): active=True) url = self.get_deactivate_url(exc, self.gopp) with self.temporarily_switch_to_user( - self.get_default_instructor_user()): + self.get_default_instructor_user( + self.get_default_course_identifier())): resp = self.client.get(url) self.assertEqual(resp.status_code, 405) exc.refresh_from_db()