Merge dev to branch

api/migrations/0001_initial.py

@@ -1,5 +1,6 @@
-# Generated by Django 3.2.25 on 2024-07-16 12:27
+# Generated by Django 3.2.25 on 2024-07-22 14:17
 
+import django.core.validators
 from django.db import migrations, models
 import django.db.models.deletion
 
@@ -79,31 +80,10 @@ class Migration(migrations.Migration):
         migrations.CreateModel(
             name="AgreementSignature",
             fields=[
-                (
-                    "id",
-                    models.AutoField(
-                        auto_created=True,
-                        primary_key=True,
-                        serialize=False,
-                        verbose_name="ID",
-                    ),
-                ),
-                (
-                    "agreement_id",
-                    models.CharField(
-                        help_text="Commit ID of the contribution agreement in workspace.",
-                        max_length=40,
-                        verbose_name="agreement id",
-                    ),
-                ),
-                ("signed_at", models.DateTimeField(verbose_name="signed at")),
-                (
-                    "contributor",
-                    models.ForeignKey(
-                        on_delete=django.db.models.deletion.CASCADE,
-                        to="api.contributor",
-                    ),
-                ),
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('agreement_id', models.CharField(help_text='Commit ID of the contribution agreement in workspace.', max_length=40, validators=[django.core.validators.MinLengthValidator(40)], verbose_name='agreement id')),
+                ('signed_at', models.DateTimeField(verbose_name='signed at')),
+                ('contributor', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='api.contributor')),
             ],
             options={
                 "verbose_name": "agreement signature",

api/models/agreement_signature.py

@@ -5,6 +5,7 @@
 
 import typing as t
 
+from django.core.validators import MinLengthValidator
 from django.db import models
 from django.utils.translation import gettext_lazy as _
 
@@ -26,6 +27,7 @@ class AgreementSignature(models.Model):
         _("agreement id"),
         max_length=40,
         help_text=_("Commit ID of the contribution agreement in workspace."),
+        validators=[MinLengthValidator(40)],
     )
     signed_at = models.DateTimeField(_("signed at"))
 

api/models/contributor.py

@@ -33,3 +33,15 @@ class Meta(TypedModelMeta):
 
     def __str__(self):
         return f"{self.name} <{self.email}>"
+
+    @property
+    def last_agreement_signature(self):
+        """The last agreement that this contributor signed."""
+        # pylint: disable-next=import-outside-toplevel,cyclic-import
+        from .agreement_signature import AgreementSignature
+
+        return (
+            AgreementSignature.objects.filter(contributor=self)
+            .order_by("signed_at")
+            .last()
+        )

api/serializers/agreement_signature.py

@@ -13,27 +13,29 @@
 
 import settings
 
-from ..models import AgreementSignature
+from ..models import AgreementSignature, Contributor
+
+# pylint: disable=missing-class-docstring
+# pylint: disable=missing-function-docstring
+# pylint: disable=too-many-ancestors
 
 # pylint: disable-next=missing-function-docstring,too-many-ancestors
 
 
 class AgreementSignatureSerializer(ModelSerializer[User, AgreementSignature]):
-    """Agreement serializer class"""
-
     class Meta:
         model = AgreementSignature
         fields = ["id", "contributor", "agreement_id", "signed_at"]
 
     def validate_signed_at(self, value: datetime):
-        """Validate the time in not in future."""
         if value > timezone.now():
             raise serializers.ValidationError(
                 "Cannot sign in the future", code="signed_in_future"
             )
+
         return value
 
-    def get_agreement_commit(self, ref: str):
+    def _get_agreement_commit(self, ref: str):
         """
         Get a commit for the agreement using GitHub's API
 
@@ -49,59 +51,49 @@ def get_agreement_commit(self, ref: str):
         Returns:
             The commit's data.
         """
-        # pylint: disable=line-too-long
-        url = f"https://api.github.com/repos/{settings.OWNER}/{settings.REPO_NAME}/commits/{ref}"
 
         # Send an API request
         response = requests.get(
-            url,
-            headers={
-                "X-GitHub-Api-Version": "2022-11-28",
-            },
-            timeout=10,
+            # pylint: disable-next=line-too-long
+            url=f"https://api.github.com/repos/{settings.GH_ORG}/{settings.GH_REPO}/commits/{ref}",
+            headers={"X-GitHub-Api-Version": "2022-11-28"},
+            timeout=5,
         )
         if not response.ok:
             raise serializers.ValidationError(
                 "Invalid commit ID", code="invalid_commit_id"
             )
