From e73e7161d51b93b14faa0a5f5babf740166aff06 Mon Sep 17 00:00:00 2001
From: Wiktork <wiktor.kepczynski@adfinis.com>
Date: Tue, 22 Feb 2022 10:06:15 +0100
Subject: [PATCH] fix(tracking): allow updating billed reports

---
 timed/tracking/serializers.py       | 13 ++++++++++---
 timed/tracking/tests/test_report.py | 16 ++++++++++++++--
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/timed/tracking/serializers.py b/timed/tracking/serializers.py
index baa94ff5..e2a43d06 100644
--- a/timed/tracking/serializers.py
+++ b/timed/tracking/serializers.py
@@ -117,6 +117,16 @@ def validate_duration(self, value):
         """Only owner is allowed to change duration."""
         return self._validate_owner_only(value, "duration")
 
+    def validate_billed(self, value):
+        """Only accountants may bill reports."""
+        if self.instance is not None:
+            if not self.context["request"].user.is_accountant and (
+                self.instance.billed != value
+            ):
+                raise ValidationError(_("Only accountants may bill reports."))
+
+        return value
+
     def validate(self, data):
         """
         Validate that verified by is only set by reviewer or superuser.
@@ -166,9 +176,6 @@ def validate(self, data):
                     _("Report can't both be set as `review` and `verified`.")
                 )
 
-        if not user.is_accountant and billed:
-            raise ValidationError(_("Only accountants may bill reports."))
-
         # update billed flag on created reports
         if not self.instance or billed is None:
             data["billed"] = task.project.billed
diff --git a/timed/tracking/tests/test_report.py b/timed/tracking/tests/test_report.py
index 59206995..9d0d6b10 100644
--- a/timed/tracking/tests/test_report.py
+++ b/timed/tracking/tests/test_report.py
@@ -1529,12 +1529,24 @@ def test_report_update_billed_user(
     assert response.status_code == status.HTTP_403_FORBIDDEN
 
 
+@pytest.mark.parametrize(
+    "is_accountant, expected",
+    [
+        (True, status.HTTP_200_OK),
+        (False, status.HTTP_400_BAD_REQUEST),
+    ],
+)
 def test_report_set_billed_by_user(
     internal_employee_client,
     report_factory,
+    is_accountant,
+    expected,
 ):
     """Test that normal user may not bill report."""
     user = internal_employee_client.user
+    if is_accountant:
+        user.is_accountant = True
+        user.save()
     report = report_factory.create(user=user)
     data = {
         "data": {
@@ -1546,12 +1558,12 @@ def test_report_set_billed_by_user(
 
     url = reverse("report-detail", args=[report.id])
     response = internal_employee_client.patch(url, data)
-    assert response.status_code == status.HTTP_400_BAD_REQUEST
+    assert response.status_code == expected
 
 
 def test_report_update_billed(internal_employee_client, report_factory, task):
     user = internal_employee_client.user
-    report = report_factory.create(user=user)
+    report = report_factory.create(user=user, billed=True)
     report.task.project.billed = True
     report.task.project.save()