Fix banner not being displayed on login page (#140688)

* Fix banner not being displayed on login page

* fix and re-enable banner FTR tests

* revert commit to config file

* add unit test

* don't use whitespace for cli config value

Co-authored-by: Tre <wayne.seymour@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

.buildkite/ftr_configs.yml

@@ -48,7 +48,6 @@ disabled:
   - x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/actions/config.ts
   - x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/telemetry/config.ts
   - x-pack/test/alerting_api_integration/spaces_only_legacy/config.ts
-  - x-pack/test/banners_functional/config.ts
   - x-pack/test/cloud_integration/config.ts
   - x-pack/test/performance/config.playwright.ts
   - x-pack/test/load/config.ts
@@ -127,6 +126,7 @@ enabled:
   - x-pack/test/apm_api_integration/basic/config.ts
   - x-pack/test/apm_api_integration/rules/config.ts
   - x-pack/test/apm_api_integration/trial/config.ts
+  - x-pack/test/banners_functional/config.ts
   - x-pack/test/cases_api_integration/security_and_spaces/config_basic.ts
   - x-pack/test/cases_api_integration/security_and_spaces/config_trial.ts
   - x-pack/test/cases_api_integration/spaces_only/config.ts

x-pack/plugins/banners/kibana.json

@@ -8,8 +8,9 @@
   "kibanaVersion": "kibana",
   "server": true,
   "ui": true,
-  "requiredPlugins": ["licensing", "screenshotMode"],
-  "optionalPlugins": [],
+  "enabledOnAnonymousPages": true,
+  "requiredPlugins": ["licensing"],
+  "optionalPlugins": ["screenshotMode"],
   "requiredBundles": ["kibanaReact"],
   "configPath": ["xpack", "banners"]
 }

x-pack/plugins/banners/public/plugin.test.tsx

@@ -40,11 +40,15 @@ describe('BannersPlugin', () => {
     });
   });
 
-  const startPlugin = async () => {
+  const startPlugin = async ({
+    screenshotPluginPresent = true,
+  }: { screenshotPluginPresent?: boolean } = {}) => {
     pluginInitContext = coreMock.createPluginInitializerContext();
     plugin = new BannersPlugin(pluginInitContext);
     plugin.setup(coreSetup);
-    plugin.start(coreStart, { screenshotMode: screenshotModeStart });
+    plugin.start(coreStart, {
+      screenshotMode: screenshotPluginPresent ? screenshotModeStart : undefined,
+    });
     // await for the `getBannerInfo` promise to resolve
     await nextTick();
   };
@@ -84,12 +88,32 @@ describe('BannersPlugin', () => {
     });
 
     it('does not register the banner in screenshot mode', async () => {
+      getBannerInfoMock.mockResolvedValue({
+        allowed: true,
+        banner: createBannerConfig({
+          placement: 'top',
+        }),
+      });
+
       screenshotModeStart.isScreenshotMode.mockReturnValue(true);
 
       await startPlugin();
 
       expect(coreStart.chrome.setHeaderBanner).not.toHaveBeenCalled();
     });
+
+    it('registers the banner if screenshotMode plugin not present', async () => {
+      getBannerInfoMock.mockResolvedValue({
+        allowed: true,
+        banner: createBannerConfig({
+          placement: 'top',
+        }),
+      });
+
+      await startPlugin({ screenshotPluginPresent: false });
+
+      expect(coreStart.chrome.setHeaderBanner).toHaveBeenCalled();
+    });
   });
 
   describe('when banner is not allowed', () => {
@@ -116,7 +140,7 @@ describe('BannersPlugin', () => {
 
       await startPlugin();
 
-      expect(coreStart.chrome.setHeaderBanner).toHaveBeenCalledTimes(0);
+      expect(coreStart.chrome.setHeaderBanner).not.toHaveBeenCalled();
     });
 
     it('does not register the banner in screenshot mode', async () => {

x-pack/plugins/banners/public/plugin.tsx

@@ -23,7 +23,7 @@ export class BannersPlugin implements Plugin<{}, {}, {}, BannerPluginStartDepend
     { chrome, uiSettings, http }: CoreStart,
     { screenshotMode }: BannerPluginStartDependencies
   ) {
-    if (!screenshotMode.isScreenshotMode()) {
+    if (!(screenshotMode?.isScreenshotMode() ?? false)) {
       getBannerInfo(http).then(
         ({ allowed, banner }) => {
           if (allowed && banner.placement === 'top') {

x-pack/plugins/banners/public/types.ts

@@ -8,5 +8,5 @@
 import type { ScreenshotModePluginStart } from '@kbn/screenshot-mode-plugin/public';
 
 export interface BannerPluginStartDependencies {
-  screenshotMode: ScreenshotModePluginStart;
+  screenshotMode?: ScreenshotModePluginStart;
 }

x-pack/test/banners_functional/config.ts

@@ -35,7 +35,7 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) {
       serverArgs: [
         ...kibanaFunctionalConfig.get('kbnTestServer.serverArgs'),
         '--xpack.banners.placement=top',
-        '--xpack.banners.textContent="global banner text"',
+        '--xpack.banners.textContent=global_banner_text',
       ],
     },
   };

x-pack/test/banners_functional/tests/global.ts

@@ -16,7 +16,7 @@ export default function ({ getPageObjects }: FtrProviderContext) {
       await PageObjects.common.navigateToApp('login');
 
       expect(await PageObjects.banners.isTopBannerVisible()).to.eql(true);
-      expect(await PageObjects.banners.getTopBannerText()).to.eql('global banner text');
+      expect(await PageObjects.banners.getTopBannerText()).to.eql('global_banner_text');
     });
   });
 }

x-pack/test/banners_functional/tests/spaces.ts

@@ -10,6 +10,7 @@ import { FtrProviderContext } from '../ftr_provider_context';
 
 export default function ({ getPageObjects, getService }: FtrProviderContext) {
   const esArchiver = getService('esArchiver');
+  const kibanaServer = getService('kibanaServer');
   const PageObjects = getPageObjects([
     'common',
     'security',
@@ -28,18 +29,15 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
     });
 
     before(async () => {
+      await kibanaServer.uiSettings.replace(
+        {
+          'banners:textContent': 'default space banner text',
+        },
+        { space: 'default' }
+      );
       await PageObjects.security.login(undefined, undefined, {
         expectSpaceSelector: true,
       });
-      await PageObjects.spaceSelector.clickSpaceCard('default');
-
-      await PageObjects.settings.navigateTo();
-      await PageObjects.settings.clickKibanaSettings();
-
-      await PageObjects.settings.setAdvancedSettingsTextArea(
-        'banners:textContent',
-        'default space banner text'
-      );
     });
 
     it('displays the space-specific banner within the space', async () => {
@@ -53,7 +51,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
       await PageObjects.common.navigateToApp('home', { basePath: '/s/another-space' });
 
       expect(await PageObjects.banners.isTopBannerVisible()).to.eql(true);
-      expect(await PageObjects.banners.getTopBannerText()).to.eql('global banner text');
+      expect(await PageObjects.banners.getTopBannerText()).to.eql('global_banner_text');
     });
 
     it('displays the global banner on the login page', async () => {
@@ -61,7 +59,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
       await PageObjects.common.navigateToApp('login');
 
       expect(await PageObjects.banners.isTopBannerVisible()).to.eql(true);
-      expect(await PageObjects.banners.getTopBannerText()).to.eql('global banner text');
+      expect(await PageObjects.banners.getTopBannerText()).to.eql('global_banner_text');
     });
   });
 }