Move chat flag evaluations to the route handler

afharo · afharo · commit 9c97aaa0d66e · 2024-08-19T20:22:16.000+02:00
diff --git a/x-pack/plugins/cloud_integrations/cloud_chat/server/plugin.ts b/x-pack/plugins/cloud_integrations/cloud_chat/server/plugin.ts
@@ -10,7 +10,6 @@ import { PluginInitializerContext, CoreSetup, Plugin } from '@kbn/core/server';
 import type { CloudSetup } from '@kbn/cloud-plugin/server';
 import { registerChatRoute } from './routes';
 import type { CloudChatConfigType } from './config';
-import type { ChatVariant } from '../common/types';
 
 interface CloudChatSetupDeps {
   cloud: CloudSetup;
@@ -36,16 +35,6 @@ export class CloudChatPlugin implements Plugin<void, void, CloudChatSetupDeps> {
         trialEndDate,
         trialBuffer,
         isDev: this.isDev,
-        getChatVariant: () =>
-          core
-            .getStartServices()
-            .then(([{ featureFlags }]) =>
-              featureFlags.getStringValue<ChatVariant>('cloud-chat.chat-variant', 'header')
-            ),
-        getChatDisabledThroughExperiments: () =>
-          core
-            .getStartServices()
-            .then(([{ featureFlags }]) => featureFlags.getBooleanValue('cloud-chat.enabled', true)),
       });
     }
   }
diff --git a/x-pack/plugins/cloud_integrations/cloud_chat/server/routes/chat.test.ts b/x-pack/plugins/cloud_integrations/cloud_chat/server/routes/chat.test.ts
@@ -16,20 +16,22 @@ import {
   httpServerMock,
   coreMock,
   securityServiceMock,
+  coreFeatureFlagsMock,
 } from '@kbn/core/server/mocks';
 import { kibanaResponseFactory } from '@kbn/core/server';
 import { type MetaWithSaml, registerChatRoute } from './chat';
