Merge pull request 2i2c-org#201 from jnywong/add-validation

Add collaborator form validation

frx_challenges/web/forms.py

@@ -5,7 +5,7 @@
 from django.urls import reverse
 from django.utils.safestring import mark_safe
 from django_jsonform.forms.fields import JSONFormField
-from web.models import Submission
+from web.models import Collaborator, Submission, User
 
 from .md import MARKDOWN_RENDERER
 
@@ -61,6 +61,7 @@ class AddCollaboratorForm(forms.Form):
     """Form to add a collaborator to a submission"""
 
     def __init__(self, *args, **kwargs):
+        self.submission_id = kwargs.pop("submission_id")
         super().__init__(*args, **kwargs)
 
         self.helper = FormHelper()
@@ -71,3 +72,23 @@ def __init__(self, *args, **kwargs):
 
         self.fields["username"] = forms.CharField()
         self.fields["username"].label = "GitHub username"
+
+    def clean(self):
+        """
+        Validate that the user exists and isn't already a collaborator.
+        """
+        cleaned_data = super().clean()
+        if "username" in cleaned_data:
+            try:
+                user = User.objects.get(username__iexact=cleaned_data["username"])
+                if Collaborator.objects.filter(
+                    submission_id=self.submission_id, user_id=user.id
+                ).exists():
+                    raise forms.ValidationError(
+                        "This collaborator is already added to the submission."
+                    )
+            except User.DoesNotExist:
+                raise forms.ValidationError(
+                    "User has not logged into this website with their GitHub account."
+                )
+        return cleaned_data

frx_challenges/web/views/collaborators.py

@@ -37,31 +37,25 @@ def add(request: HttpRequest, id: int) -> HttpResponse:
 
     if is_submission_owner:
         if request.method == "POST":
-            form = AddCollaboratorForm(request.POST)
+            form = AddCollaboratorForm(request.POST, submission_id=id)
             if form.is_valid():
                 collaborator = Collaborator()
                 collaborator.submission_id = id
-                # Handle missing users
-                try:
-                    user = User.objects.filter(
-                        username=form.cleaned_data["username"]
-                    ).get()
-                except User.DoesNotExist:
-                    raise Http404("The requested user does not exist")
+                user = User.objects.filter(username=form.cleaned_data["username"]).get()
                 collaborator.user = user
                 collaborator.is_owner = False
                 try:
                     collaborator.save()
                 except IntegrityError:
-                    raise Http404(
-                        "The requested user is already a collaborator of this submission"
-                    )
+                    raise Http404("Database integrity error.")
                 return HttpResponseRedirect(reverse("collaborators-list", args=[id]))
         else:
-            form = AddCollaboratorForm()
+            form = AddCollaboratorForm(submission_id=id)
         return render(request, "collaborator/add.html", {"form": form, "id": id})
     else:
-        raise Http404("You are not allowed to add a collaborator to this submission")
+        raise Http404(
+            "You are not the submission owner; you are not allowed to add a collaborator to this submission."
+        )
 
 
 @login_required
@@ -76,11 +70,11 @@ def delete(request: HttpRequest, id: int, collab_id: int) -> HttpResponse:
     if is_submission_owner:
         collaborator = Collaborator.objects.get(pk=collab_id)
         if collaborator.is_owner:
-            raise Http404("The owner of the submission cannot be deleted")
+            raise Http404("The owner of the submission cannot be deleted.")
         collaborator.delete()
     else:
         raise Http404(
-            "You are not allowed to remove a collaborator from this submission"
+            "You are not allowed to remove a collaborator from this submission."
         )
 
     return HttpResponseRedirect(reverse("collaborators-list", args=[id]))

frx_challenges/web/views/submissions.py

@@ -63,7 +63,7 @@ def detail(request: HttpRequest, id: int) -> HttpResponse:
     try:
         submission = Submission.objects.get(id=id)
     except Submission.DoesNotExist:
-        raise Http404("Submission does not exist")
+        raise Http404("Submission does not exist.")
     is_collaborator = _validate_collaborator(request, id)
 
     versions = submission.versions.all()

frx_challenges/web/views/versions.py

@@ -16,7 +16,7 @@
 def upload(request: HttpRequest, id: int) -> HttpResponse:
     is_collaborator = _validate_collaborator(request, id)
     if not is_collaborator:
-        raise Http404("You are not a collaborator of this submission.")
+        raise Http404("Uploads are only available to submission collaborators.")
     if request.method == "POST":
         form = UploadForm(data=request.POST, files=request.FILES, id=id)
         if form.is_valid():