UMB-CS-682-Team-03 / tracker

0 stars 0 forks source link

Browsers raise error if search status for superseder is not a valid keyword. #57

Closed rouilj closed 4 months ago

rouilj commented 4 months ago

Open a tracker issue. Select the classhelper for superseder. Make sure that the status property is not a dropdown (i.e. data-search-with="title,status,keyword[]").

In the search box for status, enter "open". Click search. A popup comes up:

 Error: Failed to load next page, check console for more details.

Console reports:

Error: Unexpected response
url: http://unixland2.home:8080/demo/rest/data/issue?%40page_index=1&%40page_size=100&%40fields=id%2Ctitle&%40sort=title&status=open
response status: 400
response body: {"error":{"status":400,"msg":"'No key (name) value \"open\" for \"status\"'"}}
    searchEvent http://unixland2.home:8080/demo/@@file/classhelper.js:1132
    handleSearchEvent http://unixland2.home:8080/demo/@@file/classhelper.js:217
    getSearchFragment http://unixland2.home:8080/demo/@@file/classhelper.js:592
    getSearchFragment http://unixland2.home:8080/demo/@@file/classhelper.js:589
    openPopUp http://unixland2.home:8080/demo/@@file/classhelper.js:921
 request data url: [object FormData]

This is a great error report.

The rest interface isn't giving us anything useful to work with though. I wonder if a new msgcode property in the error document would help here so the classhelper doesn't have to parse the msg to try to figure out if this is a recoverable error or not. A flag that indicates the user can change something to recover (if the error is reported to the user) might help as well.

I would like to say all 400 errors are recoverable, if the user has the right info to know what was wrong. But I am not sure that's true. Certainly a 403 error generated because the user isn't authorized to use the rest interface or a 404 error would not be recoverable. I assume there are unrecoverable 400 errors too.

For this case (indeed for all linked props) using a dropdown is probably the way to go, but it does prevent using a search term like: unread,chatting but for the classhelper use case it might not matter to have a more constrained search capability.

patel-malav commented 4 months ago

@rouilj just handling the 400 error in the search event as I am able to test it right now.

Not sure about the openPopup and nextPage, prevPage 400 errors should be recoverable.

Here if we got the 400, I am parsing the error message for the double-quoted strings "open" and "status" as they are value and the field on which the value is Invalid

{"error":{"status":400, "msg":"'No key (name) value \"open\" for \"status\"'"}}

If there are 2 wrong input values say status: "open" and keyword: "test", the backend only sends for the 1 possible wrong input.

Then I parse this msg string from the body using regex (ikr but its really small and easily understandable - /"(.*?)"/ ) search the input using the field value add "search-error" class show message of the value being invalid right next to input, mark the input as red (error).

Now if the 400 error message did not have the 2 double-quoted-strings and if it have a 2 double-quoted strings but second value was not a field in the search Fragment then we just throw Error and top-level error handler would log those unrecoverable 400 errors in the console and trigger fallback.

only the happy error path of 400 will show this error messages and do early return

Future enhancement could be done to use HTML setCustomValidity instead then currently just applying the search-error class

errormsg errormsg2

patel-malav commented 4 months ago

@rouilj please review #62

rouilj commented 4 months ago

I have opened #69 to move error message below input rather than next to input. On my phone the error gets cut off.

Closing this as it is complete.