fix: unwrap Enum values in get_object_identifier#1092
Open
pctablet505 wants to merge 2 commits into
Open
Conversation
get_object_identifier() returned the raw Enum member for models with an Enum primary key, so str() on it produced "ClassName.MEMBER" instead of the enum's value. This broke generated URLs and ajax identifiers for such models. Closes smithyhq#955
Member
|
Interesting. Is it somehow common to have enum field as primary key? PR seems okay, but I find it obsolete, as this is not standard and not good practice to use enum as pk |
The prior assertion (isinstance(identifier, str)) passed even without the unwrap fix, since AnimalEnum subclasses str directly. Assert type(identifier) is str instead, so the test actually fails if get_object_identifier stops unwrapping the enum.
pctablet505
marked this pull request as ready for review
July 17, 2026 09:54
4 tasks
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
2 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
get_object_identifier()returns the raw primary key value(s) for use in URLs and ajax lookups. For models whose primary key is anEnumcolumn, it was returning the Enum member itself rather than its value, sostr()on it producedAnimalEnum.DOGinstead ofdog. This broke generated edit/detail URLs for such models (e.g./admin/animal/edit/AnimalEnum.DOG).This unwraps Enum members to their
.valuebefore use, for both single and composite primary keys.Closes #955
Test plan
test_single_pk_identifier_with_enumintests/test_helpers.py, covering a model with anEnumprimary key.uv run pytest --cov=sqladmin --cov-report=term-missing— 484 passed, 4 skipped (pre-existing), coverage 97.10%.make lint(ruff check, ruff format --check, mypy) — all pass.