Code

Opened 5 months ago

Closed 5 months ago

#21391 closed New feature (fixed)

Allow model signal sender to be specified lazily

Reported by: charettes Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

With the introduction of the swappable User model there is no sane way for a third party application to connect signals to it.

Hence I propose to make the Signal.connect method of model signals (pre|post_init, pre|post_save, pre|post_delete, m2m_changed) accept a model name ('app_label.ModelName') as its sender argument.

This should allow third party application to connect signals to the swappable user model by referring to it using settings.AUTH_USER_MODEL and is consistent with the suggest way of referring to it as the target of related fields.

Will be attaching a POC with missing documentation.

Attachments (0)

Change History (8)

comment:1 Changed 5 months ago by charettes

Version 0, edited 5 months ago by charettes (next)

comment:2 Changed 5 months ago by akaariai

  • Triage Stage changed from Unreviewed to Accepted

Seems OK to me.

comment:3 Changed 5 months ago by charettes

  • Needs documentation unset
  • Patch needs improvement unset

Updated PR with documentation. It should be ready for review.

Last edited 5 months ago by charettes (previous) (diff)

comment:4 Changed 5 months ago by loic84

This patch is pretty clever in its implementation and as someone who always use full application label it has bugged me a number of times that signals didn't support this notation.

Other than the minor comments I've made on the PR, I think it's RFC.

comment:5 Changed 5 months ago by akaariai

Looks good to me. The only question is that could we somehow detect if there typoed references waiting - maybe in model validation code. At that point models should be loaded, so unresolved references there should be errors.

This isn't critical to do. But loud failure is better than silent ignore if it is easy enough to do.

comment:6 Changed 5 months ago by charettes

@akaariai I thought of it when writing the initial patch and felt like this could be a feature for third party applications. (registering signals for application that might or might not be installed).

Looking at it back those applications can always look into settings.INSTALLED_APPS before attaching lazy signals while typoed reference is less of an edgecase and would be consistent with how related fields missing reference are validated.

I'll add validation.

comment:7 Changed 5 months ago by charettes

Added validation and removed obvious warning.

comment:8 Changed 5 months ago by Simon Charette <charette.s@…>

  • Resolution set to fixed
  • Status changed from new to closed

In eb38257e5199ca06b8b32f82dd50f64fd6b0d98d:

Fixed #21391 -- Allow model signals to lazily reference their senders.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.