Skip to content

Commit

Permalink
refactor: move changelog to app rather than appversion
Browse files Browse the repository at this point in the history
  • Loading branch information
kabaros committed Dec 10, 2024
1 parent 366653d commit bd14cde
Show file tree
Hide file tree
Showing 11 changed files with 114 additions and 58 deletions.
3 changes: 2 additions & 1 deletion client/src/pages/UserApp/DetailsCard/DetailsCard.js
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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 />
Expand Down
25 changes: 12 additions & 13 deletions server/migrations/20241118143001_d2config_changelog_columns.js
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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
Expand Down
14 changes: 13 additions & 1 deletion server/src/data/createApp.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 })

Expand Down Expand Up @@ -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

Expand All @@ -54,6 +64,8 @@ const createApp = async (params, knex) => {
organisation_id: orgId,
type: appType,
core_app: coreApp,
d2config,
changelog,
})
.returning('id')

Expand Down
10 changes: 2 additions & 8 deletions server/src/data/createAppVersion.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 })

Expand Down Expand Up @@ -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 {
Expand All @@ -60,8 +58,6 @@ const createAppVersion = async (params, knex) => {
demo_url: demoUrl || '',
source_url: sourceUrl || '',
version: version || '',
changelog,
d2config,
})
.returning('id')

Expand All @@ -72,8 +68,6 @@ const createAppVersion = async (params, knex) => {
demoUrl,
sourceUrl,
version,
changelog,
d2config,
}
} catch (err) {
throw new Error(
Expand Down
1 change: 1 addition & 0 deletions server/src/data/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
43 changes: 43 additions & 0 deletions server/src/data/patchApp.js
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions server/src/routes/v1/apps/handlers/createApp.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ module.exports = {
appType,
status: AppStatus.PENDING,
coreApp: isCoreApp,
changelog,
d2config: JSON.stringify(d2config),
},
trx
)
Expand All @@ -145,8 +147,6 @@ module.exports = {
channel,
appName: name,
description: description || '',
d2config,
changelog,
},
trx
)
Expand Down
31 changes: 20 additions & 11 deletions server/src/routes/v1/apps/handlers/createAppVersion.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down Expand Up @@ -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 {
Expand All @@ -157,8 +158,6 @@ module.exports = {
demoUrl,
sourceUrl,
version,
changelog,
d2config: JSON.stringify(d2config),
},
transaction
)
Expand All @@ -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(
Expand Down
8 changes: 4 additions & 4 deletions server/src/services/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ exports.create = async (
appType,
status,
coreApp,
changelog,
d2config,
},
db
) => {
Expand All @@ -26,6 +28,8 @@ exports.create = async (
orgId: organisationId,
appType: appType,
coreApp,
changelog,
d2config,
},
db
)
Expand Down Expand Up @@ -55,8 +59,6 @@ exports.createVersionForApp = async (
channel,
appName,
description,
d2config,
changelog,
},
db
) => {
Expand All @@ -67,8 +69,6 @@ exports.createVersionForApp = async (
sourceUrl,
demoUrl,
version,
d2config,
changelog,
},
db
)
Expand Down
16 changes: 6 additions & 10 deletions server/src/services/appVersion.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down Expand Up @@ -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')
.select('app.id', 'app.changelog')
.where('app.id', appId)

const { result } = await executeQuery(query)

Expand Down
17 changes: 9 additions & 8 deletions server/src/utils/parseAppDetails.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
}
}
Expand Down

0 comments on commit bd14cde

Please sign in to comment.