Repository: temporalio/ui
Pull request #422 (2022-04-12 23:35:27)

Show version banner only if recommend > current version
<!--- Note to EXTERNAL Contributors -->
<!-- Thanks for opening a PR! 
If it is a significant code change, please **make sure there is an open issue** for this. 
We work best with you when we have accepted the idea first before you code. -->

<!--- For ALL Contributors 👇 -->

## What was changed
<!-- Describe what has changed in this PR -->

added sanity checks before showing banner about new Temporal version

## Why?
<!-- Tell your future self why have you made these changes -->

removing reliance on Temporal service and Version info tool to recommend only newer versions

## Checklist
<!--- add/delete as needed --->

1. Closes <!-- add issue number here -->

2. How was this tested:
<!--- Please describe how you tested your changes/how we can test them -->

Tried on Temporal versions `1.14.2`, `1.15.2` and `1.16.0` and verified that banner is shown accordingly

3. Any docs updates needed?
<!--- update README if applicable
      or point out where to update docs.temporal.io -->


cypress[bot]: 
<p align="center">
  <br />
  <img src="https://assets.cypress.io/github-pr-comment-icons/cypress-logo.png" height="32" />
</p>

---

### Test summary
<a href="https://dashboard.cypress.io/projects/g7xfxw/runs/129/overview?utm_source=github"><img src="https://assets.cypress.io/github-pr-comment-icons/green-check-mark.png" width="14" height="14" title="Passed" /></a> <b title="Passed">28</b>  &bull; <a href="https://dashboard.cypress.io/projects/g7xfxw/runs/129/test-results?utm_source=github&statuses=%5B%7B%22value%22%3A%22FAILED%22%2C%22label%22%3A%22FAILED%22%7D%5D"><img src="https://assets.cypress.io/github-pr-comment-icons/red-x.png" width="14" height="14" title="Failed" /></a> <b title="Failed">0</b>  &bull; <img src="https://assets.cypress.io/github-pr-comment-icons/gray-circle.png" width="14" height="14" title="Tests that did not run due to a developer annotating a test with .skip" /> <b title="Pending">0</b> &bull; <img src="https://assets.cypress.io/github-pr-comment-icons/yellow-triangle.png" width="14" height="14" title="Tests that did not run due to a failure in a mocha hook" /> <b title="Skipped">0</b> 

---

### Run details
| | |
| --- | --- |
| **Project** | Temporal UI |
| **Status** | Passed |
| **Commit** | 12208da1df <span style="cursor:help;" title="Your CI provider generated this commit by merging this branch into the target branch.">ℹ️</span> |
| **Started** | Apr 12, 2022 11:36 PM |
| **Ended** | Apr 12, 2022 11:37 PM |
| **Duration** | 00:52 <a href="https://on.cypress.io/parallelization?utm_source=github" title="Enable parallelization to reduce run duration">💡</a> |
| **OS** | Linux Ubuntu - 20.04 |
| **Browser** | Chrome 100 <img src="https://cdnjs.cloudflare.com/ajax/libs/browser-logos/48.0.4/chrome/chrome_32x32.png" width="16" height="16" /> |

