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 version_changed signal #7878

Merged
merged 1 commit into from Feb 3, 2021
Merged

Add version_changed signal #7878

merged 1 commit into from Feb 3, 2021

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented Jan 28, 2021

This is useful to know when to purge the footer.
With ext#404 we now purge the footer from all versions when we purge a version as well.
So this is the only missing place left to purge the footer.

This is useful to know when to purge the footer.
With ext#404 we now purge the footer from all versions when we purge a version as well.
So this is the only missing place left to purge the footer.
@stsewd stsewd requested review from a team and ericholscher January 28, 2021 16:28
@@ -88,6 +89,8 @@ def save(self, commit=True):
obj = super().save(commit=commit)
if obj.active and not obj.built and not obj.uploaded:
trigger_build(project=obj.project, version=obj)
if self.has_changed():
version_changed.send(sender=self.__class__, version=obj)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we not want this on the model instead of the form? Seems like having it in the model save method is better, unless there's a reason we can't do it there.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Models don't keep track of changed data. We could do this by overriding the init method of the models, but isn't guaranteed that the form will use the model in that way.

@stsewd stsewd merged commit 655bfd5 into master Feb 3, 2021
3 checks passed
@stsewd stsewd deleted the add-version-changed-signal branch February 3, 2021 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants