Remove notification dependency from uiSettings (#35239) (#35287)

* Remove notification dependency from uiSettings

* Update API docs

* Fix prettier

Still had eslint not setup correctly for TS files

* Change i18n id

docs/development/core/public/kibana-plugin-public.uisettingsclient.getupdateerrors$.md

@@ -0,0 +1,17 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index) &gt; [kibana-plugin-public](./kibana-plugin-public.md) &gt; [UiSettingsClient](./kibana-plugin-public.uisettingsclient.md) &gt; [getUpdateErrors$](./kibana-plugin-public.uisettingsclient.getupdateerrors$.md)
+
+## UiSettingsClient.getUpdateErrors$() method
+
+Returns an Observable that notifies subscribers of each error while trying to update the settings, containing the actual Error class.
+
+<b>Signature:</b>
+
+```typescript
+getUpdateErrors$(): Rx.Observable<Error>;
+```
+<b>Returns:</b>
+
+`Rx.Observable<Error>`
+

docs/development/core/public/kibana-plugin-public.uisettingsclient.md

@@ -26,6 +26,7 @@ export declare class UiSettingsClient
 |  [getAll()](./kibana-plugin-public.uisettingsclient.getall.md) |  | Gets the metadata about all uiSettings, including the type, default value, and user value for each key. |
 |  [getSaved$()](./kibana-plugin-public.uisettingsclient.getsaved$.md) |  | Returns an Observable that notifies subscribers of each update to the uiSettings, including the key, newValue, and oldValue of the setting that changed. |
 |  [getUpdate$()](./kibana-plugin-public.uisettingsclient.getupdate$.md) |  | Returns an Observable that notifies subscribers of each update to the uiSettings, including the key, newValue, and oldValue of the setting that changed. |
+|  [getUpdateErrors$()](./kibana-plugin-public.uisettingsclient.getupdateerrors$.md) |  | Returns an Observable that notifies subscribers of each error while trying to update the settings, containing the actual Error class. |
 |  [isCustom(key)](./kibana-plugin-public.uisettingsclient.iscustom.md) |  | Returns true if the setting is not a part of the uiSettingDefaults, but was either added directly via <code>set()</code>, or is an unknown setting found in the uiSettings saved object |
 |  [isDeclared(key)](./kibana-plugin-public.uisettingsclient.isdeclared.md) |  | Returns true if the key is a "known" uiSetting, meaning it is either defined in the uiSettingDefaults or was previously added as a custom setting via the <code>set()</code> method. |
 |  [isDefault(key)](./kibana-plugin-public.uisettingsclient.isdefault.md) |  | Returns true if the setting has no user-defined value or is unknown |

src/core/public/core_system.ts

