Opened 3 years ago

Closed 3 years 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


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.

Change History (8)

comment:1 Changed 3 years ago by charettes

Heres the POC.

Last edited 3 years ago by charettes (previous) (diff)

comment:2 Changed 3 years ago by akaariai

  • Triage Stage changed from Unreviewed to Accepted

Seems OK to me.

comment:3 Changed 3 years ago by charettes

  • Needs documentation unset
  • Patch needs improvement unset

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

Version 0, edited 3 years ago by charettes (next)

comment:4 Changed 3 years 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 3 years 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 3 years 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 3 years ago by charettes

Added validation and removed obvious warning.

comment:8 Changed 3 years 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.

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