-import { ChatVariant } from '../../common/types';
 
 describe('chat route', () => {
-  const getChatVariant = async (): Promise<ChatVariant> => 'header';
-  const getChatDisabledThroughExperiments = async (): Promise<boolean> => false;
   let security: ReturnType<typeof securityServiceMock.createRequestHandlerContext>;
   let requestHandlerContextMock: ReturnType<typeof coreMock.createCustomRequestHandlerContext>;
+  let featureFlags: ReturnType<typeof coreFeatureFlagsMock.createRequestHandlerContext>;
 
   beforeEach(() => {
     const core = coreMock.createRequestHandlerContext();
     security = core.security;
+    featureFlags = core.featureFlags;
+    featureFlags.getStringValue.mockResolvedValue('header');
+    featureFlags.getBooleanValue.mockResolvedValue(true);
     requestHandlerContextMock = coreMock.createCustomRequestHandlerContext({ core });
   });
 
@@ -43,8 +45,6 @@ describe('chat route', () => {
       chatIdentitySecret: 'secret',
       trialBuffer: 60,
       trialEndDate: new Date(),
-      getChatVariant,
-      getChatDisabledThroughExperiments,
     });
 
     const [_config, handler] = router.get.mock.calls[0];
@@ -78,8 +78,6 @@ describe('chat route', () => {
       chatIdentitySecret: 'secret',
       trialBuffer: 60,
       trialEndDate: new Date(),
-      getChatVariant,
-      getChatDisabledThroughExperiments,
     });
 
     const [_config, handler] = router.get.mock.calls[0];
@@ -120,8 +118,6 @@ describe('chat route', () => {
       isDev: false,
       chatIdentitySecret: 'secret',
       trialBuffer: 2,
-      getChatVariant,
-      getChatDisabledThroughExperiments,
     });
 
     const [_config, handler] = router.get.mock.calls[0];
@@ -165,8 +161,6 @@ describe('chat route', () => {
       chatIdentitySecret: 'secret',
       trialBuffer: 2,
       trialEndDate,
-      getChatVariant,
-      getChatDisabledThroughExperiments,
     });
 
     const [_config, handler] = router.get.mock.calls[0];
@@ -202,14 +196,13 @@ describe('chat route', () => {
     );
 
     const router = httpServiceMock.createRouter();
+    featureFlags.getBooleanValue.mockResolvedValueOnce(false);
     registerChatRoute({
       router,
       isDev: false,
       chatIdentitySecret: 'secret',
       trialBuffer: 60,
       trialEndDate: new Date(),
-      getChatVariant,
-      getChatDisabledThroughExperiments: async () => true,
     });
     const [_config, handler] = router.get.mock.calls[0];
     await expect(
@@ -249,8 +242,6 @@ describe('chat route', () => {
       chatIdentitySecret: 'secret',
       trialBuffer: 60,
       trialEndDate: new Date(),
-      getChatVariant,
-      getChatDisabledThroughExperiments,
     });
     const [_config, handler] = router.get.mock.calls[0];
     await expect(
@@ -297,8 +288,6 @@ describe('chat route', () => {
       chatIdentitySecret: 'secret',
       trialBuffer: 60,
       trialEndDate: new Date(),
-      getChatVariant,
-      getChatDisabledThroughExperiments,
     });
     const [_config, handler] = router.get.mock.calls[0];
     await expect(
@@ -342,14 +331,13 @@ describe('chat route', () => {
     );
 
     const router = httpServiceMock.createRouter();
+    featureFlags.getStringValue.mockResolvedValueOnce('bubble');
     registerChatRoute({
       router,
       isDev: false,
       chatIdentitySecret: 'secret',
       trialBuffer: 60,
       trialEndDate: new Date(),
-      getChatVariant: async () => 'bubble',
-      getChatDisabledThroughExperiments,
     });
     const [_config, handler] = router.get.mock.calls[0];
     await expect(
diff --git a/x-pack/plugins/cloud_integrations/cloud_chat/server/routes/chat.ts b/x-pack/plugins/cloud_integrations/cloud_chat/server/routes/chat.ts
@@ -24,28 +24,20 @@ export const registerChatRoute = ({
   trialEndDate,
   trialBuffer,
   isDev,
-  getChatVariant,
-  getChatDisabledThroughExperiments,
 }: {
   router: IRouter;
   chatIdentitySecret: string;
   trialEndDate?: Date;
   trialBuffer: number;
   isDev: boolean;
-  getChatVariant: () => Promise<ChatVariant>;
-  /**
-   * Returns true if chat is disabled in LaunchDarkly
-   * Meant to be used as a runtime kill switch
-   */
-  getChatDisabledThroughExperiments: () => Promise<boolean>;
 }) => {
   router.get(
     {
       path: GET_CHAT_USER_DATA_ROUTE_PATH,
       validate: {},
     },
     async (context, request, response) => {
-      const { security } = await context.core;
+      const { security, featureFlags } = await context.core;
       const user = security.authc.getCurrentUser();
 
       if (!user) {
@@ -85,7 +77,8 @@ export const registerChatRoute = ({
         });
       }
 
-      if (await getChatDisabledThroughExperiments()) {
+      // Meant to be used as a runtime kill switch via LaunchDarkly
+      if (!(await featureFlags.getBooleanValue('cloud-chat.enabled', true).catch(() => false))) {
         return response.badRequest({
           body: 'Chat is disabled through experiments',
         });
@@ -96,7 +89,10 @@ export const registerChatRoute = ({
         token,
         email: userEmail,
         id: userId,
-        chatVariant: await getChatVariant(),
+        chatVariant: await featureFlags.getStringValue<ChatVariant>(
+          'cloud-chat.chat-variant',
+          'header'
+        ),
       };
       return response.ok({ body });
     }
