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.

Sign up for GitHub

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

Jump to bottom

SUPESC-135: Deprecated ControllerProvider, added RouteProviderPlugin and updated code in accordance to code sniffer and phpstan. #16

Open
wants to merge 47 commits into
base: master
Choose a base branch
from
Open

SUPESC-135: Deprecated ControllerProvider, added RouteProviderPlugin and updated code in accordance to code sniffer and phpstan. #16

wants to merge 47 commits into from

Conversation

Hantixs
Copy link

@Hantixs Hantixs commented Jul 26, 2022 •
edited
Loading

Release Table

Module Release Type Constraint Updates
Easycredit minor

Module Easycredit

Change log

Improvements

  • Introduced EasycreditRouteProviderPlugin to be able to add an Easycredit success response endpoint to Router.
  • Adjusted EasycreditController::successEasycreditResponseAction() to use EasycreditRouteProviderPlugin instead of deprecated EasycreditControllerProvider.
  • Added ShopApplication module to dependencies.
  • Added Calculation module to dependencies.
  • Added Money module to dependencies.
  • Added Quote module to dependencies.
  • Added Transfer module to dependencies.
  • Introduced Payment.paymentSelection transfer property.
  • Introduced Payment.paymentProvider transfer property.
  • Introduced Payment.paymentMethod transfer property.
  • Introduced Quote.totals transfer property.
  • Introduced Quote.payment transfer property.
  • Introduced Quote.payments transfer property.
  • Introduced Quote.billingAddress transfer property.
  • Introduced Quote.items transfer property.
  • Introduced Quote.customer transfer property.
  • Introduced Quote.shippingAddress transfer property.
  • Introduced Quote.shipment transfer property.
  • Introduced Quote.expenses transfer property.
  • Introduced Expense transfer.
  • Introduced Totals transfer.
  • Introduced PaymentMethods transfer.
  • Introduced PaymentMethod transfer.
  • Introduced SaveOrder transfer.
  • Introduced Address transfer.
  • Introduced Customer transfer.
  • Introduced Shipment transfer.
  • Introduced Item transfer.

Deprecations

  • Deprecated EasycreditControllerProvider.

@Hantixs Hantixs changed the title SUPESC-135: Deprecated ControllerProvider and added RouteProviderPlugin. [WIP] SUPESC-135: Deprecated ControllerProvider and added RouteProviderPlugin. Jul 28, 2022
@Hantixs Hantixs changed the title [WIP] SUPESC-135: Deprecated ControllerProvider and added RouteProviderPlugin. SUPESC-135: Deprecated ControllerProvider, added RouteProviderPlugin and updated code in accordance to code sniffer and phpstan. Aug 4, 2022
@Hantixs Hantixs added bug Something isn't working request-review labels Aug 4, 2022
romanhavrylko
romanhavrylko reviewed Aug 9, 2022
View reviewed changes
Copy link

@romanhavrylko romanhavrylko left a comment

Choose a reason for hiding this comment

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

Please adjust changelogs according to https://spryker.atlassian.net/wiki/spaces/CORE/pages/523730976/Release+Notes+and+Changelogs

src/SprykerEco/Yves/Easycredit/Controller/EasycreditController.php Outdated Show resolved Hide resolved
src/SprykerEco/Yves/Easycredit/Plugin/Router/EasycreditRouteProviderPlugin.php Show resolved Hide resolved
src/SprykerEco/Yves/Easycredit/Processor/SuccessResponseProcessor.php Outdated Show resolved Hide resolved
src/SprykerEco/Zed/Easycredit/Communication/Plugin/Oms/Condition/IsOrderConfirmedPlugin.php Outdated Show resolved Hide resolved
...rykerEco/Zed/Easycredit/Communication/Plugin/Payment/EasycreditPaymentMethodFilterPlugin.php Show resolved Hide resolved
tests/SprykerEcoTest/Zed/Easycredit/EasycreditOrderIdentifierSaverTest.php Outdated Show resolved Hide resolved
romanhavrylko
romanhavrylko reviewed Aug 12, 2022
View reviewed changes
Copy link

@romanhavrylko romanhavrylko left a comment

Choose a reason for hiding this comment

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

Please adjust the changelogs

  1. src/SprykerEco/Yves/Easycredit/Plugin/Router/EasycreditRouteProviderPlugin.php -> EasycreditRouteProviderPlugin. Please also describe the functionality provided by this plugin.
  2. Deprecated src/SprykerEco/Yves/Easycredit/Plugin/Provider/EasycreditControllerProvider.php. -> Deprecated EasycreditControllerProvider plugin.
  3. src/SprykerEco/Yves/Easycredit/Controller/EasycreditController.php -> EasycreditController::successEasycreditResponseAction().
  4. src/SprykerEco/Yves/Easycredit/Plugin/Router/EasycreditRouteProviderPlugin.php -> EasycreditRouteProviderPlugin.
  5. src/SprykerEco/Yves/Easycredit/Plugin/Provider/EasycreditControllerProvider.php -> EasycreditControllerProvider.
  6. Usually, changelogs contain either the improvements or fixes section per module.
  7. Changes in EasycreditMapper should not be described in changelogs.

tests/SprykerEcoTest/Zed/Easycredit/_support/EasycreditTester.php Outdated Show resolved Hide resolved
composer.json Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
src/SprykerEco/Shared/Easycredit/Transfer/easycredit.transfer.xml Show resolved Hide resolved
DmitryLymarenko
DmitryLymarenko suggested changes Aug 17, 2022
View reviewed changes
Copy link

@DmitryLymarenko DmitryLymarenko left a comment

Choose a reason for hiding this comment

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

why does this PR contain 2 tickets?

kkicheglovspryker
kkicheglovspryker suggested changes Aug 17, 2022
View reviewed changes
src/SprykerEco/Shared/Easycredit/EasycreditConstants.php Show resolved Hide resolved
src/SprykerEco/Shared/Easycredit/Transfer/easycredit.transfer.xml Show resolved Hide resolved
src/SprykerEco/Zed/Easycredit/Communication/Plugin/Oms/Condition/IsOrderConfirmedPlugin.php Show resolved Hide resolved
src/SprykerEco/Zed/Easycredit/EasycreditDependencyProvider.php Outdated Show resolved Hide resolved
tests/SprykerEcoTest/Zed/Easycredit/_support/EasycreditTester.php Outdated Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
src/SprykerEco/Shared/Easycredit/Transfer/easycredit.transfer.xml Show resolved Hide resolved
src/SprykerEco/Yves/Easycredit/Handler/EasycreditPaymentHandler.php Show resolved Hide resolved
src/SprykerEco/Zed/Easycredit/Business/Mapper/EasycreditMapper.php Show resolved Hide resolved
@kkicheglovspryker
Copy link

Adjusted code in accordance to code-sniffer and phpstan.

This should not be on the change logs. Only high-level @api changes should be mentioned

@dereuromark
Copy link
Contributor

This was in review last year august with prio high
Whats the status on this now?

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

@DmitryLymarenko DmitryLymarenko DmitryLymarenko requested changes

@kkicheglovspryker kkicheglovspryker kkicheglovspryker approved these changes

@sprykerQwermus sprykerQwermus sprykerQwermus approved these changes

@romanhavrylko romanhavrylko romanhavrylko approved these changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
bug Something isn't working request-review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Hantixs @kkicheglovspryker @dereuromark @DmitryLymarenko @romanhavrylko @sprykerQwermus