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

Make Save button disabled after canceling User's password changes #6365

Merged

Conversation

hstastna
Copy link
Contributor

@hstastna hstastna commented Oct 31, 2019

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1762791

There was a bug regarding responding of Save button (Save and Reset buttons are together enabled or disabled like one) on changes of User's password: after clicking on Cancel button for canceling editing the password (not canceling editing User's info!), Save button was still enabled even when there wasn't any change in the User's info, and after clicking on Save error occurred.

The core of the problem is value vs. placeholder attribute of password_field_tag. Because of that, string with dots '●●●●●●●●' appeared in params[:password] after clicking on Cancel button, so then this value was copied to @edit[:new][:password] (which was nil originally, at the beginning of editing User's info) and here changed variable was set to true, so then Save, Reset buttons remained enabled.

Here is the place where we've set dots as a value, after canceling editing password. I don't see any reason for it. Also, the name of the variable storedPasswordPlaceholder used there also suggests that it's about setting the placeholder, not value.


Editing User's password - Save, Reset buttons enabled as expected:
cancel_passwd

Before: (after canceling User's password change Save, Reset buttons still enabled!)
before-after_cancel_passwd

After: (after canceling User's password change Save, Reset buttons disabled as expected)
after_cancel_passwd

@miq-bot
Copy link
Member

miq-bot commented Oct 31, 2019

Checked commit hstastna@79226e4 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 👍

@hstastna
Copy link
Contributor Author

hstastna commented Nov 7, 2019

@ZitaNemeckova @h-kataria @himdel Could you, please, review the changes for fixing the BZ or ping anyone else? I will appreciate any feedback on this PR. Thanks! :)

@ZitaNemeckova
Copy link
Contributor

Tested in UI. LGMT 👍

@h-kataria h-kataria self-assigned this Nov 11, 2019
@h-kataria h-kataria added this to the Sprint 124 Ending Nov 11, 2019 milestone Nov 11, 2019
@h-kataria h-kataria merged commit 5440f38 into ManageIQ:master Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants