From 1f95859a2bbd68f16144745b939b0b891f038293 Mon Sep 17 00:00:00 2001 From: SalmanAsh Date: Fri, 19 Jul 2024 10:12:20 +0000 Subject: [PATCH] Apply changed from second review --- api/serializers/agreement_signature.py | 11 ++-- api/serializers/agreement_signature_test.py | 11 ++++ api/serializers/sign_agreement.py | 4 -- api/views/agreement_signature.py | 65 ++++++++------------- api/views/contributor.py | 6 +- api/views/contributor_test.py | 4 -- 6 files changed, 39 insertions(+), 62 deletions(-) delete mode 100644 api/serializers/sign_agreement.py diff --git a/api/serializers/agreement_signature.py b/api/serializers/agreement_signature.py index d41cc53..054f0bb 100644 --- a/api/serializers/agreement_signature.py +++ b/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): diff --git a/api/serializers/agreement_signature_test.py b/api/serializers/agreement_signature_test.py index 2973a89..27e37b6 100644 --- a/api/serializers/agreement_signature_test.py +++ b/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( diff --git a/api/serializers/sign_agreement.py b/api/serializers/sign_agreement.py deleted file mode 100644 index 889dc72..0000000 --- a/api/serializers/sign_agreement.py +++ /dev/null @@ -1,4 +0,0 @@ -""" -© Ocado Group -Created on 16/07/2024 at 09:05:55(+01:00). -""" diff --git a/api/views/agreement_signature.py b/api/views/agreement_signature.py index 0bdb26b..5868462 100644 --- a/api/views/agreement_signature.py +++ b/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}, diff --git a/api/views/contributor.py b/api/views/contributor.py index f0b0390..19d8e78 100644 --- a/api/views/contributor.py +++ b/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 diff --git a/api/views/contributor_test.py b/api/views/contributor_test.py index ef1552b..dabe064 100644 --- a/api/views/contributor_test.py +++ b/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()