[Improvement-18662][API]Aligns insertSchedule with updateSchedule (which already allows offline workflows to save)

[njnu-seafish]

Was this PR generated or assisted by AI?

NO

Purpose of the pull request

close #18662

Brief change log

The new schedule created by insertSchedule is always persisted with releaseState = OFFLINE and is not registered into the Quartz scheduler, so it cannot fire anything by itself. It only takes effect after the user explicitly goes it online via onlineScheduler.

That "go online" path (doOnlineScheduler) keeps the workflow-level check — it still throws WORKFLOW_DEFINITION_NOT_RELEASE when the workflow definition is not ONLINE. So an offline workflow's schedule can never actually be triggered; the current design guard is not bypassed.

The change only aligns insertSchedule with updateSchedule (which already allows offline workflows to save), and removes an unnecessary stricter check for the "configure a timer first, go online later" flow.

Verify this pull request

This change added tests and can be verified as follows:

• Added unit tests in SchedulerServiceTest: creating a schedule for an offline workflow succeeds (persisted as OFFLINE, no trigger), while non-existent and cross-project workflows are still rejected.
• Manually verified on a locally deployed cluster: offline workflow scheduling configuration can now be saved, and it takes effect only after the schedule is manually set online.

Pull Request Notice

Pull Request Notice

If your pull request contains incompatible change, you should also add it to docs/docs/en/guide/upgrade/incompatible.md

[det101]

Looked at the insertSchedule change against updateSchedule / doOnlineScheduler.

This is not a privilege bypass: project write permission is still checked, the created schedule stays OFFLINE, and insertSchedule does not register Quartz. Online still requires the workflow to be ONLINE. Replacing orElse(null) + checkWorkflowDefinitionValid with orElseThrow plus the projectCode check is also the right existence/ownership guard.

Optional test tightening in testInsertScheduleOfflineWorkflow: assert the captured schedule is ReleaseState.OFFLINE, and verifyNoInteractions(schedulerApi) so the “saved but not scheduled” contract stays locked. A case that onlineScheduler rejects an OFFLINE workflow would help the same way.

[SbloodyS]

Preserve subworkflow validation when activating a schedule.

The removed checkWorkflowDefinitionValid() call also verified that subworkflows were online, whereas doOnlineScheduler() only checks the parent workflow.

Please keep offline schedule creation supported, but validate subworkflow readiness when activating the schedule and add coverage for this case.

[njnu-seafish]

Looked at the insertSchedule change against updateSchedule / doOnlineScheduler.

This is not a privilege bypass: project write permission is still checked, the created schedule stays OFFLINE, and insertSchedule does not register Quartz. Online still requires the workflow to be ONLINE. Replacing orElse(null) + checkWorkflowDefinitionValid with orElseThrow plus the projectCode check is also the right existence/ownership guard.

Optional test tightening in testInsertScheduleOfflineWorkflow: assert the captured schedule is ReleaseState.OFFLINE, and verifyNoInteractions(schedulerApi) so the “saved but not scheduled” contract stays locked. A case that onlineScheduler rejects an OFFLINE workflow would help the same way.

Thanks for the review. Both suggested test tightenings are now in place in commit 00c1da0:

testInsertScheduleOfflineWorkflow now asserts the captured schedule is persisted as ReleaseState.OFFLINE, and verifies no interaction with schedulerApi, so the "saved but not scheduled" contract stays locked.
Added testOnlineSchedulerRejectsOfflineWorkflow to cover that onlineScheduler still rejects an OFFLINE workflow, leaving the schedule unmodified and Quartz unregistered.

[njnu-seafish]

Preserve subworkflow validation when activating a schedule.

The removed checkWorkflowDefinitionValid() call also verified that subworkflows were online, whereas doOnlineScheduler() only checks the parent workflow.

Please keep offline schedule creation supported, but validate subworkflow readiness when activating the schedule and add coverage for this case.

Good catch — the removed checkWorkflowDefinitionValid() did include subworkflow validation via checkSubWorkflowDefinitionValid(), and doOnlineScheduler() only checked the parent workflow. That gap is now closed in commit 484cb79:

doOnlineScheduler() now calls executorService.checkSubWorkflowDefinitionValid(workflowDefinition) after the parent ONLINE check, throwing SUB_WORKFLOW_DEFINITION_NOT_RELEASE if any sub-workflow is offline.
Offline schedule creation is still supported — insertSchedule saves the schedule as OFFLINE without registering Quartz.
Added test coverage:
testOnlineSchedulerRejectsOfflineSubWorkflow — parent ONLINE but sub-workflow OFFLINE → rejected, schedule stays OFFLINE, Quartz unregistered.
testOnlineSchedulerSucceedsWhenSubWorkflowOnline — sub-workflows ready → schedule activated and Quartz registered.
Since both onlineScheduler and onlineSchedulerByWorkflowCode go through doOnlineScheduler, the validation covers both entry points.

[SbloodyS]

LGTM