fix(helpers): coerce Enum primary keys to .value in get_object_identifier#1096
Open
AdarshJ173 wants to merge 1 commit into
Open
fix(helpers): coerce Enum primary keys to .value in get_object_identifier#1096AdarshJ173 wants to merge 1 commit into
AdarshJ173 wants to merge 1 commit into
Conversation
…fier When a model uses an Enum as its primary key, getattr() returns the Enum instance (e.g. AnimalEnum.DOG). Passing this raw instance into URL construction produces paths like /admin/animal/edit/AnimalEnum.DOG instead of /admin/animal/edit/dog, breaking all edit/detail routes for such models. Fix: extract .value from any enum.Enum instance before returning or serialising. This is done via a small _coerce_pk_value() helper so the logic is applied consistently in both the single-PK fast-path and the multi-PK join path. Closes smithyhq#955
Member
|
This is duplicate of #1092, although this is better solution. If possible merge the tests of that PR to your PR (as you are missing tests). Do not copy, preferrably merge. |
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
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
Fixes #955 —
get_object_identifier()returnsAnimalEnum.DOGinstead of"dog"when the model primary key is anenum.Enum, breaking all edit/detail URLs for Enum-PK models.Root cause
get_object_identifier()callsgetattr(obj, pk.name)which returns the raw Enum instance as stored by SQLAlchemy (e.g.AnimalEnum.DOG). The single-PK fast-path returns this directly; the multi-PK path passes it throughstr(), which produces"AnimalEnum.DOG"rather than the underlying value"dog". Both paths therefore generate malformed admin URLs:What changed
sqladmin/helpers.pyAdded a private
_coerce_pk_value()helper that unwraps.valuefrom anyenum.Enuminstance and is a no-op for all other types:get_object_identifier()now applies_coerce_pk_value()to each PK value before returning or joining:enumis already imported at the top of the module — no new imports.get_object_identifier()so it reads as a unit.Acceptance criteria
get_object_identifier(dog)returns"dog"(notAnimalEnum.DOG) for astr-Enum PKget_object_identifier(status)returns1(notStatusEnum.ACTIVE) for anint-Enum PKget_object_identifier(obj)continues to return the raw value forint,str, andUUIDPKs (no regression)How to test
(Full integration test: create an
AnimalSQLModel withid: AnimalEnumas PK, add it to ansqladminview, open the admin list, click Edit — URL should be/admin/animal/edit/dog, not/admin/animal/edit/AnimalEnum.DOG.)Notes
_coerce_pk_valueis a no-op for all non-Enum values; existing integer, string, and UUID primary keys are unaffectedenummodule was already imported at the top ofhelpers.py