@@ -126,7 +126,6 @@ export class CoreSystem {
   public async setup() {
     try {
       const i18n = this.i18n.setup();
-      const notifications = this.notifications.setup({ i18n });
       const injectedMetadata = this.injectedMetadata.setup();
       const fatalErrors = this.fatalErrors.setup({ i18n });
       const http = this.http.setup({ fatalErrors });
@@ -134,11 +133,11 @@ export class CoreSystem {
       const basePath = this.basePath.setup({ injectedMetadata });
       const capabilities = this.capabilities.setup({ injectedMetadata });
       const uiSettings = this.uiSettings.setup({
-        notifications,
         http,
         injectedMetadata,
         basePath,
       });
+      const notifications = this.notifications.setup({ i18n, uiSettings });
       const chrome = this.chrome.setup({
         injectedMetadata,
         notifications,

src/core/public/notifications/notifications_service.ts

@@ -17,9 +17,12 @@
  * under the License.
  */
 
+import { i18n } from '@kbn/i18n';
+
 import { Observable, Subject, Subscription } from 'rxjs';
 import { I18nSetup } from '../i18n';
 import { ToastsService } from './toasts';
+import { UiSettingsSetup } from '../ui_settings';
 
 interface NotificationServiceParams {
   targetDomElement$: Observable<HTMLElement>;
@@ -27,6 +30,7 @@ interface NotificationServiceParams {
 
 interface NotificationsServiceDeps {
   i18n: I18nSetup;
+  uiSettings: UiSettingsSetup;
 }
 
 /** @public */
@@ -35,6 +39,7 @@ export class NotificationsService {
 
   private readonly toastsContainer$: Subject<HTMLElement>;
   private domElemSubscription?: Subscription;
+  private uiSettingsErrorSubscription?: Subscription;
   private targetDomElement?: HTMLElement;
 
   constructor(private readonly params: NotificationServiceParams) {
@@ -44,7 +49,7 @@ export class NotificationsService {
     });
   }
 
-  public setup({ i18n }: NotificationsServiceDeps) {
+  public setup({ i18n: i18nDep, uiSettings }: NotificationsServiceDeps) {
     this.domElemSubscription = this.params.targetDomElement$.subscribe({
       next: targetDomElement => {
         this.cleanupTargetDomElement();
@@ -56,7 +61,18 @@ export class NotificationsService {
       },
     });
 
-    return { toasts: this.toasts.setup({ i18n }) };
+    const notificationSetup = { toasts: this.toasts.setup({ i18n: i18nDep }) };
+
+    this.uiSettingsErrorSubscription = uiSettings.getUpdateErrors$().subscribe(error => {
+      notificationSetup.toasts.addDanger({
+        title: i18n.translate('core.notifications.unableUpdateUISettingNotificationMessageTitle', {
+          defaultMessage: 'Unable to update UI setting',
+        }),
+        text: error.message,
+      });
+    });
+
+    return notificationSetup;
   }
 
   public stop() {
@@ -66,6 +82,10 @@ export class NotificationsService {
     if (this.domElemSubscription) {
       this.domElemSubscription.unsubscribe();
     }
+
+    if (this.uiSettingsErrorSubscription) {
+      this.uiSettingsErrorSubscription.unsubscribe();
+    }
   }
 
   private cleanupTargetDomElement() {

src/core/public/public.api.md

@@ -276,6 +276,7 @@ export class UiSettingsClient {
         newValue: any;
         oldValue: any;
     }>;
+    getUpdateErrors$(): Rx.Observable<Error>;
     isCustom(key: string): boolean;
     isDeclared(key: string): boolean;
     isDefault(key: string): boolean;

src/core/public/ui_settings/__snapshots__/ui_settings_service.test.ts.snap

@@ -48,7 +48,6 @@ exports[`#setup constructs UiSettingsClient and UiSettingsApi: UiSettingsClient
         "initialSettings": Object {
           "legacyInjectedUiSettingUserValues": true,
         },
-        "onUpdateError": [Function],
       },
     ],
   ],

src/core/public/ui_settings/ui_settings_client.test.ts

@@ -28,18 +28,15 @@ function setup(options: { defaults?: any; initialSettings?: any } = {}) {
     settings: {},
   }));
 
-  const onUpdateError = jest.fn();
-
   const config = new UiSettingsClient({
     defaults,
     initialSettings,
     api: {
       batchSet,
     } as any,
-    onUpdateError,
   });
 
-  return { config, batchSet, onUpdateError };
+  return { config, batchSet };
 }
 
 describe('#get', () => {

src/core/public/ui_settings/ui_settings_client.ts

@@ -27,7 +27,6 @@ import { UiSettingsApi } from './ui_settings_api';
 /** @public */
 interface UiSettingsClientParams {
   api: UiSettingsApi;
-  onUpdateError: UiSettingsClient['onUpdateError'];
   defaults: UiSettingsState;
   initialSettings?: UiSettingsState;
 }
@@ -36,15 +35,14 @@ interface UiSettingsClientParams {
 export class UiSettingsClient {
   private readonly update$ = new Rx.Subject<{ key: string; newValue: any; oldValue: any }>();
   private readonly saved$ = new Rx.Subject<{ key: string; newValue: any; oldValue: any }>();
+  private readonly updateErrors$ = new Rx.Subject<Error>();
 
   private readonly api: UiSettingsApi;
-  private readonly onUpdateError: (error: Error) => void;
   private readonly defaults: UiSettingsState;
   private cache: UiSettingsState;
 
   constructor(readonly params: UiSettingsClientParams) {
     this.api = params.api;
-    this.onUpdateError = params.onUpdateError;
     this.defaults = cloneDeep(params.defaults);
     this.cache = defaultsDeep({}, this.defaults, cloneDeep(params.initialSettings));
   }
@@ -206,6 +204,14 @@ You can use \`config.get("${key}", defaultValue)\`, which will just return
     return this.saved$.asObservable();
   }
 
+  /**
+   * Returns an Observable that notifies subscribers of each error while trying to update
+   * the settings, containing the actual Error class.
+   */
+  public getUpdateErrors$() {
+    return this.updateErrors$.asObservable();
+  }
+
   /**
    * Prepares the uiSettingsClient to be discarded, completing any update$ observables
    * that have been created.
@@ -246,7 +252,7 @@ You can use \`config.get("${key}", defaultValue)\`, which will just return
       return true;
     } catch (error) {
       this.setLocally(key, initialVal);
-      this.onUpdateError(error);
+      this.updateErrors$.next(error);
       return false;
     }
   }

src/core/public/ui_settings/ui_settings_service.mock.ts

@@ -32,6 +32,7 @@ const createSetupContractMock = () => {
     overrideLocalDefault: jest.fn(),
     getUpdate$: jest.fn(),
     getSaved$: jest.fn(),
+    getUpdateErrors$: jest.fn(),
     stop: jest.fn(),
   };
   // we have to suppress type errors until decide how to mock es6 class

src/core/public/ui_settings/ui_settings_service.test.ts

@@ -22,13 +22,11 @@ import { MockUiSettingsApi, MockUiSettingsClient } from './ui_settings_service.t
 import { basePathServiceMock } from '../base_path/base_path_service.mock';
 import { httpServiceMock } from '../http/http_service.mock';
 import { injectedMetadataServiceMock } from '../injected_metadata/injected_metadata_service.mock';
-import { notificationServiceMock } from '../notifications/notifications_service.mock';
 import { UiSettingsService } from './ui_settings_service';
 
 const httpSetup = httpServiceMock.createSetupContract();
 
 const defaultDeps = {
-  notifications: notificationServiceMock.createSetupContract(),
   http: httpSetup,
   injectedMetadata: injectedMetadataServiceMock.createSetupContract(),
   basePath: basePathServiceMock.createSetupContract(),

src/core/public/ui_settings/ui_settings_service.ts

@@ -17,17 +17,14 @@
  * under the License.
  */
 
-import { i18n } from '@kbn/i18n';
 import { BasePathSetup } from '../base_path';
 import { HttpSetup } from '../http';
 import { InjectedMetadataSetup } from '../injected_metadata';
-import { NotificationsSetup } from '../notifications';
 
 import { UiSettingsApi } from './ui_settings_api';
 import { UiSettingsClient } from './ui_settings_client';
 
 interface UiSettingsServiceDeps {
-  notifications: NotificationsSetup;
   http: HttpSetup;
   injectedMetadata: InjectedMetadataSetup;
   basePath: BasePathSetup;
@@ -38,12 +35,7 @@ export class UiSettingsService {
   private uiSettingsApi?: UiSettingsApi;
   private uiSettingsClient?: UiSettingsClient;
 
-  public setup({
-    notifications,
-    http,
-    injectedMetadata,
-    basePath,
-  }: UiSettingsServiceDeps): UiSettingsSetup {
+  public setup({ http, injectedMetadata, basePath }: UiSettingsServiceDeps): UiSettingsSetup {
     this.uiSettingsApi = new UiSettingsApi(basePath, injectedMetadata.getKibanaVersion());
     http.addLoadingCount(this.uiSettingsApi.getLoadingCount$());
 
@@ -52,14 +44,6 @@ export class UiSettingsService {
 
     this.uiSettingsClient = new UiSettingsClient({
       api: this.uiSettingsApi,
-      onUpdateError: error => {
-        notifications.toasts.addDanger({
-          title: i18n.translate('core.uiSettings.unableUpdateUISettingNotificationMessageTitle', {
-            defaultMessage: 'Unable to update UI setting',
-          }),
-          text: error.message,
-        });
-      },
       defaults: legacyMetadata.uiSettings.defaults,
       initialSettings: legacyMetadata.uiSettings.user,
     });