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

[RefundPayment] Make the Refund customization easier #320

Merged
merged 7 commits into from
Jun 24, 2021

Conversation

GSadee
Copy link
Member

@GSadee GSadee commented Jun 24, 2021

No description provided.

@GSadee GSadee added the DX Issues and PRs aimed at improving Developer eXperience. label Jun 24, 2021
@GSadee GSadee requested a review from a team as a code owner June 24, 2021 06:14
@GSadee GSadee force-pushed the easen-customization branch from 7e21428 to 6cda314 Compare June 24, 2021 06:42
@GSadee GSadee changed the title [WIP][RefundPayment] Make the Refund customization easier [RefundPayment] Make the Refund customization easier Jun 24, 2021
UnitRefundTotalCalculatorInterface $unitRefundTotalCalculator
): void {
$this->beConstructedWith($unitRefundTotalCalculator);
public function let(RefundUnitsConverterInterface $refundUnitsConverter): void
Copy link
Member

Choose a reason for hiding this comment

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

public in spec 💃

Copy link
Member Author

Choose a reason for hiding this comment

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

Why not? public is a default one if there is not access modifier specified

spec/Factory/RefundPaymentFactorySpec.php Outdated Show resolved Hide resolved
spec/ProcessManager/RefundPaymentProcessManagerSpec.php Outdated Show resolved Hide resolved
src/Converter/RefundUnitsConverter.php Show resolved Hide resolved
Comment on lines +54 to +56
if (isset($unit['full'])) {
return null;
}
Copy link
Member

Choose a reason for hiding this comment

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

This is so bad :( I wonder, cannot we just always operate on the actual value? I don't know won't it need a avalanche of changes in other parts of the application, so could possibly be done as an improvement in a separate PR

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, but I didn't want to change the current behaviour, just extract some logic and improve extendability

@Zales0123 Zales0123 merged commit 07ca76d into Sylius:master Jun 24, 2021
@Zales0123
Copy link
Member

Thank you, Grzegorz! 🎉

lchrusciel added a commit to Sylius/Sylius that referenced this pull request Jun 24, 2021
…omization with improvements (AdamKasp, GSadee)

This PR was merged into the 1.9 branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Branch?         | 1.9
| Bug fix?        | no
| New feature?    | no
| BC breaks?      | no
| Deprecations?   | no
| Related tickets | bases on #12717 and waits for merge Sylius/RefundPlugin#320
| License         | MIT


Commits
-------

14689d7 Add cookbook about refund customization
3b689aa [Documentation] Add fixes to cookbook about refund customization
95ad5b3 [Documentation] Add fixes to cookbook about refund customization after refactoring factory
acd4b2d [Documentation] Add fixes to cookbook about refund customization after refactoring creator
1e1947e [Documentation] Minor fix to cookbook about refund customization
@GSadee GSadee deleted the easen-customization branch June 24, 2021 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX Issues and PRs aimed at improving Developer eXperience.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants