Inline /account forms #226
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
To more closely match the designs.
With some placeholder content for now.
We inline the password edit form into the /account page and ensure that successful, or unsuccessful form edits redisplay/redirect to this page. I've also added a system test to cover validation errors to give me more confidence to make the change.
Passwords can now be edited from the account page so this page is no longer needed.
To replace our use of the `shared/errors` partial.
So that I can use this to render a controller generated error message on /account
This matches the style of the ModelErrorComponent shown if the User model itself has validation errors. I've added a specific flash key so that this error box is not shown in the case where the `alert` key is set for other reasons.
Before this commit the `verified` flag on a user would be set to false even if the change to the email address failed validation (e.g. by setting the email address to one that was taken).
We're about to add another form to this page to allow the user to change their email. Since that form will also act on the User model we want to make sure that the relevant validation errors are shown on each form. This form deals with errors on the password field so we add a guard clause for those.
Note there are two fields on this page labelled "Current password" so we have to scope the Capybara selector in the system test more specifically to avoid errors.
I've decided to set a local variable `payout_detail` in the `users/show` template to either use the persisted payout detail or create a new one. I considered setting this as an instance variable in the controller, but the issue is that any controller that renders `user/show` (PasswordsController, EmailsController, PayoutDetailsController) would have to also set this instance variable. I've also updated the link in the notify_artist email to point to this new page.
chrislo
force-pushed
the
inline-my-account-forms
branch
from
217dc05 to
294826d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR inlines three settings pages (password change, email change and payout details) into the /account page.
These design changes were suggested by the designs of the admin area #225 and make it easier to extend the settings page to include other settings.
I've decided to have a button per "section" of the settings page, and show the validation errors close to the form they apply to. I think this might get a bit confusing if/when this page gets very long, but we can address that UX concern later.