-        response_json = response.json()
 
+        response_json = response.json()
         for file in response_json.get("files", []):
-            if file["filename"] == settings.FILE_NAME:
+            if file["filename"] == settings.GH_FILE:
                 return response_json
+
         raise serializers.ValidationError(
             "Agreement not in commit files", code="agreement_not_in_files"
         )
 
     def validate(self, attrs):
-        """
-        Validate the new agreement is the new version not the older version.
-        """
+        contributor: Contributor = attrs["contributor"]
+        agreement_id: str = attrs["agreement_id"]
 
         # Check validity of the new agreement_ID
-        if "agreement_id" in attrs:
-            commit = self.get_agreement_commit(attrs["agreement_id"])
-            new_agreement_version = commit["commit"]["committer"]["date"]
-
-            # Check if contributor has signed a contribution in the past
-            if "contributor" in attrs:
-                last_signature = (
-                    AgreementSignature.objects.filter(
-                        contributor=attrs["contributor"]
-                    )
-                    .order_by("-signed_at")
-                    .first()
+        commit = self._get_agreement_commit(agreement_id)
+
+        # Check if contributor has signed a contribution in the past
+        last_agreement_signature = contributor.last_agreement_signature
+        if last_agreement_signature:
+            last_commit = self._get_agreement_commit(
+                last_agreement_signature.agreement_id
+            )
+
+            if (
+                commit["commit"]["author"]["date"]
+                <= last_commit["commit"]["author"]["date"]
+            ):
+                raise serializers.ValidationError(
+                    "You tried to sign an older version of the agreement.",
+                    code="old_version",
                 )
-                if not last_signature:
-                    return attrs
-
-                commit = self.get_agreement_commit(last_signature.agreement_id)
-
-                # Compare the two versions agreement ID.
-                old_agreement_version = commit["commit"]["committer"]["date"]
-                if new_agreement_version <= old_agreement_version:
-                    raise serializers.ValidationError(
-                        "You tried to sign an older version of the agreement.",
-                        code="old_version",
-                    )
+
         return attrs

api/serializers/agreement_signature_test.py

@@ -3,66 +3,150 @@
 Created on 12/07/2024 at 17:16:58(+01:00).
 """
 
+import json
 from datetime import timedelta
+from unittest.mock import call, patch
 
+import requests
 from codeforlife.tests import ModelSerializerTestCase
 from codeforlife.user.models import User
+from django.conf import settings
 from django.utils import timezone
+from rest_framework import status
 
 from ..models import AgreementSignature, Contributor
 from .agreement_signature import AgreementSignatureSerializer
 
 
-# pylint: disable-next=missing-function-docstring,too-many-ancestors
+# pylint: disable-next=missing-class-docstring,too-many-ancestors
 class TestAgreementSignatureSerializer(
     ModelSerializerTestCase[User, AgreementSignature]
 ):
-    """Test the Agreement Signature serializers"""
-
     model_serializer_class = AgreementSignatureSerializer
     fixtures = ["contributors", "agreement_signatures"]
 
     def setUp(self):
         self.contributor = Contributor.objects.get(pk=1)
 
     def test_validate_signed_at__signed_in_future(self):
-        """Can validate the time the signature was signed at."""
-        time = timezone.now() + timedelta(hours=10)
+        """Cannot sign in the future."""
         self.assert_validate_field(
             name="signed_at",
-            value=time,
+            value=timezone.now() + timedelta(hours=10),
             error_code="signed_in_future",
         )
 
     def test_validate__invalid_commit_id(self):
-        """Can check the validity of the agreement ID"""
-        self.assert_validate(
-            attrs={
-                "contributor": 1,
-                "agreement_id": "1234567890",
-                "signed_at": "2024-02-02T12:00:00Z",
-            },
-            error_code="invalid_commit_id",
-        )
+        """Agreement id must be an existing commit SHA on GitHub."""
+        agreement_id = "1234567890"
+
+        response = requests.Response()
+        response.status_code = status.HTTP_500_INTERNAL_SERVER_ERROR
+
+        with patch.object(
+            requests, "get", return_value=response
+        ) as requests_get:
+            self.assert_validate(
+                attrs={
+                    "contributor": self.contributor,
+                    "agreement_id": agreement_id,
+                    "signed_at": timezone.now(),
+                },
+                error_code="invalid_commit_id",
+            )
+
+            requests_get.assert_called_once_with(
+                # pylint: disable-next=line-too-long
+                url=f"https://api.github.com/repos/{settings.GH_ORG}/{settings.GH_REPO}/commits/{agreement_id}",
+                headers={"X-GitHub-Api-Version": "2022-11-28"},
+                timeout=5,
+            )
 
     def test_validate__agreement_not_in_files(self):
         """Can check if Agreement not in commit files"""
-        self.assert_validate(
-            attrs={
-                "contributor": 1,
-                "agreement_id": "be894d07641a174b9000c177b92b82bd357d2e63",
-                "signed_at": "2024-02-02T12:00:00Z",
-            },
-            error_code="agreement_not_in_files",
-        )
+        agreement_id = "be894d07641a174b9000c177b92b82bd357d2e63"
+
+        response = requests.Response()
+        response.status_code = status.HTTP_200_OK
+        response.encoding = "utf-8"
+        # pylint: disable-next=protected-access
+        response._content = json.dumps({"files": []}).encode("utf-8")
+
+        with patch.object(
+            requests, "get", return_value=response
+        ) as requests_get:
+            self.assert_validate(
+                attrs={
+                    "contributor": self.contributor,
+                    "agreement_id": agreement_id,
+                    "signed_at": timezone.now(),
+                },
+                error_code="agreement_not_in_files",
+            )
+
+            requests_get.assert_called_once_with(
+                # pylint: disable-next=line-too-long
+                url=f"https://api.github.com/repos/{settings.GH_ORG}/{settings.GH_REPO}/commits/{agreement_id}",
+                headers={"X-GitHub-Api-Version": "2022-11-28"},
+                timeout=5,
+            )
 
     def test_validate__old_version(self):
         """Can check if contributor tried to sign an older version."""
-        self.assert_validate(
-            attrs={
-                "contributor": 1,
-                "agreement_id": "81efd9e68f161104071f7bef7f9256e4840c1af7",
-                "signed_at": "2024-06-02T12:00:00Z",
-            },
-            error_code="old_version",
-        )
+
+        agreement_id = "81efd9e68f161104071f7bef7f9256e4840c1af7"
+        now = timezone.now()
+
+        current_response = requests.Response()
+        current_response.status_code = status.HTTP_200_OK
+        current_response.encoding = "utf-8"
+        # pylint: disable-next=protected-access
+        current_response._content = json.dumps(
+            {
+                "files": [{"filename": settings.GH_FILE}],
+                "commit": {"author": {"date": str(now - timedelta(days=1))}},
+            }
+        ).encode("utf-8")
+
+        last_response = requests.Response()
+        last_response.status_code = status.HTTP_200_OK
+        last_response.encoding = "utf-8"
+        # pylint: disable-next=protected-access
+        last_response._content = json.dumps(
+            {
+                "files": [{"filename": settings.GH_FILE}],
+                "commit": {"author": {"date": str(now)}},
+            }
+        ).encode("utf-8")
+
+        last_agreement_signature = self.contributor.last_agreement_signature
+        assert last_agreement_signature
+
+        with patch.object(
+            requests, "get", side_effect=[current_response, last_response]
+        ) as requests_get:
+            self.assert_validate(
+                attrs={
+                    "contributor": self.contributor,
+                    "agreement_id": agreement_id,
+                    "signed_at": now - timedelta(days=1),
+                },
+                error_code="old_version",
+            )
+
+            requests_get.assert_has_calls(
+                [
+                    call(
+                        # pylint: disable-next=line-too-long
+                        url=f"https://api.github.com/repos/{settings.GH_ORG}/{settings.GH_REPO}/commits/{agreement_id}",
+                        headers={"X-GitHub-Api-Version": "2022-11-28"},
+                        timeout=5,
+                    ),
+                    call(
+                        # pylint: disable-next=line-too-long
+                        url=f"https://api.github.com/repos/{settings.GH_ORG}/{settings.GH_REPO}/commits/{last_agreement_signature.agreement_id}",
+                        headers={"X-GitHub-Api-Version": "2022-11-28"},
+                        timeout=5,
+                    ),
+                ]
+            )