Skip to content

Commit 27f3478

Browse files
alex-kolmakovAlexander Kolmakovalexeygrigorev
authored
Added non 200 validation for URLfield in homework/project submissions (#62)
* Added non 200 validation for URL field in homework and project submissions. * Added validation for project and homework submission views, added json validation for learning in public links and adjusted test * tests and extra migration --------- Co-authored-by: Alexander Kolmakov <[email protected]> Co-authored-by: Alexey Grigorev <[email protected]>
1 parent 89242cb commit 27f3478

File tree

12 files changed

+258
-39
lines changed

12 files changed

+258
-39
lines changed

‎courses/migrations/0014_alter_projectsubmission_github_link_and_more.py‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
# Generated by Django 4.2.14 on 2024-07-31 05:05
2+
3+
import courses.validators.custom_url_validators
4+
import courses.validators.validating_json_field
5+
import django.core.validators
6+
from django.db import migrations, models
7+
8+
9+
class Migration(migrations.Migration):
10+
11+
dependencies = [
12+
('courses', '0013_remove_homework_is_scored_homework_state_and_more'),
13+
]
14+
15+
operations = [
16+
migrations.AlterField(
17+
model_name='projectsubmission',
18+
name='github_link',
19+
field=models.URLField(validators=[django.core.validators.URLValidator(), courses.validators.custom_url_validators.validate_url_200]),
20+
),
21+
migrations.AlterField(
22+
model_name='projectsubmission',
23+
name='learning_in_public_links',
24+
field=courses.validators.validating_json_field.ValidatingJSONField(blank=True, null=True),
25+
),
26+
migrations.AlterField(
27+
model_name='submission',
28+
name='homework_link',
29+
field=models.URLField(blank=True, null=True, validators=[django.core.validators.URLValidator(schemes=['http', 'https', 'git']), courses.validators.custom_url_validators.validate_url_200]),
30+
),
31+
migrations.AlterField(
32+
model_name='submission',
33+
name='learning_in_public_links',
34+
field=courses.validators.validating_json_field.ValidatingJSONField(blank=True, help_text='Links where students talk about the course', null=True),
35+
),
36+
]

‎courses/models/homework.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
from django.contrib.auth import get_user_model
66

77
from .course import Course, Enrollment
8+
from courses.validators import validate_url_200
9+
from courses.validators.validating_json_field import ValidatingJSONField
810

911
User = get_user_model()
1012

@@ -153,9 +155,9 @@ class Submission(models.Model):
153155
homework_link = models.URLField(
154156
blank=True,
155157
null=True,
156-
validators=[URLValidator(schemes=["http", "https", "git"])],
158+
validators=[URLValidator(schemes=["http", "https", "git"]), validate_url_200],
157159
)
158-
learning_in_public_links = models.JSONField(
160+
learning_in_public_links = ValidatingJSONField(
159161
blank=True,
160162
null=True,
161163
help_text="Links where students talk about the course",

‎courses/models/project.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
from django.contrib.auth import get_user_model
88

99
from .course import Course, Enrollment
10+
from courses.validators import validate_url_200
11+
from courses.validators.validating_json_field import ValidatingJSONField
1012

1113
User = get_user_model()
1214

@@ -84,10 +86,10 @@ class ProjectSubmission(models.Model):
8486
student = models.ForeignKey(User, on_delete=models.CASCADE)
8587
enrollment = models.ForeignKey(Enrollment, on_delete=models.CASCADE)
8688

87-
github_link = models.URLField(validators=[URLValidator()])
89+
github_link = models.URLField(validators=[URLValidator(), validate_url_200])
8890
commit_id = models.CharField(max_length=40)
8991

90-
learning_in_public_links = models.JSONField(blank=True, null=True)
92+
learning_in_public_links = ValidatingJSONField(blank=True, null=True)
9193
faq_contribution = models.TextField(blank=True)
9294

9395
time_spent = models.FloatField(blank=True, null=True)

‎courses/tests/test_data.py‎

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from django.test import TestCase, Client
22
from django.urls import reverse
33
from django.utils import timezone
4+
from django.core.exceptions import ValidationError
45

56
from courses.models import (
67
User,
@@ -52,13 +53,16 @@ def test_homework_data_view(self):
5253
slug="test-homework",
5354
)
5455

55-
self.submission = Submission.objects.create(
56+
self.submission = Submission(
5657
homework=self.homework,
5758
student=self.user,
5859
enrollment=self.enrollment,
59-
homework_link="https://example.com/homework",
60+
homework_link="https://github.com/DataTalksClub",
6061
)
6162

63+
self.submission.full_clean()
64+
self.submission.save()
65+
6266
self.question = Question.objects.create(
6367
homework=self.homework,
6468
text="What is the capital of France?",
@@ -221,14 +225,17 @@ def test_project_data_view(self):
221225
+ timezone.timedelta(days=14),
222226
)
223227

224-
self.project_submission = ProjectSubmission.objects.create(
228+
self.project_submission = ProjectSubmission(
225229
project=self.project,
226230
student=self.user,
227231
enrollment=self.enrollment,
228-
github_link="https://github.com/test/repo",
232+
github_link="https://github.com/DataTalksClub",
229233
commit_id="abcd1234",
230234
)
231235

236+
self.project_submission.full_clean()
237+
self.project_submission.save()
238+
232239
url = reverse(
233240
"data_project",
234241
kwargs={
@@ -331,3 +338,45 @@ def test_project_data_view(self):
331338
self.assertEqual(submission['total_score'], expected_submission['total_score'])
332339
self.assertEqual(submission['reviewed_enough_peers'], expected_submission['reviewed_enough_peers'])
333340
self.assertEqual(submission['passed'], expected_submission['passed'])
341+
342+
343+
def test_homework_submission_with_404_url(self):
344+
self.homework = Homework.objects.create(
345+
course=self.course,
346+
title="Test Homework",
347+
description="Test Homework Description",
348+
due_date=timezone.now() + timezone.timedelta(days=7),
349+
state=HomeworkState.OPEN.value,
350+
slug="test-homework",
351+
)
352+
353+
self.submission = Submission(
354+
homework=self.homework,
355+
student=self.user,
356+
enrollment=self.enrollment,
357+
homework_link="https://httpbin.org/status/404",
358+
)
359+
with self.assertRaises(ValidationError):
360+
self.submission.full_clean()
361+
362+
def test_project_submission_with_404_url(self):
363+
self.project = Project.objects.create(
364+
course=self.course,
365+
slug="test-project",
366+
title="Test Project",
367+
description="Description",
368+
submission_due_date=timezone.now()
369+
+ timezone.timedelta(days=7),
370+
peer_review_due_date=timezone.now()
371+
+ timezone.timedelta(days=14),
372+
)
373+
374+
self.project_submission = ProjectSubmission(
375+
project=self.project,
376+
student=self.user,
377+
enrollment=self.enrollment,
378+
github_link="https://httpbin.org/status/404",
379+
commit_id="abcd1234",
380+
)
381+
with self.assertRaises(ValidationError):
382+
self.project_submission.full_clean()

‎courses/tests/test_homework.py‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -833,10 +833,10 @@ def test_submit_homework_with_all_fields(self):
833833
f"answer_{self.question4.id}": ["3"],
834834
f"answer_{self.question5.id}": ["3.141516"],
835835
f"answer_{self.question6.id}": ["1", "2"],
836-
"homework_url": "http://example.com/homework",
836+
"homework_url": "https://httpbin.org/status/200",
837837
"learning_in_public_links[]": [
838-
"http://example.com/link1",
839-
"http://example.com/link2",
838+
"https://httpbin.org/status/200",
839+
"https://github.com/DataTalksClub",
840840
"",
841841
],
842842
"time_spent_lectures": "5",
@@ -868,8 +868,8 @@ def test_submit_homework_with_all_fields(self):
868868
)
869869

870870
expected_learning_in_public_links = [
871-
"http://example.com/link1",
872-
"http://example.com/link2",
871+
"https://httpbin.org/status/200",
872+
"https://github.com/DataTalksClub",
873873
]
874874
self.assertEqual(
875875
submission.learning_in_public_links,
@@ -913,7 +913,7 @@ def test_submit_homework_with_all_fields_optional_empty(self):
913913
f"answer_{self.question4.id}": ["3"],
914914
f"answer_{self.question5.id}": ["3.141516"],
915915
f"answer_{self.question6.id}": ["1", "2"],
916-
"homework_url": "http://example.com/homework",
916+
"homework_url": "https://httpbin.org/status/200",
917917
"learning_in_public_links[]": [""],
918918
"time_spent_lectures": "",
919919
"time_spent_homework": "",
@@ -967,11 +967,9 @@ def test_submit_homework_learning_in_public_empty_and_duplicates(
967967
f"answer_{self.question5.id}": ["3.141516"],
968968
f"answer_{self.question6.id}": ["1", "2"],
969969
"learning_in_public_links[]": [
970-
"http://example.com/link1",
971-
"http://example.com/link2",
972-
"http://example.com/link1",
973-
"",
974-
"",
970+
"https://httpbin.org/status/200",
971+
"https://httpbin.org/status/200",
972+
"https://github.com/DataTalksClub"
975973
"",
976974
],
977975
}
@@ -994,8 +992,8 @@ def test_submit_homework_learning_in_public_empty_and_duplicates(
994992
)
995993

996994
expected_learning_in_public_links = [
997-
"http://example.com/link1",
998-
"http://example.com/link2",
995+
"https://httpbin.org/status/200",
996+
"https://github.com/DataTalksClub"
999997
]
1000998
self.assertEqual(
1001999
submission.learning_in_public_links,

‎courses/tests/test_project_view.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ def test_project_submission_post_no_submissions(self):
173173
)
174174

175175
data = {
176-
"github_link": "https://github.com/testuser/project",
176+
"github_link": "https://httpbin.org/status/200",
177177
"commit_id": "1234567",
178178
"time_spent": "2",
179179
"problems_comments": "Encountered an issue with...",
@@ -220,7 +220,7 @@ def test_project_submission_post_creates_enrollment(self):
220220
)
221221

222222
data = {
223-
"github_link": "https://github.com/testuser/project",
223+
"github_link": "https://httpbin.org/status/200",
224224
"commit_id": "1234567",
225225
"time_spent": "2",
226226
"problems_comments": "Encountered an issue with...",
@@ -242,7 +242,7 @@ def test_project_submission_post_with_submissions(self):
242242
project=self.project,
243243
student=self.user,
244244
enrollment=self.enrollment,
245-
github_link="https://github.com/testuser/project",
245+
github_link="https://httpbin.org/status/200",
246246
commit_id="123456a",
247247
)
248248

@@ -251,7 +251,7 @@ def test_project_submission_post_with_submissions(self):
251251
)
252252

253253
data = {
254-
"github_link": "https://github.com/testuser/project2",
254+
"github_link": "https://httpbin.org/status/200",
255255
"commit_id": "123456e",
256256
"time_spent": "3",
257257
"problems_comments": "No issues encountered.",
@@ -296,7 +296,7 @@ def test_project_submission_not_accepting_responses(self):
296296
)
297297

298298
data = {
299-
"github_link": "https://github.com/testuser/project",
299+
"github_link": "https://httpbin.org/status/200",
300300
"commit_id": "1234567",
301301
"time_spent": "2",
302302
"problems_comments": "Encountered an issue with...",

‎courses/tests/test_unit_url_validation.py‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import logging
2+
3+
from unittest import TestCase
4+
5+
from courses.validators.custom_url_validators import (
6+
validate_url_200,
7+
get_error_message,
8+
)
9+
from django.core.exceptions import ValidationError
10+
11+
12+
logger = logging.getLogger(__name__)
13+
14+
15+
class MockResponse:
16+
def __init__(self, status_code):
17+
self.status_code = status_code
18+
19+
20+
class UrlValidationTestCase(TestCase):
21+
22+
def test_validation_code_200_github_mock(self):
23+
mock_get = lambda url: MockResponse(200)
24+
url = "https://github.com/DataTalksClub/"
25+
validate_url_200(url, mock_get)
26+
# no exceptions should be raised
27+
28+
def test_validation_code_404_github_mock(self):
29+
mock_get = lambda url: MockResponse(404)
30+
url = "https://github.com/DataTalksClub/non-existing-repo"
31+
with self.assertRaises(ValidationError):
32+
validate_url_200(url, mock_get)
33+
34+
def test_error_message_404_github(self):
35+
url = "https://github.com/DataTalksClub/non-existing-repo"
36+
error = get_error_message(404, url)
37+
expected_error = (
38+
f"The submitted GitHub link {url} does not "
39+
+ "exist. Make sure the repository is public."
40+
)
41+
self.assertEqual(error, expected_error)
42+
43+
def test_error_message_404_github_case_insensitive(self):
44+
url = "https://GitHub.com/DataTalksClub/non-existing-repo"
45+
error = get_error_message(404, url)
46+
expected_error = (
47+
f"The submitted GitHub link {url} does not "
48+
+ "exist. Make sure the repository is public."
49+
)
50+
self.assertEqual(error, expected_error)
51+
52+
def test_error_message_404_linkedin(self):
53+
url = "https://www.linkedin.com/feed/update/urn:li:activity:7221077695688773633/"
54+
error = get_error_message(404, url)
55+
expected_error = f"The submitted link {url} does not exist."
56+
self.assertEqual(error, expected_error)
57+
58+
def test_error_message_400(self):
59+
url = "https://www.linkedin.com/whatever"
60+
error = get_error_message(400, url)
61+
expected_error = (
62+
f"The submitted link {url} does not "
63+
+ "return a 200 status code. Status code: 400."
64+
)
65+
self.assertEqual(error, expected_error)

‎courses/validators/__init__.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
from .custom_url_validators import validate_url_200
2+
from .validating_json_field import ValidatingJSONField

‎courses/validators/custom_url_validators.py‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import requests
2+
from django.core.exceptions import ValidationError
3+
4+
5+
def get_error_message(status_code, url):
6+
if status_code != 404:
7+
return (
8+
f"The submitted link {url} does not "
9+
+ "return a 200 status code. Status code: "
10+
+ f"{status_code}."
11+
)
12+
13+
# 404 status code
14+
if "github" in url.lower():
15+
return (
16+
f"The submitted GitHub link {url} does not "
17+
+ "exist. Make sure the repository is public."
18+
)
19+
20+
return f"The submitted link {url} does not exist."
21+
22+
23+
def validate_url_200(url, get_method=requests.get):
24+
try:
25+
response = get_method(url)
26+
status_code = response.status_code
27+
if status_code == 200:
28+
return
29+
error_message = get_error_message(status_code, url)
30+
raise ValidationError(error_message)
31+
except requests.exceptions.RequestException as e:
32+
raise ValidationError(
33+
f"An error occurred while trying to validate the URL: {e}"
34+
)

0 commit comments

Comments
 (0)