From bacf7d7030533ee79bce2c57397fb362dda8e930 Mon Sep 17 00:00:00 2001 From: Marco Acierno Date: Sun, 7 Jan 2024 21:55:42 +0100 Subject: [PATCH] Fix crash when sharing submission pages --- backend/api/submissions/schema.py | 12 +++- .../api/submissions/tests/test_submission.py | 59 +++++++++++++++++-- .../api/submissions/tests/test_submissions.py | 11 ---- backend/conftest.py | 11 ++++ backend/schedule/tests/factories.py | 8 ++- frontend/src/pages/submission/[id]/index.tsx | 13 +++- 6 files changed, 96 insertions(+), 18 deletions(-) diff --git a/backend/api/submissions/schema.py b/backend/api/submissions/schema.py index e71c598a43..050c5e6f1f 100644 --- a/backend/api/submissions/schema.py +++ b/backend/api/submissions/schema.py @@ -1,5 +1,6 @@ import random import typing +from api.submissions.permissions import CanSeeSubmissionRestrictedFields import strawberry @@ -20,9 +21,18 @@ class SubmissionsQuery: @strawberry.field def submission(self, info, id: strawberry.ID) -> typing.Optional[Submission]: try: - return SubmissionModel.objects.get_by_hashid(id) + submission = SubmissionModel.objects.get_by_hashid(id) except SubmissionModel.DoesNotExist: return None + except IndexError: + return None + + if not CanSeeSubmissionRestrictedFields().has_permission( + source=submission, info=info + ): + return None + + return submission @strawberry.field(permission_classes=[IsAuthenticated]) def submissions( diff --git a/backend/api/submissions/tests/test_submission.py b/backend/api/submissions/tests/test_submission.py index 71b5966f62..d2b4bbc1ac 100644 --- a/backend/api/submissions/tests/test_submission.py +++ b/backend/api/submissions/tests/test_submission.py @@ -1,9 +1,11 @@ from pytest import mark from api.helpers.ids import encode_hashid +from schedule.tests.factories import ScheduleItemFactory + +pytestmark = mark.django_db -@mark.django_db def test_returns_none_when_missing(graphql_client): resp = graphql_client.query( """query SubmissionQuery($id: ID!) { @@ -18,6 +20,20 @@ def test_returns_none_when_missing(graphql_client): assert resp["data"]["submission"] is None +def test_returns_none_with_invalid_id_string(graphql_client): + resp = graphql_client.query( + """query SubmissionQuery($id: ID!) { + submission(id: $id) { + id + } + }""", + variables={"id": "invalid"}, + ) + + assert not resp.get("errors") + assert resp["data"]["submission"] is None + + def test_returns_correct_submission(graphql_client, user, submission_factory): graphql_client.force_login(user) submission = submission_factory(speaker_id=user.id) @@ -35,7 +51,6 @@ def test_returns_correct_submission(graphql_client, user, submission_factory): assert resp["data"]["submission"]["id"] == submission.hashid -@mark.django_db def test_user_can_edit_submission_if_within_cfp_time_and_is_the_owner( graphql_client, user, submission_factory ): @@ -57,12 +72,12 @@ def test_user_can_edit_submission_if_within_cfp_time_and_is_the_owner( assert response["data"]["submission"]["canEdit"] is True -@mark.django_db def test_cannot_edit_submission_if_not_the_owner( graphql_client, user, submission_factory ): graphql_client.force_login(user) submission = submission_factory(conference__active_cfp=True) + ScheduleItemFactory(submission=submission) response = graphql_client.query( """query Submission($id: ID!) { @@ -77,7 +92,6 @@ def test_cannot_edit_submission_if_not_the_owner( assert response["data"]["submission"] == {"id": submission.hashid, "canEdit": False} -@mark.django_db def test_can_edit_submission_if_cfp_is_closed(graphql_client, user, submission_factory): graphql_client.force_login(user) submission = submission_factory(speaker_id=user.id, conference__active_cfp=False) @@ -95,3 +109,40 @@ def test_can_edit_submission_if_cfp_is_closed(graphql_client, user, submission_f ) assert response["data"]["submission"]["canEdit"] is True + + +def test_cannot_see_submissions_if_restricted(graphql_client, user, submission_factory): + graphql_client.force_login(user) + submission = submission_factory(conference__active_cfp=True) + + response = graphql_client.query( + """query Submission($id: ID!) { + submission(id: $id) { + id + } + }""", + variables={"id": submission.hashid}, + ) + + assert response["data"]["submission"] is None + + +def test_can_see_submissions_while_voting_with_ticket( + graphql_client, user, submission_factory, mock_has_ticket +): + graphql_client.force_login(user) + submission = submission_factory( + conference__active_cfp=False, conference__active_voting=True + ) + mock_has_ticket(submission.conference) + + response = graphql_client.query( + """query Submission($id: ID!) { + submission(id: $id) { + id + } + }""", + variables={"id": submission.hashid}, + ) + + assert response["data"]["submission"]["id"] == submission.hashid diff --git a/backend/api/submissions/tests/test_submissions.py b/backend/api/submissions/tests/test_submissions.py index 79b6e6770c..2aad20f839 100644 --- a/backend/api/submissions/tests/test_submissions.py +++ b/backend/api/submissions/tests/test_submissions.py @@ -3,17 +3,6 @@ pytestmark = pytest.mark.django_db -@pytest.fixture -def mock_has_ticket(requests_mock, settings): - def wrapper(conference): - requests_mock.post( - f"{settings.PRETIX_API}organizers/{conference.pretix_organizer_id}/events/{conference.pretix_event_id}/tickets/attendee-has-ticket/", - json={"user_has_admission_ticket": True}, - ) - - return wrapper - - def test_returns_submissions_paginated(graphql_client, user, submission_factory): graphql_client.force_login(user) diff --git a/backend/conftest.py b/backend/conftest.py index cd09afd502..10feead201 100644 --- a/backend/conftest.py +++ b/backend/conftest.py @@ -118,3 +118,14 @@ def wrapper(filename: str = "test.jpg"): @pytest.fixture def locale(): return lambda code: Locale.objects.get_or_create(language_code=code)[0] + + +@pytest.fixture +def mock_has_ticket(requests_mock, settings): + def wrapper(conference): + requests_mock.post( + f"{settings.PRETIX_API}organizers/{conference.pretix_organizer_id}/events/{conference.pretix_event_id}/tickets/attendee-has-ticket/", + json={"user_has_admission_ticket": True}, + ) + + return wrapper diff --git a/backend/schedule/tests/factories.py b/backend/schedule/tests/factories.py index 966b904660..461dd76ae8 100644 --- a/backend/schedule/tests/factories.py +++ b/backend/schedule/tests/factories.py @@ -54,8 +54,14 @@ class ScheduleItemFactory(DjangoModelFactory): @classmethod def _create(cls, model_class, *args, **kwargs): - _type = kwargs.get("type", None) + if "submission" in kwargs and "type" not in kwargs: + kwargs["type"] = ( + ScheduleItem.TYPES.talk + if kwargs["submission"].type.name == "talk" + else ScheduleItem.TYPES.training + ) + _type = kwargs.get("type", None) if _type == ScheduleItem.TYPES.custom: kwargs.pop("submission", None) diff --git a/frontend/src/pages/submission/[id]/index.tsx b/frontend/src/pages/submission/[id]/index.tsx index 024df77257..4f15af2340 100644 --- a/frontend/src/pages/submission/[id]/index.tsx +++ b/frontend/src/pages/submission/[id]/index.tsx @@ -15,6 +15,7 @@ import { createHref } from "~/components/link"; import { ScheduleEventDetail } from "~/components/schedule-event-detail"; import { prefetchSharedQueries } from "~/helpers/prefetch"; import { useCurrentLanguage } from "~/locale/context"; +import NotFoundPage from "~/pages/404"; import { getType } from "~/pages/event/[slug]"; import { queryIsVotingClosed, @@ -54,6 +55,10 @@ export const SubmissionPage = () => { const otherLanguage = viewInLanguage === "it" ? "en" : "it"; + if (!italianSubmission && !englishSubmission) { + return ; + } + return ( { const client = getApolloClient(null, req.cookies); - await Promise.all([ + const [_, englishSubmission, italianSubmission] = await Promise.all([ prefetchSharedQueries(client, locale), queryIsVotingClosed(client, { conference: process.env.conferenceCode, @@ -155,6 +160,12 @@ export const getServerSideProps: GetServerSideProps = async ({ }), ]); + if (!englishSubmission && !italianSubmission) { + return { + notFound: true, + }; + } + return addApolloState( client, {