fix(filter): properly include version if maxDhisVersion is empty (#593)

* fix(filter): fix return empty maxDhisVersion

* test: add more tests for appVersion filtering

* style: cleanup

* refactor: cleanup

server/seeds/04_appchannel.js

@@ -46,7 +46,20 @@ exports.seed = async knex => {
             min_dhis2_version: '2.32',
             created_by_user_id: dhis2AppVersions[2].created_by_user_id,
         },
-
+        {
+            app_version_id: dhis2AppVersions[3].id,
+            channel_id: canaryId,
+            min_dhis2_version: '2.33',
+            max_dhis2_version: '2.40',
+            created_by_user_id: dhis2AppVersions[3].created_by_user_id,
+        },
+        {
+            app_version_id: dhis2AppVersions[4].id,
+            channel_id: canaryId,
+            min_dhis2_version: '2.33',
+            max_dhis2_version: '',
+            created_by_user_id: dhis2AppVersions[4].created_by_user_id,
+        },
         {
             app_version_id: whoAppVersions[0].id,
             channel_id: stableId,

server/seeds/mock/appversions.js

@@ -42,6 +42,22 @@ const dhis2AppVersions = [
         version: '0.3-dev',
         created_at: createdAtDate(),
     },
+    {
+        id: '2812d245-96de-42c4-b651-25cb313414d6',
+        app_id: dhis2App.id,
+        created_by_user_id: dhis2App.created_by_user_id,
+        version: '1.0.0',
+        created_at: createdAtDate(),
+    },
+    {
+        id: '7be62020-5bcf-4d32-a83b-b2412bcd01bb',
+        app_id: dhis2App.id,
+        created_by_user_id: dhis2App.created_by_user_id,
+        version: '1.1.0',
+        created_at: createdAtDate(),
+    },
+
+
 ]
 const whoAppVersions = [
     {

server/seeds/mock/appversions_localized.js

@@ -76,6 +76,22 @@ const dhis2AppVersionsLocalized = [
         'En fin WHO app',
         'Detta är en mycket fin applikation, ostabil utvecklings-version.'
     ),
+    mockAppVersion(
+        'b6ce3233-90d1-40fa-a1cf-d148e3025616',
+        dhis2AppVersions,
+        3,
+        'en',
+        'A nice app',
+        'This is a really nice app by DHIS2!'
+    ),
+    mockAppVersion(
+        '564e6388-5d7e-4bac-a093-d80e3471ad1a',
+        dhis2AppVersions,
+        4,
+        'en',
+        'A nice app',
+        'This is a really nice app by DHIS2!'
+    ),
 ]
 
 const whoAppVersionsLocalized = [

server/src/utils/Filter.js

@@ -125,7 +125,7 @@ class Filters {
             query.where(builder => {
                 builder.where(nameToUse, toSQLOperator(operator, value), value)
                 if (settings.includeEmpty) {
-                    builder.orWhereRaw(`nullif( ??, ' ') is null`, nameToUse)
+                    builder.orWhereRaw(`nullif( ??, '') is null`, nameToUse)
                 }
             })
             this.markApplied(fieldName)
@@ -164,7 +164,7 @@ class Filters {
                 [colName, filter.value]
             )
             if (options.includeEmpty) {
-                builder.orWhereRaw(`nullif( ??, ' ') is null`, colName)
+                builder.orWhereRaw(`nullif( ??, '') is null`, colName)
             }
         })
         this.markApplied(filterName)

server/test/routes/v2/appVersions.js

@@ -137,10 +137,8 @@ describe('v2/appVersions', () => {
                 expect(res.statusCode).to.equal(200)
 
                 const result = res.result
-                console.log(result)
                 const versions = result.result
 
-                console.log(versions)
                 Joi.assert(versions, Joi.array().items(AppVersionModel.def))
                 versions.forEach(v => {
                     expect(v.appId).to.be.a.string()
@@ -224,7 +222,7 @@ describe('v2/appVersions', () => {
                 })
             })
 
-            it('should not fail when version include valid semver characters', async () => {
+            it('should not fail when version include valid semver tags', async () => {
                 const request = {
                     method: 'GET',
                     url: `/api/v2/apps/${dhis2App.id}/versions?maxDhisVersion=lte:2.34-beta&minDhisVersion=gte:2.29-alpha`,
@@ -250,6 +248,105 @@ describe('v2/appVersions', () => {
                     }
                 })
             })
+
+            it('should work with same version, returning a range', async () => {
+                const compatVersion = '2.30'
+                const request = {
+                    method: 'GET',
+                    url: `/api/v2/apps/${dhis2App.id}/versions?minDhisVersion=lte:${compatVersion}&maxDhisVersion=gte:${compatVersion}`,
+                }
+
+                const res = await server.inject(request)
+
+                expect(res.statusCode).to.equal(200)
+
+                const result = res.result
+                const versions = result.result
+
+                expect(versions.length).to.be.above(0)
+
+                Joi.assert(versions, Joi.array().items(AppVersionModel.def))
+                versions.forEach(v => {
+                    expect(v.appId).to.be.a.string()
+                    expect(v.version).to.be.a.string()
+                    expect(v.minDhisVersion).to.be.below(31)
+                    if (v.maxDhisVersion) {
+                        const [, major] = v.maxDhisVersion
+                            .split('.')
+                            .map(Number)
+                        expect(major).to.be.greaterThan(29)
+                    }
+                })
+            })
+
+            it('should work with patch versions', async () => {
+                const request = {
+                    method: 'GET',
+                    url: `/api/v2/apps/${dhis2App.id}/versions?minDhisVersion=gte:2.31.10&maxDhisVersion=gte:2.33.0`,
+                }
+
+                const res = await server.inject(request)
+
+                expect(res.statusCode).to.equal(200)
+
+                const result = res.result
+                const versions = result.result
+
+                expect(versions.length).to.be.above(0)
+
+                Joi.assert(versions, Joi.array().items(AppVersionModel.def))
+                versions.forEach(v => {
+                    expect(v.appId).to.be.a.string()
+                    expect(v.version).to.be.a.string()
+                    if (v.maxDhisVersion) {
+                        const [, major] = v.maxDhisVersion
+                            .split('.')
+                            .map(Number)
+                        major && expect(major).to.be.above(32)
+                    }
+                })
+            })
+
+            it('should work with maxDhisVersion if app has empty maxDhisVersion', async () => {
+                const allVersionsReq = {
+                    url: `/api/v2/apps/${dhis2App.id}/versions`,
+                }
+
+                const allVersionRes = await server.inject(allVersionsReq)
+
+                expect(allVersionRes.statusCode).to.equal(200)
+                const allVersions = allVersionRes.result.result
+                expect(allVersions.length).to.above(3) // should have at atleast 4 versions
+
+                const emptyValues = [null, '']
+                const countEmptyMaxVersion = (acc, curr) =>
+                    emptyValues.includes(curr.maxDhisVersion) ? acc + 1 : acc
+
+                const totalEmptyCount = allVersions.reduce(
+                    countEmptyMaxVersion,
+                    0
+                )
+
+                const request = {
+                    method: 'GET',
+                    url: `/api/v2/apps/${dhis2App.id}/versions?maxDhisVersion=gte:2.37`,
+                }
+
+                const res = await server.inject(request)
+
+                expect(res.statusCode).to.equal(200)
+
+                const result = res.result
+                const versions = result.result
+
+                expect(versions.length).to.be.above(0)
+
+                Joi.assert(versions, Joi.array().items(AppVersionModel.def))
+
+                // check that it still returns empty maxDhisVersions
+                const emptyCount = versions.reduce(countEmptyMaxVersion, 0)
+                expect(emptyCount).to.equal(totalEmptyCount)
+            })
         })
     })
 })