From 21327c54c0c12bd3e1183b50979ff15213022cc7 Mon Sep 17 00:00:00 2001
From: Alexandre Alves <97888974+aalves08@users.noreply.github.com>
Date: Fri, 2 Feb 2024 09:27:06 +0000
Subject: [PATCH] Update logic of 'hasError' in
 'provisioning.cattle.io.cluster' model (#10366)

* update logic of hasError in prov cluster model

* working on unit tests

* fix code logic and finish unit tests

* fix lint issue

* address pr comments

---------

Co-authored-by: Alexandre Alves <aalves@Alexandres-MacBook-Pro.local>
---
 .../provisioning.cattle.io.cluster.test.ts    | 102 ++++++++++++++++++
 .../models/provisioning.cattle.io.cluster.js  |  22 +++-
 2 files changed, 123 insertions(+), 1 deletion(-)

diff --git a/shell/models/__tests__/provisioning.cattle.io.cluster.test.ts b/shell/models/__tests__/provisioning.cattle.io.cluster.test.ts
index 8c785666f75..8d75daf9dd4 100644
--- a/shell/models/__tests__/provisioning.cattle.io.cluster.test.ts
+++ b/shell/models/__tests__/provisioning.cattle.io.cluster.test.ts
@@ -87,4 +87,106 @@ describe('class ProvCluster', () => {
     }
     );
   });
+
+  describe('hasError', () => {
+    const conditionsWithoutError = [
+      {
+        error:          false,
+        lastUpdateTime: '2022-10-17T23:09:15Z',
+        status:         'True',
+        transitioning:  false,
+        type:           'Ready'
+      },
+    ];
+
+    const conditionsWithoutReady = [
+      {
+        error:          true,
+        lastUpdateTime: '2022-10-17T23:09:15Z',
+        status:         'False',
+        message:        'some-error-message',
+        transitioning:  false,
+        type:           'Pending'
+      },
+    ];
+
+    const noConditions:[] = [];
+
+    const conditionsWithReadyLatest = [
+      {
+        error:          true,
+        lastUpdateTime: '2022-10-17T23:09:15Z',
+        status:         'False',
+        message:        'some-error-message',
+        transitioning:  false,
+        type:           'Pending'
+      },
+      {
+        error:          false,
+        lastUpdateTime: '2023-10-17T23:09:15Z',
+        status:         'True',
+        transitioning:  false,
+        type:           'Ready'
+      }
+    ];
+
+    const conditionsWithErrorLatest = [
+      {
+        error:          false,
+        lastUpdateTime: '2022-10-17T23:09:15Z',
+        status:         'True',
+        transitioning:  false,
+        type:           'Ready'
+      },
+      {
+        error:          true,
+        lastUpdateTime: '2023-10-17T23:09:15Z',
+        status:         'False',
+        message:        'some-error-message',
+        transitioning:  false,
+        type:           'Pending'
+      }
+    ];
+
+    const conditionsWithProblemInLastUpdateTimeProp = [
+      {
+        error:          true,
+        lastUpdateTime: '',
+        status:         'False',
+        message:        'some-error-message',
+        transitioning:  false,
+        type:           'Pending'
+      },
+      {
+        error:          false,
+        lastUpdateTime: '2023-10-17T23:09:15Z',
+        status:         'True',
+        transitioning:  false,
+        type:           'Ready'
+      }
+    ];
+
+    const testCases = [
+      ['conditionsWithoutError', conditionsWithoutError, false],
+      ['conditionsWithoutReady', conditionsWithoutReady, true],
+      ['noConditions', noConditions, false],
+      ['conditionsWithReadyLatest', conditionsWithReadyLatest, false],
+      ['conditionsWithErrorLatest', conditionsWithErrorLatest, true],
+      ['conditionsWithProblemInLastUpdateTimeProp', conditionsWithProblemInLastUpdateTimeProp, false],
+    ];
+
+    const resetMocks = () => {
+      // Clear all mock function calls
+      jest.clearAllMocks();
+    };
+
+    it.each(testCases)('should return the hasError value properly based on the "status.conditions" props data for testcase %p', (testName: string, conditions: Array, expected: Boolean) => {
+      const ctx = { rootGetters: { 'management/byId': jest.fn() } };
+      const cluster = new ProvCluster({ status: { conditions } }, ctx);
+
+      expect(cluster.hasError).toBe(expected);
+      resetMocks();
+    }
+    );
+  });
 });
diff --git a/shell/models/provisioning.cattle.io.cluster.js b/shell/models/provisioning.cattle.io.cluster.js
index 08b88236c01..756118561a3 100644
--- a/shell/models/provisioning.cattle.io.cluster.js
+++ b/shell/models/provisioning.cattle.io.cluster.js
@@ -891,7 +891,27 @@ export default class ProvCluster extends SteveModel {
   }
 
   get hasError() {
-    return this.status?.conditions?.some((condition) => condition.error === true);
+    // Before we were just checking for this.status?.conditions?.some((condition) => condition.error === true)
+    // but this is wrong as an error might exist but it might not be meaningful in the context of readiness of a cluster
+    // which is what this 'hasError' is used for.
+    // We now check if there's a ready condition after an error, which helps dictate the readiness of a cluster
+    // Based on the findings in https://github.com/rancher/dashboard/issues/10043
+    if (this.status?.conditions && this.status?.conditions.length) {
+      // if there are errors, we compare with how recent the "Ready" condition is compared to that error, otherwise we just move on
+      if (this.status?.conditions.some((c) => c.error === true)) {
+        // there's no ready condition and has an error, mark it
+        if (!this.status?.conditions.some((c) => c.type === 'Ready')) {
+          return true;
+        }
+
+        const filteredConditions = this.status?.conditions.filter((c) => c.error === true || c.type === 'Ready');
+        const mostRecentCondition = filteredConditions.reduce((a, b) => ((a.lastUpdateTime > b.lastUpdateTime) ? a : b));
+
+        return mostRecentCondition.error;
+      }
+    }
+
+    return false;
   }
 
   get namespaceLocation() {