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

Added hooks to templates for relationships. #1348

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mmitten
Copy link

@mmitten mmitten commented Feb 1, 2024

My attempt to resolve #445

Added into the templates for relationships as I did not see any other place that made sense for how the hooks otherwise behaved. Happy to follow up if a something was missed.

@stephenafamo
Copy link
Collaborator

While I see this as a good change, I wonder if this may cause issues for those who are used to the current behaviour 🤔

@mmitten
Copy link
Author

mmitten commented Mar 27, 2024

Now that you mention it, it could be a breaking change, as adding another hook trigger where it was previously not expected could lead to some unpleasant side effects.

I am not opposed to tabling this for a major version revision, as the use case I wrote it for has since been resolved with something else.

@stephenafamo
Copy link
Collaborator

I'll label this as breaking and may add it to an eventual v5 release.

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

Successfully merging this pull request may close these issues.

Proposal: doBeforeUpdateHooks in SetRelation function
2 participants