From 8e4acc12d7baa82c99e7d86db3ac20f33d0fe3cc Mon Sep 17 00:00:00 2001 From: joshuastegmaier Date: Wed, 13 Nov 2024 11:33:17 -0500 Subject: [PATCH 1/2] Added tests for other cases in save_transcription --- concordia/tests/test_views.py | 56 ++++++++++++++++++++++++++++++++++- concordia/views.py | 17 ++++++----- 2 files changed, 65 insertions(+), 8 deletions(-) diff --git a/concordia/tests/test_views.py b/concordia/tests/test_views.py index d478b5afe..07fd49c07 100644 --- a/concordia/tests/test_views.py +++ b/concordia/tests/test_views.py @@ -1,3 +1,4 @@ +import sys from datetime import date, timedelta from unittest.mock import patch @@ -771,6 +772,7 @@ def test_asset_reservation_tombstone_expiration(self): def test_transcription_save(self): asset = create_asset() + # Test when Turnstile validation failes with patch("concordia.turnstile.fields.TurnstileField.validate") as mock: mock.side_effect = forms.ValidationError( "Testing error", code="invalid_turnstile" @@ -798,7 +800,18 @@ def test_transcription_save(self): data = self.assertValidJSON(resp, expected_status=409) self.assertIn("error", data) - # This should work with the chain specified: + # If a transcription contains a URL, it should return an error + resp = self.client.post( + reverse("save-transcription", args=(asset.pk,)), + data={ + "text": "http://example.com", + "supersedes": asset.transcription_set.get().pk, + }, + ) + data = self.assertValidJSON(resp, expected_status=400) + self.assertIn("error", data) + + # Test that it correctly works when supersedes is set resp = self.client.post( reverse("save-transcription", args=(asset.pk,)), data={"text": "test", "supersedes": asset.transcription_set.get().pk}, @@ -806,6 +819,23 @@ def test_transcription_save(self): data = self.assertValidJSON(resp, expected_status=201) self.assertIn("submissionUrl", data) + # Test that it correctly works when supersedes is set and confirm + # ocr_originaed is properly set + transcription = asset.transcription_set.order_by("pk").last() + transcription.ocr_originated = True + transcription.save() + resp = self.client.post( + reverse("save-transcription", args=(asset.pk,)), + data={ + "text": "test", + "supersedes": asset.transcription_set.order_by("pk").last().pk, + }, + ) + data = self.assertValidJSON(resp, expected_status=201) + self.assertIn("submissionUrl", data) + new_transcription = asset.transcription_set.order_by("pk").last() + self.assertTrue(new_transcription.ocr_originated) + # We should see an error if you attempt to supersede a transcription # which has already been superseded: resp = self.client.post( @@ -818,6 +848,30 @@ def test_transcription_save(self): data = self.assertValidJSON(resp, expected_status=409) self.assertIn("error", data) + # We should get an error if you attempt to supersede a transcription + # that doesn't exist + resp = self.client.post( + reverse("save-transcription", args=(asset.pk,)), + data={ + "text": "test", + "supersedes": sys.maxsize, + }, + ) + data = self.assertValidJSON(resp, expected_status=400) + self.assertIn("error", data) + + # We should get an error if you attempt to supersede with + # with a pk that is invalid (i.e., a string instead of int) + resp = self.client.post( + reverse("save-transcription", args=(asset.pk,)), + data={ + "text": "test", + "supersedes": "bad-pk", + }, + ) + data = self.assertValidJSON(resp, expected_status=400) + self.assertIn("error", data) + # A logged in user can take over from an anonymous user: self.login_user() resp = self.client.post( diff --git a/concordia/views.py b/concordia/views.py index def878a61..bab65df8e 100644 --- a/concordia/views.py +++ b/concordia/views.py @@ -1567,14 +1567,17 @@ def get_transcription_superseded(asset, supersedes_pk): else: superseded = None else: - if asset.transcription_set.filter(supersedes=supersedes_pk).exists(): - return JsonResponse( - {"error": "This transcription has been superseded"}, status=409 - ) - try: - superseded = asset.transcription_set.get(pk=supersedes_pk) - except Transcription.DoesNotExist: + if asset.transcription_set.filter(supersedes=supersedes_pk).exists(): + return JsonResponse( + {"error": "This transcription has been superseded"}, status=409 + ) + + try: + superseded = asset.transcription_set.get(pk=supersedes_pk) + except Transcription.DoesNotExist: + return JsonResponse({"error": "Invalid supersedes value"}, status=400) + except ValueError: return JsonResponse({"error": "Invalid supersedes value"}, status=400) return superseded From 47d8cc09d0f1bafd3c4131d128c4e79ad3773ba5 Mon Sep 17 00:00:00 2001 From: joshuastegmaier Date: Wed, 13 Nov 2024 11:37:48 -0500 Subject: [PATCH 2/2] Added test for invalid tag submission --- concordia/tests/test_views.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/concordia/tests/test_views.py b/concordia/tests/test_views.py index 07fd49c07..140f86153 100644 --- a/concordia/tests/test_views.py +++ b/concordia/tests/test_views.py @@ -1151,6 +1151,22 @@ def test_tag_submission(self): self.assertEqual(sorted(test_tags), data["user_tags"]) self.assertEqual(sorted(test_tags), data["all_tags"]) + def test_invalid_tag_submission(self): + asset = create_asset() + + self.login_user() + + test_tags = ["foo", "bar"] + + with patch("concordia.models.Tag.full_clean") as mock: + mock.side_effect = forms.ValidationError("Testing error") + resp = self.client.post( + reverse("submit-tags", kwargs={"asset_pk": asset.pk}), + data={"tags": test_tags}, + ) + data = self.assertValidJSON(resp, expected_status=400) + self.assertIn("error", data) + def test_tag_submission_with_diacritics(self): asset = create_asset()