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

[API] changing password #11962

Closed
wants to merge 3 commits into from
Closed

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 the API APIs related issues and PRs. label Oct 26, 2020
@arti0090 arti0090 requested a review from a team as a code owner October 26, 2020 11:19
@@ -6,12 +6,12 @@ Feature: Changing a customer password

Background:
Given the store operates on a single channel in "United States"
And there is a customer "Francis Underwood" identified by an email "francis@underwood.com" and a password "whitehouse"
And there is a customer "Francis Underwood" identified by an email "francis@underwood.com" and a password "sylius"
Copy link
Member

Choose a reason for hiding this comment

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

Ok but unnecessary change for this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

json_encode(['email' => $user->getEmail(), 'password' => 'sylius'])

This is why it was easier to change password than refactoring for any password. We can think about this change in another PR maybe

@@ -144,6 +144,28 @@ public static function custom(string $url, string $method): RequestInterface
);
}

public static function customUpdate(?string $section, ?string $resource, ?string $id = null, ?string $urlResource = null, ?string $token = null): RequestInterface
Copy link
Member

Choose a reason for hiding this comment

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

Why did you need this method? Couldn't you use update or custom method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

custom is using ['CONTENT_TYPE' => 'application/merge-patch+json'] which does not work with this update.
And update is not designated for collection command but for item operation ( it needs id).

Other way to solve it, i can use /** @var AbstractBrowser */ and create custom request $this->client->request(), but it might cause the all accountContext to be changed. Wdyt @GSadee ??

Copy link
Member

Choose a reason for hiding this comment

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

Naming is misleading in this case. By default, POST operation suggests the creation of the object. This endpoint is declared as POST and it is a collection operation, which is not exactly true. We are operating on a particular item, but we wanted to skip user id from URI (and have account endpoint).

src/Sylius/Behat/Context/Api/Shop/AccountContext.php Outdated Show resolved Hide resolved
src/Sylius/Behat/Context/Api/Shop/CustomerContext.php Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API APIs related issues and PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants