Allow custom change_list_template in admin views using mixins

[nikhaldi]

Problem

The mixins for the admin integration override the admin change_list_template. This means that any customization of that template done by the user is effectively ignored. This also means that django-export-import is not compatible with mixins from other libraries that customize the object tools in the change list view (e.g., django-admin-sortable2 or reversion).

This issue was reported in #1450

Solution

This makes the change_list_template extend whatever customized template is already present in the class hierarchy. This solution is loosely based on the solution to the same issue in django-admin-sortable2.

Acceptance Criteria

Additional assertions in admin integration tests cover this use case.

[matthewhegarty]

Thanks @nikhaldi. Wondered if you could review @pokken-magic?

[coveralls]

Coverage increased (+0.01%) to 98.41% when pulling d5b2761 on nikhaldi:custom-change-list-template into 38e9f95 on django-import-export:main.

[pokken-magic]

Thanks @nikhaldi. Wondered if you could review @pokken-magic?

I will review but I think it's probably a good idea to have at least a glance with someone who knows django admin templates better than me; I almost exclusively work with DRF so my template fu is less robust.

[pokken-magic]

Looks very promising to me, thank you a lot for doing this. Only two minor comments from me.

[nikhaldi]

Thanks for the quick review!

[matthewhegarty]

I'm thinking this fix would be better suited to our forthcoming 3.0 release. Is it possible you could reset the base branch to be release-3-x?

[nikhaldi]

I'm thinking this fix would be better suited to our forthcoming 3.0 release. Is it possible you could reset the base branch to be release-3-x?

Makes sense, will do. Currently struggling with the mechanics of rebasing but I will figure it out and let you know when it's done.

[nikhaldi]

I'm thinking this fix would be better suited to our forthcoming 3.0 release. Is it possible you could reset the base branch to be release-3-x?

This is now rebased on release-3-x

[pokken-magic]

I think this all makes sense to me.

[matthewhegarty]

Thanks Nik for the change, and thanks Ryan for reviewing.

I will review but I think it's probably a good idea to have at least a glance with someone who knows django admin templates better than me

Likewise I don't know too much about admin templates. Looping in @PetrDlouhy in case he has time to review - that would be much appreciated.

[matthewhegarty]

@nikhaldi there is another merge from release-3-x required to fix some issues - thanks

[matthewhegarty]

Can you test using the example application? Looks like this change causes a rendering issue:

[nikhaldi]

@nikhaldi there is another merge from release-3-x required to fix some issues - thanks

Thanks, should now be properly merged.

[nikhaldi]

Can you test using the example application? Looks like this change causes a rendering issue:

It's not a rendering issue per se, it's just a text-only item that I inserted into the customized change list template. I need to insert something so I can write a test to verify it's working. I turned this into a real link (that goes nowhere) now, so it's maybe a bit less disjointed looking?

[matthewhegarty]

I'm concerned that the default button will be confusing for users, please see my alternative suggestion.

[matthewhegarty]

@nikhaldi thanks for the PR all looks good to me. One final question - do you think there is a risk that this might affect any existing implementations that users might have? If so, it will be fine for the v3 release but we may need to add a note to changelog.rst re breaking changes.

[matthewhegarty]

For some reason, the link still renders when placed in object-tools-items - hence I placed it under result_list

Please could you fix this?

[nikhaldi]

@nikhaldi thanks for the PR all looks good to me. One final question - do you think there is a risk that this might affect any existing implementations that users might have? If so, it will be fine for the v3 release but we may need to add a note to changelog.rst re breaking changes.

I would say the risk is small and it should rarely outright break something, but it can definitely affect existing implementation (e.g., it does in the real-world project I'm working on). I added a comment to the changelog but let me know if this is overkill.

[nikhaldi]

For some reason, the link still renders when placed in object-tools-items - hence I placed it under result_list

Strange. You can see in the screenshot I posted previously that the list item is in the HTML source but is not rendered in my browser, but it may well be a browser-specific issue. I changed this to a HTML comment now, that will definitely not render and does the job.