diff --git a/integration-tests/cli/CHANGELOG.md b/integration-tests/cli/CHANGELOG.md index 2037b71e0..f22a475d8 100644 --- a/integration-tests/cli/CHANGELOG.md +++ b/integration-tests/cli/CHANGELOG.md @@ -1,5 +1,12 @@ # @openfn/integration-tests-cli +## 1.0.15 + +### Patch Changes + +- Updated dependencies [58a9919] + - @openfn/project@0.14.1 + ## 1.0.14 ### Patch Changes diff --git a/integration-tests/cli/package.json b/integration-tests/cli/package.json index b60284eaa..6ad5957c3 100644 --- a/integration-tests/cli/package.json +++ b/integration-tests/cli/package.json @@ -1,7 +1,7 @@ { "name": "@openfn/integration-tests-cli", "private": true, - "version": "1.0.14", + "version": "1.0.15", "description": "CLI integration tests", "author": "Open Function Group ", "license": "ISC", diff --git a/packages/cli/CHANGELOG.md b/packages/cli/CHANGELOG.md index f42b3540f..6a71eecbf 100644 --- a/packages/cli/CHANGELOG.md +++ b/packages/cli/CHANGELOG.md @@ -1,5 +1,14 @@ # @openfn/cli +## 1.28.1 + +### Patch Changes + +- 58a9919: Fix an issue where added and removed workflows are ignored by merge +- 66e7c13: Fix deploy when the target project is a different UUID to the local project +- Updated dependencies [58a9919] + - @openfn/project@0.14.1 + ## 1.28.0 ### Minor Changes diff --git a/packages/cli/package.json b/packages/cli/package.json index 7fdfd0f05..7863b80b4 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/cli", - "version": "1.28.0", + "version": "1.28.1", "description": "CLI devtools for the OpenFn toolchain", "engines": { "node": ">=18", diff --git a/packages/cli/src/projects/deploy.ts b/packages/cli/src/projects/deploy.ts index 8e9754679..41a655759 100644 --- a/packages/cli/src/projects/deploy.ts +++ b/packages/cli/src/projects/deploy.ts @@ -22,6 +22,8 @@ import type { Provisioner } from '@openfn/lexicon/lightning'; import type { Logger } from '../util/logger'; import type { Opts } from '../options'; +export const DEFAULT_ENDPOINT = 'https://app.openfn.org'; + export type DeployOptions = Pick< Opts, | 'apiKey' @@ -33,6 +35,7 @@ export type DeployOptions = Pick< | 'logJson' | 'confirm' > & { + project?: string; // this is a CLI positional arg, not an option workspace?: string; dryRun?: boolean; new?: boolean; @@ -63,7 +66,7 @@ const printProjectName = (project: Project) => `${project.id} (${project.openfn?.uuid || ''})`; export const command: yargs.CommandModule = { - command: 'deploy', + command: 'deploy [project]', aliases: 'push', describe: `Deploy the checked out project to a Lightning Instance`, builder: (yargs: yargs.Argv) => @@ -74,7 +77,11 @@ export const command: yargs.CommandModule = { }) .example( 'deploy', - 'Deploy the checkout project to the connected instance' + 'Deploy the checked-out project its connected remote instance' + ) + .example( + 'deploy staging', + 'Deploy the checkout-out project to the remote project with alias "staging"' ), handler: ensure('project-deploy', options), }; @@ -97,9 +104,11 @@ export const hasRemoteDiverged = ( if (wf.id in refs) { const forkedVersion = refs[wf.id]; const remoteVersion = remote.getWorkflow(wf.id)?.history.at(-1); - if (!versionsEqual(forkedVersion, remoteVersion!)) { - diverged ??= []; - diverged.push(wf.id); + if (remoteVersion) { + if (!versionsEqual(forkedVersion, remoteVersion!)) { + diverged ??= []; + diverged.push(wf.id); + } } } else { // TODO what if there's no forked from for this workflow? @@ -120,26 +129,30 @@ const syncProjects = async ( config: Required, ws: Workspace, localProject: Project, + trackedProject: Project, // the project we want to update logger: Logger ): Promise => { - logger.info('Attempting to load checked-out project from workspace'); - // First step, fetch the latest version and write // this may throw! let remoteProject: Project; try { + logger.info('Fetching remote target ', printProjectName(trackedProject)); + // TODO should we prefer endpoint over alias? + // maybe if it's explicitly passed? + const endpoint = trackedProject.openfn?.endpoint ?? config.endpoint; + const { data } = await fetchProject( - config.endpoint, + endpoint, config.apiKey, - localProject.uuid ?? localProject.id, + trackedProject.uuid!, logger ); remoteProject = await Project.from('state', data!, { - endpoint: config.endpoint, + endpoint: endpoint, }); - logger.success('Downloaded latest version of project at ', config.endpoint); + logger.success('Downloaded latest version of project at ', endpoint); } catch (e) { console.log(e); throw e; @@ -150,17 +163,6 @@ const syncProjects = async ( // this will actually happen later } - // warn if the remote UUID is different to the local UUID - // This shouldn't happen? - if (!options.force && localProject.uuid !== remoteProject.uuid) { - logger.error(`UUID conflict! - -Your local project (${localProject.uuid}) has a different UUID to the remote project (${remoteProject.uuid}). - -Pass --force to override this error and deploy anyway.`); - process.exit(1); - } - const locallyChangedWorkflows = await findLocallyChangedWorkflows( ws, localProject @@ -183,9 +185,9 @@ Pass --force to override this error and deploy anyway.`); // Skip divergence testing if the remote has no history in its workflows // (this will only happen on older versions of lightning) // TODO now maybe skip if there's no forked_from - const skipVersionTest = remoteProject.workflows.find( - (wf) => wf.history.length === 0 - ); + const skipVersionTest = + options.force || + remoteProject.workflows.find((wf) => wf.history.length === 0); if (skipVersionTest) { logger.warn( @@ -222,10 +224,11 @@ Pass --force to override this error and deploy anyway.`); } logger.info('Merging changes into remote project'); - // TODO I would like to log which workflows are being updated const merged = Project.merge(localProject, remoteProject!, { - mode: 'replace', + // If pushing the same project, we use a replace strategy + // Otherwise, use the sandbox strategy to preserve UUIDs + mode: localProject.uuid === remoteProject.uuid ? 'replace' : 'sandbox', force: true, onlyUpdated: true, }); @@ -252,7 +255,34 @@ export async function handler(options: DeployOptions, logger: Logger) { name: options.name, }); + // Track the remote we want to target + // If the used passed a project alias, we need to use that + // Otherwise just sync with the local project + const tracker = ws.get(options.project ?? localProject.uuid!); + + if (!tracker) { + // Is this really an error? Unlikely to happen I thuink + console.log( + `ERROR: Failed to find tracked remote project ${ + options.project ?? localProject.uuid! + } locally` + ); + console.log('To deploy a new project, add --new to the command'); + // TODO can we automate the fetch bit? + // If it's a UUID it should be ok? + console.log( + 'You may need to fetch the project before you can safely deploy' + ); + + throw new Error('Failed to find remote project locally'); + } + + let endpoint: string = tracker.openfn?.endpoint ?? ''; + if (options.new) { + endpoint = + config.endpoint ?? localProject.openfn?.endpoint ?? DEFAULT_ENDPOINT; + // reset all metadata localProject.openfn = { endpoint: config.endpoint, @@ -261,11 +291,13 @@ export async function handler(options: DeployOptions, logger: Logger) { // generate a credential map localProject.credentials = localProject.buildCredentialMap(); - logger.success(`Loaded local project ${printProjectName(localProject)}`); + logger.success( + `Loaded checked-out project ${printProjectName(localProject)}` + ); const merged: Project = options.new ? localProject - : await syncProjects(options, config, ws, localProject, logger); + : await syncProjects(options, config, ws, localProject, tracker, logger); const state = merged.serialize('state', { format: 'json', @@ -279,9 +311,6 @@ export async function handler(options: DeployOptions, logger: Logger) { logger.debug(JSON.stringify(state, null, 2)); logger.debug(); - // TODO not totally sold on endpoint handling right now - config.endpoint ??= localProject.openfn?.endpoint!; - // TODO: I want to report diff HERE, after the merged state and stuff has been built if (options.dryRun) { @@ -292,11 +321,7 @@ export async function handler(options: DeployOptions, logger: Logger) { // The following workflows will be updated if (options.confirm) { - if ( - !(await logger.confirm( - `Ready to deploy changes to ${config.endpoint}?` - )) - ) { + if (!(await logger.confirm(`Ready to deploy changes to ${endpoint}?`))) { logger.always('Cancelled deployment'); return false; } @@ -305,7 +330,7 @@ export async function handler(options: DeployOptions, logger: Logger) { logger.info('Sending project to app...'); const { data: result } = await deployProject( - config.endpoint, + endpoint, config.apiKey, state, logger @@ -315,7 +340,7 @@ export async function handler(options: DeployOptions, logger: Logger) { 'state', result, { - endpoint: config.endpoint, + endpoint: endpoint, alias, }, merged.config @@ -332,7 +357,7 @@ export async function handler(options: DeployOptions, logger: Logger) { const fullFinalPath = await serialize(finalProject, finalOutputPath); logger.debug('Updated local project at ', fullFinalPath); - logger.success('Updated project at', config.endpoint); + logger.success('Updated project at', endpoint); } } @@ -381,4 +406,3 @@ export const reportDiff = ( return diffs; }; -``; diff --git a/packages/cli/src/projects/merge.ts b/packages/cli/src/projects/merge.ts index af237a646..6b471e39a 100644 --- a/packages/cli/src/projects/merge.ts +++ b/packages/cli/src/projects/merge.ts @@ -50,7 +50,7 @@ const options = [ const command: yargs.CommandModule = { command: 'merge ', describe: - 'Merges the specified project (by UUID, id or alias) into the currently checked out project', + 'Merges the currently checked-out project into the target project, and checks out the result. Does not update the remote project or local project.yaml file', handler: ensure('project-merge', options), builder: (yargs) => build(options, yargs), }; @@ -141,8 +141,6 @@ export const handler = async (options: MergeOptions, logger: Logger) => { logger.info('Checking out merged project to filesystem'); - // TODO support --no-checkout to merge without expanding - // Checkout after merge to expand updated files into filesystem await checkout( { diff --git a/packages/cli/src/projects/util.ts b/packages/cli/src/projects/util.ts index 47061656b..d325ec7ec 100644 --- a/packages/cli/src/projects/util.ts +++ b/packages/cli/src/projects/util.ts @@ -9,6 +9,7 @@ import { CLIError } from '../errors'; import resolvePath from '../util/resolve-path'; import { rimraf } from 'rimraf'; import { versionsEqual, Workspace } from '@openfn/project'; +import { DEFAULT_ENDPOINT } from './deploy'; export type AuthOptions = Pick; @@ -20,7 +21,7 @@ export const loadAppAuthConfig = ( const config: AuthOptions = { apiKey: options.apiKey, - endpoint: options.endpoint ?? 'https://app.openfn.org', + endpoint: options.endpoint ?? DEFAULT_ENDPOINT, }; if (!options.apiKey && OPENFN_API_KEY) { diff --git a/packages/project/CHANGELOG.md b/packages/project/CHANGELOG.md index 009374793..60ddd603a 100644 --- a/packages/project/CHANGELOG.md +++ b/packages/project/CHANGELOG.md @@ -1,5 +1,11 @@ # @openfn/project +## 0.14.1 + +### Patch Changes + +- 58a9919: Fix an issue where added and removed workflows are ignored by merge + ## 0.14.0 ### Minor Changes diff --git a/packages/project/package.json b/packages/project/package.json index 46110dd89..473fbffb3 100644 --- a/packages/project/package.json +++ b/packages/project/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/project", - "version": "0.14.0", + "version": "0.14.1", "description": "Read, serialize, replicate and sync OpenFn projects", "scripts": { "test": "pnpm ava", diff --git a/packages/project/src/util/find-changed-workflows.ts b/packages/project/src/util/find-changed-workflows.ts index baa61b2f2..ca22e5ccf 100644 --- a/packages/project/src/util/find-changed-workflows.ts +++ b/packages/project/src/util/find-changed-workflows.ts @@ -1,8 +1,9 @@ import Project from '../Project'; +import type Workflow from '../Workflow'; import { generateHash } from './version'; /** - * For a give Project, identify which workflows have changed + * For a given Project, identify which workflows have changed * Uses forked_from as the base, or history if that's unavailable */ export default (project: Project) => { @@ -23,8 +24,19 @@ export default (project: Project) => { if (hash !== base[wf.id]) { changed.push(wf); } + delete base[wf.id]; + } else { + // If a workflow doens't appear in forked_from, we assume it's new + // (and so changed!) + changed.push(wf); } } + // Anything in forked_from that hasn't been handled + // must have been removed (and so changed!) + for (const removedId in base) { + changed.push({ id: removedId, $deleted: true } as unknown as Workflow); + } + return changed; }; diff --git a/packages/project/test/merge/merge-project.test.ts b/packages/project/test/merge/merge-project.test.ts index 1ed4af421..e495f7e32 100644 --- a/packages/project/test/merge/merge-project.test.ts +++ b/packages/project/test/merge/merge-project.test.ts @@ -432,6 +432,86 @@ test('should merge two projects and preserve edge id', (t) => { t.is(target.getUUID('a-b'), resultEdge.openfn.uuid); }); +test('merge a new workflow', (t) => { + const wf1 = assignUUIDs({ + name: 'wf1', + steps: [], + }); + // Wf2 has no UUIDs + const wf2 = { + name: 'wf2', + steps: [], + }; + + const main = createProject([wf1], 'a'); + const staging = createProject([wf1, wf2], 'b'); + t.is(main.workflows.length, 1) + t.is(staging.workflows.length, 2) + + const result = merge(staging, main); + t.is(result.workflows.length, 2) +}); + +test('merge a new workflow with onlyUpdated: true', (t) => { + const wf1 = assignUUIDs({ + name: 'wf1', + steps: [], + }); + // Wf2 has no UUIDs + const wf2 = { + name: 'wf2', + steps: [], + }; + + const main = createProject([wf1], 'a'); + const staging = createProject([wf1, wf2], 'b'); + t.is(main.workflows.length, 1) + t.is(staging.workflows.length, 2) + + const result = merge(staging, main, { onlyUpdated: true }); + t.is(result.workflows.length, 2) +}); + +test('remove a workflow', (t) => { + const wf1 = assignUUIDs({ + name: 'wf1', + steps: [], + }); + const wf2 = assignUUIDs({ + name: 'wf2', + steps: [], + }); + + const main = createProject([wf1, wf2], 'a'); + const staging = createProject([wf1], 'b'); + + t.is(main.workflows.length, 2) + t.is(staging.workflows.length, 1) + + const result = merge(staging, main); + t.is(result.workflows.length, 1) +}); + +test('remove a workflow with onlyUpdated: true', (t) => { + const wf1 = assignUUIDs({ + name: 'wf1', + steps: [], + }); + const wf2 = assignUUIDs({ + name: 'wf2', + steps: [], + }); + + const main = createProject([wf1, wf2], 'a'); + const staging = createProject([wf1], 'b'); + + t.is(main.workflows.length, 2) + t.is(staging.workflows.length, 1) + + const result = merge(staging, main, { onlyUpdated: true }); + t.is(result.workflows.length, 1) +}); + test('id match: same workflow in source and target project', (t) => { const source = generateWorkflow('a-b'); const target = generateWorkflow('a-b'); diff --git a/packages/project/test/util/find-changed-workflows.test.ts b/packages/project/test/util/find-changed-workflows.test.ts index c3b144ae9..2ccd969cc 100644 --- a/packages/project/test/util/find-changed-workflows.test.ts +++ b/packages/project/test/util/find-changed-workflows.test.ts @@ -36,6 +36,37 @@ test('should return 1 changed workflows from forked_from', (t) => { t.is(changed[0].id, 'b'); }); +test('should return 1 removed workflow', (t) => { + const project = generateProject('proj', ['@id a a-b', '@id b x-y']); + const [a, b] = project.workflows; + + project.cli.forked_from = { + [a.id]: generateHash(a), + [b.id]: generateHash(b), + }; + + // remove workflow b + project.workflows.pop() + + const changed = findChangedWorkflows(project); + t.is(changed.length, 1); + t.is(changed[0].id, 'b'); +}); + +test('should return 1 added workflow', (t) => { + const project = generateProject('proj', ['@id a a-b', '@id b x-y']); + const [a, b] = project.workflows; + + project.cli.forked_from = { + [a.id]: generateHash(a), + // Do not include b in forked_from - it's new! + }; + + const changed = findChangedWorkflows(project); + t.is(changed.length, 1); + t.is(changed[0].id, 'b'); +}); + test.todo('changed from history'); test.todo('multiple changed workflows'); test.todo('if no base available, assume a change');