Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve feedback on error in the application #173

Open
jaragunde opened this issue Mar 9, 2016 · 6 comments
Open

Improve feedback on error in the application #173

jaragunde opened this issue Mar 9, 2016 · 6 comments

Comments

@jaragunde
Copy link
Member

[Trac import]
Reported by: jaragunde
Original date: Wednesday, 29 February 2012 14:36

The feedback when an error happens in !PhpReport is too generic and doesn't help the user to know what's wrong. Some examples:

  • Try to delete a project with assigned tasks. You can't, the application says there was an error but not the reason of the error.
  • Try to save tasks with overlapping times. The application says there was an error and shows a generic message.

Besides, the error notifications are too similar to 'ok' notifications, and they vanish too soon. It might happen that a user thinks he has saved his tasks correctly and he hasn't.

@jaragunde
Copy link
Member Author

As a first step, the web UI should report the error message returned by the backend to the user. There are only a few of these error messages, but the amount of potential cases would grow when addressing #496.

jaragunde added a commit that referenced this issue Feb 23, 2023
We start reporting errors on the task create operation, in the
following situations:
* Overlapping tasks detected.
* Trying to write to a date that has been locked.
* Trying to assign a task to a project that's closed.

These errors are detected prior to launching the SQL query. We haven't
addressed SQL errors yet.
@jaragunde
Copy link
Member Author

Danielle has started addressing #496, fixing this too in the process.

jaragunde added a commit that referenced this issue Feb 24, 2023
This allows batchCreate to return an array of OperationResults, each
potentially containing one error, and makes the UI go through all of
them to report on screen.
jaragunde added a commit that referenced this issue Feb 28, 2023
This allows batchCreate to return an array of OperationResults, each
potentially containing one error, and makes the UI go through all of
them to report on screen.
jaragunde added a commit that referenced this issue Feb 28, 2023
We start reporting errors on the task create operation, in the
following situations:
* Overlapping tasks detected.
* Trying to write to a date that has been locked.
* Trying to assign a task to a project that's closed.

These errors are detected prior to launching the SQL query. We haven't
addressed SQL errors yet.
jaragunde added a commit that referenced this issue Feb 28, 2023
This allows batchCreate to return an array of OperationResults, each
potentially containing one error, and makes the UI go through all of
them to report on screen.
@jaragunde
Copy link
Member Author

Merged in #616:

