Skip to content

#65 Fix for partial refund of shipping fees#94

Merged
Zales0123 merged 3 commits intoSylius:masterfrom
laurent35240:65_fix_partial_refund_shipping
Nov 2, 2018
Merged

#65 Fix for partial refund of shipping fees#94
Zales0123 merged 3 commits intoSylius:masterfrom
laurent35240:65_fix_partial_refund_shipping

Conversation

@laurent35240
Copy link
Contributor

@laurent35240 laurent35240 commented Oct 28, 2018

Fix for #65

@laurent35240 laurent35240 requested a review from a team as a code owner October 28, 2018 16:45
@laurent35240 laurent35240 force-pushed the 65_fix_partial_refund_shipping branch from a971f01 to 8932a91 Compare October 28, 2018 16:55
Copy link
Contributor

@Zales0123 Zales0123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this fix Laurent! To fix it, we need two more things:

  • ShipmentCreditMemoGeneratorSpec adjusted (we need to check one more exception throw)
  • new scenario in having_credit_memo_generated.feature for partial shipment refund credit memo

Do you think you'll be able to do that? One more time, thank you for the contribution!

@Zales0123 Zales0123 added Bug Confirmed bugs or bugfixes. Hacktoberfest labels Oct 29, 2018
@laurent35240
Copy link
Contributor Author

Sure I will try to add this

@laurent35240
Copy link
Contributor Author

I managed to do refacto and add phpSpec test but I am stuck for the moment with behat.
I did not find yet the proper way to run behat locally. I will try more probably tomorrow

@laurent35240 laurent35240 force-pushed the 65_fix_partial_refund_shipping branch from 88e674b to b871b52 Compare November 2, 2018 09:55
@laurent35240
Copy link
Contributor Author

laurent35240 commented Nov 2, 2018

@Zales0123 Behat test added. All clear for final code review 😉

Copy link
Contributor

@Zales0123 Zales0123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two small issues that can be fixed in a separate PR :)

And it should be issued in "United States" channel
And its total should be "$5.50"


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doubled blank line

And the customer bought 2 "Mr. Meeseeks T-Shirt" products
And the customer chose "Galaxy Post" shipping method to "United States" with "Space money" payment
And the order "#00000023" is already paid
And I am logged in as an administrator
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would move this one to the end of the Background

@Zales0123 Zales0123 merged commit a4bed5f into Sylius:master Nov 2, 2018
@Zales0123
Copy link
Contributor

Seems nice, thank you, Laurent, for this contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Confirmed bugs or bugfixes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants