django icon indicating copy to clipboard operation
django copied to clipboard

Fixed #12090 -- Show admin actions on the edit pages too

Open mgaligniana opened this issue 1 year ago • 9 comments

Hi!

It fixes ticket 12090

  • New discussion thread: https://forum.djangoproject.com/t/admin-actions-also-in-the-object-details-view/22927/4

  • When save_on_top is True looks like:

image

  • To run tests please do: ./runtests.py admin_views.test_actions.AdminDetailActionsTest

mgaligniana avatar Aug 30 '22 02:08 mgaligniana

Another point that is worth mentioning: Django Admin Inline Actions have the description attribute which can be used to give a more friendly human-readable name to the action.

e.g

@admin.action(description='Mark selected stories as published')
def make_published(modeladmin, request, queryset):
    queryset.update(status='p')

The problem this poses is that this attribute was always designed to be used as part of an action against multiple instances of a given model and thus over the years people (me..) would have used names written in the plural-form like "Regenerate contracts", or "Refresh indexes".

Therefore using those names in the context of the edit page would cause them to not make sense because it's a singular object being "actioned" rather than multiple. Others would have followed this naming pattern too, especially as the documentation example uses the wording "Mark selected stories as published" which would not work in singular form.

In an ideal world, inline actions would need two variables to do a good job of this - as Django does in many other places (verbose_name vs verbose_name_plural) etc. But this poses another problem in that the known-plural version is currently called description so it's not as simple as adding a new description_plural parameter.

djm avatar Sep 05 '22 12:09 djm

@mgaligniana Do you have time to keep working on this?

felixxm avatar Jun 21 '23 07:06 felixxm

@felixxm Yes! This week I'm going to add some progress! 🙏 Sorry, I've been busy these months.

mgaligniana avatar Jun 21 '23 11:06 mgaligniana

Another point that is worth mentioning: Django Admin Inline Actions have the description attribute which can be used to give a more friendly human-readable name to the action.

e.g

@admin.action(description='Mark selected stories as published')
def make_published(modeladmin, request, queryset):
    queryset.update(status='p')

The problem this poses is that this attribute was always designed to be used as part of an action against multiple instances of a given model and thus over the years people (me..) would have used names written in the plural-form like "Regenerate contracts", or "Refresh indexes".

Therefore using those names in the context of the edit page would cause them to not make sense because it's a singular object being "actioned" rather than multiple. Others would have followed this naming pattern too, especially as the documentation example uses the wording "Mark selected stories as published" which would not work in singular form.

In an ideal world, inline actions would need two variables to do a good job of this - as Django does in many other places (verbose_name vs verbose_name_plural) etc. But this poses another problem in that the known-plural version is currently called description so it's not as simple as adding a new description_plural parameter.

Has been passed almost 1 year! Sorry djm, but finally I've returned to work on this! 🙏

~As I defined other different variable for detail view actions: detail_actions. The description could be written in singular way! That solve the problem?~

mgaligniana avatar Jul 09 '23 17:07 mgaligniana

@nessita I rollback the approach of the new variable! Now uses the same actions.

I mentioned in the forum the tests that I added to show the cases covered.

I also added some questions/doubts I have (💭 )

mgaligniana avatar Aug 23 '23 15:08 mgaligniana

Hi @nessita!

I applied the description_plural paramenter and added a new test.

Two questions/doubts:

  • I pass down the change parameter (to know if its a change or list view) until _get_action_description but not sure if it is the best approach. What do you think? Is there another clean way?
  • Following the forum discussion: The next step would be remove the Delete button? (as it was replaced by the action inside the dropdown)

Thank you! And sorry the delay!

mgaligniana avatar Oct 01 '23 02:10 mgaligniana

Hi @nessita! Just as a 🎗️, no rush. Any thoughts about https://github.com/django/django/pull/16012#issuecomment-1741925611 and https://github.com/django/django/pull/16012#discussion_r1342654481 ? To move forward and add more tests cases if its necessary. Thank you!

mgaligniana avatar Nov 04 '23 13:11 mgaligniana

Hi @nessita! Just as a 🎗️, no rush. Any thoughts about #16012 (comment) and #16012 (comment) ? To move forward and add more tests cases if its necessary. Thank you!

Hi! This this definitely in my ToDo list, but I've been prioritizing release blockers for 5.0 and some DjangoCon PRs (to capitalize on the momentum). I should be able to review this soon! Thank you for your patience :100:

nessita avatar Nov 06 '23 12:11 nessita

@sarahboyce Thank you Sarah!! Sorry for not answering previosly! For sure, I'm going back to work on this and update the Tarcker when is ready! 💪

mgaligniana avatar Apr 23 '24 12:04 mgaligniana