-
Notifications
You must be signed in to change notification settings - Fork 122
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
[Assistant views] Remove 'assistant-search' & '{conversationId}' views #8109
Conversation
|
57d1643
to
13f4835
Compare
Description --- - Removes 'assistant-search' view, which was actually exactly the same as "list". The comment indicating inactive global agents were returned was erroneous - Removes '{conversationId}' view, since: - it was actually only used in AssistantSuggestion, not anywhere else; - the view didn't actually work, it was ~equivalent to list; - the use case it was supposed to enable, viewing a shared conversation with somebody else's personal assistant, is already somewhat working: user can view conversation but not mention assistant or see its details which is kind of fine. We could aim in a follow-up PR to reinstate the conversation view cleanly; that would require more work and is IMO not a priority. Risks --- Low, since it's just a view change. Tested locally. Regarding conversationId: - handled all explicit calls - looked for agentsGetView with potential implicit `agentsGetView: { conversationId }`: there are - the internal api `asisstant/agent_configurations`, which is only called via swr's `useAgentConfigurations` => checked it does not use conversationId (since it also takes agentsGetView as param) - the v1 api DustApi.getAgentsConfiguration call => here also, conversationId not used Deploy --- front
13f4835
to
003fa4b
Compare
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.
LGTM
A lint run related with cacheWithRedis is visible here. Since you touched it recently maybe we can add a comment to disable it or use unknown?
Done |
53377da
to
c1210ca
Compare
c1210ca
to
6b14597
Compare
Description
Removes 'assistant-search' view, which was actually exactly the same as "list". The comment indicating inactive global agents were returned was erroneous
Removes '{conversationId}' view, since:
We could aim in a follow-up PR to reinstate the conversation view cleanly; that would require more work and is IMO low value and not a priority.
Risks
Low, since it's just a view change. Tested locally
Regarding the conversationId removal specifically:
conversationId
agentsGetView: { conversationId }
: there areasisstant/agent_configurations
, which is only called via swr'suseAgentConfigurations
=> checked it does not use conversationId (since it also takes agentsGetView as param)
Deploy
front