search

learningequality / kolibri

Kolibri Learning Platform: the offline app for universal education
https://learningequality.org/kolibri/
MIT License
807 stars 678 forks source link

Better error handling for various edge cases (like 404s from deleting non-existent records) #5305

Open jonboiser opened 5 years ago

jonboiser commented 5 years ago

Observed behavior

Here are some places where we could replace the catch-all 'AppError' (appears with 'handleApiError' dispatches) with in-UI error messages (or no error at all).

  1. Deleting a user from the facility when that user has already been deleted. (No error should be shown)
  2. Deleting a class that has already been deleted
  3. ~Visiting a Coach link for a class that has been deleted (it's handled like a 403 error)~
  4. ~Repeat for the various other things that can be deleted in Kolibri, as these will all lead to 404 errors of some kind.~
  5. Assigning an existing user a username that has already been given (by another user in another session, for example)
  6. Creating a new user in a similar situation as above. There is incorrect logic that is intended to handle this in UserCreateModal (but the code incorrect, so this situation ends up showing the AppError page)
  7. Attempting to access a classroom page when you are a coach not assigned to the classroom.
  8. Attempting to view a Classroom List or Assignments page as a Learner when not logged in (#5360 )

These errors fall into several broad categories:

  1. Attempting access a non-existent object (e.g. because it's been deleted or a UUID has been mis-typed). Issue #5666
  2. Attempting to modify a deleted object
  3. Attempting to delete a deleted object
  4. Attempting to create an object, and the new object violates some rule (e.g. unique name)
  5. Attempting to access an object for which you don't have the proper permissions.

In general, look where handleApiError is used and ask whether this can be replaced with no error, or an error placed within the current UI.

Expected behavior

In the edge cases described above, errors are handled as prescribed.

User-facing consequences

…

Errors and logs

…

Steps to reproduce

…

Context

…

indirectlylit commented 5 years ago

do these situations need design and/or user-facing strings?

jonboiser commented 5 years ago

Some of these don't require any new UI, like deleting something that's already deleted. (e.g. if you deleted an already-deleted faciltiyuser, dont show an error, remove the facilityuser from the table, the app user doesn't need to see anything).

We don't handle going to links for non-existent things (e.g. visiting a class homepage for deleted class), so that will require new design and strings.

I've asked @MisRob if she can look for more overlooked edge cases to complete the list in the OP, and then maybe we can handle the different categories in more depth, design-wise.

MisRob commented 5 years ago

https://docs.google.com/document/d/1LNAwzu_hgXQtGh-IMz_A1IsN4CdCGDIcTMcxowiS9Tg/edit?usp=sharing

indirectlylit commented 5 years ago

Unhandled 404 errors are also flooding Sentry with spurious errors

indirectlylit commented 5 years ago

another instance: https://github.com/learningequality/kolibri/issues/5801