Apply changed from second review

api/serializers/agreement_signature.py

@@ -49,11 +49,8 @@ def get_agreement_commit(self, ref: str):
         Returns:
             The commit's data.
         """
-        # Repo Information
-        owner = settings.OWNER
-        repo = settings.REPO_NAME
-        file_name = settings.FILE_NAME
-        url = f"https://api.github.com/repos/{owner}/{repo}/commits/{ref}"
+        # 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, timeout=10)
@@ -64,10 +61,10 @@ def get_agreement_commit(self, ref: str):
         response_json = response.json()
 
         for file in response_json.get("files", []):
-            if file["filename"] == file_name:
+            if file["filename"] == settings.FILE_NAME:
                 return response_json
         raise serializers.ValidationError(
-            "Invalid commit ID", code="invalid_commit_id"
+            "Agreement not in commit files", code="agreement_not_in_files"
         )
 
     def validate(self, attrs):

api/serializers/agreement_signature_test.py

@@ -45,6 +45,17 @@ def test_validate__invalid_commit_id(self):
             error_code="invalid_commit_id",
         )
 
+    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",
+        )
+
     def test_validate__old_version(self):
         """Can check if contributor tried to sign an older version."""
         self.assert_validate(

api/views/agreement_signature.py

@@ -3,7 +3,7 @@
 Created on 15/07/2024 at 12:52:50(+01:00).
 """
 
-from typing import Dict
+import typing as t
 
 import requests
 from codeforlife.permissions import AllowAny
@@ -14,38 +14,26 @@
 
 import settings
 
-from ..models import AgreementSignature, Contributor
+from ..models import AgreementSignature
 from ..serializers import AgreementSignatureSerializer
 
 
-# pylint: disable-next=too-many-ancestors
+# pylint: disable-next=missing-docstring,too-many-ancestors
 class AgreementSignatureViewSet(ModelViewSet[User, AgreementSignature]):
-    """
-    An endpoint to check if a contributor has signed latest agreement,
-    return OKAY if he has otherwise return the latest commit ID.
-    """
-
-    # http_method_names = ["get", "post"]
-    queryset = AgreementSignature.objects.all()
+    http_method_names = ["get", "post"]
     permission_classes = [AllowAny]
     serializer_class = AgreementSignatureSerializer
 
-    def get_latest_commit_id(self, commits):
-        """Fetch the latest commit using github's api."""
-        owner = settings.OWNER
-        repo = settings.REPO_NAME
-        file_name = settings.FILE_NAME
-
-        params: Dict[str, str]
-        params = {"path": file_name, "per_page": commits}
-
-        url = f"https://api.github.com/repos/{owner}/{repo}/commits"
-
-        # Send an API request
-        response = requests.get(url, params=params, timeout=10)
+    def get_queryset(self):
+        queryset = AgreementSignature.objects.all()
+        if "contributor_pk" in self.kwargs:
+            queryset = queryset.filter(
+                contributor=self.kwargs["contributor_pk"]
+            )
 
-        return response
+        return queryset
 
+    # pylint: disable-next=unused-argument
     @action(
         detail=False,
         methods=["get"],
@@ -56,26 +44,22 @@ def check_signed(self, _, **url_params: str):
         Get the latest commit id and compare with contributor's
         agreement signature.
         """
-        # Repo information
-        github_pk = url_params["contributor_pk"]
+        # Send an API request
+        params: t.Dict[str, str] = {"path": settings.FILE_NAME, "per_page": 1}
+
+        # pylint: disable=line-too-long
+        url = f"https://api.github.com/repos/{settings.OWNER}/{settings.REPO_NAME}/commits"
 
         # Send an API request
-        response = self.get_latest_commit_id(commits=1)
+        response = requests.get(url, params=params, timeout=10)
+        if not response.ok:
+            return Response(status=status.HTTP_500_INTERNAL_SERVER_ERROR)
 
-        # Check the result of the API call
+        # Get the commit id from json data
         latest_commit_id = response.json()[0]["sha"]
 
-        # Retrieve contributor
-        try:
-            contributor = Contributor.objects.get(pk=github_pk)
-        except Contributor.DoesNotExist:
-            return Response(
-                data={"outcome: ": "Contributor does not exist"},
-                status=status.HTTP_404_NOT_FOUND,
-            )
-
         # Retrieve signature agreement IDs
-        signatures = AgreementSignature.objects.filter(contributor=contributor)
+        signatures = self.get_queryset()
         latest_signature = signatures.order_by("-signed_at").first()
         if not latest_signature:
             return Response(
@@ -85,10 +69,7 @@ def check_signed(self, _, **url_params: str):
 
         # Compare agreement IDs
         if latest_commit_id == latest_signature.agreement_id:
-            return Response(
-                data={"Outcome:": "Successful"},
-                status=status.HTTP_200_OK,
-            )
+            return Response()
 
         return Response(
             data={"latest_commit_id: ": latest_commit_id},

api/views/contributor.py

@@ -11,12 +11,8 @@
 from ..serializers import ContributorSerializer
 
 
-# pylint: disable-next=too-many-ancestors
+# pylint: disable-next=missing-docstring,too-many-ancestors
 class ContributorViewSet(ModelViewSet[User, Contributor]):
-    """
-    List, create, update, delete Contributors on the view.
-    """
-
     http_method_names = ["get", "post"]
     permission_classes = [AllowAny]
     serializer_class = ContributorSerializer

api/views/contributor_test.py

@@ -45,7 +45,3 @@ def test_create(self):
                 "avatar_url": "https://contributortest.github.io/",
             }
         )
-
-    # def test_delete(self):
-    #     """ Can delete a contributor. """
-    #     self.client.delete()