diff --git a/src/lib/domain/diff/diff-check/__tests__/diff-check.test.ts b/src/lib/domain/diff/diff-check/__tests__/diff-check.test.ts index 408fca34..a2eed36b 100644 --- a/src/lib/domain/diff/diff-check/__tests__/diff-check.test.ts +++ b/src/lib/domain/diff/diff-check/__tests__/diff-check.test.ts @@ -926,14 +926,22 @@ describe('generateDiff', () => { }); it('should detect relationship targetCardinality changes', () => { + // Use (many, many) -> (many, one) to test targetCardinality change detection + // This is NOT equivalent since source is 'many' in both cases const oldDiagram = createMockDiagram({ relationships: [ - createMockRelationship({ targetCardinality: 'many' }), + createMockRelationship({ + sourceCardinality: 'many', + targetCardinality: 'many', + }), ], }); const newDiagram = createMockDiagram({ relationships: [ - createMockRelationship({ targetCardinality: 'one' }), + createMockRelationship({ + sourceCardinality: 'many', + targetCardinality: 'one', + }), ], }); @@ -955,6 +963,102 @@ describe('generateDiff', () => { expect((diff as RelationshipDiffChanged)?.newValue).toBe('one'); }); + it('should NOT detect cardinality changes between (one,one) and (one,many) as they produce same DDL', () => { + // (one,one) -> (one,many): source stays 'one', equivalent from DDL perspective + const oldDiagram1 = createMockDiagram({ + relationships: [ + createMockRelationship({ + sourceCardinality: 'one', + targetCardinality: 'one', + }), + ], + }); + const newDiagram1 = createMockDiagram({ + relationships: [ + createMockRelationship({ + sourceCardinality: 'one', + targetCardinality: 'many', + }), + ], + }); + + const result1 = generateDiff({ + diagram: oldDiagram1, + newDiagram: newDiagram1, + }); + + // Should NOT detect any cardinality changes + expect( + result1.diffMap.has('relationship-sourceCardinality-rel-1') + ).toBe(false); + expect( + result1.diffMap.has('relationship-targetCardinality-rel-1') + ).toBe(false); + expect(result1.diffMap.size).toBe(0); + + // (one,many) -> (one,one): reverse direction, also equivalent + const oldDiagram2 = createMockDiagram({ + relationships: [ + createMockRelationship({ + sourceCardinality: 'one', + targetCardinality: 'many', + }), + ], + }); + const newDiagram2 = createMockDiagram({ + relationships: [ + createMockRelationship({ + sourceCardinality: 'one', + targetCardinality: 'one', + }), + ], + }); + + const result2 = generateDiff({ + diagram: oldDiagram2, + newDiagram: newDiagram2, + }); + + expect( + result2.diffMap.has('relationship-sourceCardinality-rel-1') + ).toBe(false); + expect( + result2.diffMap.has('relationship-targetCardinality-rel-1') + ).toBe(false); + expect(result2.diffMap.size).toBe(0); + }); + + it('should still detect cardinality changes that produce different DDL', () => { + // (one,one) -> (many,one): source changes from 'one' to 'many', real change + const oldDiagram = createMockDiagram({ + relationships: [ + createMockRelationship({ + sourceCardinality: 'one', + targetCardinality: 'one', + }), + ], + }); + const newDiagram = createMockDiagram({ + relationships: [ + createMockRelationship({ + sourceCardinality: 'many', + targetCardinality: 'one', + }), + ], + }); + + const result = generateDiff({ + diagram: oldDiagram, + newDiagram, + }); + + // Should detect the sourceCardinality change + expect( + result.diffMap.has('relationship-sourceCardinality-rel-1') + ).toBe(true); + expect(result.diffMap.size).toBe(1); + }); + it('should detect multiple relationship attribute changes', () => { const oldDiagram = createMockDiagram({ relationships: [ diff --git a/src/lib/domain/diff/diff-check/diff-check.ts b/src/lib/domain/diff/diff-check/diff-check.ts index 994ff00a..6fac168f 100644 --- a/src/lib/domain/diff/diff-check/diff-check.ts +++ b/src/lib/domain/diff/diff-check/diff-check.ts @@ -1290,19 +1290,46 @@ function compareRelationshipProperties({ ) { changedAttributes.push('targetFieldId'); } + // Check cardinality changes, but exclude changes that produce the same DDL. + // In DDL export, when source is 'one', the relationship direction stays consistent. + // Therefore (one, one) ↔ (one, many) are equivalent from DDL perspective. + const shouldCheckCardinality = + attributesToCheck.includes('sourceCardinality') || + attributesToCheck.includes('targetCardinality'); - if ( - attributesToCheck.includes('sourceCardinality') && - oldRelationship.sourceCardinality !== newRelationship.sourceCardinality - ) { - changedAttributes.push('sourceCardinality'); - } + if (shouldCheckCardinality) { + const oldSource = oldRelationship.sourceCardinality; + const oldTarget = oldRelationship.targetCardinality; + const newSource = newRelationship.sourceCardinality; + const newTarget = newRelationship.targetCardinality; - if ( - attributesToCheck.includes('targetCardinality') && - oldRelationship.targetCardinality !== newRelationship.targetCardinality - ) { - changedAttributes.push('targetCardinality'); + // Check if this is an "equivalent" cardinality change that produces the same DDL + // Equivalent pairs: (one, one) ↔ (many, one) - both put FK on source table + const isEquivalentCardinalityChange = + (oldSource === 'one' && + oldTarget === 'one' && + newTarget === 'many' && + newSource === 'one') || + (oldTarget === 'many' && + oldSource === 'one' && + newSource === 'one' && + newTarget === 'one'); + + if (!isEquivalentCardinalityChange) { + if ( + attributesToCheck.includes('sourceCardinality') && + oldSource !== newSource + ) { + changedAttributes.push('sourceCardinality'); + } + + if ( + attributesToCheck.includes('targetCardinality') && + oldTarget !== newTarget + ) { + changedAttributes.push('targetCardinality'); + } + } } if (changedAttributes.length > 0) {