diff --git a/server/seeds/04_appchannel.js b/server/seeds/04_appchannel.js index 6cd9cae6a..bb69bf635 100644 --- a/server/seeds/04_appchannel.js +++ b/server/seeds/04_appchannel.js @@ -30,12 +30,14 @@ exports.seed = async knex => { app_version_id: dhis2AppVersions[0].id, channel_id: stableId, min_dhis2_version: '2.28', + max_dhis2_version: '2.30', created_by_user_id: dhis2AppVersions[0].created_by_user_id, }, { app_version_id: dhis2AppVersions[1].id, channel_id: developmentId, min_dhis2_version: '2.29', + max_dhis2_version: '2.34', created_by_user_id: dhis2AppVersions[1].created_by_user_id, }, { diff --git a/server/src/services/appVersion.js b/server/src/services/appVersion.js index 611077862..094a97eee 100644 --- a/server/src/services/appVersion.js +++ b/server/src/services/appVersion.js @@ -20,13 +20,15 @@ const getAppVersionQuery = knex => 'app_version.version', knex.ref('app_version.app_id').as('appId'), knex.ref('app_version.created_at').as('createdAt'), + knex.ref('app_version.updated_at').as('updatedAt'), knex.ref('app_version.source_url').as('sourceUrl'), knex.ref('app_version.demo_url').as('demoUrl'), knex.ref('channel.name').as('channel'), knex.ref('ac.min_dhis2_version').as('minDhisVersion'), knex.ref('ac.max_dhis2_version').as('maxDhisVersion') ) - .distinct() // app_version_localised may return multiple versions + .where('language_code', 'en') // only english is supported for now + .orderBy('app_version.created_at', 'desc') class AppVersionService extends Schmervice.Service { constructor(server, schmerviceOptions) { @@ -46,6 +48,12 @@ class AppVersionService extends Schmervice.Service { }) } + // null-values are allowed for maxDhisVersion, so include these if filter is present + filters.applyVersionFilter(query, 'maxDhisVersion', { + includeEmpty: true, + }) + filters.applyVersionFilter(query, 'minDhisVersion') + return executeQuery(query, { filters, pager, diff --git a/server/src/utils/Filter.js b/server/src/utils/Filter.js index ab01edb5d..3d9285f51 100644 --- a/server/src/utils/Filter.js +++ b/server/src/utils/Filter.js @@ -103,22 +103,32 @@ class Filters { * Applies the filter * @param {*} query * @param {*} fieldName - * @param {*} options + * @param {*} options options object, merged with options to Filters object + * @param {*} overrideColumnName overrides the column name, takes precedence over tableName + * @param {*} includeEmpty includes empty values (null or '') for the filter */ applyOneToQuery(query, fieldName, options) { const colName = this.getFilterColumn(fieldName) const filter = this.getFilter(fieldName) - + const settings = { + ...this.options, + ...options, + } if (filter) { const nameToUse = options.overrideColumnName || - (options.tableName - ? `${options.tableName}.${colName}` + (settings.tableName + ? `${settings.tableName}.${colName}` : colName) const { value, operator } = filter + query.where(builder => { + builder.where(nameToUse, toSQLOperator(operator, value), value) + if (settings.includeEmpty) { + builder.orWhereRaw(`nullif( ??, ' ') is null`, nameToUse) + } + }) this.markApplied(fieldName) - query.where(nameToUse, toSQLOperator(operator, value), value) } else { throw new Error( `Failed to apply filter to query, ${colName} is not a valid filter.` @@ -126,6 +136,40 @@ class Filters { } } + /** Applies a filter for comparing versions + * eg. this will match correctly for for 1.9.0 < 1.10.0, + * opposed to normal string matching. + */ + applyVersionFilter(query, filterName, options = {}) { + const filter = this.getFilter(filterName) + + if (!filter) { + return + } + + const colName = + options.overrideColumnName || this.getFilterColumn(filterName) + + if (!filter) { + throw new Error( + `Failed to apply filter to query, ${colName} is not a valid filter.` + ) + } + + query.where(builder => { + builder.whereRaw( + `string_to_array( regexp_replace(??, '[^0-9.]+', '', 'g'), '.')::int[] ${toSQLOperator( + filter.operator + )} string_to_array( regexp_replace(?, '[^0-9.]+', '', 'g'), '.')::int[]`, + [colName, filter.value] + ) + if (options.includeEmpty) { + builder.orWhereRaw(`nullif( ??, ' ') is null`, colName) + } + }) + this.markApplied(filterName) + } + /** * Marks the filter as applied. Can be useful when creating a custom query, * and you don't want the filter to be applied when running applyAllToQuery(). diff --git a/server/src/utils/filterUtils.js b/server/src/utils/filterUtils.js index 7d2a2ddb1..3b4128e60 100644 --- a/server/src/utils/filterUtils.js +++ b/server/src/utils/filterUtils.js @@ -15,6 +15,7 @@ const operatorMap = { lt: '<', gt: '>', lte: '<=', + gte: '>=', ne: '<>', } const allOperatorsMap = { diff --git a/server/test/data/index.js b/server/test/data/index.js index ad1ff09c3..0a5fed843 100644 --- a/server/test/data/index.js +++ b/server/test/data/index.js @@ -261,7 +261,7 @@ describe('@data::updateAppVersion', () => { ) expect(app.version_id).to.equal(appVersionIdToUpdate) expect(app.version).to.equal('0.1') - expect(app.max_dhis2_version).to.equal(null) + expect(app.max_dhis2_version).to.equal('2.30') expect(app.min_dhis2_version).to.equal('2.28') expect(app.demo_url).to.equal(null) diff --git a/server/test/routes/v2/appVersions.js b/server/test/routes/v2/appVersions.js index c0142d06d..c50057c76 100644 --- a/server/test/routes/v2/appVersions.js +++ b/server/test/routes/v2/appVersions.js @@ -99,5 +99,155 @@ describe('v2/appVersions', () => { expect(v.minDhisVersion).to.be.a.string() }) }) + + describe('with filters', () => { + it('should handle channel filter', async () => { + const request = { + method: 'GET', + url: `/api/v2/apps/${dhis2App.id}/versions?channel=development`, + } + + const res = await server.inject(request) + + expect(res.statusCode).to.equal(200) + + const result = res.result + const versions = res.result.result + + Joi.assert(versions, Joi.array().items(AppVersionModel.def)) + expect(result.pager).to.be.an.object() + // check some keys as well + versions.forEach(v => { + expect(v.appId).to.be.a.string() + expect(v.version).to.be.a.string() + expect(v.channel).to.be.equal('development') + }) + }) + + it('should handle minDhisVersion', async () => { + const request = { + method: 'GET', + url: `/api/v2/apps/${dhis2App.id}/versions?minDhisVersion=eq:2.29`, + } + + const res = await server.inject(request) + + 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() + expect(v.version).to.be.a.string() + expect(v.minDhisVersion).to.be.equal('2.29') + }) + }) + + it('should handle minDhisVersion with lte and gte', async () => { + const request = { + method: 'GET', + url: `/api/v2/apps/${dhis2App.id}/versions?minDhisVersion=lte:2.28`, + } + + const res = await server.inject(request) + + expect(res.statusCode).to.equal(200) + + const result = res.result + const versions = result.result + + 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('2.29') + }) + }) + + it('should handle maxDhisVersion', async () => { + const request = { + method: 'GET', + url: `/api/v2/apps/${dhis2App.id}/versions?maxDhisVersion=lte:2.34`, + } + + 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) + expect(major).to.be.lessThan(35) + } + }) + }) + + it('should handle both minDhisVersion and maxDhisVersion', async () => { + const request = { + method: 'GET', + url: `/api/v2/apps/${dhis2App.id}/versions?maxDhisVersion=lte:2.34&minDhisVersion=gte:2.29`, + } + + 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) + expect(major).to.be.between(28, 35) + } + }) + }) + + it('should not fail when version include valid semver characters', async () => { + const request = { + method: 'GET', + url: `/api/v2/apps/${dhis2App.id}/versions?maxDhisVersion=lte:2.34-beta&minDhisVersion=gte:2.29-alpha`, + } + + 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) + expect(major).to.be.between(28, 35) + } + }) + }) + }) }) })