diff --git a/client/src/components/Versions/ChangeLogViewer.js b/client/src/components/Versions/ChangeLogViewer.js index ec2ace06..115bab8a 100644 --- a/client/src/components/Versions/ChangeLogViewer.js +++ b/client/src/components/Versions/ChangeLogViewer.js @@ -1,22 +1,12 @@ -import { Modal, ModalContent } from '@dhis2/ui' +import { CircularLoader, Modal, ModalContent } from '@dhis2/ui' import { useQuery } from 'src/api' import React from 'react' import ReactMarkdown from 'react-markdown' // ToDo: this is a just a placeholder for a basic Changelog viewer -const ChangeLogViewer = ({ - appId, - versionId, - modalShown, - onCloseChangelog, -}) => { - const { data, error } = useQuery(`apps/${appId}/changelog`) - - // ToDo: fall back to any version? - const matchedVersion = - data?.find((v) => v.version == versionId) ?? - data?.find((v) => !!v.changelog) +const ChangeLogViewer = ({ appId, modalShown, onCloseChangelog }) => { + const { data, loading } = useQuery(`apps/${appId}/changelog`) if (!modalShown) { return null @@ -24,7 +14,10 @@ const ChangeLogViewer = ({ return ( - {matchedVersion?.changelog} +
+ {loading && } + {data?.changelog} +
) diff --git a/client/src/components/Versions/VersionsTable/VersionsTable.js b/client/src/components/Versions/VersionsTable/VersionsTable.js index 1505c13e..31665109 100644 --- a/client/src/components/Versions/VersionsTable/VersionsTable.js +++ b/client/src/components/Versions/VersionsTable/VersionsTable.js @@ -52,16 +52,15 @@ const VersionsTable = ({ setChangelogVisible(false) } - // ToDO: we can infer change log from one change log - // const anyVersionHasChanges = !!versions.find((v) => v.hasChangelog) - return ( <> - + {changelogVisible && ( + + )} diff --git a/client/src/pages/UserApp/DetailsCard/DetailsCard.js b/client/src/pages/UserApp/DetailsCard/DetailsCard.js index ccca805e..5aca51ad 100644 --- a/client/src/pages/UserApp/DetailsCard/DetailsCard.js +++ b/client/src/pages/UserApp/DetailsCard/DetailsCard.js @@ -1,4 +1,4 @@ -import { Card, Button, Divider } from '@dhis2/ui' +import { Card, Button, Divider, Tag } from '@dhis2/ui' import PropTypes from 'prop-types' import { useState, useRef } from 'react' import { Link } from 'react-router-dom' @@ -91,6 +91,7 @@ const DetailsCard = ({ app, mutate }) => { by {appDeveloper} {appType} + {app.hasPlugin && Plugin} diff --git a/server/migrations/20241118143001_d2config_changelog_columns.js b/server/migrations/20241118143001_d2config_changelog_columns.js index 1752945c..3d1247cb 100644 --- a/server/migrations/20241118143001_d2config_changelog_columns.js +++ b/server/migrations/20241118143001_d2config_changelog_columns.js @@ -1,22 +1,22 @@ exports.up = async (knex) => { await knex.schema.table('app_version', (table) => { - table.json('d2config').notNullable().default('{}') + table.text('change_summary').nullable() + }) + await knex.schema.table('app', (table) => { + table.jsonb('d2config').nullable().notNullable().default('{}') table.text('changelog').nullable() }) - // await knex.schema.table('app', (table) => { - // table.text('d2config').nullable().notNullable().default('{}') - // table.text('changelog').nullable() - // }) - await knex.raw('DROP VIEW apps_view') // update app-view with column await knex.raw(` + DROP VIEW apps_view; CREATE VIEW apps_view AS SELECT app.id AS app_id, app.type, app.core_app, - appver.version, appver.id AS version_id, appver.created_at AS version_created_at, appver.source_url, appver.demo_url, appver.d2config, appver.changelog, - json_extract_path(d2config , 'entryPoints', 'plugin') AS plugin, + appver.version, appver.id AS version_id, appver.created_at AS version_created_at, appver.source_url, appver.demo_url, appver.change_summary, + app.d2config, app.changelog, + jsonb_extract_path_text(d2config , 'entryPoints', 'plugin') <> '' AS has_plugin, media.app_media_id AS media_id, media.original_filename, media.created_at AS media_created_at, media.media_type, localisedapp.language_code, localisedapp.name, localisedapp.description, localisedapp.slug AS appver_slug, s.status, s.created_at AS status_created_at, @@ -56,15 +56,14 @@ exports.up = async (knex) => { exports.down = async (knex) => { await knex.schema.table('app_version', (table) => { + table.dropColumn('version_change_summary') + }) + + await knex.schema.table('app', (table) => { table.dropColumn('d2config') table.dropColumn('changelog') }) - // await knex.schema.table('app', (table) => { - // table.dropColumn('d2config') - // table.dropColumn('changelog') - // }) - await knex.raw('DROP VIEW apps_view') await knex.raw(` CREATE VIEW apps_view AS diff --git a/server/src/data/createApp.js b/server/src/data/createApp.js index b56b3402..3a489eb3 100644 --- a/server/src/data/createApp.js +++ b/server/src/data/createApp.js @@ -13,6 +13,8 @@ const paramsSchema = joi .required() .valid(...AppTypes), coreApp: joi.bool(), + changelog: joi.string().allow('', null), + d2config: joi.string().allow('', null), }) .options({ allowUnknown: true }) @@ -41,7 +43,15 @@ const createApp = async (params, knex) => { } debug('params: ', params) - const { userId, contactEmail, orgId, appType, coreApp } = params + const { + userId, + contactEmail, + orgId, + appType, + coreApp, + d2config, + changelog, + } = params //generate a new uuid to insert @@ -54,6 +64,8 @@ const createApp = async (params, knex) => { organisation_id: orgId, type: appType, core_app: coreApp, + d2config, + changelog, }) .returning('id') diff --git a/server/src/data/createAppVersion.js b/server/src/data/createAppVersion.js index f0cf03a7..df45622d 100644 --- a/server/src/data/createAppVersion.js +++ b/server/src/data/createAppVersion.js @@ -12,8 +12,7 @@ const paramsSchema = joi demoUrl: joi.string().uri().allow('', null), sourceUrl: joi.string().uri().allow('', null), version: joi.string().allow(''), - changelog: joi.string().allow('', null), - d2config: joi.string().allow('', null), + change_summary: joi.string().allow('', null), }) .options({ allowUnknown: true }) @@ -43,8 +42,7 @@ const createAppVersion = async (params, knex) => { throw paramsValidation.error } - const { userId, appId, demoUrl, sourceUrl, version, changelog, d2config } = - params + const { userId, appId, demoUrl, sourceUrl, version } = params debug('got params: ', params) try { @@ -60,8 +58,6 @@ const createAppVersion = async (params, knex) => { demo_url: demoUrl || '', source_url: sourceUrl || '', version: version || '', - changelog, - d2config, }) .returning('id') @@ -72,8 +68,6 @@ const createAppVersion = async (params, knex) => { demoUrl, sourceUrl, version, - changelog, - d2config, } } catch (err) { throw new Error( diff --git a/server/src/data/index.js b/server/src/data/index.js index 8be89e18..4d2dba14 100644 --- a/server/src/data/index.js +++ b/server/src/data/index.js @@ -21,6 +21,7 @@ module.exports = { getOrganisationsByName: require('./getOrganisationsByName'), getUserByEmail: require('./getUserByEmail'), setImageAsLogoForApp: require('./setImageAsLogoForApp'), + patchApp: require('./patchApp'), updateApp: require('./updateApp'), updateAppVersion: require('./updateAppVersion'), updateImageMeta: require('./updateImageMeta'), diff --git a/server/src/data/patchApp.js b/server/src/data/patchApp.js new file mode 100644 index 00000000..3cc3dd99 --- /dev/null +++ b/server/src/data/patchApp.js @@ -0,0 +1,43 @@ +const joi = require('joi') + +const paramsSchema = joi + .object() + .keys({ + changelog: joi.string().allow('', null), + d2config: joi.string().allow('', null), + id: joi.string().uuid().required(), + }) + .options({ allowUnknown: false }) + +/** + * Patches an app instance + * + * @param {object} params + * @param {string} params.id id of the app to update + * @param {number} params.changelog The changelog of the app + * @param {string} params.d2config The latest d2config of the app + * @returns {Promise} + */ +const patchApp = async (params, knex) => { + const validation = paramsSchema.validate(params) + + if (validation.error !== undefined) { + throw new Error(validation.error) + } + + if (!knex) { + throw new Error('Missing parameter: knex') + } + + const { id, ...rest } = params + + try { + await knex('app').update(rest).where({ + id, + }) + } catch (err) { + throw new Error(`Could not update app: ${id}. ${err.message}`) + } +} + +module.exports = patchApp diff --git a/server/src/routes/v1/apps/handlers/createApp.js b/server/src/routes/v1/apps/handlers/createApp.js index be11ddc8..fbbea387 100644 --- a/server/src/routes/v1/apps/handlers/createApp.js +++ b/server/src/routes/v1/apps/handlers/createApp.js @@ -129,6 +129,8 @@ module.exports = { appType, status: AppStatus.PENDING, coreApp: isCoreApp, + changelog, + d2config: JSON.stringify(d2config), }, trx ) @@ -145,8 +147,6 @@ module.exports = { channel, appName: name, description: description || '', - d2config, - changelog, }, trx ) diff --git a/server/src/routes/v1/apps/handlers/createAppVersion.js b/server/src/routes/v1/apps/handlers/createAppVersion.js index c0af5d7f..5afdbd8f 100644 --- a/server/src/routes/v1/apps/handlers/createAppVersion.js +++ b/server/src/routes/v1/apps/handlers/createAppVersion.js @@ -21,7 +21,11 @@ const createLocalizedAppVersion = require('../../../../data/createLocalizedAppVe const addAppVersionToChannel = require('../../../../data/addAppVersionToChannel') const Organisation = require('../../../../services/organisation') -const { getOrganisationAppsByUserId, getAppsById } = require('../../../../data') +const { + getOrganisationAppsByUserId, + getAppsById, + patchApp, +} = require('../../../../data') const { convertAppToV1AppVersion } = require('../formatting') @@ -134,19 +138,16 @@ module.exports = { } = appVersionJson const [dbApp] = dbAppRows + + // Todo: FIXME a workround - seems like the source url is at app version level but it doesn't get saved after the very first time + const appSourceUrl = dbAppRows?.find((a) => !!a.source_url)?.source_url + debug(`Adding version to app ${dbApp.name}`) let appVersion = null const { changelog, d2config } = await parseAppDetails({ buffer: file._data, - appRepo: sourceUrl, - }) - - verifyBundle({ - buffer: file._data, - appId: dbApp.app_id, // null, // this can never be identical on first upload - appName: dbApp.name, - version, + appRepo: appSourceUrl, }) try { @@ -157,8 +158,6 @@ module.exports = { demoUrl, sourceUrl, version, - changelog, - d2config: JSON.stringify(d2config), }, transaction ) @@ -168,6 +167,16 @@ module.exports = { throw Boom.boomify(err) } + await patchApp( + { + id: dbApp.app_id, + changelog, + d2config: JSON.stringify(d2config), + }, + db, + transaction + ) + //Add the texts as english language, only supported for now try { await createLocalizedAppVersion( diff --git a/server/src/routes/v2/apps.js b/server/src/routes/v2/apps.js index fe81e5d9..06144535 100644 --- a/server/src/routes/v2/apps.js +++ b/server/src/routes/v2/apps.js @@ -166,9 +166,9 @@ module.exports = [ config: { auth: false, tags: ['api', 'v2'], - // cache: { - // expiresIn: 24 * 3600 * 1000, - // }, + cache: { + expiresIn: 6 * 3600 * 1000, + }, validate: { params: Joi.object({ appId: Joi.string().required(), diff --git a/server/src/security/verifyBundle.js b/server/src/security/verifyBundle.js index 006c1e7b..d5367613 100644 --- a/server/src/security/verifyBundle.js +++ b/server/src/security/verifyBundle.js @@ -48,7 +48,6 @@ module.exports = ({ buffer, appId, appName, version, organisationName }) => { const entries = zip.getEntries().map((e) => e.entryName) const manifestPath = 'manifest.webapp' const d2ConfigPath = 'd2.config.json' - const changelogPath = 'CHANGELOG.md' const canBeCoreApp = organisationName === 'DHIS2' if (!entries.includes(manifestPath)) { @@ -59,7 +58,6 @@ module.exports = ({ buffer, appId, appName, version, organisationName }) => { throw new Error(`${manifestPath} is not valid JSON`) } const manifest = JSON.parse(manifestJson) - checkManifest({ manifest, appId, @@ -76,9 +74,6 @@ module.exports = ({ buffer, appId, appName, version, organisationName }) => { if (!isValidJSON(d2ConfigJson)) { throw new Error(`${d2ConfigPath} is not valid JSON`) } - - const changeLog = zip.readAsText(changelogPath) - const d2Config = JSON.parse(d2ConfigJson) checkD2Config({ d2Config, @@ -91,6 +86,5 @@ module.exports = ({ buffer, appId, appName, version, organisationName }) => { return { manifest, d2Config, - changeLog, } } diff --git a/server/src/services/app.js b/server/src/services/app.js index caf39919..128a9084 100644 --- a/server/src/services/app.js +++ b/server/src/services/app.js @@ -16,6 +16,8 @@ exports.create = async ( appType, status, coreApp, + changelog, + d2config, }, db ) => { @@ -26,6 +28,8 @@ exports.create = async ( orgId: organisationId, appType: appType, coreApp, + changelog, + d2config, }, db ) @@ -55,8 +59,6 @@ exports.createVersionForApp = async ( channel, appName, description, - d2config, - changelog, }, db ) => { @@ -67,8 +69,6 @@ exports.createVersionForApp = async ( sourceUrl, demoUrl, version, - d2config, - changelog, }, db ) diff --git a/server/src/services/appVersion.js b/server/src/services/appVersion.js index 3fdee84a..60921abd 100644 --- a/server/src/services/appVersion.js +++ b/server/src/services/appVersion.js @@ -22,10 +22,12 @@ const getAppVersionQuery = (knex) => 'app_version.app_id' ) .innerJoin(knex.ref('channel'), 'ac.channel_id', 'channel.id') + .innerJoin(knex.ref('app'), 'app.id', 'app_version.app_id') + .select( 'app_version.id', 'app_version.version', - knex.raw('app_version.changelog<>\'\' as "hasChangelog"'), + knex.raw('app.changelog<>\'\' as "hasChangelog"'), knex.ref('app_version.app_id').as('appId'), knex.ref('app_version.created_at').as('createdAt'), knex.ref('app_version.updated_at').as('updatedAt'), @@ -113,15 +115,9 @@ class AppVersionService extends Schmervice.Service { } async getChangelog(appId, knex) { - const query = knex('app_version') - .select( - 'app_version.id', - 'app_version.version', - 'app_version.changelog' - ) - .where('app_version.app_id', appId) - .where('app_version.changelog', '<>', '') - // .distinct() + const query = knex('app') + .first('app.id', 'app.changelog') + .where('app.id', appId) const { result } = await executeQuery(query) diff --git a/server/src/utils/parseAppDetails.js b/server/src/utils/parseAppDetails.js index a9f3f40c..2e2dcb32 100644 --- a/server/src/utils/parseAppDetails.js +++ b/server/src/utils/parseAppDetails.js @@ -7,12 +7,15 @@ const getChangeLog = async (appRepo) => { try { const targetUrl = appRepo - .replace(/\/$/, '') - .replace( + ?.replace(/\/$/, '') + ?.replace( 'https://github.com', 'https://raw.githubusercontent.com' - ) + '/refs/heads/main/CHANGELOG.md' + ) + '/refs/heads/master/CHANGELOG.md' // todo: check main branch as well? + if (!targetUrl) { + return null + } debug(`Getting changelog from: ${targetUrl}`) const response = await request.get({ url: targetUrl, @@ -42,17 +45,15 @@ module.exports = async ({ buffer, appRepo }) => { } } - const d2Config = JSON.parse(d2ConfigJson) + const d2config = JSON.parse(d2ConfigJson) return { - d2Config, + d2config, changelog, } } catch (err) { - // throw err - // console.error(err) return { - d2Config: null, + d2config: null, changelog: null, } }