fix: correctly compare versions to find the latest version on app hub…

… [DHIS2-10859] (#127)

* fix: correctly compare versions to find the latest version on app hub

* fix: use semver instead of manually parsing versions

package.json

@@ -1,6 +1,6 @@
 {
     "name": "app-management-app",
-    "version": "31.0.0",
+    "version": "31.2.0",
     "description": "The Application Management App provides the ability to upload webapps in .zip files, as well as installing apps directly from the official DHIS 2 App Store",
     "scripts": {
         "start": "d2-app-scripts start",
@@ -47,6 +47,7 @@
         "react": "^16.13.1",
         "react-dom": "^16.13.1",
         "rxjs": "^5.4.3",
+        "semver": "^7.3.2",
         "susy": "^3.0.0"
     }
 }

src/components/SelfUpdateNoticeBox.component.js

@@ -5,32 +5,12 @@ import { NoticeBox, Button } from '@dhis2/ui'
 import PropTypes from 'prop-types'
 import React from 'react'
 import actions from '../actions'
+import { getTargetVersion } from '../utils/versions'
 
 const currentVersion = __VERSION__
 const appManagementAppOrg = 'DHIS2'
 const appManagementAppName = 'App Management'
 
-const parseVersion = versionString => versionString.split('.').map(Number)
-const needsUpdate = (current, candidate) => {
-    const currentVersion = parseVersion(current)
-    const candidateVersion = parseVersion(candidate)
-
-    return currentVersion.some((v, i) => v < candidateVersion[i])
-}
-
-const getTargetVersion = (current, versions) => {
-    if (!versions) return null
-
-    let targetVersion
-    versions.forEach(candidate => {
-        if (needsUpdate(targetVersion?.version || current, candidate.version)) {
-            targetVersion = candidate
-        }
-    })
-
-    return targetVersion
-}
-
 export const SelfUpdateNoticeBox = ({ appHub }) => {
     if (!appHub.apps) return null
 

src/utils/versions.js

@@ -0,0 +1,23 @@
+import { valid, lt } from 'semver'
+
+export const isUpgradeCandidate = (current, candidate) =>
+    valid(current) === null ||
+    (valid(candidate) !== null && lt(current, candidate))
+
+export const getTargetVersion = (current, candidates) => {
+    if (!candidates || !candidates.length) return null
+
+    let targetVersion
+    candidates.forEach(candidate => {
+        if (
+            isUpgradeCandidate(
+                targetVersion?.version || current,
+                candidate.version
+            )
+        ) {
+            targetVersion = candidate
+        }
+    })
+
+    return targetVersion
+}

src/utils/versions.test.js

@@ -0,0 +1,64 @@
+const { getTargetVersion, isUpgradeCandidate } = require('./versions')
+
+describe('utils::versions', () => {
+    describe('isUpgradeCandidate', () => {
+        it('Should return true iff candidate is higher than current', () => {
+            //lower
+            expect(isUpgradeCandidate('3.4.5', '1.0.0')).toBe(false)
+            expect(isUpgradeCandidate('3.4.5', '2.7.6')).toBe(false)
+            expect(isUpgradeCandidate('3.4.5', '3.4.4')).toBe(false)
+
+            //equal
+            expect(isUpgradeCandidate('1.0.0', '1.0.0')).toBe(false)
+
+            //higher
+            expect(isUpgradeCandidate('1.0.0', '1.0.1')).toBe(true)
+            expect(isUpgradeCandidate('1.0.0', '1.1.0')).toBe(true)
+            expect(isUpgradeCandidate('1.0.0', '3.0.4')).toBe(true)
+        })
+        it('Should ignore invalid candidates', () => {
+            expect(isUpgradeCandidate('1.0.0', '1.0.1b')).toBe(false)
+            expect(isUpgradeCandidate('1.0.0', '1.1')).toBe(false)
+            expect(isUpgradeCandidate('1.0.0', '2')).toBe(false)
+        })
+        it('Should ignore invalid current', () => {
+            expect(isUpgradeCandidate('1.0', '1.0.0')).toBe(true)
+        })
+        it('Should consider prereleases valid candidates', () => {
+            expect(isUpgradeCandidate('1.0.0', '1.0.1-alpha.0')).toBe(true)
+        })
+    })
+
+    describe('getTargetVersion', () => {
+        it('Should detect the highest version', () => {
+            const candidates = [
+                { version: '2.0.0' },
+                { version: '2.1.3' },
+                { version: '2.3.4-beta.5' },
+                { version: '2.3.4-beta.3' },
+            ]
+            expect(getTargetVersion('1.0.0', candidates)).toStrictEqual({
+                version: '2.3.4-beta.5',
+            })
+
+            const candidates2 = [
+                { version: '2.0.0' },
+                { version: '4.3.3' },
+                { version: '2.3.4-beta.5' },
+                { version: '2.3.4-beta.3' },
+            ]
+
+            expect(getTargetVersion('2.1.1', candidates2)).toStrictEqual({
+                version: '4.3.3',
+            })
+        })
+        it('Should return undefined if no upgrade candidate found', () => {
+            const candidates = [
+                { version: '2.0.0' },
+                { version: '4.3.3' },
+                { version: '3.3.4-beta.3' },
+            ]
+            expect(getTargetVersion('5.0.0', candidates)).toBeUndefined()
+        })
+    })
+})