refactor: move changelog to app rather than appversion

client/src/components/Versions/ChangeLogViewer.js

@@ -1,30 +1,23 @@
-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
     }
     return (
         <Modal onClose={onCloseChangelog}>
             <ModalContent>
-                <ReactMarkdown>{matchedVersion?.changelog}</ReactMarkdown>
+                <div>
+                    {loading && <CircularLoader large />}
+                    <ReactMarkdown>{data?.changelog}</ReactMarkdown>
+                </div>
             </ModalContent>
         </Modal>
     )

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 (
         <>
-            <ChangeLogViewer
-                appId={appId}
-                modalShown={changelogVisible}
-                onCloseChangelog={onCloseChangelog}
-            />
+            {changelogVisible && (
+                <ChangeLogViewer
+                    appId={appId}
+                    modalShown={changelogVisible}
+                    onCloseChangelog={onCloseChangelog}
+                />
+            )}
             <Table className={styles.table}>
                 <TableHead>
                     <TableRowHead>

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}
                     </span>
                     <span className={styles.detailsCardType}>{appType}</span>
+                    {app.hasPlugin && <Tag neutral>Plugin</Tag>}
                 </div>
             </section>
             <Divider />

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

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')
 

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(

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'),

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<CreateUserResult>}
+ */
+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

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
             )

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(

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(),

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,
     }
 }