CiviCRM core issue: Refund not communicated back to CiviCRM properly

This is a core issue but I thought it was working. See dev/financial#87 for background on "partial" refunds but this was a full refund so I expected to see the contribution status change to Refunded.

I also started some work around improving the CiviCRM UI in relation to showing the actual state: dev/financial#103 as it would be much more useful to show the contribution amount as well as the actual amount paid without having to click multiple times.

added Needs funding enhancement labels

changed title from Refund not communicated back to CiviCRM properly to CiviCRM core issue: Refund not communicated back to CiviCRM properly

Related to #82 (closed)

It definitely used to work when initiated on Stripe side as of 6.4.2

@andyburns Just to confirm the actual issue here. You issued a full refund but the contribution stayed "Completed" instead of changing to "Refunded". It is CiviCRM core that is meant to change the contribution to "Refunded" so if that's not happening maybe we need to investigate. Did you change CiviCRM versions as well or does it behave differently on the same version with 6.4.2 and 6.5?

Correct. Civi 5.28.3 with Stripe 6.5. The Civi version has been constant so yes I do believe that 6.4.2 this was working. I can re-test on staging to confirm that.

Hi @mattwire @andyburns - I tracked down the exact cause of this today.

Matt is correct that he submitted PR 16148 to handle this exact scenario (i.e. set Contribution status to "Refunded" if the total refunds equal the contribution amount).

However, this function returns early if the cancelled_payment_id is set. Stripe 6.5 sets that value (as it should). So that code never runs.

I'm going to try patching this locally in core by moving the code from PR 16148 up above the return, and if it works I'll write a PR/test. I'm not 100% sure this is the ideal solution, but creating the test will let folks refactor safely.

mentioned in issue dev/financial#156 (closed)

OK, after reading the PR that causes the early return I have a lot more confidence in my solution. I'm going to put it into production as soon as tests pass on Github.

Andy, if you want to do the same, the PR is here: https://github.com/civicrm/civicrm-core/pull/18930

Matt, not sure if you want to close this because the issue lies in core, I'll leave that to you.