Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #316: Allow combining admin mixins by storing original change_list_template #317

Closed
wants to merge 2 commits into from

Conversation

PetrDlouhy
Copy link
Contributor

Fix #316

The problem is caused by django-admin-sortable2 ignoring the value of change_list_template property from previous mixins.
This PR stores renames the changoe_list_template to sortable_change_list_template to allow access to the original property value. Then it sets the new value in __init__, but uses the original value as base template for admin/change_list.html so that overriding of these templates is chained together.

@PetrDlouhy
Copy link
Contributor Author

I have added test for the new behavior (which also tests, that GET will put the JSON values in the response).
I rebased to 2.0.2, which is the last version with passing tests, but I still got timeouts. I don't know what is the problem, but I expect it is not the new code.

@jrief Could you please try to remember or test this code in your environment to determine what caused the "it didn't work" problem? I am willing to fix all the problems, but right now I couldn't think how the mechanism I have used can fail.

Or if somebody else could test this to confirm/disprove the problems, I would be really glad. I am using this on different project than the original PR and it works fine.

@PetrDlouhy PetrDlouhy marked this pull request as ready for review June 10, 2022 15:45
@jrief
Copy link
Owner

jrief commented Jun 10, 2022

Unit tests run on my local machiche. However, for reasons unknown to me, they fail on GH actions. I have to investigate further.
I'll merge this solution soon.
Thanks for investigating.

@jrief
Copy link
Owner

jrief commented Jun 17, 2022

@PetrDlouhy I used your pull request as a starting point but implemented it in a slightly simpler way. I also created a unit test to check if the template is overridden. Please have a look at the HEAD of the repository.

@jrief jrief force-pushed the master branch 11 times, most recently from da08943 to 7ce50a2 Compare June 22, 2022 22:13
@PetrDlouhy
Copy link
Contributor Author

@jrief The new version seems to work nicely. Thank you very much.

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.

SortableAdminMixin not compatible with other mixins (at least ImportExportMixin from django-import-export)
2 participants