cms icon indicating copy to clipboard operation
cms copied to clipboard

Add user profile and change password form tags

Open jacksleight opened this issue 2 years ago • 1 comments

Continuing with what @edalzell started in https://github.com/statamic/cms/pull/6023, merging in what I've implemented in my addon.

This PR adds the following:

  • [x] Adds a {{ user:profile_form }} tag that allows users to edit their information
    • [x] Supports uploading assets in the profile form
  • [x] Adds a {{ user:password_form }} tag that allows users to change their password
  • [x] Adds support for uploading assets in the register form
  • [x] Tests

Couple of notes:

  1. Profile Form Defaults I'm sure the way I've passed the default values from the user data to the profile form (including the change to getRenderableField) probably isn't the best way to do that, but I wasn't really sure how else to do it.
  2. Assets At the moment it's not possible for a user to remove assets from their accounts through the profile form, only replace them. I'm not sure if that should be supported and if so how best to do it?
  3. Protected Fields In my addon I implemented the profile form with a list of fields that were allowed to be submitted, defined via a config option. I've not implemented that here as the registration form doesn't have that. But perhaps it would be a good idea, to stop people submitting values they shouldn't have the ability to change? Maybe a block list instead of an allow list?

Also the tests have to check for either validation.current_password or The password is incorrect. because there's no validation.current_password string when running prefer-lowest.

Closes https://github.com/statamic/ideas/issues/676.

jacksleight avatar Jul 29 '22 15:07 jacksleight

Great work on this Jack

edalzell avatar Aug 11 '22 15:08 edalzell

Hi @jacksleight! Thanks for your work! Any ideas, what's missing or why it is still not merged?

j3ll3yfi5h avatar Oct 15 '22 18:10 j3ll3yfi5h

Think they just haven’t got to it yet, sure it'll be reviewed in due course. It's a fairly big PR so might take some time.

jacksleight avatar Oct 15 '22 21:10 jacksleight

This is really great, thank you.

But the asset fields are complicated. As it stands, if you have an avatar, and you submit the form, the avatar field gets wiped.

So while you said:

At the moment it's not possible for a user to remove assets from their accounts through the profile form, only replace them

It is possible... It just does it when you probably don't want it to. There's actually no way to maintain the value.

For now, so this can get merged and used, I'm going to filter out asset fields.

We can figure out asset field handling in a separate PR.

jasonvarga avatar Dec 01 '22 21:12 jasonvarga

I could be wrong (can't test it right now) but I think that's accounted for by these two bits:

request()->hasFile($field->handle())
->only(array_keys($values))

These should prevent blank files overwriting the existing values by filtering out the item from the collection if nothing has been uploaded.

Will do some testing to check though, I'm fairly sure that works OK in the addon (where most of this comes from).

jacksleight avatar Dec 01 '22 22:12 jacksleight

It wiped it when trying here. That hasFile only happens in the uploadAssetFiles, but not normalizeAssetValues, so the null value still gets submitted.

There's also the issue of how to display the file when one already exists, how to let a user remove it, and what to do when you have other assets fields that may have max_files != 1.

jasonvarga avatar Dec 01 '22 22:12 jasonvarga

  • Added profile and password routes
  • Added user:profile_form, user:password_form tags
  • Updated UserController to handle the new forms (and added a few helper methods)
  • Removed asset fields from registration form values so they don't get validated as required when not present in request data
  • Added a new test file
  • Created tests for the user:password_form tag
  • Added a new test file
  • Created the class ProfileFormTest and added use statements for NormalizesHtml, PreventSavingStacheItemsToDisk, TestCase traits
  • Used trait to prevent saving stache items to disk in tests/PreventSavingStacheItemsToDisk.php
  • Wrote it_renders_form() method that asserts if form is rendered with correct attributes like action url etc.. using assertStringContainsString(), assertStringEndsWith() methods of PHPUnit\Framework\AssertionFailedError class which extends from Exception Class (PHPUnit)
  • Wrote it_renders_form_with params() method that asserts if form is rendered with redirect attribute value as /submitted and error-redirect attribute value as /errors using assertEquals($expectedRedirectUrl, $actualRedirectUrl),assertEquals($expectedErrorRedirectUrl ,$actualErrorRedirctURL) methods of PHPUnit\Framework\AssertionFailedError class which extends from Exception Class (PHPUnit). Also used Parse::template(string $content = '', array $data = []) static function provided by Statamic Facade to parse template tags within content string passed into this function

what-the-diff[bot] avatar Dec 01 '22 22:12 what-the-diff[bot]

Ah OK, must not have spotted that before, no worries. 👍 And yeah there are other issues to consider as well.

jacksleight avatar Dec 01 '22 23:12 jacksleight