Properly call destroy method so after_commit callback is called. #67
Conversation
nashby
force-pushed
the
after-commit-callback
branch
from
302777a to
0986c14
Compare
|
I fixed merge conflicts. |
test/test_helper.rb
Outdated
|
|
||
| attr_reader :after_commited | ||
|
|
||
| def foo |
There was a problem hiding this comment.
Thanks for the PR @nashby. Could you change the name of this method to something more descriptive. This will provide insight into why we're including this in the test_helper.rb down the line.
Something like set_after_committed_flag should do.
nashby
force-pushed
the
after-commit-callback
branch
from
0986c14 to
0488f44
Compare
|
@michaeldupuisjr thanks for the review! I've updated PR. |
| comment = Comment.create | ||
| comment.destroy | ||
|
|
||
| comment.after_committed.must_equal true |
There was a problem hiding this comment.
This spec does not demonstrate that destroying objects fires the after_commit callback. Indeed, it will still pass if line 266 is removed altogether. This is because the #create also runs the after_commit callbacks, so the flag is already set before #destroy is even run.
There was a problem hiding this comment.
hmm, but I have after_commit with on: :destroy option. It shouldn't be called on create, should it?
There was a problem hiding this comment.
Yes, I overlooked that; you are absolutely correct.
deletes original comment
|
Great minds think alike! 😉 It seems that I have duplicated this effort in #71. |
|
Sorry @nashby but it looks like this branch may need to be rebased. I'll get this merged in once tests are green. |
nashby
force-pushed
the
after-commit-callback
branch
from
0488f44 to
ae82efb
Compare
|
@michaeldupuisjr no problem. I rebased. Let's wait for specs to be 💚 . |
3 participants
I couldn't find an easy way to test that method is called in minitest though.
fixes #56