Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for post-save signals that depend on related objects #329

Merged

Conversation

hancush
Copy link
Contributor

@hancush hancush commented Mar 3, 2020

Description

This PR adds a line to save new objects after related objects are created. This allows use of Django's post_save signal to perform operations that depend on related objects.

Take our use case as an example: We want to use some custom logic to determine a bill's last action date. This logic depends on two types of related objects, bill actions and event related entities. The change in this PR allows us to query related objects after Event and Bill save in order to determine the appropriate last action date.

As ever, thanks for your work!

@jamesturk
Copy link
Member

I'm curious if it'd make sense to just fire the signal artificially in this case & skip the database write. From a quick glance it looks like that should be safe here, any thoughts on pros/cons?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 94.772% when pulling d2b768f on hancush:feature/hec/post-save-signal-support into 195a68c on opencivicdata:master.

@coveralls
Copy link

coveralls commented Mar 3, 2020

Coverage Status

Coverage increased (+0.007%) to 94.775% when pulling 86006cf on hancush:feature/hec/post-save-signal-support into 195a68c on opencivicdata:master.

@hancush
Copy link
Contributor Author

hancush commented Mar 3, 2020

@jamesturk That's a great point. The only con I can think of is that artificially firing post_save would only support post-save signals that expect related objects, but I can't imagine you'd want to write a pre-save signal that expects data to already exist. Let me give it a shot.

@hancush
Copy link
Contributor Author

hancush commented Mar 3, 2020

@jamesturk Firing the signal directly seems to work as expected! Revised per your suggestion in 86006cf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants