diff --git a/backend/data/bod.dump b/backend/data/bod.dump index b88079c122..3d4a0555ed 100644 Binary files a/backend/data/bod.dump and b/backend/data/bod.dump differ diff --git a/backend/src/apps/common/api/__init__.py b/backend/src/apps/common/api/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/backend/src/apps/common/api/internal/__init__.py b/backend/src/apps/common/api/internal/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/backend/src/apps/common/api/internal/mutations/__init__.py b/backend/src/apps/common/api/internal/mutations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/backend/src/apps/common/api/internal/mutations/common.py b/backend/src/apps/common/api/internal/mutations/common.py new file mode 100644 index 0000000000..442ed43471 --- /dev/null +++ b/backend/src/apps/common/api/internal/mutations/common.py @@ -0,0 +1,75 @@ +"""Shared GraphQL mutation building blocks.""" + +import functools +import inspect +from collections.abc import Callable +from typing import Any + +import pydantic +import strawberry +from strawberry.utils.str_converters import to_camel_case + + +@strawberry.type +class FieldError: + """Per-field validation error.""" + + field: str + message: str + + +def pydantic_errors_to_field_errors(exc: pydantic.ValidationError) -> list[FieldError]: + """Convert a Pydantic ValidationError into a list of FieldError. + + Args: + exc (pydantic.ValidationError): Error raised by to_pydantic(). + + Returns: + list[FieldError]: One entry per validation issue. + + """ + return [ + FieldError( + field=".".join(to_camel_case(str(part)) for part in err["loc"]), + message=err["msg"], + ) + for err in exc.errors() + ] + + +def validate_pydantic_input( + result_cls: type[Any], + input_arg: str = "input_data", +) -> Callable: + """Wrap a mutation resolver with Pydantic input validation. + + Args: + result_cls (type): The Result GraphQL type used to return validation errors. + input_arg (str, optional): Name of the resolver's input argument to validate. + Defaults to "input_data". + + Returns: + Callable: A decorator that validates the resolver's input. + + """ + + def decorator(func: Callable) -> Callable: + signature = inspect.signature(func) + + @functools.wraps(func) + def wrapper(*args, **kwargs): + input_data = signature.bind(*args, **kwargs).arguments[input_arg] + try: + object.__setattr__(input_data, "validated_data", input_data.to_pydantic()) + except pydantic.ValidationError as e: + return result_cls( + ok=False, + code="VALIDATION_ERROR", + message="Some fields are invalid.", + field_errors=pydantic_errors_to_field_errors(e), + ) + return func(*args, **kwargs) + + return wrapper + + return decorator diff --git a/backend/src/apps/owasp/admin/board_of_directors.py b/backend/src/apps/owasp/admin/board_of_directors.py index d2c1e24009..41eeb5a685 100644 --- a/backend/src/apps/owasp/admin/board_of_directors.py +++ b/backend/src/apps/owasp/admin/board_of_directors.py @@ -8,7 +8,7 @@ class BoardOfDirectorsAdmin(admin.ModelAdmin): """Admin for Snapshot model.""" - filter_horizontal = ("reviewers",) + filter_horizontal = ("claim_reviewers",) list_filter = ("year",) ordering = ("-year",) search_fields = ("year",) diff --git a/backend/src/apps/owasp/api/internal/mutations/board_candidate_claim.py b/backend/src/apps/owasp/api/internal/mutations/board_candidate_claim.py index cf8d4f17d5..22252647f5 100644 --- a/backend/src/apps/owasp/api/internal/mutations/board_candidate_claim.py +++ b/backend/src/apps/owasp/api/internal/mutations/board_candidate_claim.py @@ -2,6 +2,7 @@ import logging +import pydantic import strawberry from django.core.exceptions import ValidationError from django.db import transaction @@ -9,6 +10,7 @@ from django.utils import timezone from strawberry.types import Info +from apps.common.api.internal.mutations.common import FieldError, validate_pydantic_input from apps.nest.api.internal.permissions import IsAuthenticated from apps.owasp.api.internal.nodes.board_candidate_claim import BoardCandidateClaimNode from apps.owasp.models.board_candidate_claim import BoardCandidateClaim @@ -21,57 +23,81 @@ GENERIC_ERROR_MSG = "Something went wrong." -@strawberry.input +class CreateClaimPydanticInput(pydantic.BaseModel): + """Pydantic validation for creating a claim.""" + + description: str + name: str = pydantic.Field(max_length=200) + year: int + + +@strawberry.experimental.pydantic.input(model=CreateClaimPydanticInput, all_fields=True) class CreateClaimInput: """Input for creating a claim.""" - description: str - name: str + +class UpdateClaimPydanticInput(pydantic.BaseModel): + """Pydantic validation for updating a claim.""" + + description: str | None = None + key: str = pydantic.Field(max_length=100) + name: str | None = pydantic.Field(default=None, max_length=200) year: int -@strawberry.input +@strawberry.experimental.pydantic.input(model=UpdateClaimPydanticInput, all_fields=True) class UpdateClaimInput: """Input for updating a claim.""" - description: str | None = None - key: str - name: str | None = None + +class DiscardClaimPydanticInput(pydantic.BaseModel): + """Pydantic validation for discarding a claim.""" + + key: str = pydantic.Field(max_length=100) year: int -@strawberry.input +@strawberry.experimental.pydantic.input(model=DiscardClaimPydanticInput, all_fields=True) class DiscardClaimInput: """Input for discarding a claim.""" - key: str + +class SubmitClaimPydanticInput(pydantic.BaseModel): + """Pydantic validation for submitting a claim.""" + + key: str = pydantic.Field(max_length=100) year: int -@strawberry.input +@strawberry.experimental.pydantic.input(model=SubmitClaimPydanticInput, all_fields=True) class SubmitClaimInput: """Input for submitting a claim.""" - key: str + +class WithdrawClaimPydanticInput(pydantic.BaseModel): + """Pydantic validation for withdrawing a claim.""" + + key: str = pydantic.Field(max_length=100) + withdrawn_reason: str year: int -@strawberry.input +@strawberry.experimental.pydantic.input(model=WithdrawClaimPydanticInput, all_fields=True) class WithdrawClaimInput: """Input for withdrawing a claim.""" - key: str - withdrawn_reason: str + +class ReorderClaimsPydanticInput(pydantic.BaseModel): + """Pydantic validation for reordering claims.""" + + keys: list[str] year: int -@strawberry.input +@strawberry.experimental.pydantic.input(model=ReorderClaimsPydanticInput, all_fields=True) class ReorderClaimsInput: """Input for reordering claims.""" - keys: list[str] - year: int - @strawberry.type class ReorderClaimsResult: @@ -81,6 +107,7 @@ class ReorderClaimsResult: code: str | None = None message: str | None = None claims: list[BoardCandidateClaimNode] | None = None + field_errors: list[FieldError] | None = None @strawberry.type @@ -91,17 +118,18 @@ class ClaimResult: code: str | None = None message: str | None = None claim: BoardCandidateClaimNode | None = None + field_errors: list[FieldError] | None = None def _validate_reorder_claims( login: str, - input_data: ReorderClaimsInput, + input_data: ReorderClaimsPydanticInput, ) -> tuple[list[str], ReorderClaimsResult | None]: """Validate reorder claims input. Args: login (str): The login of the candidate. - input_data (ReorderClaimsInput): Input containing claim keys to reorder. + input_data (ReorderClaimsPydanticInput): Input containing claim keys to reorder. Returns: tuple of (list[str], ReorderClaimsResult | None) @@ -142,21 +170,23 @@ class BoardCandidateClaimMutations: @strawberry.mutation(permission_classes=[IsAuthenticated]) @transaction.atomic + @validate_pydantic_input(ClaimResult) def create_board_candidate_claim( self, info: Info, input_data: CreateClaimInput ) -> ClaimResult: """Create a new draft claim for a candidate.""" + validated = input_data.validated_data # type: ignore[attr-defined] user = info.context.request.user if user.github_user is None: return ClaimResult(ok=False, code="FORBIDDEN", message=ACCESS_DENIED_MSG) try: - board = BoardOfDirectors.objects.get(year=input_data.year) + board = BoardOfDirectors.objects.get(year=validated.year) except BoardOfDirectors.DoesNotExist: return ClaimResult( ok=False, code="NOT_FOUND", - message=f"No board election found for the year {input_data.year}.", + message=f"No board election found for the year {validated.year}.", ) candidate = board.get_candidate(login=user.github_user.login) @@ -171,14 +201,14 @@ def create_board_candidate_claim( claim = BoardCandidateClaim.objects.create( board=board, candidate=candidate, - description=input_data.description, - name=input_data.name, + description=validated.description, + name=validated.name, ) except IntegrityError: logger.warning( "Error creating Board Candidate Claim for candidate %s, year %s", candidate.member.login, - input_data.year, + validated.year, ) return ClaimResult( ok=False, @@ -204,19 +234,21 @@ def create_board_candidate_claim( @strawberry.mutation(permission_classes=[IsAuthenticated]) @transaction.atomic + @validate_pydantic_input(ClaimResult) def update_board_candidate_claim( self, info: Info, input_data: UpdateClaimInput ) -> ClaimResult: """Update a draft claim.""" + validated = input_data.validated_data # type: ignore[attr-defined] user = info.context.request.user if user.github_user is None: return ClaimResult(ok=False, code="FORBIDDEN", message=ACCESS_DENIED_MSG) try: claim = BoardCandidateClaim.objects.select_for_update().get( - board__year=input_data.year, + board__year=validated.year, candidate__member__login=user.github_user.login, - key=input_data.key, + key=validated.key, ) except BoardCandidateClaim.DoesNotExist: return ClaimResult(ok=False, code="NOT_FOUND", message=CLAIM_NOT_FOUND_MSG) @@ -225,12 +257,12 @@ def update_board_candidate_claim( return ClaimResult(ok=False, code="LOCKED", message="Cannot update a locked claim.") update_fields = [] - if input_data.name: - claim.name = input_data.name + if validated.name: + claim.name = validated.name update_fields.append("name") update_fields.append("key") - if input_data.description: - claim.description = input_data.description + if validated.description: + claim.description = validated.description update_fields.append("description") try: @@ -239,7 +271,7 @@ def update_board_candidate_claim( logger.warning( "Error updating Board Candidate Claim for candidate %s, key %s", claim.candidate.member.login, - input_data.key, + validated.key, ) return ClaimResult( ok=False, @@ -265,19 +297,21 @@ def update_board_candidate_claim( @strawberry.mutation(permission_classes=[IsAuthenticated]) @transaction.atomic + @validate_pydantic_input(ClaimResult) def discard_board_candidate_claim( self, info: Info, input_data: DiscardClaimInput ) -> ClaimResult: """Discard a claim.""" + validated = input_data.validated_data # type: ignore[attr-defined] user = info.context.request.user if user.github_user is None: return ClaimResult(ok=False, code="FORBIDDEN", message=ACCESS_DENIED_MSG) try: claim = BoardCandidateClaim.objects.select_for_update().get( - board__year=input_data.year, + board__year=validated.year, candidate__member__login=user.github_user.login, - key=input_data.key, + key=validated.key, ) except BoardCandidateClaim.DoesNotExist: return ClaimResult(ok=False, code="NOT_FOUND", message=CLAIM_NOT_FOUND_MSG) @@ -322,19 +356,21 @@ def discard_board_candidate_claim( @strawberry.mutation(permission_classes=[IsAuthenticated]) @transaction.atomic + @validate_pydantic_input(ClaimResult) def submit_board_candidate_claim( self, info: Info, input_data: SubmitClaimInput ) -> ClaimResult: """Submit a claim.""" + validated = input_data.validated_data # type: ignore[attr-defined] user = info.context.request.user if user.github_user is None: return ClaimResult(ok=False, code="FORBIDDEN", message=ACCESS_DENIED_MSG) try: claim = BoardCandidateClaim.objects.select_for_update().get( - board__year=input_data.year, + board__year=validated.year, candidate__member__login=user.github_user.login, - key=input_data.key, + key=validated.key, ) except BoardCandidateClaim.DoesNotExist: return ClaimResult(ok=False, code="NOT_FOUND", message=CLAIM_NOT_FOUND_MSG) @@ -389,19 +425,21 @@ def submit_board_candidate_claim( @strawberry.mutation(permission_classes=[IsAuthenticated]) @transaction.atomic + @validate_pydantic_input(ClaimResult) def withdraw_board_candidate_claim( self, info: Info, input_data: WithdrawClaimInput ) -> ClaimResult: """Withdraw a claim.""" + validated = input_data.validated_data # type: ignore[attr-defined] user = info.context.request.user if user.github_user is None: return ClaimResult(ok=False, code="FORBIDDEN", message=ACCESS_DENIED_MSG) try: claim = BoardCandidateClaim.objects.select_for_update().get( - board__year=input_data.year, + board__year=validated.year, candidate__member__login=user.github_user.login, - key=input_data.key, + key=validated.key, ) except BoardCandidateClaim.DoesNotExist: return ClaimResult(ok=False, code="NOT_FOUND", message=CLAIM_NOT_FOUND_MSG) @@ -418,7 +456,7 @@ def withdraw_board_candidate_claim( try: claim.status = BoardCandidateClaim.Status.WITHDRAWN - claim.withdrawn_reason = input_data.withdrawn_reason + claim.withdrawn_reason = validated.withdrawn_reason claim.withdrawn_at = timezone.now() claim.save() except IntegrityError: @@ -451,23 +489,25 @@ def withdraw_board_candidate_claim( @strawberry.mutation(permission_classes=[IsAuthenticated]) @transaction.atomic + @validate_pydantic_input(ReorderClaimsResult) def reorder_board_candidate_claims( self, info: Info, input_data: ReorderClaimsInput ) -> ReorderClaimsResult: """Reorder claims for a candidate in a board year.""" + validated = input_data.validated_data # type: ignore[attr-defined] user = info.context.request.user if user.github_user is None: return ReorderClaimsResult(ok=False, code="FORBIDDEN", message=ACCESS_DENIED_MSG) login = user.github_user.login - keys, error = _validate_reorder_claims(login, input_data) + keys, error = _validate_reorder_claims(login, validated) if error: return error claims = list( BoardCandidateClaim.objects.filter( - board__year=input_data.year, + board__year=validated.year, candidate__member__login=login, key__in=keys, ) @@ -490,7 +530,7 @@ def reorder_board_candidate_claims( ordered_claims = ( BoardCandidateClaim.objects.filter( - board__year=input_data.year, + board__year=validated.year, candidate__member__login=login, key__in=keys, ) diff --git a/backend/src/apps/owasp/api/internal/mutations/board_candidate_claim_evidence.py b/backend/src/apps/owasp/api/internal/mutations/board_candidate_claim_evidence.py index a4a7ca1389..bfa2e75e45 100644 --- a/backend/src/apps/owasp/api/internal/mutations/board_candidate_claim_evidence.py +++ b/backend/src/apps/owasp/api/internal/mutations/board_candidate_claim_evidence.py @@ -2,6 +2,7 @@ import logging +import pydantic import strawberry from django.core.exceptions import ValidationError from django.db import transaction @@ -10,6 +11,7 @@ from strawberry.file_uploads import Upload from strawberry.types import Info +from apps.common.api.internal.mutations.common import FieldError, validate_pydantic_input from apps.nest.api.internal.permissions import IsAuthenticated from apps.owasp.api.internal.nodes.board_candidate_claim_evidence import ( BoardCandidateClaimEvidenceNode, @@ -25,41 +27,55 @@ GENERIC_ERROR_MSG = "Something went wrong." -@strawberry.input +class CreateEvidencePydanticInput(pydantic.BaseModel): + """Pydantic validation for creating claim evidence.""" + + claim_key: str = pydantic.Field(max_length=100) + description: str + name: str = pydantic.Field(max_length=200) + source_url: pydantic.HttpUrl | None = None + year: int + + +@strawberry.experimental.pydantic.input(model=CreateEvidencePydanticInput, all_fields=True) class CreateEvidenceInput: """Input for creating claim evidence.""" - claim_key: str - description: str - file: Upload | None = None - name: str - source_url: str | None = None + file: Upload | None = strawberry.field(default=None) + + +class UpdateEvidencePydanticInput(pydantic.BaseModel): + """Pydantic validation for updating claim evidence.""" + + claim_key: str = pydantic.Field(max_length=100) + description: str | None = None + key: str = pydantic.Field(max_length=100) + name: str | None = pydantic.Field(default=None, max_length=200) + source_url: pydantic.HttpUrl | None = None year: int -@strawberry.input +@strawberry.experimental.pydantic.input(model=UpdateEvidencePydanticInput, all_fields=True) class UpdateEvidenceInput: """Input for updating claim evidence.""" - claim_key: str - description: str | None = None - file: Upload | None = None - key: str - name: str | None = None - source_url: str | None = None - year: int + file: Upload | None = strawberry.field(default=None) -@strawberry.input -class RemoveEvidenceInput: - """Input for removing claim evidence.""" +class RemoveEvidencePydanticInput(pydantic.BaseModel): + """Pydantic validation for removing claim evidence.""" - claim_key: str - key: str + claim_key: str = pydantic.Field(max_length=100) + key: str = pydantic.Field(max_length=100) removed_reason: str | None = None year: int +@strawberry.experimental.pydantic.input(model=RemoveEvidencePydanticInput, all_fields=True) +class RemoveEvidenceInput: + """Input for removing claim evidence.""" + + @strawberry.type class EvidenceResult: """Result for claim evidence mutations.""" @@ -68,6 +84,7 @@ class EvidenceResult: code: str | None = None message: str | None = None evidence: BoardCandidateClaimEvidenceNode | None = None + field_errors: list[FieldError] | None = None @strawberry.type @@ -76,19 +93,21 @@ class BoardCandidateClaimEvidenceMutations: @strawberry.mutation(permission_classes=[IsAuthenticated]) @transaction.atomic + @validate_pydantic_input(EvidenceResult) def create_board_candidate_claim_evidence( self, info: Info, input_data: CreateEvidenceInput ) -> EvidenceResult: """Create evidence for a claim.""" + validated = input_data.validated_data # type: ignore[attr-defined] user = info.context.request.user if user.github_user is None: return EvidenceResult(ok=False, code="FORBIDDEN", message=ACCESS_DENIED_MSG) try: claim = BoardCandidateClaim.objects.select_for_update().get( - board__year=input_data.year, + board__year=validated.year, candidate__member__login=user.github_user.login, - key=input_data.claim_key, + key=validated.claim_key, ) except BoardCandidateClaim.DoesNotExist: return EvidenceResult(ok=False, code="NOT_FOUND", message=CLAIM_NOT_FOUND_MSG) @@ -103,10 +122,10 @@ def create_board_candidate_claim_evidence( try: evidence = BoardCandidateClaimEvidence.objects.create( claim=claim, - description=input_data.description, + description=validated.description, file=input_data.file, - name=input_data.name, - source_url=input_data.source_url or "", + name=validated.name, + source_url=str(validated.source_url) if validated.source_url else "", ) except IntegrityError: logger.warning( @@ -137,20 +156,22 @@ def create_board_candidate_claim_evidence( @strawberry.mutation(permission_classes=[IsAuthenticated]) @transaction.atomic + @validate_pydantic_input(EvidenceResult) def update_board_candidate_claim_evidence( self, info: Info, input_data: UpdateEvidenceInput ) -> EvidenceResult: """Update evidence for a claim.""" + validated = input_data.validated_data # type: ignore[attr-defined] user = info.context.request.user if user.github_user is None: return EvidenceResult(ok=False, code="FORBIDDEN", message=ACCESS_DENIED_MSG) try: evidence = BoardCandidateClaimEvidence.objects.select_for_update().get( - claim__board__year=input_data.year, + claim__board__year=validated.year, claim__candidate__member__login=user.github_user.login, - claim__key=input_data.claim_key, - key=input_data.key, + claim__key=validated.claim_key, + key=validated.key, ) except BoardCandidateClaimEvidence.DoesNotExist: return EvidenceResult(ok=False, code="NOT_FOUND", message=EVIDENCE_NOT_FOUND_MSG) @@ -163,16 +184,15 @@ def update_board_candidate_claim_evidence( ) update_fields = [] - if input_data.name is not None: - evidence.name = input_data.name + if validated.name is not None: + evidence.name = validated.name update_fields.append("name") update_fields.append("key") - if input_data.description is not None: - evidence.description = input_data.description + if validated.description is not None: + evidence.description = validated.description update_fields.append("description") - if input_data.source_url is not None: - evidence.source_url = input_data.source_url - update_fields.append("source_url") + evidence.source_url = str(validated.source_url) if validated.source_url else "" + update_fields.append("source_url") if input_data.file is not None: evidence.file = input_data.file update_fields.extend(["file", "file_name", "file_size"]) @@ -208,20 +228,22 @@ def update_board_candidate_claim_evidence( @strawberry.mutation(permission_classes=[IsAuthenticated]) @transaction.atomic + @validate_pydantic_input(EvidenceResult) def remove_board_candidate_claim_evidence( self, info: Info, input_data: RemoveEvidenceInput ) -> EvidenceResult: """Remove evidence for a claim.""" + validated = input_data.validated_data # type: ignore[attr-defined] user = info.context.request.user if user.github_user is None: return EvidenceResult(ok=False, code="FORBIDDEN", message=ACCESS_DENIED_MSG) try: evidence = BoardCandidateClaimEvidence.objects.select_for_update().get( - claim__board__year=input_data.year, + claim__board__year=validated.year, claim__candidate__member__login=user.github_user.login, - claim__key=input_data.claim_key, - key=input_data.key, + claim__key=validated.claim_key, + key=validated.key, ) except BoardCandidateClaimEvidence.DoesNotExist: return EvidenceResult(ok=False, code="NOT_FOUND", message=EVIDENCE_NOT_FOUND_MSG) @@ -238,7 +260,7 @@ def remove_board_candidate_claim_evidence( evidence.file = None evidence.is_removed = True evidence.removed_at = timezone.now() - evidence.removed_reason = input_data.removed_reason or "" + evidence.removed_reason = validated.removed_reason or "" evidence.save(update_fields=["file", "is_removed", "removed_reason", "removed_at"]) if old_file: transaction.on_commit(lambda f=old_file: f.delete(save=False)) diff --git a/backend/src/apps/owasp/api/internal/mutations/board_candidate_claim_review.py b/backend/src/apps/owasp/api/internal/mutations/board_candidate_claim_review.py index 5274f462a4..02b0b055b0 100644 --- a/backend/src/apps/owasp/api/internal/mutations/board_candidate_claim_review.py +++ b/backend/src/apps/owasp/api/internal/mutations/board_candidate_claim_review.py @@ -2,12 +2,14 @@ import logging +import pydantic import strawberry from django.core.exceptions import ValidationError from django.db import transaction from django.db.utils import IntegrityError from strawberry.types import Info +from apps.common.api.internal.mutations.common import FieldError, validate_pydantic_input from apps.nest.api.internal.permissions import IsAuthenticated from apps.nest.models.user import User from apps.owasp.api.internal.nodes.board_candidate_claim_review import ( @@ -27,17 +29,21 @@ INVALID_STATUS_MSG = "Review can only be added to submitted claims." -@strawberry.input -class CreateReviewInput: - """Input for creating claim review.""" +class CreateReviewPydanticInput(pydantic.BaseModel): + """Pydantic validation for creating a claim review.""" - claim_key: str + claim_key: str = pydantic.Field(max_length=100) claim_member_login: str notes: str = "" status: ReviewStatusEnum year: int +@strawberry.experimental.pydantic.input(model=CreateReviewPydanticInput, all_fields=True) +class CreateReviewInput: + """Input for creating claim review.""" + + @strawberry.type class ReviewResult: """Result for claim review mutations.""" @@ -46,6 +52,7 @@ class ReviewResult: code: str | None = None message: str | None = None review: BoardCandidateClaimReviewNode | None = None + field_errors: list[FieldError] | None = None def _validate_review_eligibility( @@ -58,11 +65,7 @@ def _validate_review_eligibility( message=INVALID_STATUS_MSG, ) - if ( - claim.board - and reviewer.github_user - and claim.board.get_candidate(login=reviewer.github_user.login) - ): + if reviewer.github_user and claim.board.get_candidate(login=reviewer.github_user.login): return ReviewResult( ok=False, code="FORBIDDEN", @@ -84,23 +87,25 @@ class BoardCandidateClaimReviewMutations: @strawberry.mutation(permission_classes=[IsAuthenticated]) @transaction.atomic + @validate_pydantic_input(ReviewResult) def create_board_candidate_claim_review( self, info: Info, input_data: CreateReviewInput ) -> ReviewResult: """Create review for a claim.""" + validated = input_data.validated_data # type: ignore[attr-defined] user = info.context.request.user is_reviewer = BoardOfDirectors.objects.filter( - year=input_data.year, reviewers=user + year=validated.year, claim_reviewers=user ).exists() if not user.github_user or not is_reviewer: return ReviewResult(ok=False, code="FORBIDDEN", message=ACCESS_DENIED_MSG) try: claim = BoardCandidateClaim.objects.select_for_update().get( - board__year=input_data.year, - candidate__member__login=input_data.claim_member_login, - key=input_data.claim_key, + board__year=validated.year, + candidate__member__login=validated.claim_member_login, + key=validated.claim_key, ) except BoardCandidateClaim.DoesNotExist: return ReviewResult(ok=False, code="NOT_FOUND", message=CLAIM_NOT_FOUND_MSG) @@ -113,15 +118,15 @@ def create_board_candidate_claim_review( try: review = BoardCandidateClaimReview.objects.create( claim=claim, - status=input_data.status.value, - notes=input_data.notes, + status=validated.status.value, + notes=validated.notes, reviewer=user, ) except IntegrityError: logger.warning( "Error creating Board Candidate Claim Review for claim %s of user %s", - input_data.claim_key, - input_data.claim_member_login, + validated.claim_key, + validated.claim_member_login, ) return ReviewResult( ok=False, diff --git a/backend/src/apps/owasp/api/internal/nodes/board_candidate_claim.py b/backend/src/apps/owasp/api/internal/nodes/board_candidate_claim.py index 62bceb9b2c..43d3c59f56 100644 --- a/backend/src/apps/owasp/api/internal/nodes/board_candidate_claim.py +++ b/backend/src/apps/owasp/api/internal/nodes/board_candidate_claim.py @@ -57,16 +57,8 @@ def reviews( and root.candidate.member is not None and user.github_user == root.candidate.member ) - if is_self or root.status == BoardCandidateClaim.Status.APPROVED: + if is_self or root.status in BoardCandidateClaim.PUBLIC_STATUSES: return root.reviews.all() - - is_reviewer = ( - user.is_authenticated - and root.board is not None - and root.board.reviewers.filter(id=user.id).exists() - ) - if is_reviewer: - return root.reviews.filter(reviewer=user) return [] @strawberry_django.field diff --git a/backend/src/apps/owasp/api/internal/nodes/board_of_directors.py b/backend/src/apps/owasp/api/internal/nodes/board_of_directors.py index a5f12d8732..63da6a6682 100644 --- a/backend/src/apps/owasp/api/internal/nodes/board_of_directors.py +++ b/backend/src/apps/owasp/api/internal/nodes/board_of_directors.py @@ -43,6 +43,8 @@ def owasp_url(self, root: BoardOfDirectors) -> str: def reviewer(self, root: BoardOfDirectors, login: str) -> UserNode | None: """Resolve board election reviewer.""" user = ( - root.reviewers.select_related("github_user").filter(github_user__login=login).first() + root.claim_reviewers.select_related("github_user") + .filter(github_user__login=login) + .first() ) return user.github_user if user else None diff --git a/backend/src/apps/owasp/api/internal/queries/board_candidate_claim.py b/backend/src/apps/owasp/api/internal/queries/board_candidate_claim.py index 1cac976733..0cf369706a 100644 --- a/backend/src/apps/owasp/api/internal/queries/board_candidate_claim.py +++ b/backend/src/apps/owasp/api/internal/queries/board_candidate_claim.py @@ -7,7 +7,6 @@ from apps.owasp.api.internal.nodes.board_candidate_claim import BoardCandidateClaimNode from apps.owasp.models.board_candidate_claim import BoardCandidateClaim from apps.owasp.models.board_candidate_claim_evidence import BoardCandidateClaimEvidence -from apps.owasp.models.board_of_directors import BoardOfDirectors @strawberry.type @@ -30,13 +29,7 @@ def board_candidate_claims( """ user = info.context.request.user - is_reviewer = ( - user.is_authenticated - and BoardOfDirectors.objects.filter(year=year, reviewers=user).exists() - ) - claims = BoardCandidateClaim.objects.filter( - board__year=year, - ) + claims = BoardCandidateClaim.objects.filter(board__year=year) if login is not None: is_self = ( @@ -45,33 +38,15 @@ def board_candidate_claims( and user.github_user.login == login ) claims = claims.filter(candidate__member__login=login) - - if not is_self and not is_reviewer: - claims = claims.filter(status=BoardCandidateClaim.Status.APPROVED) - elif is_reviewer and not is_self: - claims = claims.filter( - status__in=[ - BoardCandidateClaim.Status.SUBMITTED, - BoardCandidateClaim.Status.APPROVED, - ] - ) - elif is_reviewer: - claims = claims.filter( - Q(candidate__member=user.github_user) - | Q( - status__in=[ - BoardCandidateClaim.Status.SUBMITTED, - BoardCandidateClaim.Status.APPROVED, - ] - ) - ) + if not is_self: + claims = claims.filter(status__in=BoardCandidateClaim.PUBLIC_STATUSES) elif user.is_authenticated and user.github_user: claims = claims.filter( Q(candidate__member=user.github_user) - | Q(status=BoardCandidateClaim.Status.APPROVED) + | Q(status__in=BoardCandidateClaim.PUBLIC_STATUSES) ) else: - claims = claims.filter(status=BoardCandidateClaim.Status.APPROVED) + claims = claims.filter(status__in=BoardCandidateClaim.PUBLIC_STATUSES) return ( claims.annotate( @@ -126,14 +101,5 @@ def board_candidate_claim( and user.github_user is not None and user.github_user == claim.candidate.member ) - is_reviewer = user.is_authenticated and claim.board.reviewers.filter(id=user.id).exists() - return ( - claim - if ( - is_self - or (is_reviewer and claim.status == BoardCandidateClaim.Status.SUBMITTED) - or claim.status == BoardCandidateClaim.Status.APPROVED - ) - else None - ) + return claim if is_self or claim.status in BoardCandidateClaim.PUBLIC_STATUSES else None diff --git a/backend/src/apps/owasp/api/internal/queries/board_candidate_claim_evidence.py b/backend/src/apps/owasp/api/internal/queries/board_candidate_claim_evidence.py index dfecf8a096..d9890b90b4 100644 --- a/backend/src/apps/owasp/api/internal/queries/board_candidate_claim_evidence.py +++ b/backend/src/apps/owasp/api/internal/queries/board_candidate_claim_evidence.py @@ -43,17 +43,10 @@ def get_claim_evidence( and evidence.claim.candidate.member is not None and user.github_user == evidence.claim.candidate.member ) - is_reviewer = ( - user.is_authenticated and evidence.claim.board.reviewers.filter(id=user.id).exists() - ) return ( evidence - if ( - is_self - or (is_reviewer and evidence.claim.status == BoardCandidateClaim.Status.SUBMITTED) - or evidence.claim.status == BoardCandidateClaim.Status.APPROVED - ) + if is_self or evidence.claim.status in BoardCandidateClaim.PUBLIC_STATUSES else None ) @@ -92,15 +85,10 @@ def board_candidate_claim_evidences( and claim.candidate.member is not None and user.github_user == claim.candidate.member ) - is_reviewer = user.is_authenticated and claim.board.reviewers.filter(id=user.id).exists() return ( claim.evidences.filter(is_removed=False) - if ( - is_self - or (is_reviewer and claim.status == BoardCandidateClaim.Status.SUBMITTED) - or claim.status == BoardCandidateClaim.Status.APPROVED - ) + if is_self or claim.status in BoardCandidateClaim.PUBLIC_STATUSES else [] ) diff --git a/backend/src/apps/owasp/migrations/0081_alter_boardcandidateclaim_board_and_more.py b/backend/src/apps/owasp/migrations/0081_alter_boardcandidateclaim_board_and_more.py new file mode 100644 index 0000000000..b84b96a14f --- /dev/null +++ b/backend/src/apps/owasp/migrations/0081_alter_boardcandidateclaim_board_and_more.py @@ -0,0 +1,42 @@ +# Generated by Django 6.0.6 on 2026-08-08 15:32 + +import django.core.validators +import django.db.models.deletion +from django.db import migrations, models + +import apps.owasp.models.board_candidate_claim_evidence +import apps.owasp.validators + + +class Migration(migrations.Migration): + dependencies = [ + ("owasp", "0080_boardcandidateclaimreview_boardofdirectors_reviewers_and_more"), + ] + + operations = [ + migrations.AlterField( + model_name="boardcandidateclaim", + name="board", + field=models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="claims", + to="owasp.boardofdirectors", + ), + ), + migrations.AlterField( + model_name="boardcandidateclaimevidence", + name="file", + field=models.FileField( + blank=True, + null=True, + upload_to=apps.owasp.models.board_candidate_claim_evidence.uuid_upload_to, + validators=[ + django.core.validators.FileExtensionValidator( + allowed_extensions=["jpeg", "jpg", "pdf", "png", "webp"] + ), + apps.owasp.validators.validate_evidence_file_size, + ], + verbose_name="File", + ), + ), + ] diff --git a/backend/src/apps/owasp/migrations/0082_remove_boardofdirectors_reviewers_and_more.py b/backend/src/apps/owasp/migrations/0082_remove_boardofdirectors_reviewers_and_more.py new file mode 100644 index 0000000000..d07f139ff3 --- /dev/null +++ b/backend/src/apps/owasp/migrations/0082_remove_boardofdirectors_reviewers_and_more.py @@ -0,0 +1,29 @@ +# Generated by Django 6.0.6 on 2026-08-08 15:51 + +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("owasp", "0081_alter_boardcandidateclaim_board_and_more"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.RemoveField( + model_name="boardofdirectors", + name="reviewers", + ), + migrations.AddField( + model_name="boardofdirectors", + name="claim_reviewers", + field=models.ManyToManyField( + blank=True, + help_text="Reviewers for this year's board election claims.", + related_name="+", + to=settings.AUTH_USER_MODEL, + verbose_name="Claim reviewers", + ), + ), + ] diff --git a/backend/src/apps/owasp/models/board_candidate_claim.py b/backend/src/apps/owasp/models/board_candidate_claim.py index 88b93bb221..1befdefa43 100644 --- a/backend/src/apps/owasp/models/board_candidate_claim.py +++ b/backend/src/apps/owasp/models/board_candidate_claim.py @@ -45,6 +45,7 @@ class Status(models.TextChoices): FINALIZED_STATUSES = frozenset( {Status.APPROVED, Status.DISCARDED, Status.REJECTED, Status.WITHDRAWN} ) + PUBLIC_STATUSES = frozenset({Status.APPROVED, Status.REJECTED, Status.SUBMITTED}) VALID_TRANSITIONS = { Status.DRAFT: {Status.SUBMITTED, Status.DISCARDED}, Status.SUBMITTED: {Status.APPROVED, Status.REJECTED, Status.WITHDRAWN}, @@ -55,9 +56,7 @@ class Status(models.TextChoices): } WITHDRAWAL_ALLOWED_FIELDS = frozenset({"status", "withdrawn_reason", "withdrawn_at"}) - board = models.ForeignKey( - BoardOfDirectors, blank=True, null=True, on_delete=models.SET_NULL, related_name="claims" - ) + board = models.ForeignKey(BoardOfDirectors, on_delete=models.CASCADE, related_name="claims") candidate = models.ForeignKey(EntityMember, on_delete=models.CASCADE, related_name="claims") description = models.TextField(default="", verbose_name="Description") is_locked = models.BooleanField( @@ -139,7 +138,7 @@ def save(self, *args, **kwargs) -> None: self.full_clean() - if not self.pk and self.candidate_id and self.board_id: + if not self.pk: max_order = ( BoardCandidateClaim.objects.filter( candidate_id=self.candidate_id, @@ -155,6 +154,11 @@ def save(self, *args, **kwargs) -> None: super().save(*args, **kwargs) + def set_status_approved(self) -> None: + """Set claim status to approved.""" + self.status = self.Status.APPROVED + self.save() + @staticmethod def bulk_save(claims: list, fields: list | None = None) -> None: # type: ignore[override] """Bulk save claims. @@ -165,3 +169,16 @@ def bulk_save(claims: list, fields: list | None = None) -> None: # type: ignore """ BulkSaveModel.bulk_save(BoardCandidateClaim, claims, fields=fields) + + @classmethod + def bulk_set_status_approved(cls, claims: list[BoardCandidateClaim]) -> None: + """Bulk-approve and lock claims. + + Args: + claims (list[BoardCandidateClaim]): Claims to approve. + + """ + for claim in claims: + claim.status = cls.Status.APPROVED + claim.is_locked = True + cls.objects.bulk_update(claims, ["is_locked", "status"]) diff --git a/backend/src/apps/owasp/models/board_candidate_claim_review.py b/backend/src/apps/owasp/models/board_candidate_claim_review.py index 5959099589..fa73451ef3 100644 --- a/backend/src/apps/owasp/models/board_candidate_claim_review.py +++ b/backend/src/apps/owasp/models/board_candidate_claim_review.py @@ -64,16 +64,11 @@ def clean(self) -> None: err = "Review can only be added to submitted claims." raise ValidationError(err) - if ( - not self.claim.board - or not self.claim.board.reviewers.filter(id=self.reviewer.id).exists() - ): + if not self.claim.board.claim_reviewers.filter(id=self.reviewer.id).exists(): err = "Only Claim Reviewers can review claims." raise ValidationError(err) - if self.claim.board and self.claim.board.get_candidate( - login=self.reviewer.github_user.login - ): + if self.claim.board.get_candidate(login=self.reviewer.github_user.login): err = "A candidate cannot review claims in the same election year." raise ValidationError(err) diff --git a/backend/src/apps/owasp/models/board_of_directors.py b/backend/src/apps/owasp/models/board_of_directors.py index 1e4eb16e46..8a3fc8f1a9 100644 --- a/backend/src/apps/owasp/models/board_of_directors.py +++ b/backend/src/apps/owasp/models/board_of_directors.py @@ -30,9 +30,9 @@ class Meta: created_at = models.DateTimeField(auto_now_add=True) updated_at = models.DateTimeField(auto_now=True) - reviewers = models.ManyToManyField( + claim_reviewers = models.ManyToManyField( "nest.User", - verbose_name="Reviewers", + verbose_name="Claim reviewers", related_name="+", blank=True, help_text="Reviewers for this year's board election claims.", diff --git a/backend/src/apps/owasp/signals/board_candidate_claim_review.py b/backend/src/apps/owasp/signals/board_candidate_claim_review.py index fa97984d58..024a5cca21 100644 --- a/backend/src/apps/owasp/signals/board_candidate_claim_review.py +++ b/backend/src/apps/owasp/signals/board_candidate_claim_review.py @@ -25,8 +25,7 @@ def review_post_save_finalize_claim_status(sender, instance, **kwargs): # noqa: ).count() if approved_count >= threshold: - claim.status = BoardCandidateClaim.Status.APPROVED - claim.save() + claim.set_status_approved() logger.info( "Claim '%s' auto-approved with %d approvals (threshold: %d).", claim.key, diff --git a/backend/src/apps/owasp/signals/board_of_directors.py b/backend/src/apps/owasp/signals/board_of_directors.py index 7e11ac63d6..28d291a519 100644 --- a/backend/src/apps/owasp/signals/board_of_directors.py +++ b/backend/src/apps/owasp/signals/board_of_directors.py @@ -24,12 +24,10 @@ def board_post_save_re_evaluate_claims(sender, instance, **kwargs): # noqa: ARG ).count() if approved_count >= threshold: - claim.status = BoardCandidateClaim.Status.APPROVED - claim.is_locked = True claims_to_approve.append(claim) if claims_to_approve: - BoardCandidateClaim.objects.bulk_update(claims_to_approve, ["is_locked", "status"]) + BoardCandidateClaim.bulk_set_status_approved(claims_to_approve) logger.info( "Approved %d claims after threshold change on board %d.", len(claims_to_approve), diff --git a/backend/src/apps/owasp/utils/file.py b/backend/src/apps/owasp/utils/file.py index 97aa55812b..7c747070cf 100644 --- a/backend/src/apps/owasp/utils/file.py +++ b/backend/src/apps/owasp/utils/file.py @@ -20,7 +20,7 @@ ".png": "image/png", ".webp": "image/webp", } -IMAGE_EXTENSIONS = frozenset({".jpeg", ".jpg", ".png", ".webp"}) +IMAGE_EXTENSIONS = frozenset(IMAGE_CONTENT_TYPE_MAP) IMAGE_FORMAT_MAP = { ".jpeg": "JPEG", ".jpg": "JPEG", @@ -51,16 +51,16 @@ def strip_file_metadata(file: UploadedFile | None) -> UploadedFile | None: ext = Path(file.name).suffix.lower() if ext in IMAGE_EXTENSIONS: - return _strip_image_metadata(file, ext) + return strip_image_metadata(file, ext) if ext == PDF_EXTENSION: - return _strip_pdf_metadata(file) + return strip_pdf_metadata(file) msg = f"Unsupported file type for metadata stripping: {ext}" raise ValidationError(msg) -def _strip_image_metadata(file: UploadedFile, ext: str) -> SimpleUploadedFile: +def strip_image_metadata(file: UploadedFile, ext: str) -> SimpleUploadedFile: """Strip EXIF/XMP metadata from an image file. Opens the image with Pillow and re-saves it without metadata. @@ -100,7 +100,7 @@ def _strip_image_metadata(file: UploadedFile, ext: str) -> SimpleUploadedFile: ) -def _strip_pdf_metadata(file: UploadedFile) -> SimpleUploadedFile: +def strip_pdf_metadata(file: UploadedFile) -> SimpleUploadedFile: """Strip metadata from a PDF file. Uses pypdf to read and rewrite the PDF, removing the /Info diff --git a/backend/src/settings/local.py b/backend/src/settings/local.py index ae649331ff..c4bc3908ae 100644 --- a/backend/src/settings/local.py +++ b/backend/src/settings/local.py @@ -1,13 +1,9 @@ """OWASP Nest local configuration.""" -from pathlib import Path - from configurations import values from settings.base import Base -BASE_DIR = Path(__file__).resolve().parent.parent - class Local(Base): """Local configuration.""" @@ -24,7 +20,7 @@ class Local(Base): DEBUG = True IS_LOCAL_ENVIRONMENT = True LOGGING = {} - MEDIA_ROOT = BASE_DIR / "media" + MEDIA_ROOT = Base.BASE_DIR / "media" MEDIA_URL = "/media/" PUBLIC_IP_ADDRESS = values.Value() diff --git a/backend/src/settings/production.py b/backend/src/settings/production.py index 851e8b767b..c3922d1192 100644 --- a/backend/src/settings/production.py +++ b/backend/src/settings/production.py @@ -17,6 +17,7 @@ class Production(Base): traces_sample_rate=0.5, ) + AWS_MEDIA_BUCKET_NAME = values.Value(environ_name="AWS_MEDIA_BUCKET_NAME") AWS_STORAGE_BUCKET_NAME = values.Value(environ_name="AWS_STORAGE_BUCKET_NAME") AWS_S3_CUSTOM_DOMAIN = f"{AWS_STORAGE_BUCKET_NAME}.s3.amazonaws.com" AWS_S3_OBJECT_PARAMETERS = { @@ -31,6 +32,10 @@ class Production(Base): STORAGES = { "default": { "BACKEND": "storages.backends.s3.S3Storage", + "OPTIONS": { + "bucket_name": AWS_MEDIA_BUCKET_NAME, + "custom_domain": None, + }, }, "staticfiles": { "BACKEND": "storages.backends.s3.S3Storage", diff --git a/backend/src/settings/staging.py b/backend/src/settings/staging.py index c3389fed58..0a11dcdb09 100644 --- a/backend/src/settings/staging.py +++ b/backend/src/settings/staging.py @@ -18,6 +18,7 @@ class Staging(Base): traces_sample_rate=0.5, ) + AWS_MEDIA_BUCKET_NAME = values.Value(environ_name="AWS_MEDIA_BUCKET_NAME") AWS_STORAGE_BUCKET_NAME = values.Value(environ_name="AWS_STORAGE_BUCKET_NAME") AWS_S3_CUSTOM_DOMAIN = f"{AWS_STORAGE_BUCKET_NAME}.s3.amazonaws.com" AWS_S3_OBJECT_PARAMETERS = { @@ -30,6 +31,10 @@ class Staging(Base): STORAGES = { "default": { "BACKEND": "storages.backends.s3.S3Storage", + "OPTIONS": { + "bucket_name": AWS_MEDIA_BUCKET_NAME, + "custom_domain": None, + }, }, "staticfiles": { "BACKEND": "storages.backends.s3.S3Storage", diff --git a/backend/tests/unit/apps/common/api/__init__.py b/backend/tests/unit/apps/common/api/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/backend/tests/unit/apps/common/api/internal/__init__.py b/backend/tests/unit/apps/common/api/internal/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/backend/tests/unit/apps/common/api/internal/mutations/__init__.py b/backend/tests/unit/apps/common/api/internal/mutations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/backend/tests/unit/apps/common/api/internal/mutations/common_test.py b/backend/tests/unit/apps/common/api/internal/mutations/common_test.py new file mode 100644 index 0000000000..6685ac831b --- /dev/null +++ b/backend/tests/unit/apps/common/api/internal/mutations/common_test.py @@ -0,0 +1,113 @@ +"""Tests for shared GraphQL mutation building blocks.""" + +from dataclasses import dataclass +from unittest.mock import MagicMock + +import pydantic +import pytest + +from apps.common.api.internal.mutations.common import ( + FieldError, + pydantic_errors_to_field_errors, + validate_pydantic_input, +) + + +class SampleModel(pydantic.BaseModel): + """Sample pydantic model for validating tests.""" + + name: str = pydantic.Field(max_length=5) + + +@dataclass +class SampleResult: + """Sample Result class mirroring the mutation Result shape.""" + + ok: bool + code: str | None = None + message: str | None = None + field_errors: list[FieldError] | None = None + + +class TestPydanticErrorsToFieldErrors: + """Tests for pydantic_errors_to_field_errors.""" + + def test_returns_one_entry_per_error(self): + with pytest.raises(pydantic.ValidationError) as exc_info: + SampleModel(name="x" * 10) + + result = pydantic_errors_to_field_errors(exc_info.value) + + assert len(result) == 1 + assert result[0].field == "name" + assert "at most 5 characters" in result[0].message + + def test_joins_nested_loc_with_dot(self): + class Nested(pydantic.BaseModel): + inner: SampleModel + + with pytest.raises(pydantic.ValidationError) as exc_info: + Nested(inner={"name": "x" * 10}) + + result = pydantic_errors_to_field_errors(exc_info.value) + + assert result[0].field == "inner.name" + + def test_converts_snake_case_field_to_camel_case(self): + class SnakeModel(pydantic.BaseModel): + source_url: pydantic.HttpUrl + + with pytest.raises(pydantic.ValidationError) as exc_info: + SnakeModel(source_url="not-a-url") + + result = pydantic_errors_to_field_errors(exc_info.value) + + assert result[0].field == "sourceUrl" + + +class TestValidatePydanticInput: + """Tests for validate_pydantic_input decorator.""" + + def test_attaches_validated_and_calls_wrapped_on_success(self): + captured = {} + + @validate_pydantic_input(SampleResult) + def resolver(self, info, input_data): # noqa: ARG001 + captured["validated"] = input_data.validated_data + return SampleResult(ok=True, code="SUCCESS") + + data = MagicMock() + data.to_pydantic.return_value = SampleModel(name="ok") + + result = resolver(None, MagicMock(), data) + + assert result.ok + assert result.code == "SUCCESS" + assert captured["validated"].name == "ok" + + def test_returns_field_errors_on_pydantic_failure(self): + @validate_pydantic_input(SampleResult) + def resolver(self, info, input_data): # noqa: ARG001 + pytest.fail("Resolver body should not run when validation fails.") + + data = MagicMock() + with pytest.raises(pydantic.ValidationError) as exc_info: + SampleModel(name="x" * 10) + data.to_pydantic.side_effect = exc_info.value + + result = resolver(None, MagicMock(), data) + + assert not result.ok + assert result.code == "VALIDATION_ERROR" + assert result.message == "Some fields are invalid." + assert result.field_errors is not None + assert {fe.field for fe in result.field_errors} == {"name"} + + def test_preserves_wrapped_function_signature_for_introspection(self): + @validate_pydantic_input(SampleResult) + def resolver(self, info, input_data: SampleModel) -> SampleResult: # noqa: ARG001 + """Docstring stays.""" + return SampleResult(ok=True) + + assert resolver.__name__ == "resolver" + assert resolver.__doc__ == "Docstring stays." diff --git a/backend/tests/unit/apps/owasp/admin/board_of_directors_test.py b/backend/tests/unit/apps/owasp/admin/board_of_directors_test.py index c9bbabd4ac..1a85bc8049 100644 --- a/backend/tests/unit/apps/owasp/admin/board_of_directors_test.py +++ b/backend/tests/unit/apps/owasp/admin/board_of_directors_test.py @@ -11,10 +11,10 @@ class TestBoardOfDirectorsAdmin: """Tests for BoardOfDirectorsAdmin.""" def test_filter_horizontal(self): - """Test filter_horizontal includes reviewers.""" + """Test filter_horizontal includes claim_reviewers.""" admin_instance = BoardOfDirectorsAdmin(BoardOfDirectors, AdminSite()) - assert admin_instance.filter_horizontal == ("reviewers",) + assert admin_instance.filter_horizontal == ("claim_reviewers",) def test_list_filter(self): """Test list_filter includes year.""" diff --git a/backend/tests/unit/apps/owasp/api/internal/mutations/board_candidate_claim_evidence_test.py b/backend/tests/unit/apps/owasp/api/internal/mutations/board_candidate_claim_evidence_test.py index 3a921ea709..da230e8726 100644 --- a/backend/tests/unit/apps/owasp/api/internal/mutations/board_candidate_claim_evidence_test.py +++ b/backend/tests/unit/apps/owasp/api/internal/mutations/board_candidate_claim_evidence_test.py @@ -3,12 +3,16 @@ from datetime import UTC, datetime from unittest.mock import MagicMock, patch +import pydantic import pytest from django.core.exceptions import ValidationError from django.db.utils import IntegrityError from apps.owasp.api.internal.mutations.board_candidate_claim_evidence import ( BoardCandidateClaimEvidenceMutations, + CreateEvidencePydanticInput, + RemoveEvidencePydanticInput, + UpdateEvidencePydanticInput, ) from apps.owasp.models.board_candidate_claim import BoardCandidateClaim from apps.owasp.models.board_candidate_claim_evidence import BoardCandidateClaimEvidence @@ -49,6 +53,7 @@ def _make_input_data( ) input_data.name = name input_data.year = year + input_data.to_pydantic.return_value = input_data return input_data @patch("apps.owasp.api.internal.mutations.board_candidate_claim_evidence.BoardCandidateClaim") @@ -209,6 +214,7 @@ def _make_input_data( input_data.name = name input_data.claim_key = claim_key input_data.year = year + input_data.to_pydantic.return_value = input_data return input_data @patch("apps.owasp.api.internal.mutations.board_candidate_claim_evidence.BoardCandidateClaim") @@ -262,6 +268,7 @@ def test_update_partial_success(self, mock_evidence_model, mock_claim_model): input_data.name = "Updated Name" input_data.claim_key = "test-claim-key" input_data.year = 2025 + input_data.to_pydantic.return_value = input_data evidence = MagicMock() evidence.claim.candidate.member = mock_github_user @@ -275,7 +282,8 @@ def test_update_partial_success(self, mock_evidence_model, mock_claim_model): assert result.ok assert result.code == "SUCCESS" assert evidence.name == "Updated Name" - evidence.save.assert_called_once_with(update_fields=["name", "key"]) + assert evidence.source_url == "" + evidence.save.assert_called_once_with(update_fields=["name", "key", "source_url"]) @patch("apps.owasp.api.internal.mutations.board_candidate_claim_evidence.BoardCandidateClaim") @patch( @@ -300,6 +308,7 @@ def test_update_with_file_replacement(self, mock_evidence_model, mock_claim_mode ) input_data.claim_key = "test-claim-key" input_data.year = 2025 + input_data.to_pydantic.return_value = input_data evidence = MagicMock() evidence.claim.candidate.member = mock_github_user @@ -434,12 +443,14 @@ def _make_input_data( claim_key="test-claim-key", year=2025, ): - return MagicMock( + data = MagicMock( key=evidence_key, removed_reason=removed_reason, claim_key=claim_key, year=year, ) + data.to_pydantic.return_value = data + return data @pytest.mark.parametrize( "status", @@ -639,3 +650,119 @@ def test_remove_validation_error(self, mock_evidence_model, mock_claim_model): assert not result.ok assert result.code == "VALIDATION_ERROR" + + +class TestCreateEvidencePydanticValidation: + """Tests for CreateEvidencePydanticInput and its resolver handling.""" + + def test_pydantic_rejects_name_over_max_length(self): + with pytest.raises(pydantic.ValidationError) as exc_info: + CreateEvidencePydanticInput(claim_key="k", description="d", name="x" * 201, year=2025) + + assert any(err["loc"] == ("name",) for err in exc_info.value.errors()) + + @pytest.mark.parametrize("bad_url", ["not a url", "javascript:alert(1)", "ftp:/host", "://x"]) + def test_pydantic_rejects_invalid_source_url(self, bad_url): + with pytest.raises(pydantic.ValidationError) as exc_info: + CreateEvidencePydanticInput( + claim_key="k", description="d", name="n", source_url=bad_url, year=2025 + ) + + assert any(err["loc"] == ("source_url",) for err in exc_info.value.errors()) + + @pytest.mark.parametrize("url", [None, "https://example.com/path?q=1"]) + def test_pydantic_accepts_absent_or_valid_source_url(self, url): + model = CreateEvidencePydanticInput( + claim_key="k", description="d", name="n", source_url=url, year=2025 + ) + + if url: + assert model.source_url.host == "example.com" + assert model.source_url.scheme == "https" + else: + assert model.source_url is None + + def test_resolver_returns_field_errors_when_pydantic_fails(self): + with pytest.raises(pydantic.ValidationError) as exc_info: + CreateEvidencePydanticInput(claim_key="x" * 101, description="d", name="n", year=2025) + + data = MagicMock() + data.to_pydantic.side_effect = exc_info.value + info = _make_info(MagicMock()) + + result = BoardCandidateClaimEvidenceMutations().create_board_candidate_claim_evidence( + info, data + ) + + assert not result.ok + assert result.code == "VALIDATION_ERROR" + assert result.field_errors is not None + assert {fe.field for fe in result.field_errors} == {"claimKey"} + + +class TestUpdateEvidencePydanticValidation: + """Tests for UpdateEvidencePydanticInput and its resolver handling.""" + + def test_pydantic_rejects_key_over_max_length(self): + with pytest.raises(pydantic.ValidationError) as exc_info: + UpdateEvidencePydanticInput(claim_key="k", key="x" * 101, year=2025) + + assert any(err["loc"] == ("key",) for err in exc_info.value.errors()) + + def test_pydantic_rejects_invalid_source_url(self): + with pytest.raises(pydantic.ValidationError) as exc_info: + UpdateEvidencePydanticInput(claim_key="k", key="e", source_url="not-a-url", year=2025) + + assert any(err["loc"] == ("source_url",) for err in exc_info.value.errors()) + + def test_pydantic_accepts_valid_source_url(self): + model = UpdateEvidencePydanticInput( + claim_key="k", key="e", source_url="https://example.com/x", year=2025 + ) + + assert model.source_url.host == "example.com" + assert model.source_url.scheme == "https" + + def test_resolver_returns_field_errors_when_pydantic_fails(self): + with pytest.raises(pydantic.ValidationError) as exc_info: + UpdateEvidencePydanticInput(claim_key="k", key="x" * 101, year=2025) + + data = MagicMock() + data.to_pydantic.side_effect = exc_info.value + info = _make_info(MagicMock()) + + result = BoardCandidateClaimEvidenceMutations().update_board_candidate_claim_evidence( + info, data + ) + + assert not result.ok + assert result.code == "VALIDATION_ERROR" + assert result.field_errors is not None + assert {fe.field for fe in result.field_errors} == {"key"} + + +class TestRemoveEvidencePydanticValidation: + """Tests for RemoveEvidencePydanticInput and its resolver handling.""" + + def test_pydantic_rejects_key_over_max_length(self): + with pytest.raises(pydantic.ValidationError) as exc_info: + RemoveEvidencePydanticInput(claim_key="k", key="x" * 101, year=2025) + + assert any(err["loc"] == ("key",) for err in exc_info.value.errors()) + + def test_resolver_returns_field_errors_when_pydantic_fails(self): + with pytest.raises(pydantic.ValidationError) as exc_info: + RemoveEvidencePydanticInput(claim_key="k", key="x" * 101, year=2025) + + data = MagicMock() + data.to_pydantic.side_effect = exc_info.value + info = _make_info(MagicMock()) + + result = BoardCandidateClaimEvidenceMutations().remove_board_candidate_claim_evidence( + info, data + ) + + assert not result.ok + assert result.code == "VALIDATION_ERROR" + assert result.field_errors is not None + assert {fe.field for fe in result.field_errors} == {"key"} diff --git a/backend/tests/unit/apps/owasp/api/internal/mutations/board_candidate_claim_review_test.py b/backend/tests/unit/apps/owasp/api/internal/mutations/board_candidate_claim_review_test.py index 6c9269665f..9cce301e34 100644 --- a/backend/tests/unit/apps/owasp/api/internal/mutations/board_candidate_claim_review_test.py +++ b/backend/tests/unit/apps/owasp/api/internal/mutations/board_candidate_claim_review_test.py @@ -2,13 +2,16 @@ from unittest.mock import MagicMock, patch +import pydantic import pytest from django.core.exceptions import ValidationError from django.db.utils import IntegrityError from apps.owasp.api.internal.mutations.board_candidate_claim_review import ( BoardCandidateClaimReviewMutations, + CreateReviewPydanticInput, ) +from apps.owasp.api.internal.nodes.enum import ReviewStatusEnum from apps.owasp.models.board_candidate_claim import BoardCandidateClaim from apps.owasp.models.board_candidate_claim_review import BoardCandidateClaimReview @@ -43,6 +46,7 @@ def _make_input_data( data.status.value = status data.year = year data.notes = notes + data.to_pydantic.return_value = data return data @@ -324,3 +328,40 @@ def test_create_review_validation_error( assert not result.ok assert result.code == "VALIDATION_ERROR" + + +class TestCreateReviewPydanticValidation: + """Tests for CreateReviewPydanticInput and its resolver handling.""" + + def test_pydantic_rejects_claim_key_over_max_length(self): + with pytest.raises(pydantic.ValidationError) as exc_info: + CreateReviewPydanticInput( + claim_key="x" * 101, + claim_member_login="alice", + status=ReviewStatusEnum.APPROVED, + year=2025, + ) + + assert any(err["loc"] == ("claim_key",) for err in exc_info.value.errors()) + + def test_resolver_returns_field_errors_when_pydantic_fails(self): + with pytest.raises(pydantic.ValidationError) as exc_info: + CreateReviewPydanticInput( + claim_key="x" * 101, + claim_member_login="alice", + status=ReviewStatusEnum.APPROVED, + year=2025, + ) + + data = MagicMock() + data.to_pydantic.side_effect = exc_info.value + info = _make_info(MagicMock()) + + result = BoardCandidateClaimReviewMutations().create_board_candidate_claim_review( + info, data + ) + + assert not result.ok + assert result.code == "VALIDATION_ERROR" + assert result.field_errors is not None + assert {fe.field for fe in result.field_errors} == {"claimKey"} diff --git a/backend/tests/unit/apps/owasp/api/internal/mutations/board_candidate_claim_test.py b/backend/tests/unit/apps/owasp/api/internal/mutations/board_candidate_claim_test.py index e88e080f62..25ab6db088 100644 --- a/backend/tests/unit/apps/owasp/api/internal/mutations/board_candidate_claim_test.py +++ b/backend/tests/unit/apps/owasp/api/internal/mutations/board_candidate_claim_test.py @@ -3,12 +3,19 @@ from datetime import UTC, datetime from unittest.mock import MagicMock, patch +import pydantic import pytest from django.core.exceptions import ValidationError from django.db.utils import IntegrityError from apps.owasp.api.internal.mutations.board_candidate_claim import ( BoardCandidateClaimMutations, + CreateClaimPydanticInput, + DiscardClaimPydanticInput, + ReorderClaimsPydanticInput, + SubmitClaimPydanticInput, + UpdateClaimPydanticInput, + WithdrawClaimPydanticInput, _validate_reorder_claims, ) from apps.owasp.models.board_candidate_claim import BoardCandidateClaim @@ -35,7 +42,9 @@ class TestDiscardBoardCandidateClaim: """Tests for discard_board_candidate_claim mutation.""" def _make_input_data(self, key, year=2025): - return MagicMock(key=key, year=year) + data = MagicMock(key=key, year=year) + data.to_pydantic.return_value = data + return data @patch("apps.owasp.api.internal.mutations.board_candidate_claim.BoardCandidateClaim") def test_discard_claim_success(self, mock_claim_model): @@ -108,7 +117,9 @@ class TestSubmitBoardCandidateClaim: """Tests for submit_board_candidate_claim mutation.""" def _make_input_data(self, key, year=2025): - return MagicMock(key=key, year=year) + data = MagicMock(key=key, year=year) + data.to_pydantic.return_value = data + return data @patch("apps.owasp.api.internal.mutations.board_candidate_claim.BoardCandidateClaim") def test_submit_claim_success(self, mock_claim_model): @@ -183,11 +194,13 @@ class TestWithdrawBoardCandidateClaim: """Tests for withdraw_board_candidate_claim mutation.""" def _make_input_data(self, key, withdrawn_reason="No longer relevant", year=2025): - return MagicMock( + data = MagicMock( key=key, withdrawn_reason=withdrawn_reason, year=year, ) + data.to_pydantic.return_value = data + return data @patch("apps.owasp.api.internal.mutations.board_candidate_claim.BoardCandidateClaim") @patch("apps.owasp.api.internal.mutations.board_candidate_claim.timezone") @@ -328,7 +341,9 @@ class TestReorderBoardCandidateClaims: """Tests for reorder_board_candidate_claims mutation.""" def _make_input_data(self, keys, year=2025): - return MagicMock(keys=list(keys), year=year) + data = MagicMock(keys=list(keys), year=year) + data.to_pydantic.return_value = data + return data @patch("apps.owasp.api.internal.mutations.board_candidate_claim.BoardCandidateClaim") def test_reorder_claims_success(self, mock_claim_model): @@ -521,6 +536,7 @@ def _make_input_data(self, name="Test Claim", description="Test description", ye data.name = name data.description = description data.year = year + data.to_pydantic.return_value = data return data @patch("apps.owasp.api.internal.mutations.board_candidate_claim.BoardOfDirectors") @@ -660,6 +676,31 @@ def test_create_claim_validation_error(self, mock_claim_model, mock_board_model) assert "required" in result.message +class TestCreateClaimPydanticValidation: + """Tests for CreateClaimPydanticInput validation and its resolver handling.""" + + def test_pydantic_rejects_name_over_max_length(self): + with pytest.raises(pydantic.ValidationError) as exc_info: + CreateClaimPydanticInput(description="ok", name="x" * 201, year=2025) + + assert any(err["loc"] == ("name",) for err in exc_info.value.errors()) + + def test_resolver_returns_field_errors_when_pydantic_fails(self): + with pytest.raises(pydantic.ValidationError) as exc_info: + CreateClaimPydanticInput(description="ok", name="x" * 201, year=2025) + + data = MagicMock() + data.to_pydantic.side_effect = exc_info.value + info = _make_info(MagicMock()) + + result = BoardCandidateClaimMutations().create_board_candidate_claim(info, data) + + assert not result.ok + assert result.code == "VALIDATION_ERROR" + assert result.field_errors is not None + assert {fe.field for fe in result.field_errors} == {"name"} + + class TestUpdateBoardCandidateClaim: """Tests for update_board_candidate_claim mutation.""" @@ -671,6 +712,7 @@ def _make_input_data( data.name = name data.description = description data.year = year + data.to_pydantic.return_value = data return data @patch("apps.owasp.api.internal.mutations.board_candidate_claim.BoardCandidateClaim") @@ -708,6 +750,7 @@ def test_update_claim_partial(self, mock_claim_model): info = _make_info(user) input_data = MagicMock(key="test-key", description=None, year=2025) input_data.name = "Updated Name" + input_data.to_pydantic.return_value = input_data claim = MagicMock() claim.candidate.member = mock_github_user @@ -808,3 +851,131 @@ def test_update_claim_validation_error(self, mock_claim_model): assert not result.ok assert result.code == "VALIDATION_ERROR" + + +class TestUpdateClaimPydanticValidation: + """Tests for UpdateClaimPydanticInput and its resolver handling.""" + + def test_pydantic_rejects_name_over_max_length(self): + with pytest.raises(pydantic.ValidationError) as exc_info: + UpdateClaimPydanticInput(key="k", name="x" * 201, year=2025) + + assert any(err["loc"] == ("name",) for err in exc_info.value.errors()) + + def test_pydantic_rejects_key_over_max_length(self): + with pytest.raises(pydantic.ValidationError) as exc_info: + UpdateClaimPydanticInput(key="x" * 101, year=2025) + + assert any(err["loc"] == ("key",) for err in exc_info.value.errors()) + + def test_resolver_returns_field_errors_when_pydantic_fails(self): + with pytest.raises(pydantic.ValidationError) as exc_info: + UpdateClaimPydanticInput(key="x" * 101, year=2025) + + data = MagicMock() + data.to_pydantic.side_effect = exc_info.value + info = _make_info(MagicMock()) + + result = BoardCandidateClaimMutations().update_board_candidate_claim(info, data) + + assert not result.ok + assert result.code == "VALIDATION_ERROR" + assert result.field_errors is not None + assert {fe.field for fe in result.field_errors} == {"key"} + + +class TestDiscardClaimPydanticValidation: + """Tests for DiscardClaimPydanticInput and its resolver handling.""" + + def test_pydantic_rejects_key_over_max_length(self): + with pytest.raises(pydantic.ValidationError) as exc_info: + DiscardClaimPydanticInput(key="x" * 101, year=2025) + + assert any(err["loc"] == ("key",) for err in exc_info.value.errors()) + + def test_resolver_returns_field_errors_when_pydantic_fails(self): + with pytest.raises(pydantic.ValidationError) as exc_info: + DiscardClaimPydanticInput(key="x" * 101, year=2025) + + data = MagicMock() + data.to_pydantic.side_effect = exc_info.value + info = _make_info(MagicMock()) + + result = BoardCandidateClaimMutations().discard_board_candidate_claim(info, data) + + assert not result.ok + assert result.code == "VALIDATION_ERROR" + assert result.field_errors is not None + assert {fe.field for fe in result.field_errors} == {"key"} + + +class TestSubmitClaimPydanticValidation: + """Tests for SubmitClaimPydanticInput and its resolver handling.""" + + def test_pydantic_rejects_key_over_max_length(self): + with pytest.raises(pydantic.ValidationError) as exc_info: + SubmitClaimPydanticInput(key="x" * 101, year=2025) + + assert any(err["loc"] == ("key",) for err in exc_info.value.errors()) + + def test_resolver_returns_field_errors_when_pydantic_fails(self): + with pytest.raises(pydantic.ValidationError) as exc_info: + SubmitClaimPydanticInput(key="x" * 101, year=2025) + + data = MagicMock() + data.to_pydantic.side_effect = exc_info.value + info = _make_info(MagicMock()) + + result = BoardCandidateClaimMutations().submit_board_candidate_claim(info, data) + + assert not result.ok + assert result.code == "VALIDATION_ERROR" + assert result.field_errors is not None + assert {fe.field for fe in result.field_errors} == {"key"} + + +class TestWithdrawClaimPydanticValidation: + """Tests for WithdrawClaimPydanticInput and its resolver handling.""" + + def test_pydantic_rejects_key_over_max_length(self): + with pytest.raises(pydantic.ValidationError) as exc_info: + WithdrawClaimPydanticInput(key="x" * 101, withdrawn_reason="ok", year=2025) + + assert any(err["loc"] == ("key",) for err in exc_info.value.errors()) + + def test_resolver_returns_field_errors_when_pydantic_fails(self): + with pytest.raises(pydantic.ValidationError) as exc_info: + WithdrawClaimPydanticInput(key="x" * 101, withdrawn_reason="ok", year=2025) + + data = MagicMock() + data.to_pydantic.side_effect = exc_info.value + info = _make_info(MagicMock()) + + result = BoardCandidateClaimMutations().withdraw_board_candidate_claim(info, data) + + assert not result.ok + assert result.code == "VALIDATION_ERROR" + assert result.field_errors is not None + assert {fe.field for fe in result.field_errors} == {"key"} + + +class TestReorderClaimsPydanticValidation: + """Tests for ReorderClaimsPydanticInput and its resolver handling.""" + + def test_pydantic_accepts_valid_input(self): + ReorderClaimsPydanticInput(keys=["k1", "k2"], year=2025) + + def test_resolver_returns_field_errors_when_pydantic_fails(self): + with pytest.raises(pydantic.ValidationError) as exc_info: + ReorderClaimsPydanticInput(keys=["k1"], year="not-an-int") # type: ignore[arg-type] + + data = MagicMock() + data.to_pydantic.side_effect = exc_info.value + info = _make_info(MagicMock()) + + result = BoardCandidateClaimMutations().reorder_board_candidate_claims(info, data) + + assert not result.ok + assert result.code == "VALIDATION_ERROR" + assert result.field_errors is not None + assert {fe.field for fe in result.field_errors} == {"year"} diff --git a/backend/tests/unit/apps/owasp/api/internal/nodes/board_candidate_claim_test.py b/backend/tests/unit/apps/owasp/api/internal/nodes/board_candidate_claim_test.py index b2a4fda1bd..8669e5e0bd 100644 --- a/backend/tests/unit/apps/owasp/api/internal/nodes/board_candidate_claim_test.py +++ b/backend/tests/unit/apps/owasp/api/internal/nodes/board_candidate_claim_test.py @@ -117,7 +117,7 @@ def test_reviews_approved_sees_all(self): mock_claim.reviews.all.assert_called_once() assert result == mock_queryset - def test_reviews_reviewer_sees_own(self): + def test_reviews_submitted_sees_all(self): user = MagicMock() user.is_authenticated = True user.github_user = Mock() @@ -126,18 +126,17 @@ def test_reviews_reviewer_sees_own(self): mock_claim = Mock() mock_claim.candidate.member = Mock() mock_claim.status = BoardCandidateClaim.Status.SUBMITTED - mock_claim.board.reviewers.filter.return_value.exists.return_value = True mock_queryset = MagicMock() mock_claim.reviews = MagicMock() - mock_claim.reviews.filter.return_value = mock_queryset + mock_claim.reviews.all.return_value = mock_queryset field = self._get_field_by_name("reviews", BoardCandidateClaimNode) result = field.base_resolver.wrapped_func(None, mock_claim, info) - mock_claim.reviews.filter.assert_called_once_with(reviewer=user) + mock_claim.reviews.all.assert_called_once() assert result == mock_queryset - def test_reviews_non_reviewer_gets_empty_on_submitted(self): + def test_reviews_rejected_sees_all(self): user = MagicMock() user.is_authenticated = True user.github_user = Mock() @@ -145,23 +144,58 @@ def test_reviews_non_reviewer_gets_empty_on_submitted(self): mock_claim = Mock() mock_claim.candidate.member = Mock() - mock_claim.status = BoardCandidateClaim.Status.SUBMITTED - mock_claim.board.reviewers.filter.return_value.exists.return_value = False + mock_claim.status = BoardCandidateClaim.Status.REJECTED + mock_queryset = MagicMock() + mock_claim.reviews = MagicMock() + mock_claim.reviews.all.return_value = mock_queryset + + field = self._get_field_by_name("reviews", BoardCandidateClaimNode) + result = field.base_resolver.wrapped_func(None, mock_claim, info) + + mock_claim.reviews.all.assert_called_once() + assert result == mock_queryset + + def test_reviews_non_self_draft_gets_empty(self): + user = MagicMock() + user.is_authenticated = True + user.github_user = Mock() + info = self._make_info(user) + + mock_claim = Mock() + mock_claim.candidate.member = Mock() + mock_claim.status = BoardCandidateClaim.Status.DRAFT field = self._get_field_by_name("reviews", BoardCandidateClaimNode) result = field.base_resolver.wrapped_func(None, mock_claim, info) assert result == [] - def test_reviews_anonymous_gets_empty_on_submitted(self): + def test_reviews_anonymous_draft_gets_empty(self): user = MagicMock() user.is_authenticated = False info = self._make_info(user) mock_claim = Mock() - mock_claim.status = BoardCandidateClaim.Status.SUBMITTED + mock_claim.status = BoardCandidateClaim.Status.DRAFT field = self._get_field_by_name("reviews", BoardCandidateClaimNode) result = field.base_resolver.wrapped_func(None, mock_claim, info) assert result == [] + + def test_reviews_anonymous_submitted_sees_all(self): + user = MagicMock() + user.is_authenticated = False + info = self._make_info(user) + + mock_claim = Mock() + mock_claim.status = BoardCandidateClaim.Status.SUBMITTED + mock_queryset = MagicMock() + mock_claim.reviews = MagicMock() + mock_claim.reviews.all.return_value = mock_queryset + + field = self._get_field_by_name("reviews", BoardCandidateClaimNode) + result = field.base_resolver.wrapped_func(None, mock_claim, info) + + mock_claim.reviews.all.assert_called_once() + assert result == mock_queryset diff --git a/backend/tests/unit/apps/owasp/api/internal/nodes/board_of_directors_test.py b/backend/tests/unit/apps/owasp/api/internal/nodes/board_of_directors_test.py index 39f1d54584..f8988ed355 100644 --- a/backend/tests/unit/apps/owasp/api/internal/nodes/board_of_directors_test.py +++ b/backend/tests/unit/apps/owasp/api/internal/nodes/board_of_directors_test.py @@ -95,31 +95,25 @@ def test_reviewer_resolver_found(self): mock_nest_user.github_user = mock_github_user mock_board = Mock() - mock_board.reviewers.select_related.return_value.filter.return_value.first.return_value = ( - mock_nest_user - ) + mock_filter = mock_board.claim_reviewers.select_related.return_value.filter + mock_filter.return_value.first.return_value = mock_nest_user field = self._get_field_by_name("reviewer", BoardOfDirectorsNode) result = field.base_resolver.wrapped_func(None, mock_board, login="alice") - mock_board.reviewers.select_related.assert_called_once_with("github_user") - mock_board.reviewers.select_related.return_value.filter.assert_called_once_with( - github_user__login="alice" - ) + mock_board.claim_reviewers.select_related.assert_called_once_with("github_user") + mock_filter.assert_called_once_with(github_user__login="alice") assert result is mock_github_user def test_reviewer_resolver_not_found(self): """Test reviewer returns None when no reviewer matches the login.""" mock_board = Mock() - mock_board.reviewers.select_related.return_value.filter.return_value.first.return_value = ( - None - ) + mock_filter = mock_board.claim_reviewers.select_related.return_value.filter + mock_filter.return_value.first.return_value = None field = self._get_field_by_name("reviewer", BoardOfDirectorsNode) result = field.base_resolver.wrapped_func(None, mock_board, login="unknown") - mock_board.reviewers.select_related.assert_called_once_with("github_user") - mock_board.reviewers.select_related.return_value.filter.assert_called_once_with( - github_user__login="unknown" - ) + mock_board.claim_reviewers.select_related.assert_called_once_with("github_user") + mock_filter.assert_called_once_with(github_user__login="unknown") assert result is None diff --git a/backend/tests/unit/apps/owasp/api/internal/queries/board_candidate_claim_evidence_test.py b/backend/tests/unit/apps/owasp/api/internal/queries/board_candidate_claim_evidence_test.py index eeef2b9a08..c6dd83b920 100644 --- a/backend/tests/unit/apps/owasp/api/internal/queries/board_candidate_claim_evidence_test.py +++ b/backend/tests/unit/apps/owasp/api/internal/queries/board_candidate_claim_evidence_test.py @@ -21,6 +21,7 @@ class TestBoardCandidateClaimEvidenceQuery: @patch("apps.owasp.api.internal.queries.board_candidate_claim_evidence.BoardCandidateClaim") def test_board_candidate_claim_evidences_claim_not_found(self, mock_claim_model): mock_claim_model.Status = BoardCandidateClaim.Status + mock_claim_model.PUBLIC_STATUSES = BoardCandidateClaim.PUBLIC_STATUSES user = MagicMock() user.is_authenticated = True user.github_user = MagicMock() @@ -44,6 +45,7 @@ def test_board_candidate_claim_evidences_claim_not_found(self, mock_claim_model) @patch("apps.owasp.api.internal.queries.board_candidate_claim_evidence.BoardCandidateClaim") def test_board_candidate_claim_evidences_self(self, mock_claim_model): mock_claim_model.Status = BoardCandidateClaim.Status + mock_claim_model.PUBLIC_STATUSES = BoardCandidateClaim.PUBLIC_STATUSES user = MagicMock() user.is_authenticated = True mock_github_user = MagicMock() @@ -63,78 +65,64 @@ def test_board_candidate_claim_evidences_self(self, mock_claim_model): info, claim_key=claim_key, login=login, year=2025 ) - mock_claim_model.objects.filter.assert_called_once_with( - candidate__member__login=login, key=claim_key, board__year=2025 - ) claim.evidences.filter.assert_called_once_with(is_removed=False) assert result == evidences_qs @patch("apps.owasp.api.internal.queries.board_candidate_claim_evidence.BoardCandidateClaim") - def test_board_candidate_claim_evidences_non_self_non_approved(self, mock_claim_model): + def test_board_candidate_claim_evidences_non_self_draft_hidden(self, mock_claim_model): mock_claim_model.Status = BoardCandidateClaim.Status + mock_claim_model.PUBLIC_STATUSES = BoardCandidateClaim.PUBLIC_STATUSES user = MagicMock() user.is_authenticated = True user.github_user = MagicMock() info = _make_info(user) - claim_key = "my-key" - login = "alice" claim = MagicMock() - claim.board.reviewers.filter.return_value.exists.return_value = False claim.candidate.member = None - claim.status = BoardCandidateClaim.Status.SUBMITTED + claim.status = BoardCandidateClaim.Status.DRAFT mock_claim_model.objects.filter.return_value.first.return_value = claim query = BoardCandidateClaimEvidenceQuery() result = query.board_candidate_claim_evidences( - info, claim_key=claim_key, login=login, year=2025 + info, claim_key="my-key", login="alice", year=2025 ) - mock_claim_model.objects.filter.assert_called_once_with( - candidate__member__login=login, key=claim_key, board__year=2025 - ) assert result == [] @patch("apps.owasp.api.internal.queries.board_candidate_claim_evidence.BoardCandidateClaim") def test_board_candidate_claim_evidences_non_self_approved(self, mock_claim_model): mock_claim_model.Status = BoardCandidateClaim.Status + mock_claim_model.PUBLIC_STATUSES = BoardCandidateClaim.PUBLIC_STATUSES user = MagicMock() user.is_authenticated = True user.github_user = MagicMock() info = _make_info(user) - claim_key = "my-key" - login = "alice" claim = MagicMock() claim.candidate.member = MagicMock() - claim.status = mock_claim_model.Status.APPROVED + claim.status = BoardCandidateClaim.Status.APPROVED evidences_qs = MagicMock() claim.evidences.filter.return_value = evidences_qs mock_claim_model.objects.filter.return_value.first.return_value = claim query = BoardCandidateClaimEvidenceQuery() result = query.board_candidate_claim_evidences( - info, claim_key=claim_key, login=login, year=2025 + info, claim_key="my-key", login="alice", year=2025 ) - mock_claim_model.objects.filter.assert_called_once_with( - candidate__member__login=login, key=claim_key, board__year=2025 - ) claim.evidences.filter.assert_called_once_with(is_removed=False) assert result == evidences_qs @patch("apps.owasp.api.internal.queries.board_candidate_claim_evidence.BoardCandidateClaim") - def test_board_candidate_claim_evidences_reviewer_sees_submitted(self, mock_claim_model): + def test_board_candidate_claim_evidences_non_self_submitted(self, mock_claim_model): mock_claim_model.Status = BoardCandidateClaim.Status + mock_claim_model.PUBLIC_STATUSES = BoardCandidateClaim.PUBLIC_STATUSES user = MagicMock() user.is_authenticated = True user.github_user = MagicMock() info = _make_info(user) - claim_key = "my-key" - login = "alice" claim = MagicMock() - claim.board.reviewers.filter.return_value.exists.return_value = True claim.candidate.member = MagicMock() claim.status = BoardCandidateClaim.Status.SUBMITTED evidences_qs = MagicMock() @@ -143,12 +131,9 @@ def test_board_candidate_claim_evidences_reviewer_sees_submitted(self, mock_clai query = BoardCandidateClaimEvidenceQuery() result = query.board_candidate_claim_evidences( - info, claim_key=claim_key, login=login, year=2025 + info, claim_key="my-key", login="alice", year=2025 ) - mock_claim_model.objects.filter.assert_called_once_with( - candidate__member__login=login, key=claim_key, board__year=2025 - ) claim.evidences.filter.assert_called_once_with(is_removed=False) assert result == evidences_qs @@ -179,13 +164,6 @@ def test_board_candidate_claim_evidence_self(self): info, claim_key="test-key", key="ev-key", login="alice", year=2025 ) - mock_evidence_model.objects.get.assert_called_once_with( - claim__key="test-key", - key="ev-key", - claim__candidate__member__login="alice", - claim__board__year=2025, - is_removed=False, - ) assert result == evidence def test_board_candidate_claim_evidence_non_self_approved(self): @@ -212,15 +190,14 @@ def test_board_candidate_claim_evidence_non_self_approved(self): assert result == evidence - def test_board_candidate_claim_evidence_non_self_not_approved(self): + def test_board_candidate_claim_evidence_non_self_submitted(self): user = MagicMock() user.is_authenticated = True user.github_user = MagicMock() info = _make_info(user) evidence = MagicMock() - evidence.claim.board.reviewers.filter.return_value.exists.return_value = False - evidence.claim.candidate.member = None + evidence.claim.candidate.member = MagicMock() evidence.claim.status = BoardCandidateClaim.Status.SUBMITTED with patch( @@ -235,20 +212,24 @@ def test_board_candidate_claim_evidence_non_self_not_approved(self): info, claim_key="test-key", key="ev-key", login="alice", year=2025 ) - assert result is None + assert result == evidence - def test_board_candidate_claim_evidence_not_found(self): + def test_board_candidate_claim_evidence_non_self_draft_hidden(self): user = MagicMock() user.is_authenticated = True user.github_user = MagicMock() info = _make_info(user) + evidence = MagicMock() + evidence.claim.candidate.member = None + evidence.claim.status = BoardCandidateClaim.Status.DRAFT + with patch( "apps.owasp.api.internal.queries.board_candidate_claim_evidence" ".BoardCandidateClaimEvidence" ) as mock_evidence_model: mock_evidence_model.DoesNotExist = BoardCandidateClaimEvidence.DoesNotExist - mock_evidence_model.objects.get.side_effect = BoardCandidateClaimEvidence.DoesNotExist + mock_evidence_model.objects.get.return_value = evidence query = BoardCandidateClaimEvidenceQuery() result = query.board_candidate_claim_evidence( @@ -257,30 +238,25 @@ def test_board_candidate_claim_evidence_not_found(self): assert result is None - def test_board_candidate_claim_evidence_reviewer_sees_submitted(self): + def test_board_candidate_claim_evidence_not_found(self): user = MagicMock() user.is_authenticated = True user.github_user = MagicMock() info = _make_info(user) - evidence = MagicMock() - evidence.claim.board.reviewers.filter.return_value.exists.return_value = True - evidence.claim.candidate.member = MagicMock() - evidence.claim.status = BoardCandidateClaim.Status.SUBMITTED - with patch( "apps.owasp.api.internal.queries.board_candidate_claim_evidence" ".BoardCandidateClaimEvidence" ) as mock_evidence_model: mock_evidence_model.DoesNotExist = BoardCandidateClaimEvidence.DoesNotExist - mock_evidence_model.objects.get.return_value = evidence + mock_evidence_model.objects.get.side_effect = BoardCandidateClaimEvidence.DoesNotExist query = BoardCandidateClaimEvidenceQuery() result = query.board_candidate_claim_evidence( info, claim_key="test-key", key="ev-key", login="alice", year=2025 ) - assert result == evidence + assert result is None class TestBoardCandidateClaimEvidenceFileUrlQuery: @@ -342,16 +318,15 @@ def test_file_url_accessible_no_file(self): assert result is None - def test_file_url_not_accessible(self): + def test_file_url_not_accessible_draft(self): user = MagicMock() user.is_authenticated = True user.github_user = MagicMock() info = _make_info(user) evidence = MagicMock() - evidence.claim.board.reviewers.filter.return_value.exists.return_value = False evidence.claim.candidate.member = None - evidence.claim.status = BoardCandidateClaim.Status.SUBMITTED + evidence.claim.status = BoardCandidateClaim.Status.DRAFT evidence.file = MagicMock() evidence.file.url = "/media/test.pdf" @@ -416,15 +391,12 @@ def test_file_url_anonymous_approved(self): assert result == "https://example.com/media/test.pdf" - def test_file_url_reviewer_accessible(self): + def test_file_url_anonymous_submitted(self): user = MagicMock() - user.is_authenticated = True - user.github_user = MagicMock() + user.is_authenticated = False info = _make_info(user) evidence = MagicMock() - evidence.claim.board.reviewers.filter.return_value.exists.return_value = True - evidence.claim.candidate.member = None evidence.claim.status = BoardCandidateClaim.Status.SUBMITTED evidence.file = MagicMock() evidence.file.url = "/media/test.pdf" diff --git a/backend/tests/unit/apps/owasp/api/internal/queries/board_candidate_claim_test.py b/backend/tests/unit/apps/owasp/api/internal/queries/board_candidate_claim_test.py index 560d4f63d0..7c02e9c978 100644 --- a/backend/tests/unit/apps/owasp/api/internal/queries/board_candidate_claim_test.py +++ b/backend/tests/unit/apps/owasp/api/internal/queries/board_candidate_claim_test.py @@ -15,11 +15,10 @@ def _make_info(user): class TestBoardCandidateClaimQuery: """Tests for board_candidate_claims query.""" - @patch("apps.owasp.api.internal.queries.board_candidate_claim.BoardOfDirectors") @patch("apps.owasp.api.internal.queries.board_candidate_claim.BoardCandidateClaim") - def test_board_candidate_claims_self(self, mock_claim_model, mock_board_model): + def test_board_candidate_claims_self(self, mock_claim_model): mock_claim_model.Status = BoardCandidateClaim.Status - mock_board_model.objects.filter.return_value.exists.return_value = False + mock_claim_model.PUBLIC_STATUSES = BoardCandidateClaim.PUBLIC_STATUSES user = MagicMock() user.is_authenticated = True user.github_user = MagicMock() @@ -42,13 +41,10 @@ def test_board_candidate_claims_self(self, mock_claim_model, mock_board_model): base_qs.filter.assert_called_once_with(candidate__member__login="alice") assert result == claims - @patch("apps.owasp.api.internal.queries.board_candidate_claim.BoardOfDirectors") @patch("apps.owasp.api.internal.queries.board_candidate_claim.BoardCandidateClaim") - def test_board_candidate_claims_non_self_filters_approved( - self, mock_claim_model, mock_board_model - ): + def test_board_candidate_claims_non_self_filters_public(self, mock_claim_model): mock_claim_model.Status = BoardCandidateClaim.Status - mock_board_model.objects.filter.return_value.exists.return_value = False + mock_claim_model.PUBLIC_STATUSES = BoardCandidateClaim.PUBLIC_STATUSES user = MagicMock() user.is_authenticated = True user.github_user = MagicMock() @@ -69,16 +65,13 @@ def test_board_candidate_claims_non_self_filters_approved( result = query.board_candidate_claims(info, login="alice", year=2025) base_qs.filter.assert_called_once_with(candidate__member__login="alice") - login_qs.filter.assert_called_once_with(status=BoardCandidateClaim.Status.APPROVED) + login_qs.filter.assert_called_once_with(status__in=BoardCandidateClaim.PUBLIC_STATUSES) assert result == filtered_qs - @patch("apps.owasp.api.internal.queries.board_candidate_claim.BoardOfDirectors") @patch("apps.owasp.api.internal.queries.board_candidate_claim.BoardCandidateClaim") - def test_board_candidate_claims_anonymous_filters_approved( - self, mock_claim_model, mock_board_model - ): + def test_board_candidate_claims_anonymous_filters_public(self, mock_claim_model): mock_claim_model.Status = BoardCandidateClaim.Status - mock_board_model.objects.filter.return_value.exists.return_value = False + mock_claim_model.PUBLIC_STATUSES = BoardCandidateClaim.PUBLIC_STATUSES user = MagicMock() user.is_authenticated = False info = _make_info(user) @@ -97,39 +90,7 @@ def test_board_candidate_claims_anonymous_filters_approved( result = query.board_candidate_claims(info, login="alice", year=2025) base_qs.filter.assert_called_once_with(candidate__member__login="alice") - login_qs.filter.assert_called_once_with(status=BoardCandidateClaim.Status.APPROVED) - assert result == filtered_qs - - @patch("apps.owasp.api.internal.queries.board_candidate_claim.BoardOfDirectors") - @patch("apps.owasp.api.internal.queries.board_candidate_claim.BoardCandidateClaim") - def test_board_candidate_claims_reviewer_sees_submitted_and_approved( - self, mock_claim_model, mock_board_model - ): - mock_claim_model.Status = BoardCandidateClaim.Status - user = MagicMock() - user.is_authenticated = True - user.github_user = MagicMock() - user.github_user.login = "bob" - mock_board_model.objects.filter.return_value.exists.return_value = True - info = _make_info(user) - - filtered_qs = MagicMock() - filtered_qs.annotate.return_value.select_related.return_value.order_by.return_value = ( - filtered_qs - ) - login_qs = MagicMock() - login_qs.filter.return_value = filtered_qs - base_qs = MagicMock() - base_qs.filter.return_value = login_qs - mock_claim_model.objects.filter.return_value = base_qs - - query = BoardCandidateClaimQuery() - result = query.board_candidate_claims(info, login="alice", year=2025) - - base_qs.filter.assert_called_once_with(candidate__member__login="alice") - login_qs.filter.assert_called_once_with( - status__in=[BoardCandidateClaim.Status.SUBMITTED, BoardCandidateClaim.Status.APPROVED] - ) + login_qs.filter.assert_called_once_with(status__in=BoardCandidateClaim.PUBLIC_STATUSES) assert result == filtered_qs @@ -139,6 +100,7 @@ class TestBoardCandidateClaimSingleQuery: @patch("apps.owasp.api.internal.queries.board_candidate_claim.BoardCandidateClaim") def test_board_candidate_claim_self(self, mock_claim_model): mock_claim_model.Status = BoardCandidateClaim.Status + mock_claim_model.PUBLIC_STATUSES = BoardCandidateClaim.PUBLIC_STATUSES mock_claim_model.DoesNotExist = BoardCandidateClaim.DoesNotExist user = MagicMock() user.is_authenticated = True @@ -168,6 +130,7 @@ def test_board_candidate_claim_self(self, mock_claim_model): @patch("apps.owasp.api.internal.queries.board_candidate_claim.BoardCandidateClaim") def test_board_candidate_claim_non_self_approved(self, mock_claim_model): mock_claim_model.Status = BoardCandidateClaim.Status + mock_claim_model.PUBLIC_STATUSES = BoardCandidateClaim.PUBLIC_STATUSES mock_claim_model.DoesNotExist = BoardCandidateClaim.DoesNotExist user = MagicMock() user.is_authenticated = True @@ -175,7 +138,6 @@ def test_board_candidate_claim_non_self_approved(self, mock_claim_model): info = _make_info(user) claim = MagicMock() - claim.board.reviewers.filter.return_value.exists.return_value = False claim.candidate.member = None claim.status = BoardCandidateClaim.Status.APPROVED mock_qs = MagicMock() @@ -185,12 +147,12 @@ def test_board_candidate_claim_non_self_approved(self, mock_claim_model): query = BoardCandidateClaimQuery() result = query.board_candidate_claim(info, login="alice", key="test-key", year=2025) - mock_claim_model.objects.select_related.return_value.annotate.assert_called_once() assert result == claim @patch("apps.owasp.api.internal.queries.board_candidate_claim.BoardCandidateClaim") - def test_board_candidate_claim_non_self_not_approved(self, mock_claim_model): + def test_board_candidate_claim_non_self_submitted(self, mock_claim_model): mock_claim_model.Status = BoardCandidateClaim.Status + mock_claim_model.PUBLIC_STATUSES = BoardCandidateClaim.PUBLIC_STATUSES mock_claim_model.DoesNotExist = BoardCandidateClaim.DoesNotExist user = MagicMock() user.is_authenticated = True @@ -198,7 +160,6 @@ def test_board_candidate_claim_non_self_not_approved(self, mock_claim_model): info = _make_info(user) claim = MagicMock() - claim.board.reviewers.filter.return_value.exists.return_value = False claim.candidate.member = None claim.status = BoardCandidateClaim.Status.SUBMITTED mock_qs = MagicMock() @@ -208,36 +169,43 @@ def test_board_candidate_claim_non_self_not_approved(self, mock_claim_model): query = BoardCandidateClaimQuery() result = query.board_candidate_claim(info, login="alice", key="test-key", year=2025) - assert result is None + assert result == claim @patch("apps.owasp.api.internal.queries.board_candidate_claim.BoardCandidateClaim") - def test_board_candidate_claim_not_found(self, mock_claim_model): + def test_board_candidate_claim_non_self_rejected(self, mock_claim_model): mock_claim_model.Status = BoardCandidateClaim.Status + mock_claim_model.PUBLIC_STATUSES = BoardCandidateClaim.PUBLIC_STATUSES mock_claim_model.DoesNotExist = BoardCandidateClaim.DoesNotExist user = MagicMock() user.is_authenticated = True user.github_user = MagicMock() info = _make_info(user) + claim = MagicMock() + claim.candidate.member = None + claim.status = BoardCandidateClaim.Status.REJECTED mock_qs = MagicMock() - mock_qs.get.side_effect = BoardCandidateClaim.DoesNotExist + mock_qs.get.return_value = claim mock_claim_model.objects.select_related.return_value.annotate.return_value = mock_qs query = BoardCandidateClaimQuery() result = query.board_candidate_claim(info, login="alice", key="test-key", year=2025) - assert result is None + assert result == claim @patch("apps.owasp.api.internal.queries.board_candidate_claim.BoardCandidateClaim") - def test_board_candidate_claim_anonymous_approved(self, mock_claim_model): + def test_board_candidate_claim_non_self_draft_hidden(self, mock_claim_model): mock_claim_model.Status = BoardCandidateClaim.Status + mock_claim_model.PUBLIC_STATUSES = BoardCandidateClaim.PUBLIC_STATUSES mock_claim_model.DoesNotExist = BoardCandidateClaim.DoesNotExist user = MagicMock() - user.is_authenticated = False + user.is_authenticated = True + user.github_user = MagicMock() info = _make_info(user) claim = MagicMock() - claim.status = BoardCandidateClaim.Status.APPROVED + claim.candidate.member = None + claim.status = BoardCandidateClaim.Status.DRAFT mock_qs = MagicMock() mock_qs.get.return_value = claim mock_claim_model.objects.select_related.return_value.annotate.return_value = mock_qs @@ -245,22 +213,38 @@ def test_board_candidate_claim_anonymous_approved(self, mock_claim_model): query = BoardCandidateClaimQuery() result = query.board_candidate_claim(info, login="alice", key="test-key", year=2025) - mock_claim_model.objects.select_related.return_value.annotate.assert_called_once() - assert result == claim + assert result is None @patch("apps.owasp.api.internal.queries.board_candidate_claim.BoardCandidateClaim") - def test_board_candidate_claim_reviewer_sees_submitted(self, mock_claim_model): + def test_board_candidate_claim_not_found(self, mock_claim_model): mock_claim_model.Status = BoardCandidateClaim.Status + mock_claim_model.PUBLIC_STATUSES = BoardCandidateClaim.PUBLIC_STATUSES mock_claim_model.DoesNotExist = BoardCandidateClaim.DoesNotExist user = MagicMock() user.is_authenticated = True user.github_user = MagicMock() info = _make_info(user) + mock_qs = MagicMock() + mock_qs.get.side_effect = BoardCandidateClaim.DoesNotExist + mock_claim_model.objects.select_related.return_value.annotate.return_value = mock_qs + + query = BoardCandidateClaimQuery() + result = query.board_candidate_claim(info, login="alice", key="test-key", year=2025) + + assert result is None + + @patch("apps.owasp.api.internal.queries.board_candidate_claim.BoardCandidateClaim") + def test_board_candidate_claim_anonymous_approved(self, mock_claim_model): + mock_claim_model.Status = BoardCandidateClaim.Status + mock_claim_model.PUBLIC_STATUSES = BoardCandidateClaim.PUBLIC_STATUSES + mock_claim_model.DoesNotExist = BoardCandidateClaim.DoesNotExist + user = MagicMock() + user.is_authenticated = False + info = _make_info(user) + claim = MagicMock() - claim.board.reviewers.filter.return_value.exists.return_value = True - claim.candidate.member = None - claim.status = BoardCandidateClaim.Status.SUBMITTED + claim.status = BoardCandidateClaim.Status.APPROVED mock_qs = MagicMock() mock_qs.get.return_value = claim mock_claim_model.objects.select_related.return_value.annotate.return_value = mock_qs @@ -271,17 +255,15 @@ def test_board_candidate_claim_reviewer_sees_submitted(self, mock_claim_model): assert result == claim @patch("apps.owasp.api.internal.queries.board_candidate_claim.BoardCandidateClaim") - def test_board_candidate_claim_reviewer_blocked_from_draft(self, mock_claim_model): + def test_board_candidate_claim_anonymous_draft_hidden(self, mock_claim_model): mock_claim_model.Status = BoardCandidateClaim.Status + mock_claim_model.PUBLIC_STATUSES = BoardCandidateClaim.PUBLIC_STATUSES mock_claim_model.DoesNotExist = BoardCandidateClaim.DoesNotExist user = MagicMock() - user.is_authenticated = True - user.github_user = MagicMock() + user.is_authenticated = False info = _make_info(user) claim = MagicMock() - claim.board.reviewers.filter.return_value.exists.return_value = True - claim.candidate.member = None claim.status = BoardCandidateClaim.Status.DRAFT mock_qs = MagicMock() mock_qs.get.return_value = claim diff --git a/backend/tests/unit/apps/owasp/models/board_candidate_claim_review_test.py b/backend/tests/unit/apps/owasp/models/board_candidate_claim_review_test.py index 55c1766366..088ca20cd4 100644 --- a/backend/tests/unit/apps/owasp/models/board_candidate_claim_review_test.py +++ b/backend/tests/unit/apps/owasp/models/board_candidate_claim_review_test.py @@ -86,7 +86,7 @@ def test_clean_passes_for_claim_reviewer_role(self): board.get_candidate = MagicMock(return_value=None) with ( - patch.object(BoardOfDirectors, "reviewers") as mock_reviewers, + patch.object(BoardOfDirectors, "claim_reviewers") as mock_reviewers, patch.object(User, "github_user", new_callable=PropertyMock) as mock_github_user, ): mock_reviewers.filter.return_value.exists.return_value = True @@ -125,13 +125,20 @@ def test_clean_non_submitted_claim_raises(self, status): def test_clean_user_not_reviewer_raises(self): """Test that clean raises ValidationError when user is not a reviewer.""" reviewer_user = User() - review = self._build_review( - claim_status=BoardCandidateClaim.Status.SUBMITTED, - reviewer_user=reviewer_user, - ) + board = BoardOfDirectors() - with patch.object(User, "github_user") as mock_github_user: + with ( + patch.object(BoardOfDirectors, "claim_reviewers") as mock_reviewers, + patch.object(User, "github_user") as mock_github_user, + ): + mock_reviewers.filter.return_value.exists.return_value = False mock_github_user.login = "alice" + review = self._build_review( + claim_status=BoardCandidateClaim.Status.SUBMITTED, + reviewer_user=reviewer_user, + claim_board=board, + ) + with pytest.raises(ValidationError) as exc_info: review.clean() @@ -144,7 +151,7 @@ def test_clean_candidate_reviewing_same_election_year_raises(self): board.get_candidate = MagicMock(return_value=MagicMock()) # candidate found with ( - patch.object(BoardOfDirectors, "reviewers") as mock_reviewers, + patch.object(BoardOfDirectors, "claim_reviewers") as mock_reviewers, patch.object(User, "github_user") as mock_github_user, ): mock_reviewers.filter.return_value.exists.return_value = True @@ -169,7 +176,7 @@ def test_clean_raises_validation_error_when_no_github_user(self): """Test that clean raises ValidationError when when reviewer has no linked github user.""" reviewer_user = User() board = BoardOfDirectors() - with patch.object(BoardOfDirectors, "reviewers") as mock_reviewers: + with patch.object(BoardOfDirectors, "claim_reviewers") as mock_reviewers: mock_reviewers.filter.return_value.exists.return_value = True review = self._build_review( claim_status=BoardCandidateClaim.Status.SUBMITTED, diff --git a/backend/tests/unit/apps/owasp/models/board_candidate_claim_test.py b/backend/tests/unit/apps/owasp/models/board_candidate_claim_test.py index 6b17e35d02..c2703f71c3 100644 --- a/backend/tests/unit/apps/owasp/models/board_candidate_claim_test.py +++ b/backend/tests/unit/apps/owasp/models/board_candidate_claim_test.py @@ -61,6 +61,16 @@ def test_finalized_statuses(self): assert expected == BoardCandidateClaim.FINALIZED_STATUSES + def test_public_statuses(self): + """Test PUBLIC_STATUSES contains the correct statuses.""" + expected = { + BoardCandidateClaim.Status.APPROVED, + BoardCandidateClaim.Status.REJECTED, + BoardCandidateClaim.Status.SUBMITTED, + } + + assert expected == BoardCandidateClaim.PUBLIC_STATUSES + def test_default_status_is_draft(self): """Test default status is DRAFT.""" field = BoardCandidateClaim._meta.get_field("status") @@ -79,12 +89,12 @@ def test_default_order_zero(self): assert field.default == 0 - def test_board_field_nullable(self): - """Test board field is nullable.""" + def test_board_field_required(self): + """Test board field is required.""" field = BoardCandidateClaim._meta.get_field("board") - assert field.null - assert field.blank + assert not field.null + assert not field.blank def test_withdrawn_at_field_nullable(self): """Test withdrawn_at field is nullable.""" @@ -325,6 +335,7 @@ def test_clean_non_draft_claim_allows_status_update_only(self, mock_objects): def test_save_calls_full_clean(self, mock_super_save, mock_full_clean): """Test that save calls full_clean before saving.""" claim = BoardCandidateClaim(name="Test Claim", status=BoardCandidateClaim.Status.DRAFT) + claim.pk = 1 claim.save() @@ -346,6 +357,7 @@ def test_save_locks_claim_on_finalized_status(self, mock_super_save, mock_full_c """Test that save sets is_locked=True for finalized statuses.""" claim = BoardCandidateClaim(name="Test Claim", status=status) claim.is_locked = False + claim.pk = 1 claim.save() @@ -366,11 +378,22 @@ def test_save_does_not_lock_claim_on_non_finalized_status( """Test that save does not set is_locked=True for non-finalized statuses.""" claim = BoardCandidateClaim(name="Test Claim", status=status) claim.is_locked = False + claim.pk = 1 claim.save() assert claim.is_locked is False + @patch.object(BoardCandidateClaim, "save") + def test_set_status_approved_sets_status_and_saves(self, mock_save): + """Test set_status_approved flips status to APPROVED and saves.""" + claim = BoardCandidateClaim(name="Test Claim", status=BoardCandidateClaim.Status.SUBMITTED) + + claim.set_status_approved() + + assert claim.status == BoardCandidateClaim.Status.APPROVED + mock_save.assert_called_once() + @patch("apps.owasp.models.board_candidate_claim.BulkSaveModel.bulk_save") def test_bulk_save_delegates(self, mock_bulk_save): """Test bulk_save delegates to BulkSaveModel.bulk_save.""" @@ -380,6 +403,22 @@ def test_bulk_save_delegates(self, mock_bulk_save): mock_bulk_save.assert_called_once_with(BoardCandidateClaim, claims, fields=["order"]) + @patch("apps.owasp.models.board_candidate_claim.BoardCandidateClaim.objects") + def test_bulk_set_status_approved_updates_status_lock_and_calls_bulk_update( + self, mock_objects + ): + """Test bulk_set_status_approved flips status, locks, and issues one bulk_update.""" + claim_a = BoardCandidateClaim(name="Claim A", status=BoardCandidateClaim.Status.SUBMITTED) + claim_b = BoardCandidateClaim(name="Claim B", status=BoardCandidateClaim.Status.SUBMITTED) + claims = [claim_a, claim_b] + + BoardCandidateClaim.bulk_set_status_approved(claims) + + for claim in claims: + assert claim.status == BoardCandidateClaim.Status.APPROVED + assert claim.is_locked is True + mock_objects.bulk_update.assert_called_once_with(claims, ["is_locked", "status"]) + @patch.object(BoardCandidateClaim, "full_clean") @patch("apps.owasp.models.board_candidate_claim.TimestampedModel.save") @patch("apps.owasp.models.board_candidate_claim.BoardCandidateClaim.objects") diff --git a/backend/tests/unit/apps/owasp/signals/board_candidate_claim_review_test.py b/backend/tests/unit/apps/owasp/signals/board_candidate_claim_review_test.py index da89e9f129..d2a0e18fcd 100644 --- a/backend/tests/unit/apps/owasp/signals/board_candidate_claim_review_test.py +++ b/backend/tests/unit/apps/owasp/signals/board_candidate_claim_review_test.py @@ -29,8 +29,7 @@ def test_auto_approves_when_threshold_met(self, mock_logger): claim.reviews.filter.assert_called_once_with( status=BoardCandidateClaimReview.Status.APPROVED, ) - assert claim.status == BoardCandidateClaim.Status.APPROVED - claim.save.assert_called_once() + claim.set_status_approved.assert_called_once() mock_logger.info.assert_called_once_with( "Claim '%s' auto-approved with %d approvals (threshold: %d).", "test-claim", @@ -55,7 +54,7 @@ def test_does_not_approve_when_threshold_not_met(self, mock_logger): status=BoardCandidateClaimReview.Status.APPROVED, ) assert claim.status == BoardCandidateClaim.Status.SUBMITTED - claim.save.assert_not_called() + claim.set_status_approved.assert_not_called() mock_logger.info.assert_not_called() @patch("apps.owasp.signals.board_candidate_claim_review.logger") @@ -68,7 +67,7 @@ def test_does_nothing_when_claim_not_submitted(self, mock_logger): review_post_save_finalize_claim_status(sender=None, instance=instance) - claim.save.assert_not_called() + claim.set_status_approved.assert_not_called() mock_logger.info.assert_not_called() @patch("apps.owasp.signals.board_candidate_claim_review.logger") @@ -88,6 +87,5 @@ def test_auto_approves_when_approvals_exactly_equal_threshold(self, mock_logger) claim.reviews.filter.assert_called_once_with( status=BoardCandidateClaimReview.Status.APPROVED, ) - assert claim.status == BoardCandidateClaim.Status.APPROVED - claim.save.assert_called_once() + claim.set_status_approved.assert_called_once() mock_logger.info.assert_called_once() diff --git a/backend/tests/unit/apps/owasp/signals/board_of_directors_test.py b/backend/tests/unit/apps/owasp/signals/board_of_directors_test.py index 3c9175c491..8b18fbd23d 100644 --- a/backend/tests/unit/apps/owasp/signals/board_of_directors_test.py +++ b/backend/tests/unit/apps/owasp/signals/board_of_directors_test.py @@ -32,12 +32,7 @@ def test_approves_claims_when_threshold_met(self, mock_claim_model, mock_logger) board_post_save_re_evaluate_claims(sender=None, instance=instance) - assert claim_a.status == BoardCandidateClaim.Status.APPROVED - assert claim_a.is_locked is True - assert claim_b.status == BoardCandidateClaim.Status.SUBMITTED - mock_claim_model.objects.bulk_update.assert_called_once_with( - [claim_a], ["is_locked", "status"] - ) + mock_claim_model.bulk_set_status_approved.assert_called_once_with([claim_a]) mock_logger.info.assert_called_once_with( "Approved %d claims after threshold change on board %d.", 1, @@ -60,7 +55,7 @@ def test_approves_no_claims_when_threshold_not_met(self, mock_claim_model, mock_ board_post_save_re_evaluate_claims(sender=None, instance=instance) - mock_claim_model.objects.bulk_update.assert_not_called() + mock_claim_model.bulk_set_status_approved.assert_not_called() mock_logger.info.assert_not_called() @patch("apps.owasp.signals.board_of_directors.logger") @@ -83,13 +78,7 @@ def test_approves_all_eligible_claims(self, mock_claim_model, mock_logger): board_post_save_re_evaluate_claims(sender=None, instance=instance) - assert claim_a.status == BoardCandidateClaim.Status.APPROVED - assert claim_a.is_locked is True - assert claim_b.status == BoardCandidateClaim.Status.APPROVED - assert claim_b.is_locked is True - mock_claim_model.objects.bulk_update.assert_called_once_with( - [claim_a, claim_b], ["is_locked", "status"] - ) + mock_claim_model.bulk_set_status_approved.assert_called_once_with([claim_a, claim_b]) mock_logger.info.assert_called_once() @patch("apps.owasp.signals.board_of_directors.BoardCandidateClaim") @@ -103,4 +92,4 @@ def test_runs_when_threshold_updated(self, mock_claim_model): board_post_save_re_evaluate_claims(sender=None, instance=instance) instance.claims.filter.assert_called_once_with(status=BoardCandidateClaim.Status.SUBMITTED) - mock_claim_model.objects.bulk_update.assert_not_called() + mock_claim_model.bulk_set_status_approved.assert_not_called() diff --git a/backend/tests/unit/apps/owasp/utils/file_test.py b/backend/tests/unit/apps/owasp/utils/file_test.py index 478b981162..d0de2c4449 100644 --- a/backend/tests/unit/apps/owasp/utils/file_test.py +++ b/backend/tests/unit/apps/owasp/utils/file_test.py @@ -7,6 +7,7 @@ from django.core.exceptions import ValidationError from django.core.files.uploadedfile import SimpleUploadedFile from PIL import Image +from PIL.ExifTags import Base from pypdf import PdfReader, PdfWriter from apps.owasp.utils.file import ( @@ -14,13 +15,15 @@ IMAGE_EXTENSIONS, PDF_CONTENT_TYPE, PDF_EXTENSION, - _strip_image_metadata, - _strip_pdf_metadata, strip_file_metadata, + strip_image_metadata, + strip_pdf_metadata, ) +EXIF_ORIENTATION_ROTATE_90_CW = 6 -def _create_test_image_simple(fmt="JPEG", ext=".jpg"): + +def create_test_image_simple(fmt="JPEG", ext=".jpg"): """Create a simple test image file without external EXIF libraries.""" image = Image.new("RGB", (10, 10), color="red") output = io.BytesIO() @@ -34,12 +37,12 @@ def _create_test_image_simple(fmt="JPEG", ext=".jpg"): ) -def _create_jpeg_with_exif(): +def create_jpeg_with_exif(): """Create a JPEG with EXIF data embedded via Pillow.""" image = Image.new("RGB", (10, 10), color="blue") output = io.BytesIO() exif = image.getexif() - exif[271] = "TestCamera" + exif[Base.Make] = "TestCamera" image.save(output, format="JPEG", exif=exif) content = output.getvalue() @@ -51,12 +54,12 @@ def _create_jpeg_with_exif(): return file, content -def _create_jpeg_with_orientation(): +def create_jpeg_with_orientation(): """Create a 10x5 JPEG image with EXIF Orientation = 6 (requires 90 CW rotation).""" image = Image.new("RGB", (10, 5), color="blue") output = io.BytesIO() exif = image.getexif() - exif[0x0112] = 6 # Orientation tag + exif[Base.Orientation] = EXIF_ORIENTATION_ROTATE_90_CW image.save(output, format="JPEG", exif=exif) content = output.getvalue() return SimpleUploadedFile( @@ -66,7 +69,7 @@ def _create_jpeg_with_orientation(): ) -def _create_test_pdf(*, with_metadata=True): +def create_test_pdf(*, with_metadata=True): """Create a test PDF file with optional metadata.""" writer = PdfWriter() writer.add_blank_page(width=72, height=72) @@ -92,7 +95,7 @@ def _create_test_pdf(*, with_metadata=True): ) -def _create_test_pdf_with_xmp(): +def create_test_pdf_with_xmp(): """Create a test PDF file containing an XMP /Metadata stream.""" xmp_xml = ( b'' @@ -134,7 +137,7 @@ def test_dispatches_to_image_handler_for_image_extensions(self, ext): mock_file.name = f"test.{ext}" with patch( - "apps.owasp.utils.file._strip_image_metadata", return_value=mock_file + "apps.owasp.utils.file.strip_image_metadata", return_value=mock_file ) as mock_strip: result = strip_file_metadata(mock_file) @@ -147,7 +150,7 @@ def test_dispatches_to_pdf_handler_for_pdf_extensions(self, ext): mock_file.name = f"test.{ext}" with patch( - "apps.owasp.utils.file._strip_pdf_metadata", return_value=mock_file + "apps.owasp.utils.file.strip_pdf_metadata", return_value=mock_file ) as mock_strip: result = strip_file_metadata(mock_file) @@ -168,7 +171,7 @@ def test_handles_uppercase_extension(self): mock_file.name = "test.JPEG" with patch( - "apps.owasp.utils.file._strip_image_metadata", return_value=mock_file + "apps.owasp.utils.file.strip_image_metadata", return_value=mock_file ) as mock_strip: strip_file_metadata(mock_file) @@ -179,7 +182,7 @@ def test_handles_mixed_case_extension(self): mock_file.name = "test.Pdf" with patch( - "apps.owasp.utils.file._strip_pdf_metadata", return_value=mock_file + "apps.owasp.utils.file.strip_pdf_metadata", return_value=mock_file ) as mock_strip: strip_file_metadata(mock_file) @@ -196,7 +199,7 @@ def test_corrupt_file_raises_validation_error(self): class TestStripImageMetadata: - """Tests for _strip_image_metadata.""" + """Tests for strip_image_metadata.""" @pytest.mark.parametrize( ("fmt", "ext"), @@ -208,9 +211,9 @@ class TestStripImageMetadata: ], ) def test_strips_metadata_and_preserves_image_content(self, fmt, ext): - mock_file = _create_test_image_simple(fmt=fmt, ext=ext) + mock_file = create_test_image_simple(fmt=fmt, ext=ext) - result = _strip_image_metadata(mock_file, ext) + result = strip_image_metadata(mock_file, ext) result_image = Image.open(io.BytesIO(result.read())) assert result_image.size == (10, 10) @@ -226,9 +229,9 @@ def test_strips_metadata_and_preserves_image_content(self, fmt, ext): ], ) def test_preserves_file_name(self, fmt, ext): - mock_file = _create_test_image_simple(fmt=fmt, ext=ext) + mock_file = create_test_image_simple(fmt=fmt, ext=ext) - result = _strip_image_metadata(mock_file, ext) + result = strip_image_metadata(mock_file, ext) assert result.name == f"test_image.{ext.lstrip('.')}" @@ -242,25 +245,25 @@ def test_preserves_file_name(self, fmt, ext): ], ) def test_sets_correct_content_type(self, fmt, ext): - mock_file = _create_test_image_simple(fmt=fmt, ext=ext) + mock_file = create_test_image_simple(fmt=fmt, ext=ext) - result = _strip_image_metadata(mock_file, ext) + result = strip_image_metadata(mock_file, ext) assert result.content_type == IMAGE_CONTENT_TYPE_MAP[ext] def test_result_has_valid_size(self): - mock_file = _create_test_image_simple(fmt="JPEG", ext=".jpg") + mock_file = create_test_image_simple(fmt="JPEG", ext=".jpg") - result = _strip_image_metadata(mock_file, ".jpg") + result = strip_image_metadata(mock_file, ".jpg") assert result.size > 0 def test_strips_exif_data_from_jpeg(self): - mock_file, original_bytes = _create_jpeg_with_exif() + mock_file, original_bytes = create_jpeg_with_exif() assert b"TestCamera" in original_bytes - result = _strip_image_metadata(mock_file, ".jpg") + result = strip_image_metadata(mock_file, ".jpg") result_image = Image.open(io.BytesIO(result.read())) assert not result_image.getexif() @@ -272,24 +275,24 @@ def test_corrupt_image_raises_validation_error(self): content_type="image/jpeg", ) with pytest.raises(ValidationError, match="Invalid or corrupt image"): - _strip_image_metadata(mock_file, ".jpg") + strip_image_metadata(mock_file, ".jpg") def test_exif_transpose_preserves_orientation(self): - mock_file = _create_jpeg_with_orientation() - result = _strip_image_metadata(mock_file, ".jpg") + mock_file = create_jpeg_with_orientation() + result = strip_image_metadata(mock_file, ".jpg") result_image = Image.open(io.BytesIO(result.read())) assert result_image.size == (5, 10) - assert result_image.getexif().get(0x0112) is None + assert result_image.getexif().get(Base.Orientation) is None class TestStripPdfMetadata: - """Tests for _strip_pdf_metadata.""" + """Tests for strip_pdf_metadata.""" def test_strips_metadata_from_pdf(self): - mock_file = _create_test_pdf(with_metadata=True) + mock_file = create_test_pdf(with_metadata=True) - result = _strip_pdf_metadata(mock_file) + result = strip_pdf_metadata(mock_file) reader = PdfReader(io.BytesIO(result.read())) metadata = reader.metadata @@ -297,46 +300,46 @@ def test_strips_metadata_from_pdf(self): assert not metadata.get("/Title") def test_preserves_page_count(self): - mock_file = _create_test_pdf(with_metadata=True) + mock_file = create_test_pdf(with_metadata=True) - result = _strip_pdf_metadata(mock_file) + result = strip_pdf_metadata(mock_file) reader = PdfReader(io.BytesIO(result.read())) assert len(reader.pages) == 1 def test_preserves_file_name(self): - mock_file = _create_test_pdf() + mock_file = create_test_pdf() - result = _strip_pdf_metadata(mock_file) + result = strip_pdf_metadata(mock_file) assert result.name == "test_document.pdf" def test_sets_correct_content_type(self): - mock_file = _create_test_pdf() + mock_file = create_test_pdf() - result = _strip_pdf_metadata(mock_file) + result = strip_pdf_metadata(mock_file) assert result.content_type == PDF_CONTENT_TYPE def test_result_has_valid_size(self): - mock_file = _create_test_pdf() + mock_file = create_test_pdf() - result = _strip_pdf_metadata(mock_file) + result = strip_pdf_metadata(mock_file) assert result.size > 0 def test_handles_pdf_without_metadata(self): - mock_file = _create_test_pdf(with_metadata=False) + mock_file = create_test_pdf(with_metadata=False) - result = _strip_pdf_metadata(mock_file) + result = strip_pdf_metadata(mock_file) assert result.size > 0 assert result.name == "test_document.pdf" def test_producer_and_creator_are_cleared(self): - mock_file = _create_test_pdf(with_metadata=True) + mock_file = create_test_pdf(with_metadata=True) - result = _strip_pdf_metadata(mock_file) + result = strip_pdf_metadata(mock_file) reader = PdfReader(io.BytesIO(result.read())) metadata = reader.metadata @@ -344,9 +347,9 @@ def test_producer_and_creator_are_cleared(self): assert not metadata.get("/Creator") def test_strips_xmp_metadata_stream(self): - mock_file = _create_test_pdf_with_xmp() + mock_file = create_test_pdf_with_xmp() - result = _strip_pdf_metadata(mock_file) + result = strip_pdf_metadata(mock_file) cleaned_bytes = result.read() reader = PdfReader(io.BytesIO(cleaned_bytes)) @@ -360,4 +363,4 @@ def test_corrupt_pdf_raises_validation_error(self): content_type="application/pdf", ) with pytest.raises(ValidationError, match="Invalid or corrupt PDF"): - _strip_pdf_metadata(mock_file) + strip_pdf_metadata(mock_file) diff --git a/e2e/pages/BoardCandidateClaimDetails.spec.ts b/e2e/pages/BoardCandidateClaimDetails.spec.ts index f20f5d96e9..a3c938e62a 100644 --- a/e2e/pages/BoardCandidateClaimDetails.spec.ts +++ b/e2e/pages/BoardCandidateClaimDetails.spec.ts @@ -61,12 +61,6 @@ test.describe('Board Candidate Claim Details Page', () => { await expect(page.getByText("Sorry, the claim you're looking for doesn't exist.")).toBeVisible() }) - test('shows access denied when viewing another user profile', async ({ page }) => { - await mockClaimAuth(page, mockData, 'otheruser', ['GetClaimAndEvidences']) - await page.goto(baseUrl) - await expect(page.getByText('Access Denied')).toBeVisible() - }) - test('breadcrumb renders correct segments', async ({ page }) => { await expectBreadCrumbsToBeVisible(page, [ 'Home', diff --git a/e2e/pages/BoardCandidateClaimEvidenceDetails.spec.ts b/e2e/pages/BoardCandidateClaimEvidenceDetails.spec.ts index 11ede2a4a5..6270f34529 100644 --- a/e2e/pages/BoardCandidateClaimEvidenceDetails.spec.ts +++ b/e2e/pages/BoardCandidateClaimEvidenceDetails.spec.ts @@ -71,10 +71,4 @@ test.describe('Board Candidate Claim Evidence Details Page', () => { page.getByText("Sorry, the evidence you're looking for doesn't exist.") ).toBeVisible() }) - - test('shows access denied when viewing another user profile', async ({ page }) => { - await mockClaimAuth(page, mockData, 'otheruser', ['GetClaimAndEvidences']) - await page.goto(baseUrl) - await expect(page.getByText('Access Denied')).toBeVisible() - }) }) diff --git a/frontend/__tests__/unit/components/ClaimActions.test.tsx b/frontend/__tests__/unit/components/ClaimActions.test.tsx index 18f403a2f1..0bbd0d8e7a 100644 --- a/frontend/__tests__/unit/components/ClaimActions.test.tsx +++ b/frontend/__tests__/unit/components/ClaimActions.test.tsx @@ -55,6 +55,7 @@ const renderClaimActions = (claim: Claim) => year="2025" hasReviewed={false} isReviewer={undefined} + isSelf={true} /> ) @@ -66,6 +67,7 @@ const renderAsReviewer = (claim: Claim) => year="2025" hasReviewed={false} isReviewer={true} + isSelf={false} /> ) @@ -162,19 +164,17 @@ describe('ClaimActions', () => { year="2025" hasReviewed={true} isReviewer={true} + isSelf={false} /> ) expect(screen.queryByRole('button', { name: /actions menu/i })).not.toBeInTheDocument() }) - it('shows only edit option for DRAFT when reviewer', () => { + it('hides dropdown for DRAFT when reviewer is not the owner', () => { renderAsReviewer(baseClaim) - fireEvent.click(screen.getByRole('button', { name: /actions menu/i })) - expect(screen.getByText('Edit Claim')).toBeInTheDocument() - expect(screen.queryByText('Submit Claim')).not.toBeInTheDocument() - expect(screen.queryByText('Discard Claim')).not.toBeInTheDocument() + expect(screen.queryByRole('button', { name: /actions menu/i })).not.toBeInTheDocument() }) it('hides dropdown for non-DRAFT non-SUBMITTED statuses when reviewer', () => { @@ -182,6 +182,21 @@ describe('ClaimActions', () => { expect(screen.queryByRole('button', { name: /actions menu/i })).not.toBeInTheDocument() }) + + it('hides dropdown for a non-self public viewer', () => { + render( + + ) + + expect(screen.queryByRole('button', { name: /actions menu/i })).not.toBeInTheDocument() + }) }) describe('submit action', () => { diff --git a/frontend/__tests__/unit/components/EvidenceForm.test.tsx b/frontend/__tests__/unit/components/EvidenceForm.test.tsx index 6d314e3dbb..4959377ac9 100644 --- a/frontend/__tests__/unit/components/EvidenceForm.test.tsx +++ b/frontend/__tests__/unit/components/EvidenceForm.test.tsx @@ -7,8 +7,10 @@ const mockOnSubmit = jest.fn() const TestWrapper = ({ initialData, + initialBackendErrors, }: { initialData?: { name: string; description: string; sourceUrl: string; file: File | null } + initialBackendErrors?: Record }) => { const [formData, setFormData] = useState( initialData ?? { @@ -18,11 +20,16 @@ const TestWrapper = ({ file: null as File | null, } ) + const [backendErrors, setBackendErrors] = useState>( + initialBackendErrors ?? {} + ) return ( { }) }) - describe('GraphQL backend errors', () => { - it('displays and clears backend validation errors on name field', async () => { - const gqlError = { - graphQLErrors: [ - { message: 'Name is required', extensions: { code: 'VALIDATION_ERROR', field: 'name' } }, - ], - } - mockOnSubmit.mockRejectedValue(gqlError) + describe('backend errors prop', () => { + it('displays backend error passed via prop after fields are touched', async () => { + render( + + ) - render() + const submitButton = screen.getByRole('button', { name: /add evidence/i }) + fireEvent.click(submitButton) - const nameInput = screen.getByPlaceholderText('Enter evidence name') - fireEvent.change(nameInput, { target: { value: 'Valid Name' } }) - const descInput = screen.getByPlaceholderText('Enter evidence description') - fireEvent.change(descInput, { target: { value: 'Valid description' } }) - const urlInput = screen.getByPlaceholderText('https://example.com/document.pdf') - fireEvent.change(urlInput, { target: { value: 'https://example.com/doc' } }) + await waitFor(() => { + expect(screen.getByText('Name is required')).toBeInTheDocument() + }) + }) + + it('clears backend error for the field when the user edits it', async () => { + render( + + ) const submitButton = screen.getByRole('button', { name: /add evidence/i }) fireEvent.click(submitButton) @@ -169,6 +193,7 @@ describe('EvidenceForm', () => { expect(screen.getByText('Name is required')).toBeInTheDocument() }) + const nameInput = screen.getByPlaceholderText('Enter evidence name') fireEvent.change(nameInput, { target: { value: 'Updated Name' } }) await waitFor(() => { @@ -176,40 +201,34 @@ describe('EvidenceForm', () => { }) }) - it('clears backend file error when file is changed after validation error', async () => { - const gqlError = { - graphQLErrors: [ - { message: 'File is too large', extensions: { code: 'VALIDATION_ERROR', field: 'file' } }, - ], - } - mockOnSubmit.mockRejectedValue(gqlError) - - render() - - const nameInput = screen.getByPlaceholderText('Enter evidence name') - fireEvent.change(nameInput, { target: { value: 'Valid Name' } }) - const descInput = screen.getByPlaceholderText('Enter evidence description') - fireEvent.change(descInput, { target: { value: 'Valid description' } }) - const urlInput = screen.getByPlaceholderText('https://example.com/document.pdf') - fireEvent.change(urlInput, { target: { value: 'https://example.com/doc' } }) + it('clears sourceUrl backend error when a file is selected', async () => { + render( + + ) const submitButton = screen.getByRole('button', { name: /add evidence/i }) fireEvent.click(submitButton) await waitFor(() => { - expect(mockOnSubmit).toHaveBeenCalled() + expect(screen.getByText('Either a file or source URL is required.')).toBeInTheDocument() }) - mockOnSubmit.mockResolvedValue(undefined) - const fileInput = screen.getByLabelText(/file/i) - const file = new File(['content'], 'test.pdf', { type: 'application/pdf' }) + const file = new File(['dummy content'], 'test.pdf', { type: 'application/pdf' }) fireEvent.change(fileInput, { target: { files: [file] } }) - fireEvent.click(submitButton) - await waitFor(() => { - expect(mockOnSubmit).toHaveBeenCalledTimes(2) + expect( + screen.queryByText('Either a file or source URL is required.') + ).not.toBeInTheDocument() }) }) }) diff --git a/frontend/__tests__/unit/pages/ClaimDetailsPage.test.tsx b/frontend/__tests__/unit/pages/ClaimDetailsPage.test.tsx index 46556d9ed9..26fe3201b4 100644 --- a/frontend/__tests__/unit/pages/ClaimDetailsPage.test.tsx +++ b/frontend/__tests__/unit/pages/ClaimDetailsPage.test.tsx @@ -87,7 +87,7 @@ describe('ClaimDetailsPage', () => { expect(screen.getByTestId('error-title')).toHaveTextContent('Claim Not Found') }) - test('renders access denied when viewing another user profile', () => { + test('renders claim details when viewing another user profile', async () => { mockUseDjangoSession.mockReturnValue({ isSyncing: false, session: { user: { login: 'otheruser' } }, @@ -96,7 +96,10 @@ describe('ClaimDetailsPage', () => { render() - expect(screen.getByText('Access Denied')).toBeInTheDocument() + await waitFor(() => { + expect(screen.getByText('Claim Details')).toBeInTheDocument() + }) + expect(screen.queryByText('Access Denied')).not.toBeInTheDocument() }) test('renders claim name in claim details metadata', async () => { diff --git a/frontend/__tests__/unit/pages/CreateClaimPage.test.tsx b/frontend/__tests__/unit/pages/CreateClaimPage.test.tsx index 66a584b552..d04d9afe2f 100644 --- a/frontend/__tests__/unit/pages/CreateClaimPage.test.tsx +++ b/frontend/__tests__/unit/pages/CreateClaimPage.test.tsx @@ -186,12 +186,17 @@ describe('CreateClaimPage', () => { }) test('shows backend validation errors from GraphQL', async () => { - const gqlError = { - graphQLErrors: [ - { message: 'Name is required', extensions: { code: 'VALIDATION_ERROR', field: 'name' } }, - ], - } - mockCreateFn.mockRejectedValue(gqlError) + mockCreateFn.mockResolvedValue({ + data: { + createBoardCandidateClaim: { + ok: false, + code: 'VALIDATION_ERROR', + message: 'Some fields are invalid.', + fieldErrors: [{ field: 'name', message: 'Name is required' }], + claim: null, + }, + }, + }) render() @@ -213,12 +218,17 @@ describe('CreateClaimPage', () => { }) test('clears backend error when user types after validation error', async () => { - const gqlError = { - graphQLErrors: [ - { message: 'Name is required', extensions: { code: 'VALIDATION_ERROR', field: 'name' } }, - ], - } - mockCreateFn.mockRejectedValue(gqlError) + mockCreateFn.mockResolvedValue({ + data: { + createBoardCandidateClaim: { + ok: false, + code: 'VALIDATION_ERROR', + message: 'Some fields are invalid.', + fieldErrors: [{ field: 'name', message: 'Name is required' }], + claim: null, + }, + }, + }) render() diff --git a/frontend/__tests__/unit/pages/CreateEvidencePage.test.tsx b/frontend/__tests__/unit/pages/CreateEvidencePage.test.tsx index e8682db348..824f206614 100644 --- a/frontend/__tests__/unit/pages/CreateEvidencePage.test.tsx +++ b/frontend/__tests__/unit/pages/CreateEvidencePage.test.tsx @@ -173,15 +173,17 @@ describe('CreateEvidencePage', () => { }) test('shows backend validation errors from GraphQL', async () => { - const gqlError = { - graphQLErrors: [ - { - message: 'Name is required', - extensions: { code: 'VALIDATION_ERROR', field: 'name' }, + mockCreateFn.mockResolvedValue({ + data: { + createBoardCandidateClaimEvidence: { + ok: false, + code: 'VALIDATION_ERROR', + message: 'Some fields are invalid.', + fieldErrors: [{ field: 'name', message: 'Name is required' }], + evidence: null, }, - ], - } - mockCreateFn.mockRejectedValue(gqlError) + }, + }) render() @@ -210,15 +212,17 @@ describe('CreateEvidencePage', () => { }) test('clears backend error when user types after validation error', async () => { - const gqlError = { - graphQLErrors: [ - { - message: 'Name is required', - extensions: { code: 'VALIDATION_ERROR', field: 'name' }, + mockCreateFn.mockResolvedValue({ + data: { + createBoardCandidateClaimEvidence: { + ok: false, + code: 'VALIDATION_ERROR', + message: 'Some fields are invalid.', + fieldErrors: [{ field: 'name', message: 'Name is required' }], + evidence: null, }, - ], - } - mockCreateFn.mockRejectedValue(gqlError) + }, + }) render() diff --git a/frontend/__tests__/unit/pages/EditClaimPage.test.tsx b/frontend/__tests__/unit/pages/EditClaimPage.test.tsx index 4f67fe2c27..071273a69b 100644 --- a/frontend/__tests__/unit/pages/EditClaimPage.test.tsx +++ b/frontend/__tests__/unit/pages/EditClaimPage.test.tsx @@ -196,12 +196,17 @@ describe('EditClaimPage', () => { }) test('shows backend validation errors from GraphQL', async () => { - const gqlError = { - graphQLErrors: [ - { message: 'Name is required', extensions: { code: 'VALIDATION_ERROR', field: 'name' } }, - ], - } - mockUpdateFn.mockRejectedValue(gqlError) + mockUpdateFn.mockResolvedValue({ + data: { + updateBoardCandidateClaim: { + ok: false, + code: 'VALIDATION_ERROR', + message: 'Some fields are invalid.', + fieldErrors: [{ field: 'name', message: 'Name is required' }], + claim: null, + }, + }, + }) render() @@ -221,12 +226,17 @@ describe('EditClaimPage', () => { }) test('clears backend error when user types after validation error', async () => { - const gqlError = { - graphQLErrors: [ - { message: 'Name is required', extensions: { code: 'VALIDATION_ERROR', field: 'name' } }, - ], - } - mockUpdateFn.mockRejectedValue(gqlError) + mockUpdateFn.mockResolvedValue({ + data: { + updateBoardCandidateClaim: { + ok: false, + code: 'VALIDATION_ERROR', + message: 'Some fields are invalid.', + fieldErrors: [{ field: 'name', message: 'Name is required' }], + claim: null, + }, + }, + }) render() diff --git a/frontend/__tests__/unit/pages/EditEvidencePage.test.tsx b/frontend/__tests__/unit/pages/EditEvidencePage.test.tsx index 78da298677..920edde30a 100644 --- a/frontend/__tests__/unit/pages/EditEvidencePage.test.tsx +++ b/frontend/__tests__/unit/pages/EditEvidencePage.test.tsx @@ -202,15 +202,17 @@ describe('EditEvidencePage', () => { }) test('shows backend validation errors from GraphQL', async () => { - const gqlError = { - graphQLErrors: [ - { - message: 'Name is required', - extensions: { code: 'VALIDATION_ERROR', field: 'name' }, + mockUpdateFn.mockResolvedValue({ + data: { + updateBoardCandidateClaimEvidence: { + ok: false, + code: 'VALIDATION_ERROR', + message: 'Some fields are invalid.', + fieldErrors: [{ field: 'name', message: 'Name is required' }], + evidence: null, }, - ], - } - mockUpdateFn.mockRejectedValue(gqlError) + }, + }) render() @@ -230,15 +232,17 @@ describe('EditEvidencePage', () => { }) test('clears backend error when user types after validation error', async () => { - const gqlError = { - graphQLErrors: [ - { - message: 'Name is required', - extensions: { code: 'VALIDATION_ERROR', field: 'name' }, + mockUpdateFn.mockResolvedValue({ + data: { + updateBoardCandidateClaimEvidence: { + ok: false, + code: 'VALIDATION_ERROR', + message: 'Some fields are invalid.', + fieldErrors: [{ field: 'name', message: 'Name is required' }], + evidence: null, }, - ], - } - mockUpdateFn.mockRejectedValue(gqlError) + }, + }) render() diff --git a/frontend/__tests__/unit/pages/EvidenceDetailsPage.test.tsx b/frontend/__tests__/unit/pages/EvidenceDetailsPage.test.tsx index fd4c250a99..807fbc6987 100644 --- a/frontend/__tests__/unit/pages/EvidenceDetailsPage.test.tsx +++ b/frontend/__tests__/unit/pages/EvidenceDetailsPage.test.tsx @@ -154,7 +154,7 @@ describe('EvidenceDetailsPage', () => { expect(screen.getByTestId('error-title')).toHaveTextContent('Evidence Not Found') }) - test('renders access denied when viewing another user profile', () => { + test('renders evidence details when viewing another user profile', async () => { mockUseDjangoSession.mockReturnValue({ isSyncing: false, session: { user: { login: 'otheruser' } }, @@ -163,7 +163,10 @@ describe('EvidenceDetailsPage', () => { render() - expect(screen.getByText('Access Denied')).toBeInTheDocument() + await waitFor(() => { + expect(screen.getByText('Evidence Details')).toBeInTheDocument() + }) + expect(screen.queryByText('Access Denied')).not.toBeInTheDocument() }) test('renders evidence details', async () => { diff --git a/frontend/__tests__/unit/utils/handleGraphQLError.test.ts b/frontend/__tests__/unit/utils/handleGraphQLError.test.ts index 5228524779..9bc8f954dd 100644 --- a/frontend/__tests__/unit/utils/handleGraphQLError.test.ts +++ b/frontend/__tests__/unit/utils/handleGraphQLError.test.ts @@ -1,9 +1,15 @@ +import { addToast } from '@heroui/toast' import { extractGraphQLErrors, + handleMutationPayloadErrors, isForbiddenGraphQLError, isAccessDeniedGraphQLError, } from 'utils/helpers/handleGraphQLError' +jest.mock('@heroui/toast', () => ({ + addToast: jest.fn(), +})) + describe('extractGraphQLErrors', () => { describe('ApolloError-like errors with graphQLErrors', () => { it('extracts validation errors when code is VALIDATION_ERROR and field is present', () => { @@ -253,6 +259,92 @@ describe('extractGraphQLErrors', () => { }) }) +describe('handleMutationPayloadErrors', () => { + const setBackendErrors = jest.fn() + + beforeEach(() => { + jest.clearAllMocks() + }) + + it('returns true and does nothing when payload is ok', () => { + const result = handleMutationPayloadErrors({ ok: true }, 'Failed.', setBackendErrors) + + expect(result).toBe(true) + expect(setBackendErrors).not.toHaveBeenCalled() + expect(addToast).not.toHaveBeenCalled() + }) + + it('maps fieldErrors to backendErrors when present', () => { + const payload = { + ok: false, + fieldErrors: [ + { field: 'name', message: 'Name is required.' }, + { field: 'sourceUrl', message: 'Invalid URL.' }, + ], + } + + const result = handleMutationPayloadErrors(payload, 'Failed.', setBackendErrors) + + expect(result).toBe(false) + expect(setBackendErrors).toHaveBeenCalledWith({ + name: 'Name is required.', + sourceUrl: 'Invalid URL.', + }) + expect(addToast).not.toHaveBeenCalled() + }) + + it('shows a toast with payload.message when no fieldErrors', () => { + const result = handleMutationPayloadErrors( + { ok: false, message: 'Server rejected the request.' }, + 'Fallback.', + setBackendErrors + ) + + expect(result).toBe(false) + expect(setBackendErrors).not.toHaveBeenCalled() + expect(addToast).toHaveBeenCalledWith( + expect.objectContaining({ description: 'Server rejected the request.', color: 'danger' }) + ) + }) + + it('falls back to the provided fallback message when payload.message is missing', () => { + handleMutationPayloadErrors({ ok: false }, 'Fallback used.', setBackendErrors) + + expect(addToast).toHaveBeenCalledWith( + expect.objectContaining({ description: 'Fallback used.', color: 'danger' }) + ) + }) + + it('falls back to the provided fallback message when payload is null', () => { + handleMutationPayloadErrors(null, 'Null fallback.', setBackendErrors) + + expect(addToast).toHaveBeenCalledWith( + expect.objectContaining({ description: 'Null fallback.' }) + ) + }) + + it('falls back to the provided fallback message when payload is undefined', () => { + handleMutationPayloadErrors(undefined, 'Undefined fallback.', setBackendErrors) + + expect(addToast).toHaveBeenCalledWith( + expect.objectContaining({ description: 'Undefined fallback.' }) + ) + }) + + it('shows a toast when fieldErrors is an empty array', () => { + handleMutationPayloadErrors( + { ok: false, fieldErrors: [], message: 'Nothing to map.' }, + 'Fallback.', + setBackendErrors + ) + + expect(setBackendErrors).not.toHaveBeenCalled() + expect(addToast).toHaveBeenCalledWith( + expect.objectContaining({ description: 'Nothing to map.' }) + ) + }) +}) + describe('isForbiddenGraphQLError', () => { it('returns true when graphQLErrors includes FORBIDDEN', () => { const error = { diff --git a/frontend/src/app/board/[year]/candidates/[login]/claims/[claimKey]/edit/page.tsx b/frontend/src/app/board/[year]/candidates/[login]/claims/[claimKey]/edit/page.tsx index c95ce1207c..0d4dc87361 100644 --- a/frontend/src/app/board/[year]/candidates/[login]/claims/[claimKey]/edit/page.tsx +++ b/frontend/src/app/board/[year]/candidates/[login]/claims/[claimKey]/edit/page.tsx @@ -7,7 +7,7 @@ import React, { useEffect, useState } from 'react' import { ErrorDisplay, handleAppError } from 'app/global-error' import { UpdateBoardCandidateClaimDocument } from 'types/__generated__/claimMutations.generated' import { GetBoardCandidateClaimDocument } from 'types/__generated__/claimQueries.generated' -import { extractGraphQLErrors } from 'utils/helpers/handleGraphQLError' +import { handleMutationPayloadErrors } from 'utils/helpers/handleGraphQLError' import AccessDeniedDisplay from 'components/AccessDeniedDisplay' import ClaimForm from 'components/ClaimForm' import LoadingSpinner from 'components/LoadingSpinner' @@ -31,6 +31,7 @@ const EditClaimPage = () => { description: '', name: '', }) + const [backendErrors, setBackendErrors] = useState>({}) useEffect(() => { if (graphQLRequestError) { @@ -80,14 +81,14 @@ const EditClaimPage = () => { const handleSubmit = async (e: React.FormEvent) => { e.preventDefault() - try { - const input = { - description: formData.description, - key: claimKey, - name: formData.name, - year: Number.parseInt(year), - } + const input = { + description: formData.description, + key: claimKey, + name: formData.name, + year: Number.parseInt(year), + } + try { const result = await updateClaim({ variables: { input }, update(cache, { data }) { @@ -101,8 +102,9 @@ const EditClaimPage = () => { }, }) - if (!result.data?.updateBoardCandidateClaim?.ok) { - throw new Error(result.data?.updateBoardCandidateClaim?.message ?? 'Claim update failed.') + const payload = result.data?.updateBoardCandidateClaim + if (!handleMutationPayloadErrors(payload, 'Claim update failed.', setBackendErrors)) { + return } addToast({ @@ -113,22 +115,17 @@ const EditClaimPage = () => { color: 'success', }) - const updatedClaim = result.data?.updateBoardCandidateClaim?.claim + const updatedClaim = payload.claim if (updatedClaim?.key) { router.push(`/board/${year}/candidates/${login}/claims/${updatedClaim.key}`) } - } catch (err) { - const { hasValidationErrors } = extractGraphQLErrors(err) - if (!hasValidationErrors) { - addToast({ - description: - err instanceof Error ? err.message : 'Unable to complete the requested operation.', - timeout: 3000, - shouldShowTimeoutProgress: true, - color: 'danger', - }) - } - throw err + } catch (error) { + addToast({ + description: error instanceof Error ? error.message : 'Claim update failed.', + timeout: 3000, + shouldShowTimeoutProgress: true, + color: 'danger', + }) } } @@ -136,6 +133,8 @@ const EditClaimPage = () => { { file: null as File | null, sourceUrl: '', }) + const [backendErrors, setBackendErrors] = useState>({}) useEffect(() => { if (graphQLRequestError) { @@ -89,17 +90,17 @@ const EditEvidencePage = () => { const handleSubmit = async (e: React.FormEvent) => { e.preventDefault() - try { - const input = { - claimKey: claimKey, - description: formData.description, - file: formData.file || null, - key: evidenceKey, - name: formData.name, - sourceUrl: formData.sourceUrl, - year: Number.parseInt(year), - } + const input = { + claimKey: claimKey, + description: formData.description, + file: formData.file || null, + key: evidenceKey, + name: formData.name, + sourceUrl: formData.sourceUrl.trim() || null, + year: Number.parseInt(year), + } + try { const result = await updateEvidence({ variables: { input }, update(cache, { data }) { @@ -113,10 +114,9 @@ const EditEvidencePage = () => { }, }) - if (!result.data?.updateBoardCandidateClaimEvidence?.ok) { - throw new Error( - result.data?.updateBoardCandidateClaimEvidence?.message ?? 'Evidence update failed.' - ) + const payload = result.data?.updateBoardCandidateClaimEvidence + if (!handleMutationPayloadErrors(payload, 'Evidence update failed.', setBackendErrors)) { + return } addToast({ @@ -127,24 +127,19 @@ const EditEvidencePage = () => { color: 'success', }) - const updatedEvidence = result.data?.updateBoardCandidateClaimEvidence?.evidence + const updatedEvidence = payload.evidence if (updatedEvidence?.key) { router.push( `/board/${year}/candidates/${login}/claims/${claimKey}/evidences/${updatedEvidence.key}` ) } - } catch (err) { - const { hasValidationErrors } = extractGraphQLErrors(err) - if (!hasValidationErrors) { - addToast({ - description: - err instanceof Error ? err.message : 'Unable to complete the requested operation.', - timeout: 3000, - shouldShowTimeoutProgress: true, - color: 'danger', - }) - } - throw err + } catch (error) { + addToast({ + description: error instanceof Error ? error.message : 'Evidence update failed.', + timeout: 3000, + shouldShowTimeoutProgress: true, + color: 'danger', + }) } } @@ -152,6 +147,8 @@ const EditEvidencePage = () => { { const { isSyncing, session } = useDjangoSession() const { data, loading, error } = useQuery(GetClaimAndEvidencesDocument, { fetchPolicy: 'cache-and-network', - skip: isSyncing || !claimKey || !login || !year || !session?.user?.login, + skip: isSyncing || !claimKey || !login || !year, variables: { key: claimKey, login, @@ -39,7 +38,7 @@ const EvidenceDetailsPage = () => { }, }) - const isReviewer = data?.boardOfDirectors?.reviewer != null + const isSelf = session?.user?.login === login const [fetchFileUrl] = useLazyQuery(GetBoardCandidateClaimEvidenceFileUrlDocument) const claim = data?.boardCandidateClaim @@ -54,12 +53,6 @@ const EvidenceDetailsPage = () => { if (loading || isSyncing) return - if (session?.user?.login !== login && !isReviewer) { - return ( - - ) - } - if (error) { return ( { {'Download Evidence'} )} - {!isReviewer && ( + {isSelf && ( { file: null as File | null, sourceUrl: '', }) + const [backendErrors, setBackendErrors] = useState>({}) if (isSyncing) { return @@ -42,16 +43,16 @@ const CreateEvidencePage = () => { const handleSubmit = async (e: React.FormEvent) => { e.preventDefault() - try { - const input = { - claimKey: claimKey, - description: formData.description, - file: formData.file, - name: formData.name, - sourceUrl: formData.sourceUrl, - year: Number.parseInt(year), - } + const input = { + claimKey: claimKey, + description: formData.description, + file: formData.file, + name: formData.name, + sourceUrl: formData.sourceUrl.trim() || null, + year: Number.parseInt(year), + } + try { const result = await createEvidence({ variables: { input }, update(cache, { data }) { @@ -87,10 +88,9 @@ const CreateEvidencePage = () => { }, }) - if (!result.data?.createBoardCandidateClaimEvidence?.ok) { - throw new Error( - result.data?.createBoardCandidateClaimEvidence?.message ?? 'Evidence creation failed.' - ) + const payload = result.data?.createBoardCandidateClaimEvidence + if (!handleMutationPayloadErrors(payload, 'Evidence creation failed.', setBackendErrors)) { + return } addToast({ @@ -102,18 +102,13 @@ const CreateEvidencePage = () => { }) router.push(`/board/${year}/candidates/${login}/claims/${claimKey}`) - } catch (err) { - const { hasValidationErrors } = extractGraphQLErrors(err) - if (!hasValidationErrors) { - addToast({ - description: - err instanceof Error ? err.message : 'Unable to complete the requested operation.', - timeout: 3000, - shouldShowTimeoutProgress: true, - color: 'danger', - }) - } - throw err + } catch (error) { + addToast({ + description: error instanceof Error ? error.message : 'Evidence creation failed.', + timeout: 3000, + shouldShowTimeoutProgress: true, + color: 'danger', + }) } } @@ -121,6 +116,8 @@ const CreateEvidencePage = () => { { error: graphQLRequestError, } = useQuery(GetClaimAndEvidencesDocument, { fetchPolicy: 'cache-and-network', - skip: isSyncing || !claimKey || !year || !session?.user?.login, + skip: isSyncing || !claimKey || !year, variables: { key: claimKey, login, @@ -43,6 +42,7 @@ const ClaimDetailsPage = () => { }) const isReviewer = graphQLData?.boardOfDirectors?.reviewer != null + const isSelf = session?.user?.login === login const claim = graphQLData?.boardCandidateClaim const evidences = graphQLData?.boardCandidateClaimEvidences ?? [] const hasReviewed = @@ -65,12 +65,6 @@ const ClaimDetailsPage = () => { if (isLoading || isSyncing) return - if (session?.user?.login !== login && !isReviewer) { - return ( - - ) - } - if (graphQLRequestError) { return ( {

@{login}

- {claim.status === ClaimStatusEnum.Draft && session?.user?.login === login && ( + {claim.status === ClaimStatusEnum.Draft && isSelf && ( {'Add Evidence'} @@ -123,6 +117,7 @@ const ClaimDetailsPage = () => { claim={claim} hasReviewed={hasReviewed} isReviewer={isReviewer} + isSelf={isSelf} login={login} year={year} /> diff --git a/frontend/src/app/board/[year]/candidates/[login]/claims/create/page.tsx b/frontend/src/app/board/[year]/candidates/[login]/claims/create/page.tsx index f627c1d32b..c9e45eef38 100644 --- a/frontend/src/app/board/[year]/candidates/[login]/claims/create/page.tsx +++ b/frontend/src/app/board/[year]/candidates/[login]/claims/create/page.tsx @@ -9,7 +9,7 @@ import { ErrorDisplay, handleAppError } from 'app/global-error' import { GetBoardCandidateDocument } from 'types/__generated__/boardQueries.generated' import { CreateBoardCandidateClaimDocument } from 'types/__generated__/claimMutations.generated' import { GetBoardCandidateClaimsDocument } from 'types/__generated__/claimQueries.generated' -import { extractGraphQLErrors } from 'utils/helpers/handleGraphQLError' +import { handleMutationPayloadErrors } from 'utils/helpers/handleGraphQLError' import AccessDeniedDisplay from 'components/AccessDeniedDisplay' import ClaimForm from 'components/ClaimForm' import LoadingSpinner from 'components/LoadingSpinner' @@ -25,6 +25,7 @@ const CreateClaimPage = () => { description: '', name: '', }) + const [backendErrors, setBackendErrors] = useState>({}) const { data: candidateGraphQLData, @@ -69,13 +70,13 @@ const CreateClaimPage = () => { const handleSubmit = async (e: React.FormEvent) => { e.preventDefault() - try { - const input = { - description: formData.description, - name: formData.name, - year: Number.parseInt(year), - } + const input = { + description: formData.description, + name: formData.name, + year: Number.parseInt(year), + } + try { const result = await createClaim({ variables: { input }, update(cache, { data }) { @@ -95,8 +96,9 @@ const CreateClaimPage = () => { }, }) - if (!result.data?.createBoardCandidateClaim?.ok) { - throw new Error(result.data?.createBoardCandidateClaim?.message ?? 'Claim creation failed.') + const payload = result.data?.createBoardCandidateClaim + if (!handleMutationPayloadErrors(payload, 'Claim creation failed.', setBackendErrors)) { + return } addToast({ @@ -108,18 +110,13 @@ const CreateClaimPage = () => { }) router.push(`/board/${year}/candidates/${login}/claims`) - } catch (err) { - const { hasValidationErrors } = extractGraphQLErrors(err) - if (!hasValidationErrors) { - addToast({ - description: - err instanceof Error ? err.message : 'Unable to complete the requested operation.', - timeout: 3000, - shouldShowTimeoutProgress: true, - color: 'danger', - }) - } - throw err + } catch (error) { + addToast({ + description: error instanceof Error ? error.message : 'Claim creation failed.', + timeout: 3000, + shouldShowTimeoutProgress: true, + color: 'danger', + }) } } @@ -127,6 +124,8 @@ const CreateClaimPage = () => { return (