search

TrangPham / django-admin-confirm

AdminConfirmMixin is a mixin for ModelAdmin that adds confirmations to changes, additions and actions.
Other
128 stars 16 forks source link

Fixing a missing import from the code sample #39

Closed oliwarner closed 1 year ago

oliwarner commented 2 years ago

The import could probably be neater if this is a first-class export, but that's not up to me to decide.

Fixes

Proposed changes:

TrangPham commented 1 year ago

What did you mean by first-class import? Does that mean it should be able to do from admin_confirm import confirm_action?

oliwarner commented 1 year ago

What did you mean by first-class import? Does that mean it should be able to do from admin_confirm import confirm_action?

That's what I meant, yeah. So you could just import both confirm_action and AdminConfirmMixin in one import statement. Two imports that you need in every instance you use this just messy.

Edit: I see you've done that now from the .admin module. If it were me, I'd probably provide both in the parent __init__.py. There's the only things you always need from the project, right?

Another idea might be getting rid of confirm_action entirely. You could have a confirm_actions = [...] class variable and patch the action caller. Is it better? Probably not, but it's one less import.

TrangPham commented 1 year ago

I'm adding the change about the top level import here: https://github.com/TrangPham/django-admin-confirm/pull/45