Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrading plugins with dependencies #2245

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { installPlugin, uninstallPlugin, waitForApplication } from '../../utils'

const plugin = '@govuk-prototype-kit/common-templates'
const pluginVersion = '1.1.1'
const pluginsPage = '/manage-prototype/plugins'

const dependencyPlugin = 'govuk-frontend'
const dependencyPluginName = 'GOV.UK Frontend'

describe('Handle a plugin update', () => {
it('when a dependency is now required', () => {
installPlugin(plugin, pluginVersion)
uninstallPlugin(dependencyPlugin)

waitForApplication(pluginsPage)

cy.get('[data-plugin-group-status="available"]')
.find(`[data-plugin-package-name="${dependencyPlugin}"]`)
.find('button')
.contains('Install')

cy.get('[data-plugin-group-status="installed"]')
.find(`[data-plugin-package-name="${plugin}"]`)
.find('button')
.contains('Update')
.click()

cy.get('#plugin-action-confirmation')
.find('ul')
.contains(dependencyPluginName)

cy.get('#plugin-action-button').click()

cy.get('#panel-complete', { timeout: 20000 })
.should('be.visible')
.contains('Update complete')

cy.get('#instructions-complete a')
.contains('Back to plugins')
.click()

cy.get('[data-plugin-group-status="installed"]')
.find(`[data-plugin-package-name="${dependencyPlugin}"]`)

cy.get('[data-plugin-group-status="installed"]')
.find(`[data-plugin-package-name="${plugin}"]`)
})
})
28 changes: 14 additions & 14 deletions lib/manage-prototype-handlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,10 @@ function getCommand (mode, chosenPlugin) {
dependentPlugins
} = chosenPlugin
const dependents = dependentPlugins?.map(({ packageName }) => packageName).join(' ')
const dependencies = dependencyPlugins?.map(({ packageName }) => packageName).join(' ')
const dependencies = dependencyPlugins?.map(({
packageName,
latestVersion
}) => packageName + '@' + latestVersion).join(' ')

if (version && installCommand) {
installCommand += `@${version}`
Expand Down Expand Up @@ -474,19 +477,22 @@ const verbs = {
title: 'Update',
para: 'update',
status: 'updated',
progressive: 'updating'
progressive: 'updating',
other: 'install'
},
install: {
title: 'Install',
para: 'install',
status: 'installed',
progressive: 'installing'
progressive: 'installing',
other: 'install'
},
uninstall: {
title: 'Uninstall',
para: 'uninstall',
status: 'uninstalled',
progressive: 'uninstalling'
progressive: 'uninstalling',
other: 'uninstall'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, I think there's likely to be a better name than other. if you/we can find a better one before merging that could work or we can just keep an eye out for a better name over time.

}
}

Expand Down Expand Up @@ -533,21 +539,15 @@ async function getPluginForRequest (req) {
}
}

const dependentPlugins = mode === 'uninstall'
? (await getDependentPackages(chosenPlugin.packageName))
.filter(({ installed }) => installed)
.map(buildPluginData)
: []
const dependentPlugins = (await getDependentPackages(chosenPlugin.packageName, version, mode))
.filter(({ installed }) => installed || mode !== 'uninstall')
.map(buildPluginData)

if (dependentPlugins.length) {
chosenPlugin.dependentPlugins = dependentPlugins
}

const dependencyPlugins = mode !== 'uninstall'
? (await getDependencyPackages(chosenPlugin.packageName, version))
.filter(({ installed }) => !installed)
.map(buildPluginData)
: []
const dependencyPlugins = (await getDependencyPackages(chosenPlugin.packageName, version, mode)).map(buildPluginData)

