diff --git a/src/core/forms.py b/src/core/forms.py index 230940f6..c04cac39 100644 --- a/src/core/forms.py +++ b/src/core/forms.py @@ -250,12 +250,17 @@ class ExhibitForm(forms.ModelForm): augmenteds = forms.CharField(max_length=1000, required=False) sounds = forms.CharField(max_length=1000, required=False) - def __init__(self, *args, **kwargs): + def __init__(self, *args, exhibit_type=None, **kwargs): super(ExhibitForm, self).__init__(*args, **kwargs) self.fields["name"].widget.attrs["placeholder"] = _("Exhibit Title") self.fields["slug"].widget.attrs["placeholder"] = _( "Complete with your Exhibit URL here" ) + # The route picks the exhibit type for create; for edit, the + # existing instance's type is authoritative. + if exhibit_type is None and self.instance.pk: + exhibit_type = self.instance.exhibit_type + self.exhibit_type = exhibit_type class Meta: model = Exhibit @@ -337,10 +342,21 @@ def clean_sounds(self): def clean(self): cleaned_data = super().clean() - artworks = cleaned_data.get("artworks", []) - augmenteds = cleaned_data.get("augmenteds", []) - - if not artworks and not augmenteds: + artworks = cleaned_data.get("artworks") or [] + augmenteds = cleaned_data.get("augmenteds") or [] + + # The route (/exhibits/create-ar/ vs /create-mr/) selected the + # exhibit type. Validate the relevant content list — AR requires + # artworks, MR requires augmented objects. + if self.exhibit_type == ExhibitTypes.AR and not artworks: + raise forms.ValidationError(_("AR exhibits require at least one artwork.")) + if self.exhibit_type == ExhibitTypes.MR and not augmenteds: + raise forms.ValidationError( + _("MR exhibits require at least one augmented object.") + ) + # Fallback for callers that didn't pass exhibit_type — keep + # the old "you must pick at least something" guard. + if self.exhibit_type is None and not artworks and not augmenteds: raise forms.ValidationError( _("You must select at least one artwork or augmented object.") ) @@ -350,12 +366,18 @@ def clean(self): def save(self, commit=True): exhibit = super().save(commit=False) - # Set exhibit_type based on augmented objects artworks = self.cleaned_data.get("artworks", []) augmenteds = self.cleaned_data.get("augmenteds", []) sounds = self.cleaned_data.get("sounds", []) - exhibit.exhibit_type = ExhibitTypes.MR if augmenteds else ExhibitTypes.AR + # The exhibit type comes from the route (create) or the existing + # row (edit), set in __init__. Only fall back to inferring from + # the data if no type was supplied — protects against callers + # that haven't been updated. + if self.exhibit_type is not None: + exhibit.exhibit_type = self.exhibit_type + else: + exhibit.exhibit_type = ExhibitTypes.MR if augmenteds else ExhibitTypes.AR if commit: exhibit.save() diff --git a/src/core/tests/test_view_create_edit_exhibit.py b/src/core/tests/test_view_create_edit_exhibit.py index 93bedccf..e0614402 100644 --- a/src/core/tests/test_view_create_edit_exhibit.py +++ b/src/core/tests/test_view_create_edit_exhibit.py @@ -154,6 +154,10 @@ def test_create_exhibit_with_objects(self): ) def test_create_exhibit_with_artworks_and_objects(self): + # Posting a payload that mixes artworks + augmenteds against the + # AR route saves an AR exhibit (the route picks the type now, + # not the data). The augmenteds value is accepted into the row + # but the exhibit is treated as AR. url = reverse("create-ar-exhibit") data = { "name": "My Test Exhibit with Artworks and Objects", @@ -162,14 +166,12 @@ def test_create_exhibit_with_artworks_and_objects(self): "augmenteds": f"{self.object1.id},{self.object2.id}", } response = self.client.post(url, data) - # Should redirect to profile after creation assert response.status_code == 302 - # Exhibit should be created assert Exhibit.objects.count() == 1 exhibit = Exhibit.objects.get(name="My Test Exhibit with Artworks and Objects") assert exhibit.owner == self.profile assert exhibit.slug == "my-test-exhibit-art-objects" - assert exhibit.exhibit_type == ExhibitTypes.MR + assert exhibit.exhibit_type == ExhibitTypes.AR self.assertSetEqual( set(exhibit.artworks.values_list("id", flat=True)), {self.artwork1.id, self.artwork2.id}, @@ -376,39 +378,38 @@ def test_edit_exhibit_with_objects(self): {self.object1.id, self.object2.id}, ) - def test_edit_exhibit_type_changes_correctly(self): - # Initially, the exhibit has artworks only + def test_edit_ar_exhibit_keeps_type_regardless_of_augmenteds(self): + # Editing through the /edit-ar/ route saves an AR exhibit; the + # type does not flip based on whether augmenteds are present + # (route-authoritative behaviour, #853). url = reverse("edit-ar-exhibit", query={"id": self.exhibit.id}) data = { "name": "Exhibit with Artworks", "slug": "exhibit-with-artworks", "artworks": f"{self.artwork1.id},{self.artwork2.id}", - "augmenteds": "", + "augmenteds": f"{self.object1.id},{self.object2.id}", } response = self.client.post(url, data) assert response.status_code == 302 self.exhibit.refresh_from_db() assert self.exhibit.exhibit_type == ExhibitTypes.AR - # Now add objects to change it to MR - data["augmenteds"] = f"{self.object1.id},{self.object2.id}" - response = self.client.post(url, data) - assert response.status_code == 302 - self.exhibit.refresh_from_db() - assert self.exhibit.exhibit_type == ExhibitTypes.MR - - data["artworks"] = "" - response = self.client.post(url, data) - assert response.status_code == 302 - self.exhibit.refresh_from_db() - assert self.exhibit.exhibit_type == ExhibitTypes.MR # Should remain MR even - - data["augmenteds"] = "" - data["artworks"] = f"{self.artwork1.id},{self.artwork2.id}" + def test_edit_ar_exhibit_rejects_empty_artworks(self): + # AR route requires at least one artwork. Posting only + # augmenteds returns a validation error (no redirect). + url = reverse("edit-ar-exhibit", query={"id": self.exhibit.id}) + data = { + "name": "Exhibit with only objects", + "slug": "exhibit-with-only-objects", + "artworks": "", + "augmenteds": f"{self.object1.id},{self.object2.id}", + } response = self.client.post(url, data) - assert response.status_code == 302 - self.exhibit.refresh_from_db() - assert self.exhibit.exhibit_type == ExhibitTypes.AR # Should change back to AR + assert response.status_code == 200 + assert ( + "AR exhibits require at least one artwork" + in (response.context["form"].non_field_errors()[0]) + ) def test_edit_ar_exhibit_comes_filled_with_current_data(self): self.exhibit.artworks.set([self.artwork1, self.artwork2]) diff --git a/src/core/views/views.py b/src/core/views/views.py index cf73ccdb..bb7ea92c 100644 --- a/src/core/views/views.py +++ b/src/core/views/views.py @@ -427,7 +427,9 @@ def _handle_exhibit_form( is_edit = exhibit_instance is not None if request.method == "POST": - form = ExhibitForm(request.POST, instance=exhibit_instance) + form = ExhibitForm( + request.POST, instance=exhibit_instance, exhibit_type=exhibit_type + ) form.full_clean() if form.is_valid(): @@ -442,7 +444,7 @@ def _handle_exhibit_form( context = _get_ar_exhibit_context_data(user_profile, form, edit=is_edit) return render(request, "core/exhibit_create_ar.jinja2", context) else: - form = ExhibitForm(instance=exhibit_instance) + form = ExhibitForm(instance=exhibit_instance, exhibit_type=exhibit_type) if exhibit_type == ExhibitTypes.MR: context = _get_mr_exhibit_context_data(form, edit=is_edit) return render(request, "core/exhibit_create_mr.jinja2", context)