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

EZP-31825: Use "updateUserPassword" if newer ezplatform-kernel is available #80

Merged
merged 2 commits into from
Oct 22, 2020

Conversation

Steveb-p
Copy link
Contributor

Question Answer
Tickets EZP-31825
Bug fix? yes
New feature? yes
BC breaks? no
Tests pass? yes/no
Doc needed? no
License GPL-2.0

Related to ezsystems/ezplatform-kernel#117

Checklist:

  • Implement tests
  • Coding standards ($ composer fix-cs)

@Steveb-p Steveb-p changed the title EZP-31825: Use "updateUserPassword" is newer ezplatform-kernel is available EZP-31825: Use "updateUserPassword" if newer ezplatform-kernel is available Oct 15, 2020
@@ -78,7 +78,18 @@ public function userPasswordChangeAction(Request $request)
$newPassword = $data->getNewPassword();
$userUpdateStruct = $this->userService->newUserUpdateStruct();
$userUpdateStruct->password = $newPassword;
$this->userService->updateUser($user, $userUpdateStruct);
if (method_exists($this->userService, 'updateUserPassword')) {
Copy link
Contributor

@ViniTou ViniTou Oct 15, 2020

Choose a reason for hiding this comment

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

There is no point of doing it that way, both versions (this and kernel) gonna released at the same time. What you should do is rebase this PR against 2.1 branch.

Copy link
Member

@alongosz alongosz Oct 15, 2020

Choose a reason for hiding this comment

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

There is no point of doing it that way, both versions (this and kernel) gonna released at the same time. What you should do is rebase this PR against 2.1 branch.

@Steveb-p ☝️ bump required ezplatform-kernel version instead, please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alongosz @ViniTou what version should be targetted?

Copy link
Member

Choose a reason for hiding this comment

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

@@ -78,7 +78,18 @@ public function userPasswordChangeAction(Request $request)
$newPassword = $data->getNewPassword();
$userUpdateStruct = $this->userService->newUserUpdateStruct();
$userUpdateStruct->password = $newPassword;
$this->userService->updateUser($user, $userUpdateStruct);
if (method_exists($this->userService, 'updateUserPassword')) {
Copy link
Member

@alongosz alongosz Oct 15, 2020

Choose a reason for hiding this comment

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

There is no point of doing it that way, both versions (this and kernel) gonna released at the same time. What you should do is rebase this PR against 2.1 branch.

@Steveb-p ☝️ bump required ezplatform-kernel version instead, please

@Steveb-p Steveb-p marked this pull request as ready for review October 20, 2020 17:20
@Steveb-p Steveb-p requested a review from a team October 20, 2020 17:20
Copy link
Contributor

@tomaszszopinski tomaszszopinski left a comment

Choose a reason for hiding this comment

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

QA approved on eZPlatform-ee 3.1.2 with diff & path.

@lserwatka lserwatka merged commit 5352f23 into master Oct 22, 2020
@lserwatka lserwatka deleted the EZP-31825 branch October 22, 2020 11:46
Steveb-p added a commit that referenced this pull request Oct 22, 2020
…ilable (#80)

* EZP-31825: Use "updateUserPassword" is newer ezplatform-kernel is available

* Update password change and password reset controllers

(cherry picked from commit 5352f23)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants