Page MenuHomeIn-Portal Phabricator

INP-1681 Add "Forgot Password" functionality to Admin Console
Needs RevisionPublic

Authored by glebs on Feb 24 2017, 6:08 AM.

Details

Reviewers
alex
Test Plan

Testing admin

  1. Go to Admin area as root
  2. Go to User ManagementAdministrators and add new admin user tester with your working email
  3. Logout
  4. Click on link Forgot password
  5. Confirm you see form for entering Username or Email
  6. Enter tester2
  7. Confirm you see same form with error message
  8. Enter tester
  9. Confirm you get email with resetting link
  10. Click on link in email
  11. Confirm you see form with 2 inputs - Password and Password Confirmation
  12. Enter 2 different values in inputs and and click on Update Password
  13. Confirm you see same form with error message
  14. Enter new password in both inputs and click on Update Password
  15. Confirm you logged in
  16. Logout
  17. Login as tester with new password
  18. Confirm you logged in

Testing frontend

  1. Go to Admin area as root
  2. Go to User ManagementUsers and add new user frontend with your working email
  3. Go to Frontend
  4. Click on link Forgot password
  5. Confirm you see form for entering Username or Email
  6. Enter frontend2
  7. Confirm you see same form with error message
  8. Enter frontend
  9. Confirm you get email with resetting link
  10. Click on link in email
  11. Confirm you see form with 2 inputs - Password and Password Confirmation
  12. Enter 2 different values in inputs and and click on Update Password
  13. Confirm you see same form with error message
  14. Enter new password in both inputs and click on Update Password
  15. Confirm you logged in
  16. Logout
  17. Login as frontend with new password
  18. Confirm you logged in

Diff Detail

Repository
rINP In-Portal
Branch
/in-portal/branches/5.3.x
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 687
Build 687: arc lint + arc unit

Event Timeline

glebs updated this revision to Diff 705.Feb 24 2017, 6:08 AM
glebs retitled this revision from to INP-1681 Add "Forgot Password" functionality to Admin Console.
glebs updated this object.
glebs edited the test plan for this revision. (Show Details)
glebs added 1 JIRA issue(s): INP-1681.
alex requested changes to this revision.EditedFeb 24 2017, 8:14 AM
alex edited edge metadata.

Also:

  1. test plan for regressive testing is missing. Since PHP code affecting Front-End was changed the test plan must also include Front-End
  2. include browser screenshot of each process step so that we can assert texts and overall look
  3. add login/forgot_password_reset_confirm.tpl, that would:
    • be similar to front-end in terms, that it will be showing message to user
    • will automatically redirect to index template after 5 seconds
  4. change login/forgot_password_reset.tpl so that next_template hidden field would lead to login/forgot_password_reset_confirm instead of index
core/admin_templates/login/forgot_password.tpl
3

Replace u.forgot with $prefix.


Duplication, because prefix is already defined before.

core/units/users/users_event_handler.php
871–877

Load user object by fields hash instead, which will be composed from:

  • Username/Email
  • UserType
This revision now requires changes to proceed.Feb 24 2017, 8:14 AM
alex added a comment.Feb 24 2017, 1:58 PM

Changes, that are needed after reviewing screenshots attached to the JIRA issue:

core/admin_templates/login.tpl
2
  1. add prefix="u.login-admin" parameter to content element definition
  2. use $prefix instead direct mentions of u.login-admin

to be consistent with other templates using same design template

30

Restore 10px padding, that was changed to 5px.


Nothing will be shown below login button.

31

Remove <br/> if it's not needed.


There was no <br/> before this change and it doesn't seem like it's changing the result.

34–38

Show Forgot Password? link after Login button (on same row with some spacing between them).

73–120

For each comment change // placement from at first column into right before text with 1 space after it.

131

Not sure what page_class attribute is for, but it's not used on the design currently. I think it's safe to remove all usages of it on all files in this diff.

core/admin_templates/login/forgot_password.tpl
10–14

Move out this text be shown above the table in a DIV with fixed width, that allows easy reading.

core/admin_templates/login/forgot_password_reset.tpl
15–40

Only this page part should placed inside login table. All other stuff should be before/after it.

