diff --git a/packages/nodes-base/nodes/Schedule/GenericFunctions.ts b/packages/nodes-base/nodes/Schedule/GenericFunctions.ts index 84e55a9e393..c9502a4def3 100644 --- a/packages/nodes-base/nodes/Schedule/GenericFunctions.ts +++ b/packages/nodes-base/nodes/Schedule/GenericFunctions.ts @@ -115,7 +115,14 @@ export const toCronExpression = (interval: ScheduleInterval, nodeKey: string): C if (interval.field === 'minutes') return `${second} */${interval.minutesInterval} * * * *`; const minute = interval.triggerAtMinute ?? stableInt(nodeKey, 'minute', 0, 60); - if (interval.field === 'hours') return `${second} ${minute} */${interval.hoursInterval} * * *`; + if (interval.field === 'hours') { + const hours = interval.hoursInterval; + if (24 % hours === 0) return `${second} ${minute} */${hours} * * *`; + // `*/${hours}` fires only at clock hours divisible by ${hours}: for 18 h + // that is 00:xx and 18:xx — an 18 h gap then a 6 h gap, not a steady + // 18 h rhythm. Fire every hour; recurrenceCheck enforces elapsed time. + return `${second} ${minute} * * * *`; + } // Since Cron does not support `*/` for days or weeks, all following expressions trigger more often, but are then filtered by `recurrenceCheck` const hour = interval.triggerAtHour ?? stableInt(nodeKey, 'hour', 0, 24); diff --git a/packages/nodes-base/nodes/Schedule/test/GenericFunctions.test.ts b/packages/nodes-base/nodes/Schedule/test/GenericFunctions.test.ts index d9a75e51761..eaf9c090af2 100644 --- a/packages/nodes-base/nodes/Schedule/test/GenericFunctions.test.ts +++ b/packages/nodes-base/nodes/Schedule/test/GenericFunctions.test.ts @@ -94,6 +94,15 @@ describe('toCronExpression', () => { TEST_SEED, ); expect(result1).toEqual('56 19 */3 * * *'); + + const result2 = toCronExpression( + { + field: 'hours', + hoursInterval: 18, + }, + TEST_SEED, + ); + expect(result2).toEqual('56 19 * * * *'); }); it('should return cron expression for days interval', () => { diff --git a/packages/nodes-base/nodes/Schedule/test/ScheduleTrigger.node.test.ts b/packages/nodes-base/nodes/Schedule/test/ScheduleTrigger.node.test.ts index be94cbe020f..004ecbb351d 100644 --- a/packages/nodes-base/nodes/Schedule/test/ScheduleTrigger.node.test.ts +++ b/packages/nodes-base/nodes/Schedule/test/ScheduleTrigger.node.test.ts @@ -61,6 +61,54 @@ describe('ScheduleTrigger', () => { expect(emit).toHaveBeenCalledTimes(2); }); + it('should emit repeatedly for hourly intervals that do not divide evenly into a day', async () => { + const { emit } = await testTriggerNode(ScheduleTrigger, { + timezone, + node: { parameters: { rule: { interval: [{ field: 'hours', hoursInterval: 18 }] } } }, + workflowStaticData: { recurrenceRules: [] }, + }); + + expect(emit).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(HOUR); + expect(emit).toHaveBeenCalledTimes(1); + + jest.advanceTimersByTime(17 * HOUR); + expect(emit).toHaveBeenCalledTimes(1); + + jest.advanceTimersByTime(HOUR); + expect(emit).toHaveBeenCalledTimes(2); + + jest.advanceTimersByTime(17 * HOUR); + expect(emit).toHaveBeenCalledTimes(2); + + jest.advanceTimersByTime(HOUR); + expect(emit).toHaveBeenCalledTimes(3); + }); + + it('should emit every 18 hours when triggerAtMinute is explicitly set to 0', async () => { + const { emit } = await testTriggerNode(ScheduleTrigger, { + timezone, + node: { + parameters: { + rule: { interval: [{ field: 'hours', hoursInterval: 18, triggerAtMinute: 0 }] }, + }, + }, + workflowStaticData: { recurrenceRules: [] }, + }); + + expect(emit).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(HOUR); + expect(emit).toHaveBeenCalledTimes(1); + + jest.advanceTimersByTime(17 * HOUR); + expect(emit).toHaveBeenCalledTimes(1); + + jest.advanceTimersByTime(HOUR); + expect(emit).toHaveBeenCalledTimes(2); + }); + it('should emit on schedule defined as a cron expression', async () => { const { emit } = await testTriggerNode(ScheduleTrigger, { timezone,