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

Proposal: doBeforeUpdateHooks in SetRelation function #445

Open
soolaimon opened this issue Dec 17, 2018 · 4 comments · May be fixed by #1348
Open

Proposal: doBeforeUpdateHooks in SetRelation function #445

soolaimon opened this issue Dec 17, 2018 · 4 comments · May be fixed by #1348

Comments

@soolaimon
Copy link

soolaimon commented Dec 17, 2018

I have a validation function I run as a BeforeUpdateHook on one model. It runs some checks an associated model (in the context of the local model), and it would improve code flow if I could rely on

Local.SetForeign(_, _, foreignObject)

rather than setting the foreign ID and calling local.Update.

I'd love to take a stab at this if you don't have any objections to it.

@aarondl
Copy link
Member

aarondl commented Dec 25, 2018

You're saying you would like SetForeign to actually make a call to the database? I was fairly certain that that already occurs. Though I think I'm not understanding exactly what you're bringing up here.

@soolaimon
Copy link
Author

soolaimon commented Dec 27, 2018

I'd like SetForeign to run beforeUpdateHooks, since it is actually updating the local table by setting a foreign key.

@aarondl
Copy link
Member

aarondl commented Dec 31, 2018

Ah yes. I now understand. That's a very good point that hooks are not run for these special operations. It makes sense to do that.

It also makes sense at some point to be able to opt out of hooks and timestamps by using a context value. This is of course a separate change but I wanted to mention it because I've only just now thought of it seeing as we're adding hooks in places people might not expect (or at least is a change from what's happening now)

@aarondl
Copy link
Member

aarondl commented Jan 2, 2019

FWIW I added new boil.SkipTimestamps and boil.SkipHooks features to compliment this change.

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

Successfully merging a pull request may close this issue.

2 participants