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

Bugfix: path and fx handler #189

Closed

Conversation

samroberton
Copy link
Contributor

If there is no :db effect produced by an effectful handler (presumably
because the handler is concerned with some other form of side-effect
than a DB update), then the path interceptor should not attempt to
update the DB value it returns.

If there is no `:db` effect produced by an effectful handler (presumably
because the handler is concerned with some other form of side-effect
than a DB update), then the `path` interceptor should not attempt to
update the DB value it returns.
mike-thompson-day8 added a commit that referenced this pull request Aug 8, 2016
@mike-thompson-day8
Copy link
Contributor

@samroberton a test was failing (false looked like a nil) so I created this alternative fix 6a172dd
Does it cover the problem usecases you identified?

@samroberton
Copy link
Contributor Author

@mike-thompson-day8 your commit is definitely a better fix (and yes, it does fix the issue I observed in our codebase) -- especially in that it also allows you to return nil to indicate that you want this whole path to be set to nil.

One suggestion, though: with this facility now available, the debug interceptor should use the new arity as well (on std_interceptors.clj lines 38 & 39). One of the reasons this bug took me a while to track down was that the debug interceptor was telling me that there was no DB change happening, then suddenly bits of my DB were ending up nil -- ie inconsistency between what debug reports is happening, and what actually happens.

@samroberton
Copy link
Contributor Author

Closing since 6a172dd covers the original bug, and 486c7e6 covers the suggestion to get the debug interceptor to behave the same way.

Thanks for this, Mike.

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