Opened 16 years ago

Last modified 10 years ago

#10964 new New feature

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

Reported by: Filip Gruszczyński Owned by: nobody
Component: Forms Version: dev
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

Description

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 Filip Gruszczyński 16 years ago.
10964_r17068.diff (8.4 KB ) - added by Koen Biermans 13 years ago.
reverse m2m on forms + in admin with list_horizontal

Download all attachments as: .zip

Change History (17)

comment:1 by Filip Gruszczyński, 16 years ago

Has patch: set
Needs tests: set
Version: 1.1-beta-1SVN

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.

by Filip Gruszczyński, 16 years ago

Attachment: group_admin.patch added

comment:2 by Alex Gaynor, 15 years ago

Triage Stage: UnreviewedDesign decision needed

comment:3 by _roman, 14 years ago

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 by Luke Plant, 14 years ago

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 by Filip Gruszczyński, 14 years ago

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 by Julien Phalip, 14 years ago

Severity: Normal
Type: New feature

comment:7 by Julien Phalip, 13 years ago

UI/UX: set

by Koen Biermans, 13 years ago

Attachment: 10964_r17068.diff added

reverse m2m on forms + in admin with list_horizontal

comment:8 by Koen Biermans, 13 years ago

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 by Aymeric Augustin, 13 years ago

Triage Stage: Design decision neededAccepted

I agree that the idea in general is good.

comment:10 by Aymeric Augustin, 13 years ago

Component: contrib.admincontrib.admindocs

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

comment:11 by Aymeric Augustin, 13 years ago

Component: contrib.admindocscontrib.auth

Typo :(

comment:12 by Koen Biermans, 11 years ago

I worked on this again. Progress is here: https://github.com/planop/django/tree/ticket_10964

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 by Koen Biermans <koen@…>, 11 years ago

Component: contrib.authForms
Keywords: reverse M2M added
Needs documentation: unset
Needs tests: unset

WIP still here: https://github.com/planop/django/tree/ticket_10964

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 by Collin Anderson, 10 years ago

The is basically a duplicate of #897

comment:15 by Collin Anderson, 10 years ago

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