f3f8c5f3 [#492, #496] Manage and return SQL errors on task creation. [Jacobo Aragunde Pérez]
00088a42 [#173] Allow to report multiple errors on the same creation batch. [Jacobo Aragunde Pérez]
f4700807 [#173] Report several kinds of errors on task creation. [Jacobo Aragunde Pérez]
a2a96ce8 Prevent calling task create without overlap check. [Jacobo Aragunde Pérez]

jaragunde added a commit that referenced this issue Feb 28, 2023
Make "error" font red and increase message time on screen.
jaragunde added a commit that referenced this issue Mar 1, 2023
Make "error" font red and increase message time on screen.
jaragunde added a commit that referenced this issue Mar 6, 2023
Make "error" font red and increase message time on screen.
jaragunde added a commit that referenced this issue Mar 6, 2023
Make "error" font red and increase message time on screen.
jaragunde added a commit that referenced this issue Mar 7, 2023
Make "error" font red, add an icon next to it and increase message
time on screen.
jaragunde added a commit that referenced this issue Mar 8, 2023
Modify the user creation pipeline (service, facade, DAO) to use
OperationResult and return better errors.

This affects only the postgres implementation but there is another
DAO for LDAP+DB that will be addressed in a subsequent patch.
jaragunde added a commit that referenced this issue Mar 8, 2023
The HybridUserDAO matches user data with the LDAP and keeps them in
sync with the local database, to maintain the relations with other
data tables.

We modify the create operation in this DAO to return OperationResult,
matching the new expectations of the upper layers.

We change the Hybrid DAO to extend the Postgres DAO, so we can reuse
the SQL operations. In this case, we call the create operation from
the Postgres DAO after we do the corresponding LDAP checks.
jaragunde added a commit that referenced this issue Mar 8, 2023
Modify the user update pipeline (service, facade, DAO) to use
OperationResult. HybridUserDAO will be addressed in a subsequent patch.
jaragunde added a commit that referenced this issue Mar 8, 2023
This operation did not do anything related with LDAP, so we simply run
the corresponding SQL operation. We don't modify the behavior of the
operation, so we add a comment about a detected problem.

In the web service layer, we add try/catch blocks for the
LDAPInvalidOperationException like the ones in the create and delete
user services, to prevent crashing on operations that aren't defined
in the LDAP backend (namely, assigning users to groups).
jaragunde added a commit that referenced this issue Mar 8, 2023
Modify the user delete pipeline (service, facade, DAO) to use
OperationResult. HybridUserDAO is changed to reuse the same operation
from Postgres DAO, since it did not have any code related to LDAP.
jaragunde added a commit that referenced this issue Mar 8, 2023
Make "error" font red, add an icon next to it and increase message
time on screen.
@jaragunde
Copy link
Member Author

Merged in #618:

82d4d972 [#496] Review error response codes in task operations. [Jacobo Aragunde Pérez]
ddbeb195 [#173] Make error messages more noticeable. [Jacobo Aragunde Pérez]
13e9c2b2 [#496,#513] Return more detailed errors on task delete. [Jacobo Aragunde Pérez]
e49f20e5 [#496] Return more detailed errors on task update. [Jacobo Aragunde Pérez]

jaragunde added a commit that referenced this issue Mar 16, 2023
When updating a login name in the LDAP backend there is a risk of side
effects. With this change, we are able to report the success message
from the backend through the UI instead of overwriting it with another
string, so we can warn users when doing that operation.
jaragunde added a commit that referenced this issue Mar 20, 2023
Modify the user update pipeline (service, facade, DAO) to use
OperationResult. HybridUserDAO will be addressed in a subsequent patch.
jaragunde added a commit that referenced this issue Mar 20, 2023
This operation did not do anything related with LDAP, so we simply run
the corresponding SQL operation. We don't modify the behavior of the
operation, so we add a comment about a detected problem.

In the web service layer, we add try/catch blocks for the
LDAPInvalidOperationException like the ones in the create and delete
user services, to prevent crashing on operations that aren't defined
in the LDAP backend (namely, assigning users to groups).
jaragunde added a commit that referenced this issue Mar 20, 2023
Modify the user delete pipeline (service, facade, DAO) to use
OperationResult. HybridUserDAO is changed to reuse the same operation
from Postgres DAO, since it did not have any code related to LDAP.
jaragunde added a commit that referenced this issue Mar 20, 2023
When updating a login name in the LDAP backend there is a risk of side
effects. With this change, we are able to report the success message
from the backend through the UI instead of overwriting it with another
string, so we can warn users when doing that operation.
@jaragunde
Copy link
Member Author

Merged in PR #621:

04cc1fdb [#173,#496] Report better errors on user CRUD operations [Jacobo Aragunde Pérez]
e0fd7893 [#173] Report warning message from LDAP backend to users. [Jacobo Aragunde Pérez]
73b21e5f [#173,#496] Report detailed errors on user delete. [Jacobo Aragunde Pérez]
d9b60724 [#173,#496] Report better errors from LDAP user update DAO. [Jacobo Aragunde Pérez]
999ec3f3 [#173,#496] Report detailed errors on user update. [Jacobo Aragunde Pérez]
0708ec42 [#173,#496] Report better errors from LDAP user create DAO. [Jacobo Aragunde Pérez]
2ea7e95f [#173,#496] Report detailed errors on user creation. [Jacobo Aragunde Pérez]
4d307f27 [#431] Fix calling sizeof on non-countable. [Jacobo Aragunde Pérez]

jaragunde added a commit that referenced this issue Mar 20, 2023
Merge pull request #621 from Igalia/user-create-report-errors
@dmtrek14
Copy link
Collaborator

dmtrek14 commented Sep 1, 2023

I think it is probably time to pause working on this one and spend the time on the new frontend so that we can benefit from the new technologies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants