fix(risk_acceptance): reinstate findings when expiration date updated via API (#15147)

* fix(risk_acceptance): reinstate findings when expiration date updated via API

RiskAcceptanceSerializer.update() never called ra_helper.reinstate() when
expiration_date changed, unlike the legacy Django view which does so at
engagement/views.py. This caused two bugs reported together:

1. Findings stayed Active after a user updated the expiration date from a
   past date to a future date via the Edit Risk Acceptance form (Vue UI).
   reinstate() sets them back to inactive/risk_accepted.

2. Findings stayed Inactive on subsequent expiry cycles. Because reinstate()
   was never called, expiration_date_handled was never cleared. The Celery
   expiration task filters on expiration_date_handled__isnull=True, so the
   RA was permanently excluded from every future expiry run.

Fix: capture old_expiration_date before super().update(), then call
ra_helper.reinstate() when the date changes — matching the logic that
already existed in the legacy view.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test(risk_acceptance): regression coverage for reinstate on API expiration update

Adds two API tests exercising the serializer update() reinstate path:

- Extending an expired RA's expiration_date reinstates its findings
  (active=False / risk_accepted=True) and clears expiration_date_handled
  so the Celery expiry task re-processes it.
- Editing a never-expired RA's date leaves findings untouched and keeps
  expiration_date_handled None.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Cody Maffucci <46459665+Maffooch@users.noreply.github.com>

Author: 3 people

dojo/risk_acceptance/api/serializer.py

@@ -50,6 +50,7 @@ def update(self, instance, validated_data):
         findings_to_remove = set(existing_findings) - set(new_findings)
         findings_to_add = Finding.objects.filter(id__in=[x.id for x in findings_to_add])
         findings_to_remove = Finding.objects.filter(id__in=[x.id for x in findings_to_remove])
+        old_expiration_date = self.instance.expiration_date
         # Make the update in the database
         instance = super().update(instance, validated_data)
         user = getattr(self.context.get("request", None), "user", None)
@@ -59,6 +60,9 @@ def update(self, instance, validated_data):
         for finding in findings_to_remove:
             ra_helper.remove_finding_from_risk_acceptance(user, instance, finding)
 
+        if instance.expiration_date != old_expiration_date:
+            ra_helper.reinstate(instance, old_expiration_date)
+
         # Handle orphaned risk acceptances: link to engagement if it now has findings
         # This is fine as Pro has its own model + relationshop to track links with engagements.
         if instance.accepted_findings.exists() and not instance.engagement:

unittests/test_risk_acceptance_api.py

@@ -374,6 +374,92 @@ def test_update_risk_acceptance_add_cross_engagement_fails(self):
         self.assertEqual(403, response.status_code, response.content)
         self.assertIn("multiple engagements", str(response.data))
 
+    def test_update_expired_risk_acceptance_reinstates_findings(self):
+        """
+        Regression test for the serializer update() reinstate bug.
+
+        Extending an expired risk acceptance's expiration_date via the API must call
+        ra_helper.reinstate(): its accepted findings are set back to inactive/risk-accepted
+        and expiration_date_handled is cleared so the Celery expiry task re-processes it.
+        """
+        past_date = datetime.datetime.now(datetime.UTC) - datetime.timedelta(days=5)
+        ra = Risk_Acceptance.objects.create(
+            name="Expired RA",
+            recommendation="A",
+            decision="A",
+            accepted_by="Test User",
+            owner=self.user,
+            expiration_date=past_date,
+            expiration_date_handled=past_date,
+        )
+        ra.accepted_findings.add(self.finding_a1)
+        self.engagement_a.risk_acceptance.add(ra)
+
+        # Simulate the post-expiry state: the expiry job already reactivated the finding
+        self.finding_a1.active = True
+        self.finding_a1.risk_accepted = False
+        self.finding_a1.save()
+
+        future_date = datetime.datetime.now(datetime.UTC) + datetime.timedelta(days=30)
+        payload = {
+            "expiration_date": future_date.strftime("%Y-%m-%dT%H:%M:%SZ"),
+            "accepted_findings": [self.finding_a1.id],
+        }
+
+        response = self.client.patch(f"{self.url}{ra.id}/", payload, format="json")
+        self.assertEqual(200, response.status_code, response.content)
+
+        # Finding is reinstated (only reinstate() flips these back)
+        self.finding_a1.refresh_from_db()
+        self.assertFalse(self.finding_a1.active)
+        self.assertTrue(self.finding_a1.risk_accepted)
+
+        # Flags cleared so the RA is eligible for the next Celery expiry cycle
+        ra.refresh_from_db()
+        self.assertIsNone(ra.expiration_date_handled)
+        self.assertIsNone(ra.expiration_date_warned)
+
+    def test_update_never_expired_risk_acceptance_leaves_findings_unchanged(self):
+        """
+        Regression test companion: editing the expiration_date of a risk acceptance that
+        never expired must not disturb its findings, and expiration_date_handled stays None.
+        """
+        old_date = datetime.datetime.now(datetime.UTC) + datetime.timedelta(days=30)
+        ra = Risk_Acceptance.objects.create(
+            name="Active RA",
+            recommendation="A",
+            decision="A",
+            accepted_by="Test User",
+            owner=self.user,
+            expiration_date=old_date,
+        )
+        ra.accepted_findings.add(self.finding_a1)
+        self.engagement_a.risk_acceptance.add(ra)
+
+        # Normal accepted state for a live risk acceptance
+        self.finding_a1.active = False
+        self.finding_a1.risk_accepted = True
+        self.finding_a1.save()
+
+        new_date = datetime.datetime.now(datetime.UTC) + datetime.timedelta(days=60)
+        payload = {
+            "expiration_date": new_date.strftime("%Y-%m-%dT%H:%M:%SZ"),
+            "accepted_findings": [self.finding_a1.id],
+        }
+
+        response = self.client.patch(f"{self.url}{ra.id}/", payload, format="json")
+        self.assertEqual(200, response.status_code, response.content)
+
+        # Findings untouched
+        self.finding_a1.refresh_from_db()
+        self.assertFalse(self.finding_a1.active)
+        self.assertTrue(self.finding_a1.risk_accepted)
+
+        # Never handled, so it stays None
+        ra.refresh_from_db()
+        self.assertIsNone(ra.expiration_date_handled)
+        self.assertIsNone(ra.expiration_date_warned)
+
     def test_risk_acceptance_created_filter(self):
         # 1. Create a baseline Risk Acceptance using the existing test setup
         risk_acceptance = self.create_risk_acceptance()