Opened 3 years ago

Closed 3 years ago

#32695 closed Cleanup/optimization (wontfix)

Create Permissions with create() instead of bulk_create() to send signal

Reported by: BRISBOIS Laurent Owned by: BRISBOIS Laurent
Component: contrib.auth Version: 3.2
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Hello there !

It seems Django is creating new Permission records in a post_migrate signal handler, especially in the method create_permissions which is called here.

It would be great if it was possible to act on Permission creation. I mean by using create() instead of bulk_create(), pre_save() and post_save() signals would be sent and therefore it would be possible to, for instance, assign a Permission once it is created with the post_save() signal.

I guess the only think to do would be to change this line.

I could even do it if you say it will be accepted.

Otherwise how could it be possible to achieve it ? I mean assign a Permission once it is created..?

Kind regards,

A Django Lover.

Change History (8)

comment:1 by BRISBOIS Laurent, 3 years ago

Owner: changed from nobody to BRISBOIS Laurent
Status: newassigned

comment:2 by BRISBOIS Laurent, 3 years ago

comment:3 by Tim Graham, 3 years ago

Permission creation was changed from create() to bulk_create() in 7deb25b8dd5aa1ed02b5e30cbc67cd1fb0c3d6e6 (#7596).

comment:4 by BRISBOIS Laurent, 3 years ago

I see that it seems to be only a performance reason that was behind this original change.
But what if people need to react on this (in my opinion) important step which is the creation of the Permissions ?

I think it should be considered again to put create() back here

comment:5 by BRISBOIS Laurent, 3 years ago

Has patch: set
Triage Stage: UnreviewedReady for checkin

comment:6 by Tim Graham, 3 years ago

Triage Stage: Ready for checkinUnreviewed

Please don't mark your own ticket and patch as ready for checkin. The ticket should be triaged and the patch reviewed by another person.

comment:7 by BRISBOIS Laurent, 3 years ago

@Tim Graham Huh sorry I read this and I may have made a mistake. My apologies. I didn't intend to force it at all.

comment:8 by Mariusz Felisiak, 3 years ago

Resolution: wontfix
Status: assignedclosed

But what if people need to react on this (in my opinion) important step which is the creation of the Permissions?

I think it's really niche because no one has reported this in the last 10 years. You should be able to add e.g. a post_migrate handler or a scheduler job and check for new permissions. You can also try to ask for alternatives on one of support channels. I don't think this is worth changing at the moment.

Moreover, as far as I'm aware your issue will be fixed by #29843, which propose including permissions in migrations files.

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