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

Fix 1955 edit user #2191

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Conversation

alli83
Copy link
Collaborator

@alli83 alli83 commented Feb 11, 2025

Pull request for issue: #1955

This is a pull request for the following functionalities:
add a regex for password
password is optional when I want to edit an user
captcha not required for update

How to test?

Given I am on the user admin form for a user
When I change its details
And I click Save
Then the details is saved to the database
And I am not asked to fill a captcha
Given I am on the user admin form for a user
When I change its details
And I click Save
Then the details is saved to the database
And I am not forced to provide a password for that user

How have functionalities been implemented?

Any issues with implementation?

Any changes to automated tests?

test added

Any changes to documentation?

Any technical debt repayment?

Any improvements to CI/CD pipeline?

@alli83
Copy link
Collaborator Author

alli83 commented Feb 11, 2025

imho, the separate model for changing one's own password is not required. I kept it because the goal here is to fix the bug but I believe it can be rewritten

@rija
Copy link
Contributor

rija commented Feb 17, 2025

@alli83 can you add which GitHub issue (issue number to they can be linked automatically) this PR is for, thanks.


if ($user->save(false)) {

Yii::app()->user->setFlash('notice', 'Updated');
$this->redirect(array('user/show/id/'.$user->id));
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @alli83,

issue: When the form is updated, the page will be redirected to a broken page like this:
image

suggestion: Would it be better to redirect page to the view page as below:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Reviewing Required
Development

Successfully merging this pull request may close these issues.

3 participants