From 57d28c9c0b15611b6b4aeceb37d17c47aab1e128 Mon Sep 17 00:00:00 2001
From: Erik Munson <emunson@netflix.com>
Date: Mon, 30 Sep 2019 12:28:46 -0700
Subject: [PATCH] fix(core/task): properly cleanup TaskMonitor polling, fix
 digest thrashing (#7458)

---
 .../modules/core/src/task/monitor/TaskMonitor.ts    | 10 ++++++++--
 .../modules/core/src/task/task.read.service.ts      |  6 +-----
 .../modules/core/src/task/tasks.controller.js       | 13 ++++++++++++-
 app/scripts/modules/core/src/task/tasks.html        |  4 ++--
 4 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/app/scripts/modules/core/src/task/monitor/TaskMonitor.ts b/app/scripts/modules/core/src/task/monitor/TaskMonitor.ts
index ce8207dc74e..06ecec7af3c 100644
--- a/app/scripts/modules/core/src/task/monitor/TaskMonitor.ts
+++ b/app/scripts/modules/core/src/task/monitor/TaskMonitor.ts
@@ -42,8 +42,14 @@ export class TaskMonitor {
     return {
       deferred,
       result: deferred.promise,
-      close: onClose,
-      dismiss: onDismiss || onClose,
+      close: (result: T) => {
+        deferred.resolve(result);
+        return onClose(result);
+      },
+      dismiss: (result: T) => {
+        deferred.reject(result);
+        return (onDismiss || onClose)(result);
+      },
     } as IModalServiceInstanceEmulation;
   }
 
diff --git a/app/scripts/modules/core/src/task/task.read.service.ts b/app/scripts/modules/core/src/task/task.read.service.ts
index f1b2f186875..050a7e6d4ca 100644
--- a/app/scripts/modules/core/src/task/task.read.service.ts
+++ b/app/scripts/modules/core/src/task/task.read.service.ts
@@ -26,17 +26,13 @@ export class TaskReader {
     return API.one('tasks', taskId)
       .get()
       .then((task: ITask) => {
-        OrchestratedItemTransformer.defineProperties(task);
-        if (task.steps && task.steps.length) {
-          task.steps.forEach(step => OrchestratedItemTransformer.defineProperties(step));
-        }
+        this.setTaskProperties(task);
         if (task.execution) {
           OrchestratedItemTransformer.defineProperties(task.execution);
           if (task.execution.stages) {
             task.execution.stages.forEach((stage: any) => OrchestratedItemTransformer.defineProperties(stage));
           }
         }
-        this.setTaskProperties(task);
         return task;
       })
       .catch((error: any) => $log.warn('There was an issue retrieving taskId: ', taskId, error));
diff --git a/app/scripts/modules/core/src/task/tasks.controller.js b/app/scripts/modules/core/src/task/tasks.controller.js
index 543a7f1d3d3..c8461b10289 100644
--- a/app/scripts/modules/core/src/task/tasks.controller.js
+++ b/app/scripts/modules/core/src/task/tasks.controller.js
@@ -57,6 +57,7 @@ module.exports = angular
         };
         viewState.nameFilter = $stateParams.q || '';
         viewState.loading = true;
+        viewState.cancelling = false;
         viewState.itemsPerPage = tasksViewStateCache.get('#common')
           ? tasksViewStateCache.get('#common').itemsPerPage
           : 20;
@@ -147,7 +148,17 @@ module.exports = angular
           return task.id === taskId;
         })[0];
         var submitMethod = function() {
-          return TaskWriter.cancelTask(taskId).then(() => application.tasks.refresh());
+          // cancelTask() polls aggressively waiting for a sucessful cancellation,
+          // which triggers equally aggressive updates to the runningTimeInMs field
+          // on the hydrated task object. Because we render that field in templates,
+          // updating so quickly can make Angular think we're doing Bad Things(tm)
+          // and abort its change detection. Instead, we stop rendering that field
+          // while the polling is active.
+          $scope.viewState.cancelling = true;
+          return TaskWriter.cancelTask(taskId).then(() => {
+            $scope.viewState.cancelling = false;
+            application.tasks.refresh();
+          });
         };
 
         confirmationModalService.confirm({
diff --git a/app/scripts/modules/core/src/task/tasks.html b/app/scripts/modules/core/src/task/tasks.html
index bf667b006cf..527b4cafca1 100644
--- a/app/scripts/modules/core/src/task/tasks.html
+++ b/app/scripts/modules/core/src/task/tasks.html
@@ -139,10 +139,10 @@ <h4>
                 {{ task.endTime | timestamp }}
               </td>
               <td>
-                <span ng-if="task.runningTimeInMs">
+                <span ng-if="task.runningTimeInMs && !viewState.cancelling">
                   {{ task.runningTimeInMs | duration }}
                 </span>
-                <span ng-if="!task.runningTimeInMs">
+                <span ng-if="viewState.cancelling || !task.runningTimeInMs">
                   -
                 </span>
               </td>