This pull request allows a logged in user to change their password.

slashroots / harvest-v2

The Harvest API is an open data platform for the publishing and sharing of agricultural sector data across government and with the public.

http://data.abisjamaica.com.jm

4 stars 3 forks source link

This pull request allows a logged in user to change their password. #116

Closed leechristophermurray closed 7 years ago

leechristophermurray commented 7 years ago

A logged in user can now change their password. This was implemented by:

clearing the .current_user.us_password variable in CurrentUserFactory.query so the "New Password" field of profile.html is empty when a user signs in/logs in.
setting up a function, chpass(), in user-ctrl.js to accept the inputs of the chpassForm form in profile.html.
creating a PUT request in services.js which was then allowed a route (in router-user.js) to submit through user.js.
giving the user feeback based on whether the old password was correct or incorrect.

matjames007 commented 7 years ago

The old password must always be used to ensure the request to change a password is coming from the right source.

matjames007 commented 7 years ago

@nick22891 please review

matjames007 commented 7 years ago

@thinkking please adjust as @nick22891 described.

leechristophermurray commented 7 years ago

@matjames007 and @nick22891 please see the most recent commit for a different implementation of feedback for the user. Upon successfully entering the correct old password and successful submission SUCCESS: Your password has been changed. is given in a green color. The incorrect submission of an old password will give "ERROR: Incorrect password provided. Your password was not changed." in a red color. this is displayed "Password" title in the profile form.

nickwleo commented 7 years ago

I just tested this and it seems to be working very well so I've approved this PR. Just made the recommendation to @thinkking to use Bootstrap Alerts (just to be consistent with what we've done elsewhere in the application) but I wouldn't let that prevent merging this into develop.

leechristophermurray commented 7 years ago

@nick22891 I will push an update with Bootstrap Alerts soon.

UPDATE: @nick22891 bootstrap alerts are now being used.

nickwleo commented 7 years ago

@thinkking I edited the HTML and the CSS classes slightly (to make it look more like the other bootstrap alerts in the application).