-
Notifications
You must be signed in to change notification settings - Fork 513
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
fix(ai-help): handle invalid chat ids correctly #11678
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine, but might be possible to do this with useSearchParams directly.
Also missing a useEffect depencency
client/src/plus/ai-help/use-ai.ts
Outdated
navigate( | ||
{ | ||
search: searchParams.toString(), | ||
}, | ||
{ replace: true } | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might be possible to just use useSearchParams
if the docs can be believed:
The setSearchParams function works like navigate, but only for the search portion of the URL. Also note that the second arg to setSearchParams is the same type as the second arg to navigate.
https://reactrouter.com/en/main/hooks/use-search-params
Something like:
const [_, setSearchParams] = useSearchParams();
...
setSearchParams(
(searchParams) => {
const x = new URLSearchParams(searchParams)
x.delete("c")
return x
},
{ replace: true }
)
Summary
This fixes an issue where an AI Help URL with a chat id parameter (
&c=...
) is being requested and the chat referenced eitherWhile not leaking information to unauthorized accounts, there were a set of of confusing errors displayed.
The behaviour now has changed. If the chat belongs to a different user or has been deleted, the backend's
404
response is being catched and the user gets a fresh AI Help chat with the parameter cleared from the URL.We also clear the plate if history is disabled for the user.
(MP-1455)
How did you test this change?
1
2
3
4