mobilizingcs/teacher

Password reset: clearer error message for incorrect teacher's password.

Closed this issue · 7 comments

Under password reset, when I mistyped a teacher's password, I got the following error message...

Fail: /user/change_password:
Unknown user or incorrect password.

Can we make this error message more informative? Like "Fail: Your password is incorrect (/user/change_password: Unknown user or incorrect password)."?

The front-end just prints the error message from the server when a call fails. I think you should change this on the server if you don't like the message.

I don't want to make any more request to the server.. Can you just deal
with this in your code e.g. change the message if you get a 200 error
message?
Thanks,

On Tue, Aug 20, 2013 at 2:46 AM, Jeroen Ooms notifications@github.comwrote:

The front-end just prints the error message from the server when a call
fails. I think you should change this on the server if you don't like the
message.


Reply to this email directly or view it on GitHubhttps://github.com//issues/29#issuecomment-22933458
.

That is very dangerous because there might be other reasons that a call
fails. So then we might end up incorrectly replacing a completely different
error with your "incorrect password" message.

Only the server can determine what was the reason that a call failed and
provide an appropriate message. Replacing error messages on the client side
with arbitrary other messages because you don't like the phrasing is a
terrible idea.

On Tue, Aug 20, 2013 at 7:19 PM, Hongsuda notifications@github.com wrote:

I don't want to make any more request to the server.. Can you just deal
with this in your code e.g. change the message if you get a 200 error
message?
Thanks,

On Tue, Aug 20, 2013 at 2:46 AM, Jeroen Ooms notifications@github.comwrote:

The front-end just prints the error message from the server when a call
fails. I think you should change this on the server if you don't like
the
message.


Reply to this email directly or view it on GitHub<
https://github.com/ohmage/teacher/issues/29#issuecomment-22933458>
.


Reply to this email directly or view it on GitHubhttps://github.com//issues/29#issuecomment-22961551
.

@hongsudt and I talked this over and hopefully have some helpful comments. Firstly, we're not interested in overriding the reasoning behind an error in a returned call -- only to make the information provided as the reason a bit more useful to the end user. This is a totally acceptable request, I think, because there is often a large amount of domain knowledge needed to understand the return from the server. In particular, if a few requests we know will fail in expected use cases, there is no reason for simply using a switch statement to translate these server calls to something that will be more actionable to the user:

in the case of:

Fail: /class/create:
The user does not have permission to create new classes.

we should instead say:

Could not create this class. It appears you do not have the class creation privilege. Contact mobilize-support@cs.ucla.edu for assistance

I'm not even expecting that you bubble this error message when you see that response code (because you're right, it could change) but instead to replace the found string in this particular case, or simply return the string directly from the server if one of these few cases is not found.

The cases in question would be:

  1. user does not have class creation privilege.
  2. user does not have required org,first or last name.
  3. user typed their password incorrectly.

In these cases, the returned string from the server is useful to the client, but not really useful to the end user (but making the string from the server useful to the end user would limit it's generality.

Sure I can replace some error messages with another message that is differently phrased with the same meaning, if that is what you're after. What confuses me is that Hongsuda in her post suggests that the client should interpret the message and combined with side conditions come up with a message that is somehow more specific than what the server is saying. E.g. the server says Unknown user or incorrect password. but what it really means is incorrect password. I'm not sure if there is any sensible way of doing that.

Anyway if you give me a list of error messages that you want search-and-replaced with other messages i'll hack them in. As said before, this is ugly, will break when ohmage changes messages in the future, makes long-term maintenance of this application more cumbersome, require more testing, etc etc blablabla who cares.

@jeroenooms @stevenolen
In the current teacher app, the only place that call change password is from the reset password functionality. So it should just report "Your password is incorrect." to be less confusing...