Added method for managers to update a user's email

[raul-fuentes]

References

consuldemocracy#2499

Objectives

Added method and interface for a manager user to change emails of confirmed users

Visual Changes

Notes

none

[MariaCheca]

I've left some comments, also the design I see in the gif is not the one in the issue...Please review the requirements and in case you've questions ask us :)

[MariaCheca]

There should be a block to check if the user has email, as it's warned in the issue's "Notes" section.

[raul-fuentes]

In the notes section warns about users without email, in that case the form will appear empty.
Maybe there is something that I don't understand, but any user can have an email even if they don't have one at the moment, isn't it?

[bertocq]

I think you're right @raul-fuentes. The edit_user_account form should always be displayed as scenarios are:

• User has an email: Manager should be able to change it
• User does not have an email: Manager should be able to set a value for it

[raul-fuentes]

That's the actual behaviour, should I remark it some way?

[bertocq]

No it's good 👍 I just wrote it as "I think.." because I needed to double check it afterwards

[MariaCheca]

I would indent this element more like this one here to make it more readable.

[MariaCheca]

As the email is the only field we're changing here...I would go for an "Edit email" and "Update email"

[MariaCheca]

Same as the comment above.

[bertocq]

@raul-fuentes after changing the "Edit account" texts that maria requested, you're missing some changes in the specs expectations as well https://travis-ci.org/AyuntamientoMadrid/consul/jobs/355308112 at spec/features/management/account_spec.rb

[bertocq]

@raul-fuentes it's always a good idea to comment on the PR so we can notice requested changes has been applied 👍 we don't cycle through all pending changes PR's refreshing to see if there are changes.

[raul-fuentes]

Thanks for the advise, I 'll do it from now on

[bertocq]

I'm sorry to contradict decabeza's mockups, but after reviewing consuldemocracy#2500 as well, this is going to get crazy if we keep adding more and more options as ajax forms that appear and dissapear as a toggle

We're talking about a section that is used by Public Workers only:

• Not end users, no need to fancy them with animations
• Probably on legacy systems (think Windows XP SP1 with IE6 or IE7 if we're lucky)
• Used to hardcore old school apps with menus and stuff written in Java, not in JS.

So with @decabeza 's agreement I'm proposing to:

1- Create a submenu under "Edit user account" menu item with "Edit Email" title. That menu will display the email change form (✅ working as expected) and nothing more
2- Create a second submenu under "Edit user account" with title "Delete User" that will have that pre-existing "Delete User" button that is toggled to the email form
3- Removing the "Edit email" button from the to right corner
4- Keep the new red "Change user" link in the left bottom corner 👍 I think at least the red color improves the usability as its a "destructive" action.

Also with this we can avoid those render_fails and toggle usages that really doesn't add value to the feature, only make the code harder to follow and maintain in the future if we keep adding stuff to this panel

[raul-fuentes]

I have completed the requested changes and so do @iagirre, should we merge our PR's into one? (this one) We think that doing so we could easily merge the interface

This is the new interface

• Edit email

• Delete account

[bertocq]

Great! Those changes in the menu look much better 🙇 👍 But I still have some concerns:

[1] User experience still is not good enough

After the manager sets the new email, its true on localhost we are redirected to the email the user would see... but the Manager in Production will not, he'll see:

Instead he will see the following:

The old email is still present, and there's no clue about what just happened, just a "Cambios guardados" that could be good enough if at least the manager or the user had our knowledge (that an email has been sent and everything else) but they don't, and assuming they know might get us some tickets "email change not working properly". Again, this an interface for public workers helping really-low-technical-knowledge users.

Think about showing under "Cuenta de usuario" a message that clearly explains what just happened and next steps (like you greatly did with the "Borrar cuenta" submenu 👏)

Se ha enviado un mensaje de correo electrónico a la nueva dirección de email introducida. Será necesario usar un enlace que aparece en ese mensaje para que el cambio de email se complete con éxito

An email message has been sent on to the introduced email address. It will be necessary to follow a link on that email message for the email address change to be completed effectively.

[2] Commit history needs rebase

After changes for [1] are done, I would recommend to rebase all commits as the final result will be hard to follow for a person in the future trying to figure out either what new feature we added or why a change in a certain file was done. As a personal tip, I sometimes just go:

git reset master
git add..
git commit..

as rebasing multiple commits can be far tedious than just creating them from scratch

[bertocq]

As you can see #1370 was merged and some conflicts appeared (nothing serious as far as I have seen). Your PR has a fix for an spec that was failing in itziar's 👍, and the "Change user" red link 👌 . But it would be nice it you could merge from his changes the "Dropdown" effect that the her PR added:

[raul-fuentes]

@bertocq all the requested changes are done. Just one thing.
I changed the flash message after sent the new mail conformation request this way

Its correct or is too long and maybe its better to show it in a green warn square like the warning before deleting an account?

[bertocq]

@raul-fuentes yes I was thinking about an square below the "Cuenta de usuario" title.

Think about showing under "Cuenta de usuario" a message that clearly explains what just happened and next steps (like you greatly did with the "Borrar cuenta" submenu 👏)

[raul-fuentes]

@bertocq here are the new changes.

[bertocq]

Great @raul-fuentes ! Only left the merge conflicts

[raul-fuentes]

@bertocq, also merged the conflicts
this are the results:

[bertocq]

There are some spec/i18n_spec.rb and spec/features/management/users_spec.rb test failures raul :(

[raul-fuentes]

Hello @bertocq I just saw Travis a moment ago!
I have already made the necessary changes so that the tests work properly.

[raul-fuentes]

PR updated!

[houndci-bot]

Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://github.com/bbatsov/ruby-style-guide#consistent-string-literals)

[houndci-bot]

Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://github.com/bbatsov/ruby-style-guide#consistent-string-literals)

[houndci-bot]

Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://github.com/bbatsov/ruby-style-guide#consistent-string-literals)

[houndci-bot]

Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://github.com/bbatsov/ruby-style-guide#consistent-string-literals)

[houndci-bot]

Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://github.com/bbatsov/ruby-style-guide#consistent-string-literals)

[houndci-bot]

Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://github.com/bbatsov/ruby-style-guide#consistent-string-literals)

[houndci-bot]

Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://github.com/bbatsov/ruby-style-guide#consistent-string-literals)

[houndci-bot]

Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://github.com/bbatsov/ruby-style-guide#consistent-string-literals)

[houndci-bot]

Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://github.com/bbatsov/ruby-style-guide#consistent-string-literals)

[houndci-bot]

Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://github.com/bbatsov/ruby-style-guide#consistent-string-literals)