Skip to content

Commit 2d591e6

Browse files
jmgasperjmgasper
committed
Better guards for reopening challenge phases and a migrator fix
1 parent f85174d commit 2d591e6

File tree

3 files changed

+268
-4
lines changed

3 files changed

+268
-4
lines changed

‎data-migration/src/migrators/_baseMigrator.js‎

Lines changed: 155 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ const { loadData } = require('../utils/dataLoader');
22
const { MigrationManager } = require('../migrationManager')
33
const { Prisma } = require('@prisma/client');
44

5+
const INT32_MAX = 2147483647;
6+
const INT32_MIN = -2147483648;
7+
58
/**
69
* Base migrator class that all model migrators extend
710
*/
@@ -180,11 +183,20 @@ class BaseMigrator {
180183
// Allow subclasses to modify upsert data if needed
181184
const finalUpsertData = this.customizeUpsertData(upsertData, record);
182185

183-
// Perform the upsert operation
184-
const dbData = await this.performUpsert(prisma, finalUpsertData);
186+
// Perform the upsert operation with error handling
187+
const upsertResult = await this.performUpsert(prisma, finalUpsertData);
188+
189+
if (upsertResult?.skip) {
190+
skipped++;
191+
continue;
192+
}
193+
194+
const dbData = upsertResult?.data ?? upsertResult;
185195

186196
// Allow subclasses to perform post-upsert operations
187-
await this.afterUpsert(dbData, record, prisma);
197+
if (dbData) {
198+
await this.afterUpsert(dbData, record, prisma);
199+
}
188200

189201
processed++;
190202
}
@@ -446,7 +458,146 @@ class BaseMigrator {
446458
* @returns {Promise<Object>} The result of the upsert operation
447459
*/
448460
async performUpsert(prisma, upsertData) {
449-
return await prisma[this.queryName].upsert(upsertData);
461+
try {
462+
const data = await prisma[this.queryName].upsert(upsertData);
463+
return { data };
464+
} catch (error) {
465+
const resolution = await this.handleUpsertError(error, upsertData);
466+
467+
if (resolution?.retryData) {
468+
try {
469+
const data = await prisma[this.queryName].upsert(resolution.retryData);
470+
return { data };
471+
} catch (retryError) {
472+
this.manager.logger.error(`Retry upsert failed for ${this.modelName} ${this.describeRecord(upsertData)}:`, retryError);
473+
return { skip: true };
474+
}
475+
}
476+
477+
if (resolution?.skip) {
478+
return { skip: true };
479+
}
480+
481+
throw error;
482+
}
483+
}
484+
485+
/**
486+
* Handle upsert errors and decide whether to retry or skip
487+
* @param {Error} error
488+
* @param {Object} upsertData
489+
* @returns {{retryData?: Object, skip?: boolean}|null}
490+
*/
491+
async handleUpsertError(error, upsertData) {
492+
const recordLabel = this.describeRecord(upsertData);
493+
494+
if (this.isIntOverflowError(error)) {
495+
this.manager.logger.warn(`Integer overflow detected while migrating ${this.modelName} ${recordLabel}: ${error.message}`);
496+
const sanitized = this.sanitizeOverflowingInts(upsertData);
497+
498+
if (sanitized.skip) {
499+
this.manager.logger.warn(`Skipping ${this.modelName} ${recordLabel} due to integer overflow on required field.`);
500+
return { skip: true };
501+
}
502+
503+
if (sanitized.modified) {
504+
this.manager.logger.warn(`Retrying ${this.modelName} ${recordLabel} after removing overflow values.`);
505+
return { retryData: sanitized.data };
506+
}
507+
508+
this.manager.logger.warn(`Skipping ${this.modelName} ${recordLabel}; no safe way to correct integer overflow.`);
509+
return { skip: true };
510+
}
511+
512+
return null;
513+
}
514+
515+
/**
516+
* Check if the error corresponds to an integer overflow from Prisma
517+
* @param {Error} error
518+
* @returns {boolean}
519+
*/
520+
isIntOverflowError(error) {
521+
const message = error?.message || '';
522+
return message.includes('Unable to fit integer value');
523+
}
524+
525+
/**
526+
* Sanitize integer overflow values from the upsert payload
527+
* @param {Object} upsertData
528+
* @returns {{data: Object, modified: boolean, skip: boolean}}
529+
*/
530+
sanitizeOverflowingInts(upsertData) {
531+
const optionalFields = new Set(this.optionalFields || []);
532+
const requiredFields = new Set(this.requiredFields || []);
533+
let modified = false;
534+
let skip = false;
535+
const recordLabel = this.describeRecord(upsertData);
536+
537+
const pruneOverflow = (section = {}, sectionName) => {
538+
if (!section || typeof section !== 'object') {
539+
return section;
540+
}
541+
542+
const sanitized = { ...section };
543+
544+
for (const [key, value] of Object.entries(sanitized)) {
545+
if (typeof value !== 'number' || !Number.isFinite(value)) {
546+
continue;
547+
}
548+
549+
if (value > INT32_MAX || value < INT32_MIN) {
550+
if (optionalFields.has(key)) {
551+
delete sanitized[key];
552+
modified = true;
553+
this.manager.logger.warn(`${this.modelName} ${recordLabel}: dropped optional overflow field "${key}" (${value}) from ${sectionName}.`);
554+
} else if (requiredFields.has(key)) {
555+
skip = true;
556+
this.manager.logger.error(`${this.modelName} ${recordLabel}: required field "${key}" has overflow value (${value}); cannot migrate.`);
557+
} else {
558+
delete sanitized[key];
559+
modified = true;
560+
this.manager.logger.warn(`${this.modelName} ${recordLabel}: removed overflow field "${key}" (${value}) from ${sectionName}.`);
561+
}
562+
}
563+
}
564+
565+
return sanitized;
566+
};
567+
568+
return {
569+
data: {
570+
where: { ...(upsertData?.where || {}) },
571+
update: pruneOverflow(upsertData?.update, 'update payload'),
572+
create: pruneOverflow(upsertData?.create, 'create payload')
573+
},
574+
modified,
575+
skip
576+
};
577+
}
578+
579+
/**
580+
* Describe the target record for logging
581+
* @param {Object} upsertData
582+
* @returns {string}
583+
*/
584+
describeRecord(upsertData) {
585+
const idField = this.getIdField();
586+
const identifier = upsertData?.where?.[idField];
587+
588+
if (identifier !== undefined) {
589+
return `[${idField}: ${identifier}]`;
590+
}
591+
592+
if (upsertData?.where) {
593+
try {
594+
return `[where: ${JSON.stringify(upsertData.where)}]`;
595+
} catch (err) {
596+
return '[where: unable to serialize]';
597+
}
598+
}
599+
600+
return '';
450601
}
451602

452603
/**

‎src/services/ChallengePhaseService.js‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,40 @@ async function partiallyUpdateChallengePhase(currentUser, challengeId, id, data)
340340

341341
const hasActualStartDate = !_.isNil(challengePhase.actualStartDate);
342342
const hasActualEndDate = !_.isNil(challengePhase.actualEndDate);
343+
344+
if (hasActualStartDate && hasActualEndDate) {
345+
const openPhases = await prisma.challengePhase.findMany({
346+
where: {
347+
challengeId: challengePhase.challengeId,
348+
isOpen: true,
349+
},
350+
select: {
351+
id: true,
352+
phaseId: true,
353+
predecessor: true,
354+
},
355+
});
356+
357+
if (openPhases.length > 0) {
358+
const reopenedPhaseIdentifiers = new Set([String(challengePhase.id)]);
359+
if (!_.isNil(challengePhase.phaseId)) {
360+
reopenedPhaseIdentifiers.add(String(challengePhase.phaseId));
361+
}
362+
const hasDependentOpenPhase = openPhases.some((phase) => {
363+
if (!phase || _.isNil(phase.predecessor)) {
364+
return false;
365+
}
366+
return reopenedPhaseIdentifiers.has(String(phase.predecessor));
367+
});
368+
369+
if (!hasDependentOpenPhase) {
370+
throw new errors.ForbiddenError(
371+
`Cannot reopen ${phaseName} because no currently open phase depends on it`,
372+
);
373+
}
374+
}
375+
}
376+
343377
if (hasActualStartDate) {
344378
data.actualStartDate = challengePhase.actualStartDate;
345379
} else if (!("actualStartDate" in data) || _.isNil(data.actualStartDate)) {

‎test/unit/ChallengePhaseService.test.js‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,85 @@ describe('challenge phase service unit tests', () => {
193193
actualStartMs.should.be.at.most(after.getTime())
194194
})
195195

196+
it('partially update challenge phase - allows reopening when successor phase depends on it', async () => {
197+
const startDate = new Date('2025-05-01T00:00:00.000Z')
198+
const endDate = new Date('2025-05-02T00:00:00.000Z')
199+
200+
await prisma.challengePhase.update({
201+
where: { id: data.challengePhase1Id },
202+
data: {
203+
isOpen: false,
204+
actualStartDate: startDate,
205+
actualEndDate: endDate
206+
}
207+
})
208+
await prisma.challengePhase.update({
209+
where: { id: data.challengePhase2Id },
210+
data: {
211+
isOpen: true,
212+
predecessor: data.challengePhase1Id
213+
}
214+
})
215+
216+
const challengePhase = await service.partiallyUpdateChallengePhase(authUser, data.challenge.id, data.challengePhase1Id, {
217+
isOpen: true
218+
})
219+
220+
should.equal(challengePhase.isOpen, true)
221+
should.equal(challengePhase.actualEndDate, null)
222+
223+
await prisma.challengePhase.update({
224+
where: { id: data.challengePhase2Id },
225+
data: {
226+
isOpen: false
227+
}
228+
})
229+
})
230+
231+
it('partially update challenge phase - cannot reopen when open phase is not a successor', async () => {
232+
const startDate = new Date('2025-06-01T00:00:00.000Z')
233+
const endDate = new Date('2025-06-02T00:00:00.000Z')
234+
235+
await prisma.challengePhase.update({
236+
where: { id: data.challengePhase1Id },
237+
data: {
238+
isOpen: false,
239+
actualStartDate: startDate,
240+
actualEndDate: endDate
241+
}
242+
})
243+
await prisma.challengePhase.update({
244+
where: { id: data.challengePhase2Id },
245+
data: {
246+
isOpen: true,
247+
predecessor: null
248+
}
249+
})
250+
251+
try {
252+
await service.partiallyUpdateChallengePhase(authUser, data.challenge.id, data.challengePhase1Id, {
253+
isOpen: true
254+
})
255+
} catch (e) {
256+
should.equal(e.httpStatus || e.statusCode, 403)
257+
should.equal(
258+
e.message,
259+
'Cannot reopen Registration because no currently open phase depends on it'
260+
)
261+
return
262+
} finally {
263+
await prisma.challengePhase.update({
264+
where: { id: data.challengePhase2Id },
265+
data: {
266+
isOpen: false,
267+
predecessor: data.challengePhase1Id
268+
}
269+
})
270+
}
271+
272+
throw new Error('should not reach here')
273+
})
274+
196275
it('partially update challenge phase - not found', async () => {
197276
try {
198277
await service.partiallyUpdateChallengePhase(authUser, data.taskChallenge.id, data.challengePhase2Id, { name: 'updated', duration: 7200 })

0 commit comments

Comments
 (0)