Skip to content

[Backend] Fix issue refunding uncompleted payments#6094

Merged
tvdeyen merged 2 commits intosolidusio:mainfrom
jtapia:bugfix/admin-payment-action-options
Apr 9, 2025
Merged

[Backend] Fix issue refunding uncompleted payments#6094
tvdeyen merged 2 commits intosolidusio:mainfrom
jtapia:bugfix/admin-payment-action-options

Conversation

@jtapia
Copy link
Contributor

@jtapia jtapia commented Jan 31, 2025

Summary

  • Remove Refund button for uncompleted payments
  • Remove Credit button for payment on void and invalid state

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

The following are not always needed:

  • 📖 I have updated the README to account for my changes.
  • 📑 I have documented new code with YARD.
  • 🛣️ I have opened a PR to update the guides.
  • ✅ I have added automated tests to cover my changes.
  • 📸 I have attached screenshots to demo visual changes.

@jtapia jtapia requested a review from a team as a code owner January 31, 2025 22:56
@github-actions github-actions bot added the changelog:solidus_backend Changes to the solidus_backend gem label Jan 31, 2025
@jtapia
Copy link
Contributor Author

jtapia commented Jan 31, 2025

In favor of #2932

@codecov
Copy link

codecov bot commented Jan 31, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.64%. Comparing base (a57f0c9) to head (4b11b05).
Report is 12 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6094      +/-   ##
==========================================
- Coverage   88.82%   86.64%   -2.19%     
==========================================
  Files         835      514     -321     
  Lines       18149    11891    -6258     
==========================================
- Hits        16121    10303    -5818     
+ Misses       2028     1588     -440     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jtapia jtapia changed the title Fix issue refunding uncompleted payments [Backend] Fix issue refunding uncompleted payments Jan 31, 2025
@fthobe
Copy link
Contributor

fthobe commented Feb 1, 2025

Hey @jtapia thank you for picking this up again after so much time. Seems cov was flaky. Thank you so much!

@jtapia jtapia force-pushed the bugfix/admin-payment-action-options branch from 7e914f7 to 4ce6200 Compare February 2, 2025 21:32
@jtapia
Copy link
Contributor Author

jtapia commented Feb 2, 2025

@fthobe I believe the broken test is a flaky one. Could someone help me re-running the process on CircleCI?

@kennyadsl
Copy link
Member

Hey @jtapia thanks for contributing. Can you put a :flaky on the spec definition that fails? This way we will be covered next time it happens. Thanks!

See

it 'prefills the customer addresses with the user addresses', :flaky do
and
config.around(:each, :flaky) do |example|
.

Thanks!

@jtapia jtapia force-pushed the bugfix/admin-payment-action-options branch from 4ce6200 to 6a19837 Compare February 4, 2025 00:49
@jtapia
Copy link
Contributor Author

jtapia commented Feb 4, 2025

@kennyadsl thanks, I have made the changes here: https://github.com/solidusio/solidus/pull/6094/files#diff-b8b2f84275223ed3dc20600049f3eba20911f6e12ed489c34be875590e272599R76, I noticed that codecov is still showing a decrease in coverage, even though it was only a view code change. Is there anything might be missing?

@kennyadsl
Copy link
Member

@jtapia thanks! If the view contains logic, it's perfectly legit for codecov to compute the coverage there. It means that we added some conditional paths that are not currently tested.

@tvdeyen tvdeyen force-pushed the bugfix/admin-payment-action-options branch from 6a19837 to bf82c87 Compare February 19, 2025 16:54
@jtapia jtapia force-pushed the bugfix/admin-payment-action-options branch from bf82c87 to 4b11b05 Compare February 23, 2025 17:45
@tvdeyen tvdeyen force-pushed the bugfix/admin-payment-action-options branch from 4b11b05 to a3199c1 Compare February 24, 2025 08:28
@fthobe
Copy link
Contributor

fthobe commented Feb 24, 2025

@tvdeyen we just closed an I believe 4 year old ticket with that 😅😅😅 @jtapia thank you so much for picking that up again.

@fthobe
Copy link
Contributor

fthobe commented Apr 7, 2025

@kennyadsl ready to be merged

@tvdeyen tvdeyen force-pushed the bugfix/admin-payment-action-options branch from a3199c1 to 3670ead Compare April 7, 2025 18:13
@tvdeyen tvdeyen enabled auto-merge April 7, 2025 18:13
@fthobe
Copy link
Contributor

fthobe commented Apr 7, 2025

@tvdeyen is there anything left to do or is this ok?

@tvdeyen tvdeyen disabled auto-merge April 7, 2025 20:04
@tvdeyen
Copy link
Member

tvdeyen commented Apr 7, 2025

@tvdeyen is there anything left to do or is this ok?

A second approval from @solidusio/core-team

@fthobe
Copy link
Contributor

fthobe commented Apr 9, 2025

@tvdeyen can we merge this given that requirements have been met. It's open for too much time.

@tvdeyen tvdeyen merged commit ceadf35 into solidusio:main Apr 9, 2025
19 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants