Opened 7 years ago

Last modified 2 years ago

#10964 new New feature

Admin for group doesn't allow to easily add users to the group

Reported by: gruszczy Owned by: nobody
Component: Forms Version: master
Severity: Normal Keywords: admin, groups, reverse M2M
Cc: cmawebsite@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: yes


Admin website for group provides only possibility of setting the name and permission, but there is no widget for adding users (from the whole list of users) to the group. When there are lots of users in the system it's not useful, but if the website is intended for a smaller group, it's useful to easily add all users to the group.

Attachments (2)

group_admin.patch (5.5 KB) - added by gruszczy 7 years ago.
10964_r17068.diff (8.4 KB) - added by koenb 5 years ago.
reverse m2m on forms + in admin with list_horizontal

Download all attachments as: .zip

Change History (17)

comment:1 Changed 7 years ago by gruszczy

  • Has patch set
  • Needs documentation unset
  • Needs tests set
  • Patch needs improvement unset
  • Version changed from 1.1-beta-1 to SVN

Several days ago I created custom admin for my company software and I was allowed to turn it into a patch and post it here. It changes group admin forms, so users can be added to a group directly from there.

Changed 7 years ago by gruszczy

comment:2 Changed 7 years ago by Alex

  • Triage Stage changed from Unreviewed to Design decision needed

comment:3 Changed 5 years ago by _roman

Testing the patch revealed an error so I’d like to report about it. After patching admin website has a new widget to add user to the group. But when I click on the button save or save and add another or save and continue editing appears “error 403” with message – “CSRF verification failed. Request aborted” I've used Django v1.3

comment:4 Changed 5 years ago by lukeplant

The patch would need a lot of work before it could possibly go in. _roman highlights one issue (the csrf_protect_m decorator is not applied). There are lots of others, like:

  • the overridden add_view and change_view remove lots of functionality present in the base class methods, for no apparent reason.
  • why not just set the form = GroupForm on GroupAdmin?
  • why do we need to override the template?
  • 8 spaces for indentation instead of 4.

We also have to consider the case where people have further sub-classed GroupAdmin, or provided their own template - their customised admin interfaces should not break, as far as we can ensure that.

I think that the idea in general is good. We may need to consider whether a different approach entirely is needed - the basic problem is that while M2Ms are (nearly) symmetrical in terms of the API that is produced by the ORM on the two related models, irrespective of which model the M2M is defined on, the admin responds quite differently depending on which model has the M2M defined. This isn't ideal, as which model gets the M2M can be determined simply by the order of dependencies and things like that.

comment:5 Changed 5 years ago by gruszczy

As you can see, this patch is nearly 2 years old, when there was no CSRF protection and I was making my first steps in Django. All your remarks are of course right and this patch is no good.

If you are interested, I can try to provide a better patch now. But since this ticket got very little attention, I am not entirely sure, if you want it anyway. I wouldn't like to devote time to something, that might not be in accord with the overall deisgn.

comment:6 Changed 5 years ago by julien

  • Severity set to Normal
  • Type set to New feature

comment:7 Changed 5 years ago by julien

  • UI/UX set

Changed 5 years ago by koenb

reverse m2m on forms + in admin with list_horizontal

comment:8 Changed 5 years ago by koenb

  • Easy pickings unset
  • Needs documentation set
  • Patch needs improvement set

Added a patch that allows adding a descriptor of a reverse m2m (like 'user_set') to the fields list for a form.

An extra is that you can also add the same into the fields and filter_horizontal attributes of a modeladmin.

As an example this is used into the GroupAdmin to allow easy user selection for a group.

No tests or documentation yet, since I am not completely happy with the implementation yet.

I think an implementation for reverse foreign keys would also be usefull.

Feedback welcome.

comment:9 Changed 4 years ago by aaugustin

  • Triage Stage changed from Design decision needed to Accepted

I agree that the idea in general is good.

comment:10 Changed 4 years ago by aaugustin

  • Component changed from contrib.admin to contrib.admindocs

In fact, this is specific to the auth app, not to the admin.

comment:11 Changed 4 years ago by aaugustin

  • Component changed from contrib.admindocs to contrib.auth

Typo :(

comment:12 Changed 3 years ago by koenb

I worked on this again. Progress is here:

This is working:

  • in a modelform: in the fields list, use the descriptors for reverse M2M or FK, like 'book_set'. This will add a select multiple to the form.
  • in the admin: the same + add the descriptor name to filter_horizontal and filter_vertical to have the fancy select widget versions
  • applied this to the auth admin for the Group model so you can select users in the group creation and edit views('user_set' in fields and filter_horizontal)

There are a few tests and a beginning of documentation.

I am not entirely happy with the way the descriptors are detected, but I know of no better way at the moment.
Not sure this should go in to the auth admin by default either.

comment:13 Changed 3 years ago by Koen Biermans <koen@…>

  • Component changed from contrib.auth to Forms
  • Keywords reverse M2M added
  • Needs documentation unset
  • Needs tests unset

WIP still here:

Excluded non-nullable foreignkeys, because you would be unable to remove an item via the multiplechoicefield, which would be very surprising and does not make sense.

comment:14 Changed 2 years ago by CollinAnderson

The is basically a duplicate of #897

comment:15 Changed 2 years ago by CollinAnderson

  • Cc cmawebsite@… added

It seems to me if we want to support formfield_callback for these fields, the proper way would be create a virtual field for the reverse relationship. It would also be much easier to support custom widgets, localized_fields, labels, help_texts, error_messages, and editable.

Note: See TracTickets for help on using tickets.
Back to Top