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

PRO-4544 User settings group and password change #4238

Merged
merged 7 commits into from
Jul 27, 2023

Conversation

myovchev
Copy link
Contributor

@myovchev myovchev commented Jul 26, 2023

Summary

  • Allow configurable "protected" subforms
  • Handle change password form
  • Implement subform groups (tabbed navigation)
  • Allow configurable reload and state restore after subform save

What are the specific steps to test this change?

Use Testbed branch feature-user-settings, link Apostrophe locally while in the current branch. Boot the app, force rebuild (CI=1 APOS_DEV=1 npx nodemon)

Requirements for Phase 1 and 2 should pass.

What kind of change does this PR introduce?

(Check at least one)

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • Build-related changes
  • Other

Make sure the PR fulfills these requirements:

  • It includes a) the existing issue ID being resolved, b) a convincing reason for adding this feature, or c) a clear description of the bug it resolves
  • The changelog is updated
  • Related documentation has been updated
  • Related tests have been updated

@linear
Copy link

linear bot commented Jul 26, 2023

PRO-4544 As a user, I can update my password when logged in (requires implementation of User Setting groups).

This requires the implementation of User Setting groups.

In the User Settings, which we started as a way to accommodate the need to allow users to set their admin UI language, we would like to add other settings moving forward. One of them is password.

image.png

image.png

image.png

image.png

Sketch

https://www.sketch.com/s/fbfcda4a-08a4-4c8d-ab88-fdad7532217e/p/2AFAAA0B-E913-41CE-B118-301ADC54CB63/canvas

Tech Design

  • Remove the restriction on the password field in settings.
  • Implement a custom schema field type which uses a nested schema to require the old password, and also two matching new password fields.
  • Automatically use this alternate field type when password is the field name.
  • The front end should help the user determine that the new password fields match, however the old password should NOT be checked yet (see below).
  • When valid the field emits { newPassword, oldPassword } as its value.
  • When "Update" is clicked the backend API should spot the password field and handle checking oldPassword (see self.apos.user.verifyPassword) before updating the password property to newPassword and calling convert and update normally. If the password validation fails a forbidden error should be thrown with an "incorrect password" message.
  • The front end should recognize this specific error and flag password as the invalid field.

Acceptance Criteria

  • When I open Settings I see a new row for Password (see screen 1) and when I click on that entire row, I then see three fields for setting my password: Current password, New Password, Verify New Password
  • I fill in the first one with my current password, and the second two fields for my new password have to match.
  • I click Submit and that saves my new passwords, and I see "Updated!" letting me know it's been saved.

@myovchev myovchev marked this pull request as draft July 26, 2023 13:28
@myovchev myovchev changed the base branch from main to feature-user-settings July 27, 2023 13:27
@myovchev myovchev requested a review from boutell July 27, 2023 13:29
@myovchev myovchev marked this pull request as ready for review July 27, 2023 13:33
Copy link
Member

@boutell boutell left a comment

Choose a reason for hiding this comment

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

Great stuff, but I did ask to change "protected" to "protection" because it will resolve a lot of confusion when reading the source (even for me).

modules/@apostrophecms/settings/index.js Show resolved Hide resolved
modules/@apostrophecms/settings/index.js Outdated Show resolved Hide resolved
modules/@apostrophecms/settings/index.js Outdated Show resolved Hide resolved
modules/@apostrophecms/settings/index.js Show resolved Hide resolved
modules/@apostrophecms/settings/index.js Outdated Show resolved Hide resolved
modules/@apostrophecms/settings/index.js Outdated Show resolved Hide resolved
@myovchev myovchev requested a review from boutell July 27, 2023 14:52
@myovchev myovchev merged commit e653c8e into feature-user-settings Jul 27, 2023
6 checks passed
@myovchev myovchev deleted the pro-4544-groups-pwd branch July 27, 2023 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants