Opened 9 years ago
Closed 9 years ago
#4879 closed (fixed)
post_create signal
| Reported by: | Owned by: | George Vilches | |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | master |
| Severity: | Keywords: | sprintsept14 | |
| Cc: | gav@… | Triage Stage: | Accepted |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | UI/UX: |
Description
Patch adds post_create signal to model's save() method. This signal is useful to interact with newly created objects.
Attachments (7)
Change History (14)
Changed 9 years ago by
| Attachment: | post_create_patch.diff added |
|---|
comment:1 Changed 9 years ago by
| Needs documentation: | unset |
|---|---|
| Needs tests: | unset |
| Patch needs improvement: | unset |
| Triage Stage: | Unreviewed → Accepted |
Changed 9 years ago by
| Attachment: | post_save_with_created_kw.patch added |
|---|
Adds a "created" flag to the post_save signal.
comment:3 Changed 9 years ago by
| Cc: | gav@… added |
|---|
+1 to mtredinnick's idea for just adding a flag to the post_save signal. The patch I attached does exactly this, and uses a variable that already existed in the save() method that held this information anyway (it was missing one place it needed to be set, so the patch handles that as well), so the additional overhead should be nothing.
Changed 9 years ago by
| Attachment: | post_save_with_created_kw_r5991.patch added |
|---|
Updated patch for post_save created signal against r5991.
Changed 9 years ago by
| Attachment: | django_unit_test_model_signals.diff added |
|---|
Unit test for all model signals, including testing the post_save created keyword
comment:4 Changed 9 years ago by
The django_unit_test_model_signals.diff includes a unit tests for the Django test suite covering the following signals:
pre_save post_save pre_delete post_delete
It verifies that these signals fire in the proper order, that they have the expected instance of the model, and that the created flag on post_save is correctly set.
As far as I could tell, tests for these signals do not exist anywhere.
comment:5 Changed 9 years ago by
| Owner: | changed from nobody to George Vilches |
|---|
I'll take responsibility for cleaning up this ticket for the sprint.
Changed 9 years ago by
| Attachment: | post_save_with_created_kw_r6184.patch added |
|---|
Post save patch + unit tests against r6184
comment:6 Changed 9 years ago by
| Keywords: | sprintsept14 added |
|---|
comment:7 Changed 9 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
There was a general consensus in a recent django-dev thread about this. No core developer spoke against it. The only question is whether to make it a new signal, or a flag attached to the post_save signal (signal emissions are not free). I'll have to rummage through the archives. Marking this as "accepted" though, since it's reasonable.