fix: ingest parts not being updated when rank changes

Author: Julusian

packages/job-worker/src/blueprints/ingest/MutableIngestRundownImpl.ts

@@ -347,11 +347,7 @@ export class MutableIngestRundownImpl<TRundownPayload = unknown, TSegmentPayload
 			}
 
 			// Regenerate the segment if there are substantial changes
-			if (
-				segmentInfo.segmentHasChanges ||
-				segmentInfo.partOrderHasChanged ||
-				segmentInfo.partIdsWithChanges.length > 0
-			) {
+			if (segmentInfo.segmentHasChanges || segmentInfo.partIdsWithChanges.length > 0) {
 				segmentsToRegenerate.push(ingestSegment)
 			}
 		})

packages/job-worker/src/blueprints/ingest/MutableIngestSegmentImpl.ts

@@ -20,7 +20,6 @@ export interface MutableIngestSegmentChanges {
 	allCacheObjectIds: SofieIngestDataCacheObjId[]
 	segmentHasChanges: boolean
 	partIdsWithChanges: string[]
-	partOrderHasChanged: boolean
 	originalExternalId: string
 }
 
@@ -131,11 +130,13 @@ export class MutableIngestSegmentImpl<TSegmentPayload = unknown, TPartPayload =
 			if (beforeIndex === -1) throw new Error(`Part "${beforePartExternalId}" not found`)
 
 			this.#parts.splice(beforeIndex, 0, newPart)
+
+			this.#partOrderHasChanged = true
 		} else {
 			this.#parts.push(newPart)
-		}
 
-		this.#partOrderHasChanged = true
+			// If inserting at the end, the order hasn't changed
+		}
 
 		return newPart
 	}
@@ -234,14 +235,13 @@ export class MutableIngestSegmentImpl<TSegmentPayload = unknown, TPartPayload =
 			allCacheObjectIds.push(generator.getPartObjectId(ingestPart.externalId))
 			ingestParts.push(ingestPart)
 
-			if (part.checkAndClearChangesFlags()) {
+			if (this.#partOrderHasChanged || part.checkAndClearChangesFlags()) {
 				changedCacheObjects.push(generator.generatePartObject(segmentId, ingestPart))
 				partIdsWithChanges.push(ingestPart.externalId)
 			}
 		})
 
 		const segmentHasChanges = this.#segmentHasChanges
-		const partOrderHasChanged = this.#partOrderHasChanged
 		const originalExternalId = this.#originalExternalId
 
 		// clear flags
@@ -255,7 +255,6 @@ export class MutableIngestSegmentImpl<TSegmentPayload = unknown, TPartPayload =
 			allCacheObjectIds,
 			segmentHasChanges,
 			partIdsWithChanges,
-			partOrderHasChanged,
 			originalExternalId,
 		}
 	}

packages/job-worker/src/blueprints/ingest/__tests__/MutableIngestSegmentImpl.spec.ts

@@ -5,6 +5,7 @@ import { protectString } from '@sofie-automation/corelib/dist/protectedString'
 import { getSegmentId } from '../../../ingest/lib'
 import { MutableIngestPartImpl } from '../MutableIngestPartImpl'
 import { IngestPart, IngestSegment, SofieIngestSegment } from '@sofie-automation/blueprints-integration'
