Repository: carpentries/amy
Issue: Error when "Ask to Repeat" due to non-Carpentries repo

See this trainee as an example: https://test-amy2.carpentries.org/workshops/person/74/
They have one "Get Involved" step that is to a non-Carpentries repo.  When the admin tries to mark that as "Ask to Repeat" they get an error that it is a non-Carpentries repo. 
We want the admin to be able to mark this as "Ask to repeat" so that the trainee can go back and make the appropriate changes.  

Changed files:
amy/dashboard/forms.py, amy/dashboard/tests/test_forms.py, amy/workshops/migrations/0261_migrate_lesson_contribution_to_get_involved.py, amy/api/tests/test_export.py, amy/trainings/tests/test_training_progress.py, amy/workshops/models.py

diff --git a/amy/api/tests/test_export.py b/amy/api/tests/test_export.py
index adaa8cb6..b7550327 100644
--- a/amy/api/tests/test_export.py
+++ b/amy/api/tests/test_export.py
@@ -557,7 +557,7 @@ class TestExportingPersonData(BaseExportingTest):
                 },
                 "involvement_type": {
                     "name": "GitHub Contribution",
-                    "display_name": "Submitted a contribution to a Carpentries repository",  # noqa
+                    "display_name": "Submitted a contribution to a Carpentries repository on GitHub",  # noqa
                     "url_required": True,
                     "date_required": True,
                     "notes_required": False,
diff --git a/amy/dashboard/forms.py b/amy/dashboard/forms.py
index 6616f307..004f130d 100644
--- a/amy/dashboard/forms.py
+++ b/amy/dashboard/forms.py
@@ -183,6 +183,22 @@ class GetInvolvedForm(forms.ModelForm):
     )
     helper = BootstrapHelper(add_cancel_button=True)
 
