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

[10.0][IMP] shopinvader/guest_mode: Allows to update profile in guest mode #598

Merged
merged 2 commits into from
Apr 3, 2020
Merged

[10.0][IMP] shopinvader/guest_mode: Allows to update profile in guest mode #598

merged 2 commits into from
Apr 3, 2020

Conversation

rousseldenis
Copy link
Contributor

@rousseldenis rousseldenis commented Mar 23, 2020 •
edited
Loading

If the profile address is updated, the email must be returned in store_cache
as it can leads to partner session inconsistency.

customer['email'] must be returned in store_cache. Due to https://github.com/shopinvader/locomotive-shopinvader/blob/d7cb7f14e9943699551e8eabd439e93c3d48c5e2/lib/shop_invader/services/erp_service.rb#L195

@rousseldenis
[10.0][IMP] shopinvader: Use shopinvader_response in address update
5f3180c
@rousseldenis
[10.0][IMP] shopinvader_guest_mode: Allows profile address update
3c10e8d 
If the profile address is updated, the email must be returned in store_cache
as it can leads to partner session inconsistency
@rousseldenis rousseldenis added this to the 10.0 milestone Mar 23, 2020
@rousseldenis
Copy link
Contributor Author

@Cedric-Pigeon

@Cedric-Pigeon
Copy link

@rousseldenis Could you please add a small description to let the others understand what the issue is...?

Cedric-Pigeon
Cedric-Pigeon approved these changes Mar 24, 2020
View reviewed changes
Copy link

@Cedric-Pigeon Cedric-Pigeon left a comment

Choose a reason for hiding this comment

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

Code review

@rousseldenis
Copy link
Contributor Author

Yes I did it in commit message but didn't copy it in PR description

sebastienbeau
sebastienbeau approved these changes Mar 27, 2020
View reviewed changes
Copy link
Contributor

@sebastienbeau sebastienbeau left a comment

Choose a reason for hiding this comment

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

Thanks for the fix with test (code review)

@sebastienbeau
Copy link
Contributor

/ocabot merge patch

@shopinvader-git-bot
Copy link

This PR looks fantastic, let's merge it!
Prepared branch 10.0-ocabot-merge-pr-598-by-sebastienbeau-bump-patch, awaiting test results.

shopinvader-git-bot pushed a commit that referenced this pull request Mar 27, 2020
Merge PR #598 into 10.0
60f48be 
Signed-off-by sebastienbeau
@lmignon lmignon mentioned this pull request Mar 27, 2020
Closed
77 tasks
@shopinvader-git-bot
Copy link

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@sebastienbeau
Copy link
Contributor

/ocabot merge patch

@shopinvader-git-bot
Copy link

What a great day to merge this nice PR. Let's do it!
Prepared branch 10.0-ocabot-merge-pr-598-by-sebastienbeau-bump-patch, awaiting test results.

shopinvader-git-bot pushed a commit that referenced this pull request Mar 31, 2020
Merge PR #598 into 10.0
32b6844 
Signed-off-by sebastienbeau
@shopinvader-git-bot
Copy link

@sebastienbeau your merge command was aborted due to failed check(s), which you can inspect on this commit of 10.0-ocabot-merge-pr-598-by-sebastienbeau-bump-patch.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@sebastienbeau
Copy link
Contributor

/ocabot merge patch

@shopinvader-git-bot
Copy link

On my way to merge this fine PR!
Prepared branch 10.0-ocabot-merge-pr-598-by-sebastienbeau-bump-patch, awaiting test results.

shopinvader-git-bot pushed a commit that referenced this pull request Apr 3, 2020
Merge PR #598 into 10.0
b903a63 
Signed-off-by sebastienbeau
@shopinvader-git-bot
Copy link

@sebastienbeau your merge command was aborted due to failed check(s), which you can inspect on this commit of 10.0-ocabot-merge-pr-598-by-sebastienbeau-bump-patch.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@sebastienbeau
Copy link
Contributor

/ocabot merge patch

@shopinvader-git-bot
Copy link

On my way to merge this fine PR!
Prepared branch 10.0-ocabot-merge-pr-598-by-sebastienbeau-bump-patch, awaiting test results.

shopinvader-git-bot pushed a commit that referenced this pull request Apr 3, 2020
Merge PR #598 into 10.0
8907be5 
Signed-off-by sebastienbeau
@shopinvader-git-bot
Copy link

It looks like something changed on 10.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 10.0-ocabot-merge-pr-598-by-sebastienbeau-bump-patch, awaiting test results.

@shopinvader-git-bot shopinvader-git-bot merged commit b21173b into shopinvader:10.0 Apr 3, 2020
@shopinvader-git-bot
Copy link

Congratulations, your PR was merged at b81b921. Thanks a lot for contributing to shopinvader. ❤️

@sebastienbeau sebastienbeau mentioned this pull request Dec 7, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sebastienbeau sebastienbeau sebastienbeau approved these changes

@Cedric-Pigeon Cedric-Pigeon Cedric-Pigeon approved these changes

Assignees
No one assigned
Labels
approved enhancement merged 🎉 needs review ready to merge
Projects
None yet
Milestone
10.0
Development

Successfully merging this pull request may close these issues.
4 participants
@rousseldenis @Cedric-Pigeon @sebastienbeau @shopinvader-git-bot