From ca5fa9ff1893f962fbbb6596c44dfc74080afd9b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Mon, 4 Mar 2019 15:47:51 +0100
Subject: [PATCH] display notifications in a declarative way

---
 web/static/src/ts/env.ts                      |  4 +-
 web/static/src/ts/store/notification_mixin.ts | 37 -----------------
 web/static/src/ts/store/store.ts              | 40 ++++++++++++++++---
 web/static/src/ts/ui/notification.ts          |  2 +-
 web/static/src/ts/ui/root.ts                  | 15 +------
 web/static/src/xml/templates.xml              |  6 ++-
 web/static/tests/store/store.test.ts          | 28 ++++---------
 .../tests/ui/__snapshots__/root.test.ts.snap  | 12 ++++--
 web/static/tests/ui/notification.test.ts      | 16 ++------
 9 files changed, 65 insertions(+), 95 deletions(-)
 delete mode 100644 web/static/src/ts/store/notification_mixin.ts

diff --git a/web/static/src/ts/env.ts b/web/static/src/ts/env.ts
index c9aae2c9..3c44c2e9 100644
--- a/web/static/src/ts/env.ts
+++ b/web/static/src/ts/env.ts
@@ -1,7 +1,7 @@
 import { WEnv } from "./core/component";
 import { QWeb } from "./core/qweb_vdom";
 import { idGenerator } from "./core/utils";
-import { INotification, RPC, Services, Store } from "./store/store";
+import { Notification, RPC, Services, Store } from "./store/store";
 
 //------------------------------------------------------------------------------
 // Types
