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

Backend business rules exceptions are not handled well in front end #108

Closed
timo11 opened this issue Jun 23, 2015 · 12 comments
Closed

Backend business rules exceptions are not handled well in front end #108

timo11 opened this issue Jun 23, 2015 · 12 comments
Assignees
Labels
1 - Bug Incorrect behavior of the product pri:high Must be resolved before releasing
Milestone

Comments

@timo11
Copy link
Contributor

timo11 commented Jun 23, 2015

No description provided.

@timo11 timo11 added pri:normal Should be resolved before releasing but may be deferred to the next release 1 - Bug Incorrect behavior of the product labels Jun 23, 2015
@benanhalt
Copy link
Contributor

See if these are showing in the exception catcher.

@maxpatiiuk
Copy link
Member

Currently, the back-end throws a 500 error invalid business rule.

Instead, it should return a 400 error with a payload object that specifies the offending field if applicable.

The payload object should have an error code, which is then resolved to an error message by the front-end for purposes of localization.

@grantfitzsimmons
Copy link
Member

grantfitzsimmons commented Jul 19, 2022

For pick lists, I propose maintaining the uniqueness constraint, however it should mitigate the error (which loses all progress) by appending a (2) or (3) after the name (or at least prompt the user to enter a new name).

@maxpatiiuk
Copy link
Member

maxpatiiuk commented Jul 30, 2022

Looks like @benanhalt began this in 8f92c91

Still need to modify the response code to return a 4xx instead of 5xx, as 5xx would return an HTML response instead of JSON if the back-end is running in debug mode

@benanhalt
Copy link
Contributor

Inventory the exceptions.
Can recoverable ones be identified?
Which ones should be prevented by FE?

@maxpatiiuk
Copy link
Member

I was thinking Front-End would just catch the error and display it next to the field that triggered the issue.
The error would disappear on any edits to that field, and user would be able to press "Save" again to check for business rule errors again.

Thus, in the short term, front-end won't be preventing any errors, but it would display them nicely (with localized context aware error messages)

@maxpatiiuk
Copy link
Member

@benanhalt does this sound good to you?
If so, just modify the response code for business rule exceptions from 5xx to 4xx and return a JSON response

@benanhalt
Copy link
Contributor

I think that is the basic idea, but it will be somewhat more involved. For instance I think there are instances where the exception can't be traced to one particular field.

@maxpatiiuk
Copy link
Member

maxpatiiuk commented Sep 14, 2022

In such cases a dialog message would be displayed. The front-end already does it for save blockers:
All save blockers that can be attributed to an HTML field (not subviews), is going to be shown in that field.
If all remaining save blockers are not associated with any field, a dialog message would be displayed

@maxpatiiuk
Copy link
Member

@benanhalt Another reason to modify the response code:
BussinessRule exceptions don't show the error message when in production (since they return a 500 error, and 500 errors are hidden when not in debug mode).
See #2197 (comment) for an example.

I feel like this is a really terrible situation and should be fixed soon.

@maxpatiiuk maxpatiiuk added pri:high Must be resolved before releasing and removed pri:normal Should be resolved before releasing but may be deferred to the next release labels Sep 22, 2022
@maxpatiiuk maxpatiiuk linked a pull request Sep 26, 2022 that will close this issue
@melton-jason
Copy link
Contributor

Fixed in #2702

UX Improvements automation moved this from To do to Shipped Jan 16, 2023
@specifysoftware
Copy link

This issue has been mentioned on Specify Community Forum. There might be relevant details there:

https://discourse.specifysoftware.org/t/specify-7-8-4-announcement/999/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 - Bug Incorrect behavior of the product pri:high Must be resolved before releasing
Projects
Archived in project
UX Improvements
  
Shipped
Development

Successfully merging a pull request may close this issue.

6 participants