Skip to content

Conversation

sterliakov
Copy link
Contributor

@sterliakov sterliakov commented Mar 13, 2025

Fixes #16.

Django models do not always have a field named "id". Declaring another field with primary_key=True removes the default id field.

Admin *ListFilter variations all have lookup_kwarg attribute that provides exactly what we need: explicit target lookup without any string munging in application code. This also allows us to remove isChoiceField entirely and always use the lookup_kwarg provided by Django. Autocomplete AJAX always returns PK field under id field.

I tested these changes locally on a test project and adjusted the existing test to account for new fields. To support usual whitespace wrapping, I replaced string-based asserts with HTMLParser (built-in) to verify that there is exactly one tag matching given specification.

entry: pytest -s tests/testproject/testapp -c tests/testproject/pytest.ini
language: system
types: [python]
pass_filenames: false
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this change, editing HTML files does not trigger pytest - probably not what you need?

@sterliakov sterliakov marked this pull request as ready for review March 14, 2025 14:36
@sterliakov sterliakov requested a review from vigo as a code owner March 14, 2025 14:36
@vigo
Copy link
Owner

vigo commented Mar 14, 2025

@sterliakov buddy, great work, i can not merge it, your commits are not signed. can you please sign your commits? i'm super exited to merge it.

@sterliakov
Copy link
Contributor Author

Done, but please consider adding this to CONTRIBUTING.md - signing commits is an unusual requirement (and often adds no actual security: https://softwareengineering.stackexchange.com/questions/212192/what-are-the-advantages-and-disadvantages-of-cryptographically-signing-commits-a)

@vigo
Copy link
Owner

vigo commented Mar 14, 2025

You are right, i should add it to CONTRIBUTING.md, thank you for the great support <3

@vigo vigo merged commit 20d92cd into vigo:main Mar 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Does not support models with PK not named "id"
2 participants