Login - 2.x Review #222

jniles · 2016-03-27T10:11:09Z

This issue documents the updates to the bhima login page that need to be made before the page is up to bhima-2.X standards.

App Structure/css

~~Login form should be centered vertically on any screen size (using vh css properties)~~
Login Form should be centered vertically in line with the sidebar, not the screen vertical center.
Redesign the panel-heading for clearer login message
Remove duplicate text on the login page introduced in Lang #283 and Refactoring the fr.json and en.json #197.
Login form should expand to full screen on small screen size.

Form Validation

Login form should clearly distinguish between server-sent errors and client side errors.
Find a better place for "Forgot your password?" text.

End-to-End Tests

Write tests to demonstrate the following capabilities:
- Login to a particular project (assert that the project name is correct)
- Page Refresh retains login
- Login and Logout, page refresh retains logout
- Invalid username/password credentials display informative errors

Error Handling

Application errors (no enterprises, no projects, etc) should be CLEARLY put above the login form, perhaps disabling the login capabilities. These should be checked when the application starts. Use growl-notifications introduced in (Feature) Notification service #370
Errors from HTTP should be CLEARLY displays above the login form, along with appropriate translations and error codes. Use growl-notifications introduced in (Feature) Notification service #370
Form errors should be displayed beneath the form and prompt the user to resubmit with different credentials.

Cache

The login form should remember (and select) the last project used on that computer.

Documentation

Code should produce a readable JSDoc document for future reference.

Service Architecture

Login/Logout functionality should exist in a separate service, not in the Login controller itself.

With these improvements, the Login Form should be pretty close to bhima-2.x's standards.

The text was updated successfully, but these errors were encountered:

This commit implements some of the requested changes from #222. In particular, the following have landed: 1. The login panel positioning is a little bit better centered. 2. The login/logout methods have been moved into the SessionService, which stores session information in the local $sessionStorage. 3. The panel header buttons are slightly clearer. 4. The login form remembers which project was logged in. Most importantly, the LoginController is now more focused, removing extraneous services such as `$location`, `$timeout` and `appstate`.

jniles · 2016-05-04T09:28:36Z

Note: updates on 4/5/2016.

sfount · 2016-05-04T09:33:10Z

The updates here LGTM - this would be a logical component to tackle first.

The ideas for error handling sound very useful 👍

jniles · 2016-05-04T09:45:49Z

Alright, I'll tackle this today.

jniles · 2016-05-04T10:15:06Z

Critical bug - the URL navigation still works when the user doesn't have a session. Steps to reproduce:

Load the login page.
Type #/settings into the URL.
Observe that while the URL reset to #/login, you will be looking at the Settings page.

sfount · 2016-05-04T11:46:36Z

Interesting - that must mean the only redirect that actually works is on an unauthenticated HTTP request. The settings page only makes a request to /languages which happens to be defined as a public route so it doesn't throw the error causing a redirect.

Most of the other pages do!

This commit addresses two issues reported in #222: 1. The duplicate text on the login panel has been removed. 2. The tree cannot be expanded when the user does not have a session.

This commit cleans up the auth/login messages by moving server-sent error messages into `growl-notification` service. It also exposes a new method on the `NotifyService` called `fatal()` to signal fatal errors using the same colorscheme as `NotifyService.handleError()`. The name fatal was chosen to encourage developers NOT to use this, except in very rare cases. The `danger()` method should be preferred in almost all scenarios. This commit also adds the `NotifyService` into the `SessionService` to display welcome and goodbye messages on login/logout events. This brings us closer to finishing #222, closing all error handling and text-related issues.

jniles · 2016-05-04T11:54:03Z

Yes, I found the bug and fixed it in jniles@fe8ed51. It was a fairly easy fix - we had not updated our session safeguards for $state-based routing, which knows nothing of the URL. Now, we use the $state to redirect in case the URL is invalid... Slightly hard-coded, but good enough for now, I think.

I'll write some end-to-end tests and confirm that everything behaves as expected before sending in a pull request.

This commit ensures that $state.go() doesn't get to `/login` when the user already has a session logged in. Since $state-based and URL-based routing are decoupled in bhima, we need to two checks to make sure the URL is correctly displayed (and doesn't transition) and the $state is correctly blocked and doesn't transition. Fixes bugs reported in #222.

The login procedure has been completely polished with notifications, better error handling, translated text, and a centered panel. All bugs related to the tree and/or navigation have been squashed. This is ready for a 2.x review and approval by the team. End-to-end tests have been written to future-proof the behavior of the module. Tests have been written for every case discussed in #222 and bugs found along the way. The breakpoints have been adjusted so that the login form nicely renders on all default responsive devices included in the chrome dev tools. Decisions, such as welcome messages, should be reviewed by the team. Closes #222.