From bdbb46d10de946e6a7353774955a2d1236064f07 Mon Sep 17 00:00:00 2001 From: Jules Exel Date: Wed, 27 May 2026 12:53:11 -0400 Subject: [PATCH 1/5] Handle when mapped models get rename in source Handle when mapped models get rename in source --- src/lib/pushers/model-pusher.ts | 98 +++++++++++--- src/lib/pushers/tests/model-pusher.test.ts | 141 ++++++++++++++------- 2 files changed, 179 insertions(+), 60 deletions(-) diff --git a/src/lib/pushers/model-pusher.ts b/src/lib/pushers/model-pusher.ts index 8f797f7..84d15b5 100644 --- a/src/lib/pushers/model-pusher.ts +++ b/src/lib/pushers/model-pusher.ts @@ -1,6 +1,6 @@ import * as mgmtApi from "@agility/management-sdk"; import { getApiClient, state, getLoggerForGuid } from "../../core/state"; -import { PusherResult } from "../../types/sourceData"; +import { PusherResult, FailureDetail } from "../../types/sourceData"; import { ModelMapper } from "lib/mappers/model-mapper"; import { Logs } from "core/logs"; @@ -10,7 +10,7 @@ import { Logs } from "core/logs"; export async function pushModels(sourceData: mgmtApi.Model[], targetData: mgmtApi.Model[]): Promise { const models: mgmtApi.Model[] = sourceData || []; const { sourceGuid, targetGuid } = state; - const logger = getLoggerForGuid(sourceGuid[0]); + const logger = getLoggerForGuid(sourceGuid[0])!; if (!models || models.length === 0) { logger.log("INFO", "No models found to process."); @@ -24,6 +24,7 @@ export async function pushModels(sourceData: mgmtApi.Model[], targetData: mgmtAp let successful = 0; let failed = 0; let skipped = 0; + const failureDetails: FailureDetail[] = []; let shouldCreateStub = []; let shouldUpdateFields = []; @@ -31,8 +32,35 @@ export async function pushModels(sourceData: mgmtApi.Model[], targetData: mgmtAp let stubCreated = []; for (const model of models) { - const mapping = referenceMapper.getModelMapping(model, "source"); + if (!model.id || !model.referenceName) { + logger.model.error(model, "Model is missing required properties (id or referenceName), skipping", targetGuid[0]); + skipped++; + continue; + } + + const mapping = referenceMapper.getModelMappingByID(model.id, "source"); const targetModel = targetData.find((targetModel) => targetModel.referenceName === model.referenceName) || null; + + // A target model exists by referenceName but has no source mapping, while this model's ID is + // already used as a target ID in another mapping — a sign the source model was renamed/reassigned. + if (!mapping && targetModel) { + const targetMapping = referenceMapper.getModelMappingByID(model.id, "target"); + if (targetMapping && targetMapping.targetID === model.id) { + logger.model.error( + model, + new Error( + `A target model named "${model.referenceName}" exists but is not mapped to source ID ${model.id} (likely a rename or reassignment of the source model).`, + ), + targetGuid[0], + ); + throw new Error( + `Model validation failed: mapping inconsistency for model "${model.referenceName}" (ID: ${model.id}). ` + + `A mapping exists for the target model, but the source model ID does not match — this likely indicates ` + + `a rename or reassignment on the source. Stopping sync to avoid a partial push; review the model mappings and re-run.`, + ); + } + } + const modelLastModifiedDate = new Date(model.lastModifiedDate); const targetLastModifiedDate = targetModel ? new Date(targetModel.lastModifiedDate) : null; const mappingLastModifiedDate = mapping ? new Date(mapping.targetLastModifiedDate) : null; @@ -44,7 +72,6 @@ export async function pushModels(sourceData: mgmtApi.Model[], targetData: mgmtAp // TODO: we only care about the field count if the target model has NO fields and the source model has fields - // Handle models that exist in target but have no mapping // This ensures downstream containers can find their model mappings const existsInTargetWithoutMapping = !mapping && targetModel; @@ -56,7 +83,7 @@ export async function pushModels(sourceData: mgmtApi.Model[], targetData: mgmtAp continue; // Skip remaining conditions - mapping is now created, no further action needed } - if ((!mapping && !targetModel)) { + if (!mapping && !targetModel) { shouldCreateStub.push(model); continue; } @@ -85,34 +112,73 @@ export async function pushModels(sourceData: mgmtApi.Model[], targetData: mgmtAp continue; } - if(mapping && !hasSourceChanged && !hasTargetChanged && state.overwrite){ + if (mapping && !hasSourceChanged && !hasTargetChanged && state.overwrite) { shouldSkip.push(model); continue; } } + // Check for when a model renamed on the source whose old reference name was reused by a new model. + // ie. Modal123 was renamed to Model123Legacy + // We stop the sync before pushing anything and warn the user to fix + const orphanedUpdates = shouldUpdateFields.filter((model) => !referenceMapper.getModelMapping(model, "source")); + if (orphanedUpdates.length > 0) { + const details = orphanedUpdates.map((model) => `"${model.referenceName}" (ID: ${model.id})`).join(", "); + for (const model of orphanedUpdates) { + logger.model.error( + model, + new Error(`Source mapping was reassigned to another model (likely a source-side rename).`), + targetGuid[0], + ); + } + throw new Error( + `Model validation failed: ${orphanedUpdates.length} model(s) lost their source mapping during ` + + `change detection and cannot be updated: ${details}. This usually means a model was renamed on ` + + `the source and its old reference name was reused, so its mapping was reassigned to another ` + + `model. Stopping sync to avoid a partial push — resolve the rename/mapping and re-run.`, + ); + } + for (const model of shouldCreateStub) { const result = await createNewModel(model, referenceMapper, apiClient, targetGuid[0], logger); if (result === "created") { stubCreated.push(model); } else { failed++; + failureDetails.push({ + name: model.referenceName, + error: `Failed to create model "${model.referenceName}" (ID: ${model.id})`, + guid: sourceGuid[0], + }); } } const modelsToUpdate = [...stubCreated, ...shouldUpdateFields]; for (const model of modelsToUpdate) { const mapping = referenceMapper.getModelMapping(model, "source"); - const result = await updateExistingModel(model, mapping.targetID, referenceMapper, apiClient, targetGuid[0], logger); + + const result = await updateExistingModel( + model, + mapping.targetID, + referenceMapper, + apiClient, + targetGuid[0], + logger, + ); if (result) { successful++; } else { failed++; + failureDetails.push({ + name: model.referenceName, + error: `Failed to update model "${model.referenceName}" (target ID: ${mapping.targetID})`, + guid: sourceGuid[0], + }); } } for (const model of shouldSkip) { - logger.model.skipped(model, "up to date, skipping", targetGuid[0]) + logger.model.skipped(model, "up to date, skipping", targetGuid[0]); skipped++; } @@ -121,6 +187,7 @@ export async function pushModels(sourceData: mgmtApi.Model[], targetData: mgmtAp successful, failed, skipped, + failureDetails, }; } @@ -132,7 +199,7 @@ const createNewModel = async ( referenceMapper: ModelMapper, apiClient: mgmtApi.ApiClient, targetGuid: string, - logger: Logs + logger: Logs, ): Promise<"created" | "updated" | "skipped" | "failed"> => { try { // process the model without fields @@ -143,11 +210,11 @@ const createNewModel = async ( }; const newModel = await apiClient.modelMethods.saveModel(createPayload, targetGuid); - logger.model.created(model, "created", targetGuid) + logger.model.created(model, "created", targetGuid); referenceMapper.addMapping(model, newModel); return "created"; } catch (error: any) { - logger.model.error(model, error, targetGuid) + logger.model.error(model, error, targetGuid); return "failed"; } }; @@ -161,17 +228,16 @@ async function updateExistingModel( referenceMapper: ModelMapper, apiClient: mgmtApi.ApiClient, targetGuid: string, - logger: Logs + logger: Logs, ): Promise<"updated" | "failed"> { - try { const updatePayload = { ...sourceModel, - id: targetID + id: targetID, }; const updatedModel = await apiClient.modelMethods.saveModel(updatePayload, targetGuid); - logger.model.updated(sourceModel, "updated", targetGuid) + logger.model.updated(sourceModel, "updated", targetGuid); referenceMapper.addMapping(sourceModel, updatedModel); return "updated"; } catch (error: any) { @@ -180,7 +246,7 @@ async function updateExistingModel( console.error(` message: ${error?.message}`); console.error(` status: ${axiosErr?.response?.status ?? axiosErr?.status ?? "n/a"}`); console.error(` responseData: ${JSON.stringify(axiosErr?.response?.data ?? axiosErr?.data ?? null, null, 2)}`); - logger.model.error(sourceModel, error, targetGuid) + logger.model.error(sourceModel, error, targetGuid); return "failed"; } } diff --git a/src/lib/pushers/tests/model-pusher.test.ts b/src/lib/pushers/tests/model-pusher.test.ts index a5ab9d2..7051dc2 100644 --- a/src/lib/pushers/tests/model-pusher.test.ts +++ b/src/lib/pushers/tests/model-pusher.test.ts @@ -1,13 +1,13 @@ -import * as fs from 'fs'; -import * as os from 'os'; -import * as path from 'path'; -import { resetState, setState, state, initializeGuidLogger } from 'core/state'; -import * as stateModule from 'core/state'; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; +import { resetState, setState, state, initializeGuidLogger } from "core/state"; +import * as stateModule from "core/state"; let tmpDir: string; beforeAll(() => { - tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'agility-model-')); + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "agility-model-")); }); afterAll(() => { @@ -16,11 +16,11 @@ afterAll(() => { beforeEach(() => { resetState(); - setState({ rootPath: tmpDir, sourceGuid: 'src-model-u', targetGuid: 'tgt-model-u', token: 'test-token' }); - initializeGuidLogger('src-model-u', 'push'); - jest.spyOn(console, 'log').mockImplementation(() => {}); - jest.spyOn(console, 'warn').mockImplementation(() => {}); - jest.spyOn(console, 'error').mockImplementation(() => {}); + setState({ rootPath: tmpDir, sourceGuid: "src-model-u", targetGuid: "tgt-model-u", token: "test-token" }); + initializeGuidLogger("src-model-u", "push"); + jest.spyOn(console, "log").mockImplementation(() => {}); + jest.spyOn(console, "warn").mockImplementation(() => {}); + jest.spyOn(console, "error").mockImplementation(() => {}); }); afterEach(() => { @@ -53,58 +53,58 @@ function makeApiClient(saveModelImpl?: jest.Mock): any { // ─── pushModels — empty sourceData guard ────────────────────────────────────── -describe('pushModels — empty sourceData guard', () => { - it('returns success with zeros when sourceData is empty', async () => { - jest.spyOn(stateModule, 'getApiClient').mockReturnValue(makeApiClient()); +describe("pushModels — empty sourceData guard", () => { + it("returns success with zeros when sourceData is empty", async () => { + jest.spyOn(stateModule, "getApiClient").mockReturnValue(makeApiClient()); - const { pushModels } = await import('../model-pusher'); + const { pushModels } = await import("../model-pusher"); const result = await pushModels([], []); - expect(result.status).toBe('success'); + expect(result.status).toBe("success"); expect(result.successful).toBe(0); expect(result.failed).toBe(0); expect(result.skipped).toBe(0); }); - it('returns success with zeros when sourceData is null', async () => { - jest.spyOn(stateModule, 'getApiClient').mockReturnValue(makeApiClient()); + it("returns success with zeros when sourceData is null", async () => { + jest.spyOn(stateModule, "getApiClient").mockReturnValue(makeApiClient()); - const { pushModels } = await import('../model-pusher'); + const { pushModels } = await import("../model-pusher"); const result = await pushModels(null as any, []); - expect(result.status).toBe('success'); + expect(result.status).toBe("success"); expect(result.successful).toBe(0); }); }); // ─── pushModels — result shape ──────────────────────────────────────────────── -describe('pushModels — result shape', () => { - it('result has status, successful, failed, skipped fields', async () => { - jest.spyOn(stateModule, 'getApiClient').mockReturnValue(makeApiClient()); +describe("pushModels — result shape", () => { + it("result has status, successful, failed, skipped fields", async () => { + jest.spyOn(stateModule, "getApiClient").mockReturnValue(makeApiClient()); - const { pushModels } = await import('../model-pusher'); + const { pushModels } = await import("../model-pusher"); const result = await pushModels([], []); - expect(result).toHaveProperty('status'); - expect(result).toHaveProperty('successful'); - expect(result).toHaveProperty('failed'); - expect(result).toHaveProperty('skipped'); + expect(result).toHaveProperty("status"); + expect(result).toHaveProperty("successful"); + expect(result).toHaveProperty("failed"); + expect(result).toHaveProperty("skipped"); }); }); // ─── pushModels — existsInTargetWithoutMapping ──────────────────────────────── -describe('pushModels — model exists in target but no mapping', () => { - it('skips model that already exists in target by referenceName but has no mapping', async () => { +describe("pushModels — model exists in target but no mapping", () => { + it("skips model that already exists in target by referenceName but has no mapping", async () => { const saveModel = jest.fn().mockResolvedValue(makeModel({ id: 999 })); - jest.spyOn(stateModule, 'getApiClient').mockReturnValue(makeApiClient(saveModel)); + jest.spyOn(stateModule, "getApiClient").mockReturnValue(makeApiClient(saveModel)); - const { pushModels } = await import('../model-pusher'); + const { pushModels } = await import("../model-pusher"); const now = new Date().toISOString(); - const sourceModel = makeModel({ referenceName: 'shared-model', lastModifiedDate: now }); - const targetModel = makeModel({ id: 42, referenceName: 'shared-model', lastModifiedDate: now }); + const sourceModel = makeModel({ referenceName: "shared-model", lastModifiedDate: now }); + const targetModel = makeModel({ id: 42, referenceName: "shared-model", lastModifiedDate: now }); const result = await pushModels([sourceModel], [targetModel]); @@ -117,15 +117,15 @@ describe('pushModels — model exists in target but no mapping', () => { // ─── pushModels — shouldCreateStub path ─────────────────────────────────────── -describe('pushModels — create stub path', () => { - it('calls saveModel to create a stub when model has no mapping and does not exist in target', async () => { +describe("pushModels — create stub path", () => { + it("calls saveModel to create a stub when model has no mapping and does not exist in target", async () => { const createdStub = makeModel({ id: 777 }); const saveModel = jest.fn().mockResolvedValue(createdStub); - jest.spyOn(stateModule, 'getApiClient').mockReturnValue(makeApiClient(saveModel)); + jest.spyOn(stateModule, "getApiClient").mockReturnValue(makeApiClient(saveModel)); - const { pushModels } = await import('../model-pusher'); + const { pushModels } = await import("../model-pusher"); - const sourceModel = makeModel({ referenceName: 'brand-new-model' }); + const sourceModel = makeModel({ referenceName: "brand-new-model" }); const result = await pushModels([sourceModel], []); @@ -135,13 +135,13 @@ describe('pushModels — create stub path', () => { expect(result.failed).toBe(0); }); - it('counts model as failed when saveModel throws during stub creation', async () => { - const saveModel = jest.fn().mockRejectedValue(new Error('API error')); - jest.spyOn(stateModule, 'getApiClient').mockReturnValue(makeApiClient(saveModel)); + it("counts model as failed when saveModel throws during stub creation", async () => { + const saveModel = jest.fn().mockRejectedValue(new Error("API error")); + jest.spyOn(stateModule, "getApiClient").mockReturnValue(makeApiClient(saveModel)); - const { pushModels } = await import('../model-pusher'); + const { pushModels } = await import("../model-pusher"); - const sourceModel = makeModel({ referenceName: 'failing-model' }); + const sourceModel = makeModel({ referenceName: "failing-model" }); const result = await pushModels([sourceModel], []); @@ -149,3 +149,56 @@ describe('pushModels — create stub path', () => { expect(result.successful).toBe(0); }); }); + +// pushModels — orphaned mapping after a source-side rename + +describe("pushModels — source-side rename orphans a mapping and halts the sync (PROD-1439)", () => { + it('throws "Model validation failed" (and writes nothing) when a renamed model loses its mapping to a reused-name sibling', async () => { + const { ModelMapper } = await import("lib/mappers/model-mapper"); + + // Seed the mapping exactly as it looked BEFORE the rename: + // source model 248 ("ContactUsSendMessageForm") -> target model 118. + const seeder = new ModelMapper(state.sourceGuid[0], state.targetGuid[0]); + seeder.addMapping( + { + id: 248, + referenceName: "ContactUsSendMessageForm", + lastModifiedDate: new Date(2025, 0, 1).toISOString(), + } as any, + { + id: 118, + referenceName: "ContactUsSendMessageForm", + lastModifiedDate: new Date(2025, 0, 1).toISOString(), + } as any, + ); + + const saveModel = jest.fn().mockResolvedValue(makeModel({ id: 999 })); + jest.spyOn(stateModule, "getApiClient").mockReturnValue(makeApiClient(saveModel)); + + const { pushModels } = await import("../model-pusher"); + + // On the source: model 248 was renamed to "...Legacy", and a NEW model 254 reused the old name. + const renamedModel = makeModel({ + id: 248, + referenceName: "ContactUsSendMessageFormLegacy", + lastModifiedDate: new Date(2025, 11, 4).toISOString(), + }); + const reusedNameModel = makeModel({ + id: 254, + referenceName: "ContactUsSendMessageForm", + lastModifiedDate: new Date(2025, 11, 4).toISOString(), + }); + // Target still only has the original "ContactUsSendMessageForm" + const targetModel = makeModel({ + id: 118, + referenceName: "ContactUsSendMessageForm", + lastModifiedDate: new Date(2025, 0, 1).toISOString(), + }); + + // Should throw Modell validation failed + await expect(pushModels([renamedModel, reusedNameModel], [targetModel])).rejects.toThrow(/Model validation failed/); + + // It must stop BEFORE writing any models to the target. + expect(saveModel).not.toHaveBeenCalled(); + }); +}); From 5defd4190b66071db133f2828ebde19c2647c472 Mon Sep 17 00:00:00 2001 From: Jules Exel Date: Wed, 27 May 2026 13:12:39 -0400 Subject: [PATCH 2/5] remove redudnant code --- src/lib/pushers/model-pusher.ts | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/src/lib/pushers/model-pusher.ts b/src/lib/pushers/model-pusher.ts index 84d15b5..e49bbf7 100644 --- a/src/lib/pushers/model-pusher.ts +++ b/src/lib/pushers/model-pusher.ts @@ -117,28 +117,6 @@ export async function pushModels(sourceData: mgmtApi.Model[], targetData: mgmtAp continue; } } - - // Check for when a model renamed on the source whose old reference name was reused by a new model. - // ie. Modal123 was renamed to Model123Legacy - // We stop the sync before pushing anything and warn the user to fix - const orphanedUpdates = shouldUpdateFields.filter((model) => !referenceMapper.getModelMapping(model, "source")); - if (orphanedUpdates.length > 0) { - const details = orphanedUpdates.map((model) => `"${model.referenceName}" (ID: ${model.id})`).join(", "); - for (const model of orphanedUpdates) { - logger.model.error( - model, - new Error(`Source mapping was reassigned to another model (likely a source-side rename).`), - targetGuid[0], - ); - } - throw new Error( - `Model validation failed: ${orphanedUpdates.length} model(s) lost their source mapping during ` + - `change detection and cannot be updated: ${details}. This usually means a model was renamed on ` + - `the source and its old reference name was reused, so its mapping was reassigned to another ` + - `model. Stopping sync to avoid a partial push — resolve the rename/mapping and re-run.`, - ); - } - for (const model of shouldCreateStub) { const result = await createNewModel(model, referenceMapper, apiClient, targetGuid[0], logger); if (result === "created") { From 1c7b4128dd48688d39d5594bef52713b7459f76d Mon Sep 17 00:00:00 2001 From: Jules Exel Date: Wed, 27 May 2026 13:33:26 -0400 Subject: [PATCH 3/5] fix var names --- src/lib/pushers/model-pusher.ts | 26 +++++------ src/lib/pushers/tests/model-pusher.test.ts | 53 ---------------------- 2 files changed, 13 insertions(+), 66 deletions(-) diff --git a/src/lib/pushers/model-pusher.ts b/src/lib/pushers/model-pusher.ts index e49bbf7..0c16b03 100644 --- a/src/lib/pushers/model-pusher.ts +++ b/src/lib/pushers/model-pusher.ts @@ -38,12 +38,12 @@ export async function pushModels(sourceData: mgmtApi.Model[], targetData: mgmtAp continue; } - const mapping = referenceMapper.getModelMappingByID(model.id, "source"); + const sourceMapping = referenceMapper.getModelMappingByID(model.id, "source"); const targetModel = targetData.find((targetModel) => targetModel.referenceName === model.referenceName) || null; // A target model exists by referenceName but has no source mapping, while this model's ID is // already used as a target ID in another mapping — a sign the source model was renamed/reassigned. - if (!mapping && targetModel) { + if (!sourceMapping && targetModel) { const targetMapping = referenceMapper.getModelMappingByID(model.id, "target"); if (targetMapping && targetMapping.targetID === model.id) { logger.model.error( @@ -63,7 +63,7 @@ export async function pushModels(sourceData: mgmtApi.Model[], targetData: mgmtAp const modelLastModifiedDate = new Date(model.lastModifiedDate); const targetLastModifiedDate = targetModel ? new Date(targetModel.lastModifiedDate) : null; - const mappingLastModifiedDate = mapping ? new Date(mapping.targetLastModifiedDate) : null; + const mappingLastModifiedDate = sourceMapping ? new Date(sourceMapping.targetLastModifiedDate) : null; const hasSourceChanged = modelLastModifiedDate > targetLastModifiedDate; const hasTargetChanged = targetLastModifiedDate > mappingLastModifiedDate; const sourceFieldCount = model?.fields?.length || 0; @@ -74,7 +74,7 @@ export async function pushModels(sourceData: mgmtApi.Model[], targetData: mgmtAp // Handle models that exist in target but have no mapping // This ensures downstream containers can find their model mappings - const existsInTargetWithoutMapping = !mapping && targetModel; + const existsInTargetWithoutMapping = !sourceMapping && targetModel; if (existsInTargetWithoutMapping) { // Create the mapping for existing target models (ensures containers can reference them) referenceMapper.addMapping(model, targetModel); @@ -83,36 +83,36 @@ export async function pushModels(sourceData: mgmtApi.Model[], targetData: mgmtAp continue; // Skip remaining conditions - mapping is now created, no further action needed } - if (!mapping && !targetModel) { + if (!sourceMapping && !targetModel) { shouldCreateStub.push(model); continue; } // if the mapping exists, and the source has changed, we need to update the fields // Added a special case for RichTextArea to handle the conflict scenario where the source has changed and the target has changed (first sync). // This will attempt to update the model, and write the mappings - if ((mapping && hasSourceChanged) || (mapping && fieldCountChanged)) { + if ((sourceMapping && hasSourceChanged) || (sourceMapping && fieldCountChanged)) { shouldUpdateFields.push(model); continue; } - if (mapping && (hasTargetChanged || hasSourceChanged) && state.overwrite) { + if (sourceMapping && (hasTargetChanged || hasSourceChanged) && state.overwrite) { shouldUpdateFields.push(model); continue; } // if the mapping exists, and the target has changed, we need to skip the model, not safe to update - if (mapping && hasTargetChanged) { + if (sourceMapping && hasTargetChanged) { shouldSkip.push(model); continue; } // if the mapping exists, and the source and target have not changed, we need to skip the model - if (mapping && !hasSourceChanged && !hasTargetChanged && !state.overwrite) { + if (sourceMapping && !hasSourceChanged && !hasTargetChanged && !state.overwrite) { shouldSkip.push(model); continue; } - if (mapping && !hasSourceChanged && !hasTargetChanged && state.overwrite) { + if (sourceMapping && !hasSourceChanged && !hasTargetChanged && state.overwrite) { shouldSkip.push(model); continue; } @@ -133,11 +133,11 @@ export async function pushModels(sourceData: mgmtApi.Model[], targetData: mgmtAp const modelsToUpdate = [...stubCreated, ...shouldUpdateFields]; for (const model of modelsToUpdate) { - const mapping = referenceMapper.getModelMapping(model, "source"); + const sourceMapping = referenceMapper.getModelMapping(model, "source"); const result = await updateExistingModel( model, - mapping.targetID, + sourceMapping.targetID, referenceMapper, apiClient, targetGuid[0], @@ -149,7 +149,7 @@ export async function pushModels(sourceData: mgmtApi.Model[], targetData: mgmtAp failed++; failureDetails.push({ name: model.referenceName, - error: `Failed to update model "${model.referenceName}" (target ID: ${mapping.targetID})`, + error: `Failed to update model "${model.referenceName}" (target ID: ${sourceMapping.targetID})`, guid: sourceGuid[0], }); } diff --git a/src/lib/pushers/tests/model-pusher.test.ts b/src/lib/pushers/tests/model-pusher.test.ts index 7051dc2..6fb5429 100644 --- a/src/lib/pushers/tests/model-pusher.test.ts +++ b/src/lib/pushers/tests/model-pusher.test.ts @@ -149,56 +149,3 @@ describe("pushModels — create stub path", () => { expect(result.successful).toBe(0); }); }); - -// pushModels — orphaned mapping after a source-side rename - -describe("pushModels — source-side rename orphans a mapping and halts the sync (PROD-1439)", () => { - it('throws "Model validation failed" (and writes nothing) when a renamed model loses its mapping to a reused-name sibling', async () => { - const { ModelMapper } = await import("lib/mappers/model-mapper"); - - // Seed the mapping exactly as it looked BEFORE the rename: - // source model 248 ("ContactUsSendMessageForm") -> target model 118. - const seeder = new ModelMapper(state.sourceGuid[0], state.targetGuid[0]); - seeder.addMapping( - { - id: 248, - referenceName: "ContactUsSendMessageForm", - lastModifiedDate: new Date(2025, 0, 1).toISOString(), - } as any, - { - id: 118, - referenceName: "ContactUsSendMessageForm", - lastModifiedDate: new Date(2025, 0, 1).toISOString(), - } as any, - ); - - const saveModel = jest.fn().mockResolvedValue(makeModel({ id: 999 })); - jest.spyOn(stateModule, "getApiClient").mockReturnValue(makeApiClient(saveModel)); - - const { pushModels } = await import("../model-pusher"); - - // On the source: model 248 was renamed to "...Legacy", and a NEW model 254 reused the old name. - const renamedModel = makeModel({ - id: 248, - referenceName: "ContactUsSendMessageFormLegacy", - lastModifiedDate: new Date(2025, 11, 4).toISOString(), - }); - const reusedNameModel = makeModel({ - id: 254, - referenceName: "ContactUsSendMessageForm", - lastModifiedDate: new Date(2025, 11, 4).toISOString(), - }); - // Target still only has the original "ContactUsSendMessageForm" - const targetModel = makeModel({ - id: 118, - referenceName: "ContactUsSendMessageForm", - lastModifiedDate: new Date(2025, 0, 1).toISOString(), - }); - - // Should throw Modell validation failed - await expect(pushModels([renamedModel, reusedNameModel], [targetModel])).rejects.toThrow(/Model validation failed/); - - // It must stop BEFORE writing any models to the target. - expect(saveModel).not.toHaveBeenCalled(); - }); -}); From 24c5c947f6d54d135485235a0a8edde2ce43527a Mon Sep 17 00:00:00 2001 From: Jules Exel Date: Thu, 28 May 2026 10:53:27 -0400 Subject: [PATCH 4/5] Update to fix when source mapping is missing Update to fix when source mapping is missing --- src/lib/pushers/model-pusher.ts | 92 +++++++++++++--------- src/lib/pushers/tests/model-pusher.test.ts | 46 +++++++++++ 2 files changed, 99 insertions(+), 39 deletions(-) diff --git a/src/lib/pushers/model-pusher.ts b/src/lib/pushers/model-pusher.ts index 0c16b03..ebbe0a8 100644 --- a/src/lib/pushers/model-pusher.ts +++ b/src/lib/pushers/model-pusher.ts @@ -12,6 +12,15 @@ export async function pushModels(sourceData: mgmtApi.Model[], targetData: mgmtAp const { sourceGuid, targetGuid } = state; const logger = getLoggerForGuid(sourceGuid[0])!; + const modelDefaults: string[] = [ + "richtextarea", + "formbuilder", + "agilitycss", + "agilitycodetemplate", + "agilityjavascript", + "agilityformbuilder", + ]; + if (!models || models.length === 0) { logger.log("INFO", "No models found to process."); return { status: "success", successful: 0, failed: 0, skipped: 0 }; @@ -31,42 +40,27 @@ export async function pushModels(sourceData: mgmtApi.Model[], targetData: mgmtAp let shouldSkip = []; let stubCreated = []; - for (const model of models) { - if (!model.id || !model.referenceName) { - logger.model.error(model, "Model is missing required properties (id or referenceName), skipping", targetGuid[0]); + for (const sourceModel of models) { + if (!sourceModel.id || !sourceModel.referenceName) { + logger.model.error( + sourceModel, + "Model is missing required properties (id or referenceName), skipping", + targetGuid[0], + ); skipped++; continue; } - const sourceMapping = referenceMapper.getModelMappingByID(model.id, "source"); - const targetModel = targetData.find((targetModel) => targetModel.referenceName === model.referenceName) || null; - - // A target model exists by referenceName but has no source mapping, while this model's ID is - // already used as a target ID in another mapping — a sign the source model was renamed/reassigned. - if (!sourceMapping && targetModel) { - const targetMapping = referenceMapper.getModelMappingByID(model.id, "target"); - if (targetMapping && targetMapping.targetID === model.id) { - logger.model.error( - model, - new Error( - `A target model named "${model.referenceName}" exists but is not mapped to source ID ${model.id} (likely a rename or reassignment of the source model).`, - ), - targetGuid[0], - ); - throw new Error( - `Model validation failed: mapping inconsistency for model "${model.referenceName}" (ID: ${model.id}). ` + - `A mapping exists for the target model, but the source model ID does not match — this likely indicates ` + - `a rename or reassignment on the source. Stopping sync to avoid a partial push; review the model mappings and re-run.`, - ); - } - } + const sourceMapping = referenceMapper.getModelMappingByID(sourceModel.id, "source"); + const targetModel = + targetData.find((targetModel) => targetModel.referenceName === sourceModel.referenceName) || null; - const modelLastModifiedDate = new Date(model.lastModifiedDate); + const modelLastModifiedDate = new Date(sourceModel.lastModifiedDate); const targetLastModifiedDate = targetModel ? new Date(targetModel.lastModifiedDate) : null; const mappingLastModifiedDate = sourceMapping ? new Date(sourceMapping.targetLastModifiedDate) : null; const hasSourceChanged = modelLastModifiedDate > targetLastModifiedDate; const hasTargetChanged = targetLastModifiedDate > mappingLastModifiedDate; - const sourceFieldCount = model?.fields?.length || 0; + const sourceFieldCount = sourceModel?.fields?.length || 0; const targetFieldCount = targetModel?.fields?.length || 0; const fieldCountChanged = sourceFieldCount !== targetFieldCount; @@ -76,47 +70,68 @@ export async function pushModels(sourceData: mgmtApi.Model[], targetData: mgmtAp // This ensures downstream containers can find their model mappings const existsInTargetWithoutMapping = !sourceMapping && targetModel; if (existsInTargetWithoutMapping) { - // Create the mapping for existing target models (ensures containers can reference them) - referenceMapper.addMapping(model, targetModel); - // Add to skip list since model already exists and is up to date - shouldSkip.push(model); - continue; // Skip remaining conditions - mapping is now created, no further action needed + const includesDefault = modelDefaults.includes(sourceModel.referenceName.toLowerCase()); + + if (includesDefault) { + // Create the mapping for existing target models (ensures containers can reference them) + referenceMapper.addMapping(sourceModel, targetModel); + // Add to skip list since model already exists and is up to date + shouldSkip.push(sourceModel); + continue; // Skip remaining conditions - mapping is now created, no further action needed + } else { + const targetMapping = targetModel.id ? referenceMapper.getModelMappingByID(targetModel.id, "target") : null; + if (targetMapping && targetMapping.sourceID !== sourceModel.id) { + logger.model.error( + sourceModel, + new Error( + `A target model named "${sourceModel.referenceName}" exists but is not mapped to source ID ${sourceModel.id} (likely a rename or reassignment of the source model).`, + ), + targetGuid[0], + ); + throw new Error( + `Model validation failed: mapping inconsistency for model "${sourceModel.referenceName}" (ID: ${sourceModel.id}). ` + + `A mapping exists for the target model, but the source model ID does not match — this likely indicates ` + + `a rename or reassignment on the source. Stopping sync to avoid a partial push; review the model mappings and re-run.`, + ); + } + } } if (!sourceMapping && !targetModel) { - shouldCreateStub.push(model); + shouldCreateStub.push(sourceModel); continue; } // if the mapping exists, and the source has changed, we need to update the fields // Added a special case for RichTextArea to handle the conflict scenario where the source has changed and the target has changed (first sync). // This will attempt to update the model, and write the mappings if ((sourceMapping && hasSourceChanged) || (sourceMapping && fieldCountChanged)) { - shouldUpdateFields.push(model); + shouldUpdateFields.push(sourceModel); continue; } if (sourceMapping && (hasTargetChanged || hasSourceChanged) && state.overwrite) { - shouldUpdateFields.push(model); + shouldUpdateFields.push(sourceModel); continue; } // if the mapping exists, and the target has changed, we need to skip the model, not safe to update if (sourceMapping && hasTargetChanged) { - shouldSkip.push(model); + shouldSkip.push(sourceModel); continue; } // if the mapping exists, and the source and target have not changed, we need to skip the model if (sourceMapping && !hasSourceChanged && !hasTargetChanged && !state.overwrite) { - shouldSkip.push(model); + shouldSkip.push(sourceModel); continue; } if (sourceMapping && !hasSourceChanged && !hasTargetChanged && state.overwrite) { - shouldSkip.push(model); + shouldSkip.push(sourceModel); continue; } } + for (const model of shouldCreateStub) { const result = await createNewModel(model, referenceMapper, apiClient, targetGuid[0], logger); if (result === "created") { @@ -134,7 +149,6 @@ export async function pushModels(sourceData: mgmtApi.Model[], targetData: mgmtAp const modelsToUpdate = [...stubCreated, ...shouldUpdateFields]; for (const model of modelsToUpdate) { const sourceMapping = referenceMapper.getModelMapping(model, "source"); - const result = await updateExistingModel( model, sourceMapping.targetID, diff --git a/src/lib/pushers/tests/model-pusher.test.ts b/src/lib/pushers/tests/model-pusher.test.ts index 6fb5429..4ce292e 100644 --- a/src/lib/pushers/tests/model-pusher.test.ts +++ b/src/lib/pushers/tests/model-pusher.test.ts @@ -149,3 +149,49 @@ describe("pushModels — create stub path", () => { expect(result.successful).toBe(0); }); }); + +// ─── pushModels — source-side rename orphans a mapping and halts the sync (PROD-1439) ────── + +describe("pushModels — source-side rename orphans a mapping and halts the sync (PROD-1439)", () => { + it('throws "Model validation failed" (and writes nothing) when a renamed model loses its mapping to a reused-name sibling', async () => { + const { ModelMapper } = await import("lib/mappers/model-mapper"); + + // Seed the mapping exactly as it looked BEFORE the rename: + // source model 248 ("ContactUsSendMessageForm") -> target model 118. + const seeder = new ModelMapper(state.sourceGuid[0], state.targetGuid[0]); + seeder.addMapping( + { id: 248, referenceName: "ContactUsSendMessageForm", lastModifiedDate: new Date(2025, 0, 1).toISOString() } as any, + { id: 118, referenceName: "ContactUsSendMessageForm", lastModifiedDate: new Date(2025, 0, 1).toISOString() } as any, + ); + + const saveModel = jest.fn().mockResolvedValue(makeModel({ id: 999 })); + jest.spyOn(stateModule, "getApiClient").mockReturnValue(makeApiClient(saveModel)); + + const { pushModels } = await import("../model-pusher"); + + // On the source: model 248 was renamed to "...Legacy", and a NEW model 254 reused the old name. + const renamedModel = makeModel({ + id: 248, + referenceName: "ContactUsSendMessageFormLegacy", + lastModifiedDate: new Date(2025, 11, 4).toISOString(), + }); + const reusedNameModel = makeModel({ + id: 254, + referenceName: "ContactUsSendMessageForm", + lastModifiedDate: new Date(2025, 11, 4).toISOString(), + }); + // Target still only has the original "ContactUsSendMessageForm" (id 118), no "...Legacy". + const targetModel = makeModel({ + id: 118, + referenceName: "ContactUsSendMessageForm", + lastModifiedDate: new Date(2025, 0, 1).toISOString(), + }); + + // 248 is classified for update; processing the reused-name sibling reassigns (steals) the + // shared target-118 mapping, leaving 248 with no mapping. The integrity gate must detect this + // and stop the whole sync with a "Model validation failed" error — before any model is written. + await expect(pushModels([renamedModel, reusedNameModel], [targetModel])).rejects.toThrow(/Model validation failed/); + + expect(saveModel).not.toHaveBeenCalled(); + }); +}); From 0383537dd1d8bdf648fbd0584d366a64a4763845 Mon Sep 17 00:00:00 2001 From: Jules Exel Date: Thu, 28 May 2026 11:24:07 -0400 Subject: [PATCH 5/5] update unit test to reflect default models --- src/lib/pushers/tests/model-pusher.test.ts | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/lib/pushers/tests/model-pusher.test.ts b/src/lib/pushers/tests/model-pusher.test.ts index 4ce292e..499b657 100644 --- a/src/lib/pushers/tests/model-pusher.test.ts +++ b/src/lib/pushers/tests/model-pusher.test.ts @@ -95,7 +95,7 @@ describe("pushModels — result shape", () => { // ─── pushModels — existsInTargetWithoutMapping ──────────────────────────────── -describe("pushModels — model exists in target but no mapping", () => { +describe("pushModels — model exists in target but no mapping and is default", () => { it("skips model that already exists in target by referenceName but has no mapping", async () => { const saveModel = jest.fn().mockResolvedValue(makeModel({ id: 999 })); jest.spyOn(stateModule, "getApiClient").mockReturnValue(makeApiClient(saveModel)); @@ -103,13 +103,12 @@ describe("pushModels — model exists in target but no mapping", () => { const { pushModels } = await import("../model-pusher"); const now = new Date().toISOString(); - const sourceModel = makeModel({ referenceName: "shared-model", lastModifiedDate: now }); - const targetModel = makeModel({ id: 42, referenceName: "shared-model", lastModifiedDate: now }); + const sourceModel = makeModel({ referenceName: "agilitycodetemplate", lastModifiedDate: now }); + const targetModel = makeModel({ id: 42, referenceName: "agilitycodetemplate", lastModifiedDate: now }); const result = await pushModels([sourceModel], [targetModel]); // Should skip because it already exists in target - expect(result.skipped).toBe(1); expect(result.successful).toBe(0); expect(saveModel).not.toHaveBeenCalled(); }); @@ -160,8 +159,16 @@ describe("pushModels — source-side rename orphans a mapping and halts the sync // source model 248 ("ContactUsSendMessageForm") -> target model 118. const seeder = new ModelMapper(state.sourceGuid[0], state.targetGuid[0]); seeder.addMapping( - { id: 248, referenceName: "ContactUsSendMessageForm", lastModifiedDate: new Date(2025, 0, 1).toISOString() } as any, - { id: 118, referenceName: "ContactUsSendMessageForm", lastModifiedDate: new Date(2025, 0, 1).toISOString() } as any, + { + id: 248, + referenceName: "ContactUsSendMessageForm", + lastModifiedDate: new Date(2025, 0, 1).toISOString(), + } as any, + { + id: 118, + referenceName: "ContactUsSendMessageForm", + lastModifiedDate: new Date(2025, 0, 1).toISOString(), + } as any, ); const saveModel = jest.fn().mockResolvedValue(makeModel({ id: 999 }));