From 5aa984cc6c5a737e2dfcc7806f0fe48d1b41d1c5 Mon Sep 17 00:00:00 2001
From: =?utf8?q?Stephan=20M=C3=BCller?= <smueller@suse.com>
Date: Thu, 21 Feb 2019 11:53:46 +0100
Subject: [PATCH] mgr/dashboard: Make preventDefault work with 400 errors
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

The problem was that, if a error with the status code 400 was
received by the error interceptor the "timeoutId" was not tracked,
therefor "preventDefault" didn't prevent anything as "timeoutId"
was undefined.

Fixes: https://tracker.ceph.com/issues/38418
Signed-off-by: Stephan Müller <smueller@suse.com>
---
 .../shared/services/api-interceptor.service.spec.ts    | 10 ++++++++++
 .../src/app/shared/services/api-interceptor.service.ts | 10 ++++------
 .../app/shared/services/notification.service.spec.ts   | 10 ++++++++++
 .../src/app/shared/services/notification.service.ts    |  4 ++--
 4 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/api-interceptor.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/api-interceptor.service.spec.ts
index 67e93e2a0457a..cc7fb2ba73f6b 100644
--- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/api-interceptor.service.spec.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/api-interceptor.service.spec.ts
@@ -182,6 +182,16 @@ describe('ApiInterceptorService', () => {
       expect(notificationService.save).not.toHaveBeenCalled();
     }));
 
+    it('should be able to use preventDefault with 400 errors', fakeAsync(() => {
+      httpError(
+        { task: { name: 'someName', metadata: { component: 'someComponent' } } },
+        { status: 400 },
+        (resp) => resp.preventDefault()
+      );
+      tick(10);
+      expect(notificationService.save).not.toHaveBeenCalled();
+    }));
+
     it('should prevent the default behaviour by status code', fakeAsync(() => {
       httpError(undefined, { status: 500 }, (resp) => resp.ignoreStatusCode(500));
       tick(10);
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/api-interceptor.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/api-interceptor.service.ts
index 27d41be95d20f..1e10ddd494f63 100644
--- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/api-interceptor.service.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/api-interceptor.service.ts
@@ -33,7 +33,7 @@ export class ApiInterceptorService implements HttpInterceptor {
     return next.handle(request).pipe(
       catchError((resp) => {
         if (resp instanceof HttpErrorResponse) {
-          let showNotification = true;
+          let timeoutId: number;
           switch (resp.status) {
             case 400:
               const finishedTask = new FinishedTask();
@@ -50,21 +50,19 @@ export class ApiInterceptorService implements HttpInterceptor {
 
               finishedTask.success = false;
               finishedTask.exception = resp.error;
-              this.notificationService.notifyTask(finishedTask);
-              showNotification = false;
+              timeoutId = this.notificationService.notifyTask(finishedTask);
               break;
             case 401:
               this.authStorageService.remove();
               this.router.navigate(['/login']);
-              showNotification = false;
               break;
             case 403:
               this.router.navigate(['/403']);
               break;
+            default:
+              timeoutId = this.prepareNotification(resp);
           }
 
-          const timeoutId = showNotification ? this.prepareNotification(resp) : undefined;
-
           /**
            * Decorated preventDefault method (in case error previously had
            * preventDefault method defined). If called, it will prevent a
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.spec.ts
index c0413f4404b27..76d75f9c83d14 100644
--- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.spec.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.spec.ts
@@ -107,6 +107,16 @@ describe('NotificationService', () => {
     expect(notification.message).toBe(undefined);
   }));
 
+  it('should be able to stop notifyTask from notifying', fakeAsync(() => {
+    const task = _.assign(new FinishedTask(), {
+      success: true
+    });
+    const timeoutId = service.notifyTask(task, true);
+    service.cancel(timeoutId);
+    tick(100);
+    expect(service['dataSource'].getValue().length).toBe(0);
+  }));
+
   it('should show a error task notification', fakeAsync(() => {
     const task = _.assign(
       new FinishedTask('rbd/create', {
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.ts
index 08b6ff8878b7c..14bd6b51e1b14 100644
--- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.ts
@@ -150,7 +150,7 @@ export class NotificationService {
     }"></i>`;
   }
 
-  notifyTask(finishedTask: FinishedTask, success: boolean = true) {
+  notifyTask(finishedTask: FinishedTask, success: boolean = true): number {
     let notification: CdNotificationConfig;
     if (finishedTask.success && success) {
       notification = new CdNotificationConfig(
@@ -164,7 +164,7 @@ export class NotificationService {
         this.taskMessageService.getErrorMessage(finishedTask)
       );
     }
-    this.show(notification);
+    return this.show(notification);
   }
 
   /**
-- 
2.39.5