Creates StripeRefund event publishing #17
Conversation
|
@wwahammy can you update your branch with houdini/main? I had to make mine even with it because I needed some things that were only there. |
clarissalimab
force-pushed
the
feature/event-publisher/stripe-refund
branch
from
5c06c57 to
09362ec
Compare
wwahammy
force-pushed
the
feature/event-publisher/stripe-refund
branch
from
2142704 to
21afcec
Compare
|
Rebased! |
|
For this one, we need to also test |
clarissalimab
force-pushed
the
feature/event-publisher/stripe-refund
branch
2 times, most recently
from
7271db8 to
675742c
Compare
| # @refund_amount = 100 | ||
| # @fees = CalculateFees.for_single_amount(100) | ||
| # end | ||
| describe InsertRefunds do |
There was a problem hiding this comment.
@wwahammy, when I started to work on the tests for the event publishing on InsertRefunds.with_stripe, I saw that those tests were pending for a long time, so I tried to update them a little to work with the current behavior. I think the calculations for the refund aren't right so I just "xit" the tests, but I can update the class to work properly since I'm already working on things here.
There was a problem hiding this comment.
@clarissalimab This is a good idea to get these tests working!
I'm not sure I understand exactly what you're saying. In particular:
- What calculations aren't right?
- When you say "I can update the class to work properly since I'm already working on things here" what does this all refer to?
There was a problem hiding this comment.
What calculations aren't right?
I think the calculation for the refund isn't updated, actually I'm not sure if it isn't right, but I looked at the calculation in CC and it's different.
When you say "I can update the class to work properly since I'm already working on things here" what does this all refer to?
I had to add some code to it to make it work with transactions. I meant that if the calculation is wrong I can work on it to have it working as we want it to.
There was a problem hiding this comment.
Hmmm. I'm still not sure I understand what 'calculation for the refund' you're referring to.
There was a problem hiding this comment.
Because when we do the refund, there is a calculation for the fee, I thought it could be outdated because the tests were pending (and failing) for a long time. I updated the tests to work with the current behavior, but I think the correct thing to do would be to understand how the calculations should be and write the tests with that in mind instead.
wwahammy
force-pushed
the
feature/event-publisher/stripe-refund
branch
from
21afcec to
44fabbe
Compare
|
@clarissalimab what's the status here? |
|
@wwahammy I realized I forgot to update the tests as we discussed, I'm going to do it later and let you know. |
clarissalimab
force-pushed
the
feature/event-publisher/stripe-refund
branch
2 times, most recently
from
b93b881 to
4039b9f
Compare
There was a problem hiding this comment.
Great work as always @clarissalimab
The tests are currently failing and there's a few things in the comments that need addressing. It's like 90% of the way there though I think.
app/models/subtransaction_payment.rb
Outdated
| delegated_type :paymentable, | ||
| types: %w[OfflineTransactionCharge OfflineTransactionDispute OfflineTransactionRefund StripeCharge | ||
| StripeRefund] |
There was a problem hiding this comment.
Try to split the types onto separate lines so we don't end up with those weird spacing issues
lib/insert/insert_refunds.rb
Outdated
|
|
||
| refund = Refund.find(refund_row['id']) | ||
| refund_payment = Payment.find(payment_row['id']) | ||
| refund_payment.update(refund: refund) |
There was a problem hiding this comment.
I forgot to remove it, just did it
clarissalimab
force-pushed
the
feature/event-publisher/stripe-refund
branch
from
4039b9f to
d7b95a5
Compare
clarissalimab
force-pushed
the
feature/event-publisher/stripe-refund
branch
from
d7b95a5 to
022cf91
Compare
Closes tix #2977