Implementing change password view

[douglasgabriel]

This PR is able to:

• Add a view to change password of a Physical Infra Provider

ems_physical_infra/show_list

ems_physical_infra/change_password/${id}

After click in save button

The user can follow the change password request status in the tasks page

This PR depends on:

• Adding feature to change Physical Infra Provider password manageiq#16883 - Merged
• Adding method to change password of a provider on its client manageiq#16895 - Merged

[AparnaKarve]

@douglasgabriel This looks more like a fronend validation.
Noticed that the form is not in angular.
Please convert the form to angular so that you can address these form-level validations using angular.

[AparnaKarve]

Typically flash messages are displayed when you transition to a new screen.
In this particular case, you need to stay right inside the form and report the error to the user. As mentioned above this is a form validation and can be best handled with angular directives. What needs to happen here is - the "Confirmation password is not equal to new password" message needs to be displayed right under the password field in red

[ohadlevy]

just wondering, do we need to show some indication of password strength? (e.g. weak, normal,, strong etc?)

[douglasgabriel]

Well, I think that this is a little bit more difficult to do, cause maybe all providers has its owns passwords rules. In some scenario, the MiQ UI can say that a password is strong, however the provider could reject this password because it break some password rule, what can make the user a bit confused. Don't you think? 🤔

My idea is to try change password in provider and if something goes wrong, catch the error message from provider and show to user

[douglasgabriel]

@miq-bot remove_label pending core

[AparnaKarve]

params[:id] is exposed here.
Hence can you add an rbac check for this?
Thanks.

[AparnaKarve]

Add a new line here

[AparnaKarve]

@douglasgabriel Tested this in the UI and this is looking really good.

A couple questions -

• If the new password is the same as the current password, should we add a frontend validation to say that the new password needs to be different than the current password?
• This might be a nit-pick, but do you think that the Confirm New password field should show only 1 validation message?
  For e.g. - It should be Required when there is no value in both New and Confirm
  And it should be only - Confirm password did not match when New is populated?

which actually brings me to my next question -
Are blank passwords supported for a Physical Infrastructure Provider?
If yes, then we may have to change the UX around this a bit.

[AparnaKarve]

@douglasgabriel Can you also take a look at the codeclimate issues?
I believe the first one can be resolved easily with a simple refactor on the controller method.

[douglasgabriel]

Thanks @AparnaKarve for the suggestions here.

If the new password is the same as the current password, should we add a frontend validation to say that the new password needs to be different than the current password?

Great idea! I've put this validation, as well.

This might be a nit-pick, but do you think that the Confirm New password field should show only 1 validation message?

I also agree, this way is much better. Done!

Are blank passwords supported for a Physical Infrastructure Provider?

No, at least for the Lenovo Provider.

@douglasgabriel Can you also take a look at the codeclimate issues?

• Cognitive complexity: I think this one is due to the conditionals statements used in field validation, IMO the code is already lean.

• Method too long: the changePasswordFormController is a constructor method, so it make sense to be long, also, the others controllers are following this approach that defines all functions inside the constructor method.

• Duplicated code: seems like some boilerplates

P.S. The Travis failures doesn't seems to be due this PR modifications 🤔

[AparnaKarve]

@douglasgabriel You can address the Cognitive Complexity issue by making the functions - isFieldsEquals and modelInit exterior to the controller

In modelInit, I noticed that you don't really need name and action. Can you double-check that?
All references to vm.model.name can be changed to vm.recordName

[miq-bot]

Checked commit douglasgabriel@3e26ed1 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
5 files checked, 0 offenses detected
Everything looks fine. 🍰

[douglasgabriel]

Done @AparnaKarve :D

[AparnaKarve]

LGTM
Thanks!