Opened 11 years ago

Closed 11 years ago

#21391 closed New feature (fixed)

Allow model signal sender to be specified lazily

Reported by: Simon Charette Owned by: nobody
Component: Database layer (models, ORM) Version: dev
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.

Change History (8)

comment:1 by Simon Charette, 11 years ago

Heres the POC.

Last edited 11 years ago by Simon Charette (previous) (diff)

comment:2 by Anssi Kääriäinen, 11 years ago

Triage Stage: UnreviewedAccepted

Seems OK to me.

comment:3 by Simon Charette, 11 years ago

Needs documentation: unset
Patch needs improvement: unset

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

Version 0, edited 11 years ago by Simon Charette (next)

comment:4 by loic84, 11 years ago

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 by Anssi Kääriäinen, 11 years ago

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 by Simon Charette, 11 years ago

@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 by Simon Charette, 11 years ago

Added validation and removed obvious warning.

comment:8 by Simon Charette <charette.s@…>, 11 years ago

Resolution: fixed
Status: newclosed

In eb38257e5199ca06b8b32f82dd50f64fd6b0d98d:

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

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