disable rename conversation api using feature flag #400
Conversation
Signed-off-by: Kama Huang <[email protected]>
kamahuan
requested review from
lezzago,
joshuali925,
anirudha,
ruanyl,
SuZhou-Joe,
xluo-aws,
gaobinlong,
Hailong-am,
wanglam and
raintygao
as code owners
| @@ -50,6 +52,13 @@ export const EditConversationNameModal = ({ | |||
| } | |||
| onClose?.('updated', title); | |||
| }, [onClose, conversationId, patchConversation, toasts.addSuccess, toasts.addDanger, isAborted]); | |||
| console.log('Rename conversation enabled:', configSchema.chat.allowRenameConversation); | |||
| @@ -11,6 +11,7 @@ export const configSchema = schema.object({ | |||
| enabled: schema.boolean({ defaultValue: false }), | |||
| trace: schema.boolean({ defaultValue: true }), | |||
| feedback: schema.boolean({ defaultValue: true }), | |||
| allowRenameConversation: schema.boolean({ defaultValue: true }), | |||
There was a problem hiding this comment.
What about name it as coversationManagement? This flag will be used to control conversation edit and delete, am I right?
There was a problem hiding this comment.
This flag and PR is only about disabling the rename conversation feature. Will submit another PR about disabling delete conversation api.
| color="text" | ||
| /> | ||
| </EuiFlexItem> | ||
| )} | ||
| <EuiFlexItem grow={false}> |
There was a problem hiding this comment.
we also need to hide the delete button.
There was a problem hiding this comment.
Will submit a separate PR about deleting the conversation api.
| onClose?.('cancelled'); | ||
| return null; |
There was a problem hiding this comment.
Q: curious why adding this? My understanding is there will be no place to trigger this modal as we hide the edit button from the dropdown menu. Perhaps not necessary?
There was a problem hiding this comment.
Good catch. we can remove this feature flag check and test since the edit button is already hidden when the feature is disabled. Having the check in both places is redundant. I'll update the PR to simplify this.
| jest.spyOn(services, 'getConfigSchema').mockReturnValue({ | ||
| enabled: true, | ||
| chat: { | ||
| enabled: true, | ||
| allowRenameConversation: true, | ||
| trace: true, | ||
| feedback: true, | ||
| }, | ||
| incontextInsight: { enabled: true }, | ||
| next: { enabled: false }, | ||
| text2viz: { enabled: false }, | ||
| alertInsight: { enabled: false }, | ||
| smartAnomalyDetector: { enabled: false }, | ||
| branding: { label: undefined, logo: undefined }, | ||
| }); | ||
|
|
There was a problem hiding this comment.
I see this piece of code is repeating in many test files, what about to have this mock in test/setup.jest.ts? This will make the mock work for every tests, and we only need to do manual mock at where we need to test the change the default value.
// test/setup.jest.ts
const DEFAULT_CONFIG = Object.freeze({
enabled: true,
chat: {
enabled: true,
allowRenameConversation: true,
trace: true,
feedback: true,
},
incontextInsight: { enabled: true },
next: { enabled: false },
text2viz: { enabled: false },
alertInsight: { enabled: false },
smartAnomalyDetector: { enabled: false },
branding: { label: undefined, logo: undefined },
});
jest.spyOn(services, 'getConfigSchema').mockReturnValue(DEFAULT_CONFIG);
There was a problem hiding this comment.
Yes will use a separate file to put this config
|
@kamahuan would you mind to change the target branch to |
Description
Disable the rename conversation api using feature flag. Upon checking with Binlong and Arjun, disable the api from the frontend is sufficient.
Issues Resolved
NONE
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.