+    CARPENTRIES_GITHUB_ORGS = [
+        "carpentries",
+        "datacarpentry",
+        "librarycarpentry",
+        "swcarpentry",
+        "carpentries-es",
+        "Reproducible-Science-Curriculum",
+        "CarpentryCon",
+        "CarpentryConnect",
+        "carpentries-workshops",
+        "data-lessons",
+        "carpentries-lab",
+        "carpentries-incubator",
+        "carpentrieslab",
+    ]
+
     class Meta:
         model = TrainingProgress
         fields = [
@@ -211,6 +227,33 @@ class GetInvolvedForm(forms.ModelForm):
             else:
                 return False
 
+    def clean_url(self):
+        """Check if URL is associated with a Carpentries GitHub organisation"""
+        involvement_type = self.cleaned_data["involvement_type"]
+        url = self.cleaned_data["url"]
+        if involvement_type and involvement_type.name == "GitHub Contribution":
+            if not url:
+                # This check is part of model validation, but form validation runs
+                # first. As an empty URL will trigger the next error, first replicate
+                # the "URL required" error check here for this specific Involvement.
+                msg = (
+                    "This field is required for activity "
+                    f'"{involvement_type.display_name}".'
+                )
+                raise ValidationError(msg)
+            elif not any(
+                f"github.com/{org}" in url for org in self.CARPENTRIES_GITHUB_ORGS
+            ):
+                msg = (
+                    "This URL is not associated with a repository in any of the "
+                    "GitHub organisations owned by The Carpentries. "
+                    "If you need help resolving this error, please contact us using "
+                    "the details at the top of this form."
+                )
+                raise ValidationError(msg)
+
+        return url
+
     def clean_trainee_notes(self):
         """Raise an error if the trainee has not provided notes where required.
 
diff --git a/amy/dashboard/tests/test_forms.py b/amy/dashboard/tests/test_forms.py
index 13c4b774..169be84d 100644
--- a/amy/dashboard/tests/test_forms.py
+++ b/amy/dashboard/tests/test_forms.py
@@ -151,6 +151,56 @@ class TestGetInvolvedForm(TestCase):
             {"involvement_type", "date", "url", "trainee_notes"}, form.fields.keys()
         )
 
+    def test_clean_custom_validation__url_empty(self):
+        # Arrange
+        github_contribution = Involvement.objects.get(
+            name="GitHub Contribution",
+        )
+        data = {
+            "involvement_type": github_contribution,
+            "date": date(2023, 7, 27),
+            "url": "",
+        }
+
+        # Act
+        form = GetInvolvedForm(data, instance=self.base_instance)
+
+        # Assert
+        self.assertEqual(form.is_valid(), False)
+        expected_msg = (
+            "This field is required for activity "
+            '"Submitted a contribution to a Carpentries repository on GitHub".'
+        )
+        self.assertEqual(
+            form.errors["url"],
+            [expected_msg],
+        )
+
+    def test_clean_custom_validation__url_invalid(self):
+        # Arrange
+        github_contribution = Involvement.objects.get(name="GitHub Contribution")
+        data = {
+            "involvement_type": github_contribution,
+            "date": date(2023, 7, 27),
+            "url": "https://not-a-github-url.org",
+        }
+
+        # Act
+        form = GetInvolvedForm(data, instance=self.base_instance)
+
+        # Assert
+        self.assertEqual(form.is_valid(), False)
+        expected_msg = (
+            "This URL is not associated with a repository in any of the "
+            "GitHub organisations owned by The Carpentries. "
+            "If you need help resolving this error, please contact us using the "
+            "details at the top of this form."
+        )
+        self.assertEqual(
+            form.errors["url"],
+            [expected_msg],
+        )
+
     def test_clean_custom_validation__trainee_notes(self):
         # Arrange
         data = {"involvement_type": self.involvement, "date": date(2023, 7, 27)}
diff --git a/amy/trainings/tests/test_training_progress.py b/amy/trainings/tests/test_training_progress.py
index d4a05a16..f9711b88 100644
--- a/amy/trainings/tests/test_training_progress.py
+++ b/amy/trainings/tests/test_training_progress.py
@@ -114,6 +114,7 @@ class TestTrainingProgressValidation(TestBase):
         p4.full_clean()  # involvement URLs can be optional
 
     def test_url_associated_with_github_organisation(self):
+        """Regression test for #2507: should not fail model validation"""
         github_url_required, _ = Involvement.objects.get_or_create(
             name="GitHub Contribution",
             defaults={
@@ -136,9 +137,8 @@ class TestTrainingProgressValidation(TestBase):
             url="http://example.com",
             date=datetime(2023, 5, 31),
         )
-        p1.full_clean()
-        with self.assertValidationErrors(["url"]):
-            p2.full_clean()
+        p1.full_clean()  # should pass everywhere
+        p2.full_clean()  # URL should pass here, but fail in GetInvolvedForm
 
     def test_event_is_required(self):
         p1 = TrainingProgress.objects.create(
diff --git a/amy/workshops/migrations/0261_migrate_lesson_contribution_to_get_involved.py b/amy/workshops/migrations/0261_migrate_lesson_contribution_to_get_involved.py
index bdbca863..7be1ba23 100644
--- a/amy/workshops/migrations/0261_migrate_lesson_contribution_to_get_involved.py
+++ b/amy/workshops/migrations/0261_migrate_lesson_contribution_to_get_involved.py
@@ -18,7 +18,7 @@ def migrate_lesson_contributions_to_involvements(apps, schema_editor) -> None:
 
     involvement = Involvement.objects.create(
         name="GitHub Contribution",
-        display_name="Submitted a contribution to a Carpentries repository",
+        display_name="Submitted a contribution to a Carpentries repository on GitHub",
         url_required=True,
         date_required=True,
     )
diff --git a/amy/workshops/models.py b/amy/workshops/models.py
index 29ffb63f..6ce83e05 100644
--- a/amy/workshops/models.py
+++ b/amy/workshops/models.py
@@ -2612,22 +2612,6 @@ class TrainingProgress(CreatedUpdatedMixin, models.Model):
     )
     notes = models.TextField(blank=True)
 
-    CARPENTRIES_GITHUB_ORGS = [
-        "carpentries",
-        "datacarpentry",
-        "librarycarpentry",
-        "swcarpentry",
-        "carpentries-es",
-        "Reproducible-Science-Curriculum",
-        "CarpentryCon",
-        "CarpentryConnect",
-        "carpentries-workshops",
-        "data-lessons",
-        "carpentries-lab",
-        "carpentries-incubator",
-        "carpentrieslab",
-    ]
-
     def get_absolute_url(self):
         return reverse("trainingprogress_edit", args=[str(self.id)])
 
@@ -2670,19 +2654,6 @@ class TrainingProgress(CreatedUpdatedMixin, models.Model):
         else:
             if not requirement.url_required and not requirement.involvement_required:
                 return self.get_not_required_error(involvement_type)
-            elif involvement_type and involvement_type.name == "GitHub Contribution":
-                return self.clean_github_url(self.url)
-
-    def clean_github_url(self, url):
-        """Check if URL is associated with a Carpentries GitHub organisation"""
-        if not any(f"github.com/{org}" in url for org in self.CARPENTRIES_GITHUB_ORGS):
-            msg = (
-                "This URL is not associated with a repository in any of the "
-                "GitHub organisations owned by The Carpentries. "
-                "If you need help resolving this error, please contact us using the "
-                "details at the top of this form."
-            )
-            return ValidationError(msg)
 
     def clean_event(
         self,