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

Update PasswordExpireService.php #533

Merged
merged 4 commits into from
Dec 18, 2023
Merged

Update PasswordExpireService.php #533

merged 4 commits into from
Dec 18, 2023

Conversation

niciz
Copy link
Contributor

@niciz niciz commented Nov 21, 2023

Fix PasswordExpireService return error when user model attribute "password_changed_at" is already set at null.

Q A
Is bugfix? yes
New feature? no
Breaks BC? no
Tests pass? yes
Fixed issues

Fix PasswordExpireService return error when user model attribute "password_changed_at" is already set at null.
@maxxer
Copy link
Collaborator

maxxer commented Dec 15, 2023

Out of curiosity, what error is it throwing?

Please add entry in changelog.

@niciz
Copy link
Contributor Author

niciz commented Dec 15, 2023

`

/**
 * Forces the user to change password at next login
 * @param integer $id
 */
public function actionForcePasswordChange($id)
{
    /** @var User $user */
    $user = $this->userQuery->where(['id' => $id])->one();
    if ($this->make(PasswordExpireService::class, [$user])->run()) {
        Yii::$app->session->setFlash("success", Yii::t('usuario', 'User will be required to change password at next login'));
    } else {
        Yii::$app->session->setFlash("danger", Yii::t('usuario', 'There was an error in saving user'));
    }
    $this->redirect(['index']);
}

`

Is AdminController::actionForcePasswordChange() which set an error flash message when PasswordExpireService return false. But there's no error in PasswordExpireService, because user already has "password_changed_at" at NULL.

@maxxer
Copy link
Collaborator

maxxer commented Dec 15, 2023

Ok. So the if in the patch is not really required, you could just run the updateAttributes and always return true.

remove useless if statement
@maxxer
Copy link
Collaborator

maxxer commented Dec 15, 2023

please add changelog, then I'll merge. thanks

@maxxer maxxer merged commit c6148fb into 2amigos:master Dec 18, 2023
0 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants