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

[API]maintaining cart after login and registration #11984

Merged
merged 8 commits into from
Nov 5, 2020
Merged

[API]maintaining cart after login and registration #11984

merged 8 commits into from
Nov 5, 2020

Conversation

arti0090
Copy link
Contributor

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
License MIT

@arti0090 arti0090 added Feature New feature proposals. API APIs related issues and PRs. labels Oct 31, 2020
@arti0090 arti0090 requested a review from a team as a code owner October 31, 2020 19:38
@lchrusciel lchrusciel mentioned this pull request Nov 3, 2020
Merged
@arti0090 arti0090 force-pushed the api-maintain-cart branch 2 times, most recently from 0717ef1 to e89a422 Compare November 4, 2020 07:17
lchrusciel
lchrusciel reviewed Nov 4, 2020
View reviewed changes
src/Sylius/Bundle/ApiBundle/CommandHandler/PickupCartHandler.php Outdated Show resolved Hide resolved
src/Sylius/Bundle/CoreBundle/EventListener/CartBlamerListener.php Outdated Show resolved Hide resolved
src/Sylius/Bundle/ApiBundle/CommandHandler/RegisterShopUserHandler.php
private function handleVerification(ShopUserInterface $user): void
{
/** @var ChannelInterface $channel */
$channel = $this->channelContext->getChannel();
Copy link
Member

Choose a reason for hiding this comment

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

In the long term, I would prefer to skip the usage of contexts in handlers. I think we should put data like channel into command.

Copy link
Contributor Author

@arti0090 arti0090 Nov 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm, do you have any resolution for this that we can implement now? Creating this function i was basing on ShopBundle->UserRegistrationListener. Should we create something similar or we leave it as it is and go back to it, when we make API user verification feature? @lchrusciel

GSadee added a commit that referenced this pull request Nov 5, 2020
@GSadee
feature #11998 [API][Shop] Logging in after changing password impleme…
ab9ad05 
…ntation (lchrusciel, arti0090)

This PR was merged into the 1.9-dev branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Branch?         | master
| Bug fix?        | no
| New feature?    | yes
| BC breaks?      | no
| Deprecations?   | no
| Related tickets | 
| License         | MIT

Based on #11984 and #11994 

<!--
 - Bug fixes must be submitted against the 1.7 or 1.8 branch (the lowest possible)
 - Features and deprecations must be submitted against the master branch
 - Make sure that the correct base branch is set

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->


Commits
-------

00822a8 [Behat][Account] Add scenario for logging in after password change
ec72ab4 Api changing password
13dee5a first iteration of fixes
b263090 changed tags in behat steps
2a60d1f add validation to password change
09d834b [API][Account] Adjust password change to chosen structure
461b6df [Minor] Adjust command and handler naming
955a060 [API] Extract user context and revamp change password handler
d48ceea [API][Shop] Logging in after changing password implementation
@GSadee GSadee merged commit 7ad22e0 into Sylius:master Nov 5, 2020
@GSadee
Copy link
Member

GSadee commented Nov 5, 2020

Thank you, @arti0090! 🎉

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

@lchrusciel lchrusciel lchrusciel left review comments

@AdamKasp AdamKasp AdamKasp approved these changes

@GSadee GSadee GSadee approved these changes

Assignees
No one assigned
Labels
API APIs related issues and PRs. Feature New feature proposals.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@arti0090 @GSadee @lchrusciel @AdamKasp