Fix bug in Claiming introduced in acd210de08 #946
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Organization was added as a required argument to EmailPeer, but not supplied by ClaimContribution and ClaimsController. This went uncaught because claim_contribution_spec was mocking EmailPeer. Changed in this commit to let ClaimContribution call composite interactions as it normally would.
maebeale
reviewed
Apr 20, 2021
| peer_alias: 'alias', | ||
| message: 'message', | ||
| ) | ||
| end | ||
|
|
||
| it "matches contribution and sends an email to peer", :aggregate_failures do | ||
| expect(MatchContribution).to have_received(:run!) |
There was a problem hiding this comment.
Do we use this elsewhere?
There was a problem hiding this comment.
Do you mean MatchContribution? EmailPeer?
There was a problem hiding this comment.
I meant the first, but was wondering re both while reviewing from my phone. :)
There was a problem hiding this comment.
Thanks! If you're asking about testing, this example is replaced by the two below and changed from being mocked to checking the side-effects of MatchContribution and EmailPeer.
As for production code, both are only used in ClaimContribution.
svileshina
approved these changes
Apr 22, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why
Fixes a bug in peer-to-peer claims.
What
acd210d introduced a bug where
Organizationwas added as a required argument toEmailPeer, but notsupplied by
ClaimContributionandClaimsController.Testing
This bug went uncaught because
claim_contribution_specwas mocking EmailPeer. Changing the spec to letClaimContributioncall composite interactions as it normally would surfaces any such integration problems.Next Steps
?
Outstanding Questions, Concerns and Other Notes
?
Pre-Merge Checklist