fix(login) ensure a password is set, when using password login …

[edlerd]

to improve error message.

fixes #382

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.77%. Comparing base (0ffe428) to head (9f3b14b).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #392   +/-   ##
=======================================
  Coverage   70.77%   70.77%           
=======================================
  Files          15       15           
  Lines        1728     1728           
=======================================
  Hits         1223     1223           
  Misses        442      442           
  Partials       63       63           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[edlerd]

rebased on main and resolved conflicts

[nsklikas]

LGTM, I tried it and it looks fine. Only question I have is wouldn't it be better to validate the fields before making the request? Not much of a difference tbh.

[edlerd]

LGTM, I tried it and it looks fine. Only question I have is wouldn't it be better to validate the fields before making the request? Not much of a difference tbh.

I think it is obvious that an empty password will not be a valid input that can be submitted. So an error shouldn't be a surprise or cause of frustration as it is expected not to work without a password.

[nsklikas]

That's not what I meant.

In this PR we check if the password is unset and if it is unset we set it to "' and let Kratos evaluate the error message.
What I meant is that if the password is unset, we just show an error message to the user without making a request to Kratos (thus avoiding the extra request)

[edlerd]

That's not what I meant.

In this PR we check if the password is unset and if it is unset we set it to "' and let Kratos evaluate the error message. What I meant is that if the password is unset, we just show an error message to the user without making a request to Kratos (thus avoiding the extra request)

I understand your concern was about a possible optimization to avoid sending the request for validation. It is pretty hard to do the validation client side. This is due to the architecture with kratos sending flows of objects. On submit, we can't easily set an error on the password input field, because that is in a different component. We could do it via DOM manipulation. Actually, we do that for in other cases to ease the UX. I am not sure adding this complexity is worth the hassle. Especially as the extra request for validation here should be tiny and quick. I assume in the UX it doesn't make a big difference given a reasonably stable and fast connection.

[nsklikas]

Understood, thanks