if (dependencyPlugins.length) {
chosenPlugin.dependencyPlugins = dependencyPlugins
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
</ul>

<p class="govuk-body">
To {{ verb.para }} {{ chosenPlugin.name }} you also need to {{ verb.para }} {% if chosenPlugin.dependentPlugins | length > 1 %}these plugins{% else %}this plugin{% endif %}.
To {{ verb.para }} {{ chosenPlugin.name }} you also need to {{ verb.other }} {% if chosenPlugin.dependentPlugins | length > 1 %}these plugins{% else %}this plugin{% endif %}.
</p>
<div class="govuk-button-group">
{{ govukButton({
Expand Down
38 changes: 32 additions & 6 deletions lib/plugins/packages.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ async function refreshPackageInfo (packageName, version) {
localVersion = path.resolve(installedPackageVersion.replace('file:', ''))
}

const pluginDependencies = pluginConfig?.pluginDependencies
const pluginDependencies = pluginConfig?.pluginDependencies ? normaliseDependencies(pluginConfig.pluginDependencies) : undefined

const packageInfo = {
packageName,
Expand Down Expand Up @@ -206,7 +206,21 @@ async function waitForPackagesCache () {
}
}

async function getDependentPackages (packageName) {
function normaliseDependencies (dependencies) {
return dependencies.map((dependency) => {
if (typeof dependency === 'string') {
dependency = {
packageName: dependency
}
}
return dependency
})
}

async function getDependentPackages (packageName, version, mode) {
if (mode !== 'uninstall') {
return []
}
if (!Object.keys(packagesCache).length) {
await startPackageTracker()
}
Expand All @@ -215,17 +229,29 @@ async function getDependentPackages (packageName) {
.filter(({ pluginDependencies }) => pluginDependencies?.some((pluginDependency) => pluginDependency === packageName || pluginDependency.packageName === packageName))
}

async function getDependencyPackages (packageName, version) {
async function getDependencyPackages (packageName, version, mode) {
if (!Object.keys(packagesCache).length) {
await startPackageTracker()
}
await waitForPackagesCache()
const pkg = await lookupPackageInfo(packageName, version)
return !pkg?.pluginDependencies
let pluginDependencies = pkg?.pluginDependencies
if (version || mode === 'update') {
const targetVersion = version || pkg.latestVersion
if (targetVersion !== pkg.installedVersion) {
const latestPluginConfig = await getConfigForPackage(pkg.packageName, version)
if (latestPluginConfig) {
pluginDependencies = latestPluginConfig.pluginDependencies
}
}
}
const dependencyPlugins = !pluginDependencies
? []
: await Promise.all(pkg.pluginDependencies.map((pluginDependency) => {
return typeof pluginDependency === 'string' ? lookupPackageInfo(pluginDependency) : lookupPackageInfo(pluginDependency.packageName)
: await Promise.all(normaliseDependencies(pluginDependencies).map((pluginDependency) => {
return lookupPackageInfo(pluginDependency.packageName)
}))

return dependencyPlugins.filter(({ installed }) => !installed)
}

if (!config.getConfig().isTest) {
Expand Down
19 changes: 15 additions & 4 deletions lib/plugins/packages.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,23 +43,29 @@ describe('packages', () => {
const availableInstalledPackage = {
packageName: 'available-installed-package',
installed: true,
available: true
available: true,
installedVersion: '1.0.0',
latestVersion: '1.0.0'
}
const availableUninstalledPackage = {
packageName: 'available-uninstalled-package',
installed: false,
available: true
available: true,
latestVersion: '1.0.0'
}
const unavailableInstalledPackage = {
packageName: 'unavailable-installed-package',
installed: true,
available: false,
installedVersion: '1.0.0',
latestVersion: '1.0.0',
pluginDependencies: [availableInstalledPackage]
}
const unavailableUninstalledPackage = {
packageName: 'unavailable-uninstalled-package',
installed: false,
available: false,
latestVersion: '1.0.0',
pluginDependencies: [availableUninstalledPackage]
}

Expand All @@ -86,10 +92,15 @@ describe('packages', () => {
})

describe('getDependentPackages', () => {
it('', async () => {
const dependentPackages = await getDependentPackages(availableInstalledPackage.packageName)
it('when mode is uninstall', async () => {
const dependentPackages = await getDependentPackages(availableInstalledPackage.packageName, undefined, 'uninstall')
expect(dependentPackages).toEqual([unavailableInstalledPackage])
})

it('when mode is update', async () => {
const dependentPackages = await getDependentPackages(availableInstalledPackage.packageName, '1.1.1', 'update')
expect(dependentPackages).toEqual([])
})
})

describe('getDependencyPackages', () => {
Expand Down
16 changes: 8 additions & 8 deletions lib/utils/requestHttps.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const { tmpDir } = require('./paths')
const { exists, readJson, ensureDir, writeJson } = require('fs-extra')
const { verboseLog } = require('./verboseLogger')

async function getConfigForPackage (packageName) {
async function getConfigForPackage (packageName, version) {
const timer = startPerformanceTimer()

const cacheFileDirectory = path.join(tmpDir, 'caches')
Expand All @@ -32,27 +32,27 @@ async function getConfigForPackage (packageName) {

return undefined
}
const latestTag = registry['dist-tags']?.latest
const targetVersion = version || registry['dist-tags']?.latest

if (cache[packageName] && cache[packageName][latestTag]) {
if (cache[packageName] && cache[packageName][targetVersion]) {
endPerformanceTimer('getConfigForPackage (from cache)', timer)
return cache[packageName][latestTag]
return cache[packageName][targetVersion]
}

if (!latestTag) {
endPerformanceTimer('getConfigForPackage (no latest tag)', timer)
if (!targetVersion || !registry.versions[targetVersion]) {
endPerformanceTimer(`getConfigForPackage (no ${targetVersion ? 'version ' + targetVersion : 'latest tag'})`, timer)
return
}

const url = registry.versions[latestTag].dist.tarball
const url = registry.versions[targetVersion].dist.tarball
try {
const result = await findFileInHttpsTgz(url, {
fileToFind: 'package/govuk-prototype-kit.config.json',
prepare: str => {
if (str && str.startsWith('{')) {
const result = JSON.parse(str)
cache[packageName] = cache[packageName] || {}
cache[packageName][latestTag] = result
cache[packageName][targetVersion] = result
writeJson(cacheFileReference, cache)
return result
}
Expand Down