@@ -12,7 +12,7 @@ export interface Env extends WEnv {
 
   // commands
   activateMenuItem(menuId: number): void;
-  addNotification(notif: Partial<INotification>): number;
+  addNotification(notif: Partial<Notification>): number;
   closeNotification(id: number): void;
   toggleHomeMenu(): void;
 
diff --git a/web/static/src/ts/store/notification_mixin.ts b/web/static/src/ts/store/notification_mixin.ts
deleted file mode 100644
index 59d04077..00000000
--- a/web/static/src/ts/store/notification_mixin.ts
+++ /dev/null
@@ -1,37 +0,0 @@
-import { Type } from "../core/component";
-import { BaseStore } from "./store";
-
-//------------------------------------------------------------------------------
-// Notifications Mixin
-//------------------------------------------------------------------------------
-export interface INotification {
-  id: number;
-  title: string;
-  message: string;
-  type: "notification" | "warning";
-  sticky: boolean;
-}
-
-export function notificationMixin<T extends Type<BaseStore>>(Base: T) {
-  return class extends Base {
-    addNotification(notif: Partial<INotification>): number {
-      const id = this.generateID();
-      const defaultVals = {
-        title: "",
-        message: "",
-        type: "notification",
-        sticky: false
-      };
-      const notification = Object.assign(defaultVals, notif, { id });
-      this.trigger("notification_added", notification);
-      if (!notification.sticky) {
-        setTimeout(() => this.closeNotification(id), 2500);
-      }
-      return id;
-    }
-
-    closeNotification(id: number) {
-      this.trigger("notification_closed", id);
-    }
-  };
-}
diff --git a/web/static/src/ts/store/store.ts b/web/static/src/ts/store/store.ts
index df00c900..fe28abea 100644
--- a/web/static/src/ts/store/store.ts
+++ b/web/static/src/ts/store/store.ts
@@ -4,7 +4,6 @@ import { idGenerator } from "../core/utils";
 import { RPC } from "../services/ajax";
 import { IRouter, Query } from "../services/router";
 import { actionManagerMixin, ActionWidget } from "./action_manager_mixin";
-import { notificationMixin } from "./notification_mixin";
 import { rpcMixin } from "./rpc_mixin";
 import { MenuItem } from "./store";
 
@@ -13,7 +12,6 @@ import { MenuItem } from "./store";
 //------------------------------------------------------------------------------
 
 export { ActionWidget } from "./action_manager_mixin";
-export { INotification } from "./notification_mixin";
 export { RPC } from "./rpc_mixin";
 
 export interface MenuItem {
@@ -39,6 +37,7 @@ export interface MenuInfo {
 export interface State {
   inHome: boolean;
   currentApp: MenuItem | null;
+  notifications: Notification[];
 }
 
 export interface Services {
@@ -46,13 +45,22 @@ export interface Services {
   router: IRouter;
 }
 
+export interface Notification {
+  id: number;
+  title: string;
+  message: string;
+  type: "notification" | "warning";
+  sticky: boolean;
+}
+
 //------------------------------------------------------------------------------
 // Store
 //------------------------------------------------------------------------------
 export class BaseStore extends EventBus {
   state: State = {
     inHome: false,
-    currentApp: null
+    currentApp: null,
+    notifications: []
   };
   menuInfo: MenuInfo;
   services: Services;
@@ -93,11 +101,31 @@ export class BaseStore extends EventBus {
     this.currentQuery = query;
     this.services.router.navigate(query);
   }
+
+  addNotification(notif: Partial<Notification>): number {
+    const id = this.generateID();
+    const defaultVals = {
+      title: "",
+      message: "",
+      type: "notification",
+      sticky: false
+    };
+    const notification = Object.assign(defaultVals, notif, { id });
+    const notifs = this.state.notifications.concat(notification);
+    this.update({ notifications: notifs });
+    if (!notification.sticky) {
+      setTimeout(() => this.closeNotification(id), 2500);
+    }
+    return id;
+  }
+
+  closeNotification(id: number) {
+    const notifs = this.state.notifications.filter(n => n.id !== id);
+    this.update({ notifications: notifs });
+  }
 }
 
-export class Store extends actionManagerMixin(
-  rpcMixin(notificationMixin(BaseStore))
-) {
+export class Store extends actionManagerMixin(rpcMixin(BaseStore)) {
   constructor(
     services: Services,
     menuInfo: MenuInfo,
diff --git a/web/static/src/ts/ui/notification.ts b/web/static/src/ts/ui/notification.ts
index 1b5947c1..cb7b6468 100644
--- a/web/static/src/ts/ui/notification.ts
+++ b/web/static/src/ts/ui/notification.ts
@@ -1,4 +1,4 @@
-import { INotification } from "../store/store";
+import { Notification as INotification } from "../store/store";
 import { Widget } from "./widget";
 
 export class Notification extends Widget<INotification, {}> {
diff --git a/web/static/src/ts/ui/root.ts b/web/static/src/ts/ui/root.ts
index 552daf10..48935458 100644
--- a/web/static/src/ts/ui/root.ts
+++ b/web/static/src/ts/ui/root.ts
@@ -13,9 +13,8 @@ import { Action } from "../store/action_manager_mixin";
 
 export class Root extends Widget<Store, State> {
   template = "web.web_client";
-  widgets = { Navbar, HomeMenu };
+  widgets = { Navbar, HomeMenu, Notification };
 
-  notifications: { [id: number]: Notification } = {};
   store: Store;
 
   constructor(env: Env, store: Store) {
@@ -23,22 +22,12 @@ export class Root extends Widget<Store, State> {
     this.store = store;
     this.state = store.state;
   }
+
   mounted() {
     this.store.on("state_updated", this, newState => {
       this.updateState(newState);
     });
 
-    // notifications
-    this.store.on("notification_added", this, notif => {
-      const notification = new Notification(this, notif);
-      this.notifications[notif.id] = notification;
-      notification.mount(<any>this.refs.notification_container);
-    });
-    this.store.on("notification_closed", this, id => {
-      this.notifications[id].destroy();
-      delete this.notifications[id];
-    });
-
     // loading indicator
     this.store.on("rpc_status", this, status => {
       const method = status === "loading" ? "remove" : "add";
diff --git a/web/static/src/xml/templates.xml b/web/static/src/xml/templates.xml
index d590bb72..10e0c2f7 100644
--- a/web/static/src/xml/templates.xml
+++ b/web/static/src/xml/templates.xml
@@ -5,7 +5,11 @@
     <t t-widget="Navbar" t-props="{inHome:state.inHome,app:state.currentApp}" />
     <t t-widget="HomeMenu" t-if="state.inHome" t-keep-alive="1" t-props="{menuInfo:props.menuInfo}" />
     <div class="o_content" t-att-class="state.inHome ? 'o_hidden' : ''" t-ref="content"/>
-    <div class="o_notification_container" t-ref="notification_container"/>
+    <div class="o_notification_container">
+      <t t-foreach="state.notifications" t-as="notif">
+        <t t-widget="Notification" t-props="notif"/>
+      </t>
+    </div>
     <div class="o_loading d-none" t-ref="loading_indicator">Loading</div>
   </div>
 
diff --git a/web/static/tests/store/store.test.ts b/web/static/tests/store/store.test.ts
index 1e4d33b3..e5936d51 100644
--- a/web/static/tests/store/store.test.ts
+++ b/web/static/tests/store/store.test.ts
@@ -35,56 +35,44 @@ describe("rpc", () => {
 
 describe("notifications", () => {
   test("can subscribe and add notification", () => {
-    let n = 0;
     const store = makeTestStore();
-    store.on("notification_added", null, () => n++);
-    store.on("notification_closed", null, () => n--);
-    expect(n).toBe(0);
+    expect(store.state.notifications.length).toBe(0);
     const id = store.addNotification({
       title: "test",
       message: "message"
     });
-    expect(n).toBe(1);
+    expect(store.state.notifications.length).toBe(1);
     expect(id).toBeDefined();
   });
 
   test("can close a notification", () => {
-    let n = 0;
     const store = makeTestStore();
-    store.on("notification_added", null, () => n++);
-    store.on("notification_closed", null, () => n--);
 
     const id = store.addNotification({
       title: "test",
       message: "message"
     });
-    expect(n).toBe(1);
+    expect(store.state.notifications.length).toBe(1);
 
     store.closeNotification(id);
-    expect(n).toBe(0);
+    expect(store.state.notifications.length).toBe(0);
   });
 
   test("notifications closes themselves after a while", () => {
     jest.useFakeTimers();
-    let n = 0;
     const store = makeTestStore();
-    store.on("notification_added", null, () => n++);
-    store.on("notification_closed", null, () => n--);
 
     store.addNotification({ title: "test", message: "message" });
 
     expect(setTimeout).toHaveBeenCalledTimes(1);
-    expect(n).toBe(1);
+    expect(store.state.notifications.length).toBe(1);
     jest.runAllTimers();
-    expect(n).toBe(0);
+    expect(store.state.notifications.length).toBe(0);
   });
 
   test("sticky notifications do not close themselves after a while", () => {
     jest.useFakeTimers();
-    let n = 0;
     const store = makeTestStore();
-    store.on("notification_added", null, () => n++);
-    store.on("notification_closed", null, () => n--);
 
     store.addNotification({
       title: "test",
@@ -93,9 +81,9 @@ describe("notifications", () => {
     });
 
     expect(setTimeout).toHaveBeenCalledTimes(0);
-    expect(n).toBe(1);
+    expect(store.state.notifications.length).toBe(1);
     jest.runAllTimers();
-    expect(n).toBe(1);
+    expect(store.state.notifications.length).toBe(1);
   });
 });
 
diff --git a/web/static/tests/ui/__snapshots__/root.test.ts.snap b/web/static/tests/ui/__snapshots__/root.test.ts.snap
index 2ba189cd..3a4a353c 100644
--- a/web/static/tests/ui/__snapshots__/root.test.ts.snap
+++ b/web/static/tests/ui/__snapshots__/root.test.ts.snap
@@ -27,7 +27,9 @@ exports[`can be rendered (in home menu) 1`] = `
     </div>
   </div>
     <div class=\\"o_content o_hidden\\"></div>
-    <div class=\\"o_notification_container\\"></div>
+    <div class=\\"o_notification_container\\">
+      
+    </div>
     <div class=\\"o_loading d-none\\">Loading</div>
   </div>"
 `;
@@ -74,7 +76,9 @@ exports[`if url has action_id, will render action and navigate to proper menu_id
     <button>Add notif</button>
     <button>Add sticky notif</button>
   </div></div>
-    <div class=\\"o_notification_container\\"></div>
+    <div class=\\"o_notification_container\\">
+      
+    </div>
     <div class=\\"o_loading d-none\\">Loading</div>
   </div>"
 `;
@@ -121,7 +125,9 @@ exports[`start with no action => clicks on client action => discuss is rendered
     <button>Add notif</button>
     <button>Add sticky notif</button>
   </div></div>
-    <div class=\\"o_notification_container\\"></div>
+    <div class=\\"o_notification_container\\">
+      
+    </div>
     <div class=\\"o_loading d-none\\">Loading</div>
   </div>"
 `;
diff --git a/web/static/tests/ui/notification.test.ts b/web/static/tests/ui/notification.test.ts
index ea17adf5..5f41778b 100644
--- a/web/static/tests/ui/notification.test.ts
+++ b/web/static/tests/ui/notification.test.ts
@@ -1,5 +1,5 @@
 import { Env, makeEnv } from "../../src/ts/env";
-import { INotification, Store } from "../../src/ts/store/store";
+import { Notification as INotification, Store } from "../../src/ts/store/store";
 import { Notification } from "../../src/ts/ui/notification";
 import * as helpers from "../helpers";
 
@@ -50,23 +50,15 @@ test("can be rendered", async () => {
 });
 
 test("can be closed by clicking on it (if sticky)", async () => {
-  let n = 0;
-  let notif;
-  store.on("notification_added", null, _notif => {
-    n++;
-    notif = _notif;
-  });
-  store.on("notification_closed", null, () => n--);
-
   env.addNotification({
     title: "title",
     message: "message",
     sticky: true
   });
 
-  const navbar = new Notification(env, notif);
+  const navbar = new Notification(env, store.state.notifications[0]);
   await navbar.mount(fixture);
-  expect(n).toBe(1);
+  expect(store.state.notifications.length).toBe(1);
   (<any>fixture.getElementsByClassName("o_close")[0]).click();
-  expect(n).toBe(0);
+  expect(store.state.notifications.length).toBe(0);
 });