Closed cayb0rg closed 4 months ago
Since it's related and was originally going to include changes to Msg::no_login()
and similar API error states, I'd like to consider merging https://github.com/ucfopen/Materia/pull/1537 into this PR as well
Actually maybe we just merge #1537 into the dev branch first and then this branch incorporates the upstream changes (there shouldn't be much overlap)
Great catch! The 500 error seemed to be coming from an uninitialized $results
array. user_get
should now respond with an empty array if the user_ids
are not positive integers (such as in the case where an instance ID is passed in accidentally). user_get
will respond with a 403 error if the current user is not logged in, hence the need for a gate for components like header.jsx
. However, if there's no need to gate this function from non-logged in users, this 403 error can be removed along with the need to gate it with apiAuthorVerify
. I think that could be a privacy risk, though.
I also fixed the issue with the copy success handler setting the hash to the new instance ID and added an optimistic update to add the new instance to the user's managed instances.
If you still get the 500 error, please let me know.
Updated to point to 10.2.0. I can give this a final review but there appears to be a number of merge conflicts that require resolution first.
With additional testing, I haven't found any other error states that cause issues on the front-end, so that's all looking great.
The 403 error associated with question_set_get
with new creator saves is fine - I see now the 3x retry behavior. It's unpleasant to see in the network tab, but doesn't cause any actual issues. I'd like to fix the creator to not perform that call, but I'm also happy to address that outside of this branch, since it's not really associated with these changes.
Really the only actual problem is the 401 associated with edit_perms_verify
on the My Widgets page.
widget_instance_edit_perms_verify
, called when selecting a widget in My Widgets, will return a401
if the user has insufficient edit perms - including View Scores access. React Query will continue to retry this request indefinitely, eventually erroring out the selected instance component with thePermission Denied: Failed to retrieve widget.
message.
Excellent catch! That function should probably not be returning an error in the first place, since its intended function is to return true/false for whether the user has edit permissions.
I also noticed that users with View Scores access can update widgets in the creator. I could not think of a specific use case where this should be possible, so I've limited the widget_instance_update
function to users with Full access only. Let me know if this is in error!
When saving a newly created instance in the creator for the first time,
widget_instance_save
is called followed byquestion_set_get
. However,question_set_get
is not properly called, instead sending a pair ofnulls
for arguments. This call was probably unintended to begin with, but with the update the server responds with403
and react query again enters an indefinite retry loop.
I included a temporary fix that sends the state value instance.id
instead of the parameter instId
that comes from the hash to question_set_get
for now.
Just needed to pull dev/10.2.0, as it seems you already changed it to instIdRef.current
in the Save History fix PR! Sweet.
This PR aims to solve how errors are both sent from FuelPHP's core and handled by the front-end (#1526)
Changes
Msg
objectspost_call
andget_call
infuel/app/classes/controller/api.php
are responsible for handling responses for api calls. These will now recursively search for a 'status' field in the called function result and set that as the response status rather than an erroneous 200.api.js
. This essentially checks the status code if the response returns not OK and throws an error for individual error handlers to catch. This handler accounts for the variety of responses that can be sent from the back-end. For example:Response
class rather than theMsg
class, the error will be stored in thebody
field and must be parsed as JSON.fetch
calls to theapi/json/___
route to usefetchGet
which automatically configures the POST options and invokes the global error handler.user_get
. Theuser_get
function returns a 403 error if the user is not logged in. A non-logged in user would then get spammed with 403's in the console. Instead of changinguser_get
, which needs to return 403 in some situations, I addedapiAuthorVerify
to gate this API call.Msg::no_login()
responses are only sent ifverify_session
has been called, which will logout the user and clear the session date.Additional Findings
React Query has the option of setting a global error handler when initializing the QueryCache. This is only called if
onError
is not defined in the API call.Potential issues