[View run in Cypress Dashboard ➡️](https://dashboard.cypress.io/projects/g7xfxw/runs/129/overview?utm_source=github)





---

This comment has been generated by **cypress-bot** as a result of this project's GitHub integration settings. You can manage this integration in [this project's settings in the Cypress Dashboard](https://dashboard.cypress.io/projects/g7xfxw/settings?utm_source=github)


feedmeapples: > Maybe we bust that logic out into a unit test?

done
cloudflare-pages[bot]: ## Deploying with &nbsp;<a href="https://pages.dev"><img alt="Cloudflare Pages" src="https://user-images.githubusercontent.com/23264/106598434-9e719e00-654f-11eb-9e59-6167043cfa01.png" width="16"></a> &nbsp;Cloudflare Pages

<table><tr><td><strong>Latest commit:</strong> </td><td>
<code>6373310</code>
</td></tr>
<tr><td><strong>Status:</strong></td><td>&nbsp;✅&nbsp; Deploy successful!</td></tr>
<tr><td><strong>Preview URL:</strong></td><td>
<a href='https://71f0b497.ui-c3x.pages.dev'>https://71f0b497.ui-c3x.pages.dev</a>
</td></tr>
</table>

[View logs](https://dash.cloudflare.com/?to=/:account/pages/view/temporal-pizza/71f0b497-8231-45ff-89ee-329098066d36)


Patch:
diff --git a/src/lib/components/banner.svelte b/src/lib/components/banner.svelte
index 6b6b6fcd..93362161 100644
--- a/src/lib/components/banner.svelte
+++ b/src/lib/components/banner.svelte
@@ -1,5 +1,6 @@
 <script lang="ts">
   import { closedBannerId, close } from '$lib/stores/banner';
+  import { isRecommendedVersionNewer } from '$lib/utilities/version-check';
 
   export let cluster: ClusterInformation;
 
@@ -13,7 +14,11 @@
   $: current = cluster?.versionInfo?.current;
   $: alert = cluster?.versionInfo?.alerts?.[0];
   $: severity = alert ? severities[alert.severity] : severities.Low;
-  $: show = current?.version && current.version != $closedBannerId;
+  $: show =
+    current?.version &&
+    current.version != $closedBannerId &&
+    isRecommendedVersionNewer(recommended?.version, current.version);
+
   $: message =
     severity == severities.Low
       ? `📥 v${recommended?.version} version is available`
diff --git a/src/lib/utilities/version-check.ts b/src/lib/utilities/version-check.ts
new file mode 100644
index 00000000..f03c4ed2
--- /dev/null
+++ b/src/lib/utilities/version-check.ts
@@ -0,0 +1,19 @@
+export const isRecommendedVersionNewer = (
+  recommendedVersion: string,
+  currentVersion: string,
+) => {
+  if (!recommendedVersion || !currentVersion) {
+    return false;
+  }
+
+  const [major1, minor1, patch1] = recommendedVersion.split('.').map(Number);
+  const [major2, minor2, patch2] = currentVersion.split('.').map(Number);
+
+  if (major1 !== major2) {
+    return major1 > major2;
+  } else if (minor1 !== minor2) {
+    return minor1 > minor2;
+  } else {
+    return patch1 > patch2;
+  }
+};


Test patch:
diff --git a/src/lib/utilities/version-check.test.ts b/src/lib/utilities/version-check.test.ts
new file mode 100644
index 00000000..9b247d61
--- /dev/null
+++ b/src/lib/utilities/version-check.test.ts
@@ -0,0 +1,25 @@
+import { isRecommendedVersionNewer } from './version-check';
+
+describe(isRecommendedVersionNewer, () => {
+  it('should return true when recommended version is higher than current', () => {
+    expect(isRecommendedVersionNewer('2.01.1', '1.01.1')).toBe(true);
+    expect(isRecommendedVersionNewer('1.02.1', '1.01.1')).toBe(true);
+    expect(isRecommendedVersionNewer('1.01.2', '1.01.1')).toBe(true);
+    expect(isRecommendedVersionNewer('1.02', '1.01')).toBe(true);
+    expect(isRecommendedVersionNewer('2', '1')).toBe(true);
+  });
+
+  it('should return false when recommended version is not higher than current', () => {
+    expect(isRecommendedVersionNewer('1.01.1', '1.01.1')).toBe(false);
+    expect(isRecommendedVersionNewer('1.01.1', '2.01.1')).toBe(false);
+    expect(isRecommendedVersionNewer('1.01.1', '1.02.1')).toBe(false);
+    expect(isRecommendedVersionNewer('1.01.1', '1.01.2')).toBe(false);
+    expect(isRecommendedVersionNewer(undefined, '1.01.1')).toBe(false);
+    expect(isRecommendedVersionNewer('1.01.1', undefined)).toBe(false);
+    expect(isRecommendedVersionNewer(undefined, undefined)).toBe(false);
+    expect(isRecommendedVersionNewer('xxx', '1.01.1')).toBe(false);
+    expect(isRecommendedVersionNewer('1.1', '1.01.1')).toBe(false);
+    expect(isRecommendedVersionNewer('1.01', '1.02')).toBe(false);
+    expect(isRecommendedVersionNewer('1', '2')).toBe(false);
+  });
+});