Make ExhibitForm respect the route-supplied exhibit_type (#853)#944
Open
vjpixel wants to merge 2 commits into
Open
Make ExhibitForm respect the route-supplied exhibit_type (#853)#944vjpixel wants to merge 2 commits into
vjpixel wants to merge 2 commits into
Conversation
The form was inferring exhibit_type from whichever data field was populated (augmenteds → MR, otherwise → AR), which let users post mismatched payloads and get a different type than the route suggested. Worse, an existing test (test_create_exhibit_with_artworks_and_objects) was locking that behaviour in. Make the route the source of truth: - ExhibitForm.__init__ now accepts exhibit_type. For edits, when the caller doesn't pass it, fall back to the existing instance's type so the form can't change type silently mid-edit. - clean() validates the type-relevant content list — AR requires artworks, MR requires augmenteds. The pre-existing "must have at least one artwork or augmented" guard remains as a fallback for callers that don't pass exhibit_type. - save() uses self.exhibit_type to set Exhibit.exhibit_type, with the old inference kept only as the no-type-supplied fallback. - _handle_exhibit_form passes exhibit_type to both bound and unbound form constructions. Tests: - test_create_exhibit_with_artworks_and_objects now expects the route-driven outcome — POST to /create-ar/ with mixed payload saves as AR, not MR. - test_edit_exhibit_type_changes_correctly is replaced by two tests that match the new contract: the type stays AR through edit-ar regardless of augmenteds content, and posting only augmenteds against the AR edit route returns the validation error. Closes #853 https://claude.ai/code/session_01XC1THLWgnGXGf5wgRhdyvB
Member
|
@vjpixel, the new tests are not working and this branch has some conflict after I've merged some of the previous merge, could you please fix them? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the route-authoritative direction for #853.
ExhibitFormwas inferringexhibit_typefrom whichever data field was populated (augmenteds → MR, otherwise → AR), which let users POST mismatched payloads and get a different type than the route suggested. Worse, an existing test (test_create_exhibit_with_artworks_and_objects) was locking that behaviour in.Make the route the source of truth:
ExhibitForm.__init__acceptsexhibit_typekwarg. For edits, if the caller doesn't pass it, fall back to the existing instance's type so the form can't change type silently mid-edit.clean()validates the type-relevant content list — AR requires artworks, MR requires augmenteds. The legacy "must have at least one artwork or augmented" guard remains as a fallback for callers that didn't passexhibit_type.save()usesself.exhibit_typeto setExhibit.exhibit_type, with the old inference kept only as the no-type-supplied fallback._handle_exhibit_formpassesexhibit_typeto both bound and unbound form constructions.Test plan
Updated tests:
test_create_exhibit_with_artworks_and_objects: POST to/create-ar/with mixed payload now saves as AR, not MR.test_edit_exhibit_type_changes_correctlyis replaced by two new tests reflecting the new contract:test_edit_ar_exhibit_keeps_type_regardless_of_augmentedstest_edit_ar_exhibit_rejects_empty_artworks— now returns 200 with the "AR exhibits require at least one artwork" error instead of redirecting.Open question
Editing an existing AR exhibit and changing its type to MR (or vice-versa) is not possible through this design — type is locked at creation. If type-change is a real workflow, we'd need a separate flow (delete + recreate, or an explicit "convert" action). Calling that out for review.
Closes #853
https://claude.ai/code/session_01XC1THLWgnGXGf5wgRhdyvB
Generated by Claude Code