Fix Submission permission model and crash on submission page (#3672)

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(

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

backend/api/submissions/tests/test_submission_fields.py → backend/api/submissions/tests/test_submission_permissions.py

@@ -67,32 +67,13 @@ def test_voting_open_and_user_cannot_vote(
 ):
     submission = _submission(submission_factory, user)
     graphql_client.force_login(other_user)
-    can_vote_mock = mocker.patch(
+    mocker.patch(
         "api.submissions.permissions.check_if_user_can_vote", return_value=False
     )
 
     data = _query(graphql_client, submission)
 
-    # ✔️ public
-    assert data["submission"]["title"] == submission.title.localize("en")
-    assert data["submission"]["slug"] == submission.slug
-
-    # ❌ restricted
-    assert data["submission"]["elevatorPitch"] is None
-    assert data["submission"]["abstract"] is None
-    assert data["submission"]["topic"] is None
-    assert data["submission"]["type"] is None
-    assert data["submission"]["duration"] is None
-    assert data["submission"]["audienceLevel"] is None
-    assert data["submission"]["languages"] is None
-    assert data["submission"]["tags"] is None
-
-    # ❌ private
-    assert data["submission"]["speakerLevel"] is None
-    assert data["submission"]["previousTalkVideo"] is None
-    assert data["submission"]["notes"] is None
-
-    can_vote_mock.assert_called()
+    assert data["submission"] is None
 
 
 def test_voting_open_and_user_can_vote(
@@ -106,11 +87,8 @@ def test_voting_open_and_user_can_vote(
 
     data = _query(graphql_client, submission)
 
-    # ✔️ public
     assert data["submission"]["title"] == submission.title.localize("en")
     assert data["submission"]["slug"] == submission.slug
-
-    # ✔️ restricted
     assert data["submission"]["elevatorPitch"] == submission.elevator_pitch.localize(
         "en"
     )
@@ -136,59 +114,25 @@ def test_voting_open_and_user_can_vote(
     can_vote_mock.assert_called()
 
 
-def test_voring_closed_and_user_is_authenticated(
+def test_voting_closed_and_user_is_authenticated(
     graphql_client, other_user, submission_factory, user
 ):
     submission = _submission(submission_factory, user, conference__active_voting=False)
     graphql_client.force_login(other_user)
 
     data = _query(graphql_client, submission)
 
-    # ✔️ public
-    assert data["submission"]["title"] == submission.title.localize("en")
-    assert data["submission"]["slug"] == submission.slug
+    assert data["submission"] is None
 
-    # ❌ restricted
-    assert data["submission"]["elevatorPitch"] is None
-    assert data["submission"]["abstract"] is None
-    assert data["submission"]["topic"] is None
-    assert data["submission"]["type"] is None
-    assert data["submission"]["duration"] is None
-    assert data["submission"]["audienceLevel"] is None
-    assert data["submission"]["languages"] is None
-    assert data["submission"]["tags"] is None
 
-    # ❌ private
-    assert data["submission"]["speakerLevel"] is None
-    assert data["submission"]["previousTalkVideo"] is None
-    assert data["submission"]["notes"] is None
-
-
-def test_voring_closed_and_user_is_not_authenticated(
+def test_voting_closed_and_user_is_not_authenticated(
     graphql_client, submission_factory, user
 ):
     submission = _submission(submission_factory, user, conference__active_voting=False)
 
     data = _query(graphql_client, submission)
 
-    # ✔️ public
-    assert data["submission"]["title"] == submission.title.localize("en")
-    assert data["submission"]["slug"] == submission.slug
-
-    # ❌ restricted
-    assert data["submission"]["elevatorPitch"] is None
-    assert data["submission"]["abstract"] is None
-    assert data["submission"]["topic"] is None
-    assert data["submission"]["type"] is None
-    assert data["submission"]["duration"] is None
-    assert data["submission"]["audienceLevel"] is None
-    assert data["submission"]["languages"] is None
-    assert data["submission"]["tags"] is None
-
-    # ❌ private
-    assert data["submission"]["speakerLevel"] is None
-    assert data["submission"]["previousTalkVideo"] is None
-    assert data["submission"]["notes"] is None
+    assert data["submission"] is None
 
 
 def test_accepted_submission_user_can_see_public_and_restricted_fields(
@@ -203,11 +147,8 @@ def test_accepted_submission_user_can_see_public_and_restricted_fields(
 
     data = _query(graphql_client, submission)
 
-    # ✔️ public
     assert data["submission"]["title"] == submission.title.localize("en")
     assert data["submission"]["slug"] == submission.slug
-
-    # ✔️ restricted
     assert data["submission"]["elevatorPitch"] == submission.elevator_pitch.localize(
         "en"
     )
@@ -239,11 +180,9 @@ def test_admin_user_can_see_everything(
 
     data = _query(graphql_client, submission)
 
-    # ✔️ public
     assert data["submission"]["title"] == submission.title.localize("en")
     assert data["submission"]["slug"] == submission.slug
 
-    # ✔️ restricted
     assert data["submission"]["elevatorPitch"] == submission.elevator_pitch.localize(
         "en"
     )
@@ -273,11 +212,9 @@ def test_submission_author_can_see_everything(graphql_client, submission_factory
 
     data = _query(graphql_client, submission)
 
-    # ✔️ public
     assert data["submission"]["title"] == submission.title.localize("en")
     assert data["submission"]["slug"] == submission.slug
 
-    # ✔️ restricted
     assert data["submission"]["elevatorPitch"] == submission.elevator_pitch.localize(
         "en"
     )
@@ -315,11 +252,9 @@ def test_ranked_submission_user_can_see_public_and_restricted_fields(
 
     data = _query(graphql_client, submission)
 
-    # ✔️ public
     assert data["submission"]["title"] == submission.title.localize("en")
     assert data["submission"]["slug"] == submission.slug
 
-    # ✔️ restricted
     assert data["submission"]["elevatorPitch"] == submission.elevator_pitch.localize(
         "en"
     )
@@ -353,24 +288,7 @@ def test_ranking_is_not_public_cannot_see_restricted_and_private_fields(
 
     data = _query(graphql_client, submission)
 
-    # ✔️ public
-    assert data["submission"]["title"] == submission.title.localize("en")
-    assert data["submission"]["slug"] == submission.slug
-
-    # ❌ restricted
-    assert data["submission"]["elevatorPitch"] is None
-    assert data["submission"]["abstract"] is None
-    assert data["submission"]["topic"] is None
-    assert data["submission"]["type"] is None
-    assert data["submission"]["duration"] is None
-    assert data["submission"]["audienceLevel"] is None
-    assert data["submission"]["languages"] is None
-    assert data["submission"]["tags"] is None
-
-    # ❌ private
-    assert data["submission"]["speakerLevel"] is None
-    assert data["submission"]["previousTalkVideo"] is None
-    assert data["submission"]["notes"] is None
+    assert data["submission"] is None
 
 
 def test_ranking_does_not_exists_cannot_see_restricted_and_private_fields(
@@ -384,21 +302,4 @@ def test_ranking_does_not_exists_cannot_see_restricted_and_private_fields(
 
     data = _query(graphql_client, submission)
 
-    # ✔️ public
-    assert data["submission"]["title"] == submission.title.localize("en")
-    assert data["submission"]["slug"] == submission.slug
-
-    # ❌ restricted
-    assert data["submission"]["elevatorPitch"] is None
-    assert data["submission"]["abstract"] is None
-    assert data["submission"]["topic"] is None
-    assert data["submission"]["type"] is None
-    assert data["submission"]["duration"] is None
-    assert data["submission"]["audienceLevel"] is None
-    assert data["submission"]["languages"] is None
-    assert data["submission"]["tags"] is None
-
-    # ❌ private
-    assert data["submission"]["speakerLevel"] is None
-    assert data["submission"]["previousTalkVideo"] is None
-    assert data["submission"]["notes"] is None
+    assert data["submission"] is None

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)