diff --git a/.changeset/fluffy-eggs-wink.md b/.changeset/fluffy-eggs-wink.md new file mode 100644 index 000000000..7395a2356 --- /dev/null +++ b/.changeset/fluffy-eggs-wink.md @@ -0,0 +1,5 @@ +--- +'@openfn/project': patch +--- + +On merge, warn when projects may have diverged diff --git a/.changeset/weak-gifts-work.md b/.changeset/weak-gifts-work.md new file mode 100644 index 000000000..ea45311f8 --- /dev/null +++ b/.changeset/weak-gifts-work.md @@ -0,0 +1,5 @@ +--- +'@openfn/cli': patch +--- + +Warn when merging projects which may have diverged diff --git a/packages/cli/src/merge/command.ts b/packages/cli/src/merge/command.ts index f61db6750..a07447e1d 100644 --- a/packages/cli/src/merge/command.ts +++ b/packages/cli/src/merge/command.ts @@ -1,6 +1,6 @@ import yargs from 'yargs'; import { Opts } from '../options'; -import { ensure, build } from '../util/command-builders'; +import { ensure, build, override } from '../util/command-builders'; import * as o from '../options'; export type MergeOptions = Required< @@ -12,6 +12,7 @@ export type MergeOptions = Required< | 'removeUnmapped' | 'workflowMappings' | 'log' + | 'force' > >; @@ -21,6 +22,9 @@ const options = [ o.removeUnmapped, o.workflowMappings, o.log, + override(o.force, { + description: 'Force a merge even when workflows are incompatible', + }), ]; const mergeCommand: yargs.CommandModule = { diff --git a/packages/cli/src/merge/handler.ts b/packages/cli/src/merge/handler.ts index 4c767df83..fe3ed2283 100644 --- a/packages/cli/src/merge/handler.ts +++ b/packages/cli/src/merge/handler.ts @@ -55,6 +55,7 @@ const mergeHandler = async (options: MergeOptions, logger: Logger) => { const final = Project.merge(sourceProject, targetProject, { removeUnmapped: options.removeUnmapped, workflowMappings: options.workflowMappings, + force: options.force, }); const yaml = final.serialize('state', { format: 'yaml' }); await fs.writeFile(finalPath, yaml); diff --git a/packages/project/src/merge/merge-project.ts b/packages/project/src/merge/merge-project.ts index 09becaae5..9d3b7b1a6 100644 --- a/packages/project/src/merge/merge-project.ts +++ b/packages/project/src/merge/merge-project.ts @@ -1,4 +1,3 @@ -import { Workflow } from '@openfn/lexicon'; import { defaultsDeep, isEmpty } from 'lodash-es'; import { Project } from '../Project'; @@ -6,12 +5,12 @@ import { mergeWorkflows } from './merge-node'; import mapUuids from './map-uuids'; import baseMerge from '../util/base-merge'; import getDuplicates from '../util/get-duplicates'; +import Workflow from '../Workflow'; export type MergeProjectOptions = Partial<{ workflowMappings: Record; // removeUnmapped: boolean; - - force: boolean; // TODO not implemented yet + force: boolean; }>; /** @@ -56,6 +55,27 @@ export function merge( return !!options?.workflowMappings[w.id]; }); + // mergeability + const potentialConflicts: Record = {}; + for (const sourceWorkflow of sourceWorkflows) { + const targetId = + options.workflowMappings?.[sourceWorkflow.id] ?? sourceWorkflow.id; + const targetWorkflow = target.getWorkflow(targetId); + if (targetWorkflow && !sourceWorkflow.canMergeInto(targetWorkflow)) { + potentialConflicts[sourceWorkflow.name] = targetWorkflow?.name; + } + } + + if (Object.keys(potentialConflicts).length && !options?.force) { + throw new Error( + `The below workflows can't be merged directly without losing data\n${Object.entries( + potentialConflicts + ) + .map(([from, to]) => `${from} → ${to}`) + .join('\n')}\nPass --force to force the merge anyway` + ); + } + for (const sourceWorkflow of sourceWorkflows) { const targetId = options.workflowMappings?.[sourceWorkflow.id] ?? sourceWorkflow.id; diff --git a/packages/project/test/project.test.ts b/packages/project/test/project.test.ts index ac5475322..8a0ac6bf1 100644 --- a/packages/project/test/project.test.ts +++ b/packages/project/test/project.test.ts @@ -2,7 +2,7 @@ import test from 'ava'; import type { Provisioner } from '@openfn/lexicon/lightning'; import { Project } from '../src/Project'; -import generateWorkflow from '../src/gen/generator'; +import generateWorkflow, { generateProject } from '../src/gen/generator'; // TODO move to fixtures and re-use? // Or use util function instead? @@ -154,3 +154,34 @@ test('should return UUIDs for everything', (t) => { }, }); }); + +test('incompatible-merge: should throw error when merge is incompatible', (t) => { + const source = generateWorkflow('trigger-x'); + source.pushHistory(source.getVersionHash()); + const target = generateWorkflow('trigger-y'); + target.pushHistory(target.getVersionHash()); + + t.false(source.canMergeInto(target)); + + const sourceProject = new Project({ workflows: [source] }); + const targetProject = new Project({ workflows: [target] }); + t.throws(() => Project.merge(sourceProject, targetProject), { + message: `The below workflows can't be merged directly without losing data\nWorkflow → Workflow\nPass --force to force the merge anyway`, + }); +}); + +test('incompatible-merge-force: should ignore incompatiblity and merge when forced', (t) => { + // same as the above test with force + const source = generateWorkflow('trigger-x'); + source.pushHistory(source.getVersionHash()); + const target = generateWorkflow('trigger-y'); + target.pushHistory(target.getVersionHash()); + + t.false(source.canMergeInto(target)); + + const sourceProject = new Project({ workflows: [source] }); + const targetProject = new Project({ workflows: [target] }); + t.notThrows(() => + Project.merge(sourceProject, targetProject, { force: true }) + ); +});