Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backend] Fix issue refunding uncompleted payments #6094

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

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

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.

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.

4 participants