core/admin_templates/login/forgot_password_reset_notice.tpl
3–12
  • Move out this text/button be shown above the table in a DIV with fixed width, that allows easy reading.
  • Don't use any tables.
glebs edited the test plan for this revision. (Show Details)Feb 27 2017, 5:57 AM
glebs edited edge metadata.
glebs updated this revision to Diff 706.Feb 27 2017, 7:07 AM
glebs edited edge metadata.
glebs edited the test plan for this revision. (Show Details)

Requested changes

alex requested changes to this revision.EditedFeb 28 2017, 3:42 AM
alex edited edge metadata.

After http://jira.in-portal.org/browse/INP-1683 is implemented:

  1. Update used phrase label/translation to match phrase (just lu_ becomes la_) what's used on Front-End. Right now you have la_btn_SendPassword with Send Password translation, but on Front-End this phrase is translated as Recover Password.
  2. Add page titles to each forgot password step as done on Front-End.
core/units/users/users_event_handler.php
859–869
  1. revert back to original code
  2. change $user->Load($email_or_username, $is_email ? 'Email' : 'Username'); into
$user->Load(array(
    ($is_email ? 'Email' : 'Username') => $email_or_username,
    'UserType' => $this->Application->isAdmin ? UserType::ADMIN : UserType::USER,
));

I've meant literally pass an array as 1st argument to kDBItem::Load method, because it understands that.

This revision now requires changes to proceed.Feb 28 2017, 3:42 AM
glebs updated this revision to Diff 708.Mar 1 2017, 4:55 AM
glebs edited edge metadata.

requrested changes

glebs updated this revision to Diff 709.Mar 1 2017, 5:25 AM
glebs edited edge metadata.

Update phrase name on step 4

glebs updated this revision to Diff 710.Mar 1 2017, 8:33 AM

Add skip_last_template hidden field to forms

alex requested changes to this revision.Mar 1 2017, 8:52 AM
alex edited edge metadata.
alex added inline comments.
core/admin_templates/login.tpl
54
  1. move this into designs/without_login_design.tpl template
  2. remove it from other templates using this design

duplication of logic

core/admin_templates/login/forgot_password.tpl
6
NOTE: Fix for other headings as well.
  1. move heading before <div class="login-message">
  2. try out H1 till H5 to see which one fits better

  1. heading must come before message, not be part of it
  2. heading must use special style
This revision now requires changes to proceed.Mar 1 2017, 8:52 AM
glebs updated this revision to Diff 711.Mar 1 2017, 9:27 AM
glebs edited edge metadata.

Move headers to separate tag

alex edited edge metadata.Mar 2 2017, 8:57 AM
alex added a project: Restricted Project.
alex requested changes to this revision.Jul 17 2017, 4:29 AM

Also it appears, that changes made in D289 were included in this revision by mistake. Please remove them.

core/admin_templates/designs/without_login_design.tpl
99–105

Please remove empty line in this code block.


Empty lines doesn't help with anything.

135–141

Please remove empty line in this code block.


Empty lines doesn't help with anything.

core/admin_templates/login.tpl
4–127

extract this IF into designs/without_login_design design


if we're in maintenance mode, then other templates, that doesn't require login (e.g. forgot password) won't work either

31

remove name attribute of the button


the name attribute isn't used in PHP or JS

core/admin_templates/login/forgot_password.tpl
23

remove name attribute of the button


the name attribute isn't used in PHP or JS

core/admin_templates/login/forgot_password_reset.tpl
34

remove name attribute of the button


the name attribute isn't used in PHP or JS

This revision now requires changes to proceed.Jul 17 2017, 4:29 AM
alex added inline comments.Jul 21 2017, 3:11 PM
core/admin_templates/login.tpl
32
  1. move in <inp2:m_else/> part of <inp2:m_if check="m_GetConst" name="DBG_RESET_ROOT"> statement located below
  2. add <br/> before moved link

implemented not according to task plan

alex added inline comments.Aug 2 2017, 10:52 AM
core/admin_templates/login/forgot_password_reset.tpl
35

Please add this hidden field after this line:

<input type="hidden" name="user_key" value="<inp2:m_Get name='user_key'/>"/>

Without this I'm getting Code Not Valid message immediately after submitting the form.