+import { SofieIngestDataCacheObjPart } from '@sofie-automation/corelib/dist/dataModel/SofieIngestDataCache'
 
 describe('MutableIngestSegmentImpl', () => {
 	function getBasicIngestSegment(): SofieIngestSegment<any> {
@@ -70,7 +71,6 @@ describe('MutableIngestSegmentImpl', () => {
 			allCacheObjectIds: ingestSegment.parts.map((p) => ingestObjectGenerator.getPartObjectId(p.externalId)),
 			segmentHasChanges: false,
 			partIdsWithChanges: [],
-			partOrderHasChanged: false,
 			originalExternalId: ingestSegment.externalId,
 		}
 	}
@@ -87,6 +87,17 @@ describe('MutableIngestSegmentImpl', () => {
 		return mutableSegment.parts.map((p) => p.externalId)
 	}
 
+	function pushAllPartsToChanges(
+		expectedChanges: MutableIngestSegmentChanges,
+		ingestSegment: SofieIngestSegment
+	): void {
+		const segmentId = getSegmentId(ingestObjectGenerator.rundownId, ingestSegment.externalId)
+		for (const ingestPart of ingestSegment.parts) {
+			expectedChanges.partIdsWithChanges.push(ingestPart.externalId)
+			expectedChanges.changedCacheObjects.push(ingestObjectGenerator.generatePartObject(segmentId, ingestPart))
+		}
+	}
+
 	test('create basic', () => {
 		const ingestSegment = getBasicIngestSegment()
 		const mutableSegment = new MutableIngestSegmentImpl(clone(ingestSegment))
@@ -114,11 +125,7 @@ describe('MutableIngestSegmentImpl', () => {
 		// check it has no changes
 		const expectedChanges = createNoChangesObject(ingestSegment)
 		expectedChanges.segmentHasChanges = true
-		const segmentId = getSegmentId(ingestObjectGenerator.rundownId, ingestSegment.externalId)
-		for (const ingestPart of ingestSegment.parts) {
-			expectedChanges.partIdsWithChanges.push(ingestPart.externalId)
-			expectedChanges.changedCacheObjects.push(ingestObjectGenerator.generatePartObject(segmentId, ingestPart))
-		}
+		pushAllPartsToChanges(expectedChanges, ingestSegment)
 		expect(mutableSegment.intoChangesInfo(ingestObjectGenerator)).toEqual(expectedChanges)
 
 		// check changes have been cleared
@@ -279,7 +286,7 @@ describe('MutableIngestSegmentImpl', () => {
 			const expectedIngestSegment = clone(ingestSegment)
 			removePartFromIngestSegment(expectedIngestSegment, 'part1')
 			const expectedChanges = createNoChangesObject(expectedIngestSegment)
-			expectedChanges.partOrderHasChanged = true
+			pushAllPartsToChanges(expectedChanges, expectedIngestSegment)
 			expect(mutableSegment.intoChangesInfo(ingestObjectGenerator)).toEqual(expectedChanges)
 
 			// try removing a second time
@@ -347,14 +354,12 @@ describe('MutableIngestSegmentImpl', () => {
 			expectedIngestSegment.parts.push({ ...newPart, rank: 3, userEditStates: {} })
 
 			const expectedChanges = createNoChangesObject(expectedIngestSegment)
-			expectedChanges.partOrderHasChanged = true
-			expectedChanges.partIdsWithChanges.push('part1')
-			expectedChanges.changedCacheObjects.push(
-				ingestObjectGenerator.generatePartObject(
-					getSegmentId(ingestObjectGenerator.rundownId, ingestSegment.externalId),
-					{ ...newPart, rank: 3, userEditStates: {} }
-				)
+			pushAllPartsToChanges(expectedChanges, expectedIngestSegment)
+			const generatedCacheObject = expectedChanges.changedCacheObjects.find(
+				(o): o is SofieIngestDataCacheObjPart => o.type === 'part' && o.data.externalId === 'part1'
 			)
+			expect(generatedCacheObject).toBeDefined()
+			expect(generatedCacheObject?.data.rank).toBe(3)
 
 			expect(mutableSegment.intoChangesInfo(ingestObjectGenerator)).toEqual(expectedChanges)
 		})
@@ -386,7 +391,6 @@ describe('MutableIngestSegmentImpl', () => {
 			expectedIngestSegment.parts.push({ ...newPart, rank: 4, userEditStates: {} })
 
 			const expectedChanges = createNoChangesObject(expectedIngestSegment)
-			expectedChanges.partOrderHasChanged = true
 			expectedChanges.partIdsWithChanges.push('partX')
 			expectedChanges.changedCacheObjects.push(
 				ingestObjectGenerator.generatePartObject(
@@ -425,8 +429,14 @@ describe('MutableIngestSegmentImpl', () => {
 			expect(mutableSegment.replacePart(newPart, 'part2')).toBeDefined()
 			expect(getPartIdOrder(mutableSegment)).toEqual(['part0', 'part1', 'partX', 'part2', 'part3'])
 
-			// Only the one should have changes
-			expect(mutableSegment.intoChangesInfo(ingestObjectGenerator).partIdsWithChanges).toEqual(['partX'])
+			// All should report changes because of their rank property
+			expect(mutableSegment.intoChangesInfo(ingestObjectGenerator).partIdsWithChanges).toEqual([
+				'part0',
+				'part1',
+				'partX',
+				'part2',
+				'part3',
+			])
 
 			// Try inserting before itself
 			expect(() => mutableSegment.replacePart(newPart, newPart.externalId)).toThrow(
@@ -470,7 +480,7 @@ describe('MutableIngestSegmentImpl', () => {
 
 			// Only the one should have changes
 			const expectedChanges = createNoChangesObject(ingestSegment)
-			expectedChanges.partOrderHasChanged = true
+			pushAllPartsToChanges(expectedChanges, ingestSegment)
 			expect(mutableSegment.intoChangesInfo(ingestObjectGenerator)).toEqual(expectedChanges)
 
 			// Try inserting before itself
@@ -511,9 +521,9 @@ describe('MutableIngestSegmentImpl', () => {
 			mutableSegment.movePartAfter('part1', 'part0')
 			expect(getPartIdOrder(mutableSegment)).toEqual(['part0', 'part1', 'part2', 'part3'])
 
-			// Only the one should have changes
+			// All the parts should be reported as having changed
 			const expectedChanges = createNoChangesObject(ingestSegment)
-			expectedChanges.partOrderHasChanged = true
+			pushAllPartsToChanges(expectedChanges, ingestSegment)
 			expect(mutableSegment.intoChangesInfo(ingestObjectGenerator)).toEqual(expectedChanges)
 
 			// Try inserting before itself