Skip to content

[Detail Bug] Scheduler: Toggling On Demand → Schedule overwrites an existing cron with the period default #33067

Description

@detail-app

Detail Bug Report

https://app.detail.dev/org_3377c26d-da48-4ccd-b83a-22c542f4fe83/bugs/bug_d4bfccfe-f2f8-4212-a874-f8f02aac3b88

Introduced in #32082 by @shah-harshit on Sep 6, 2026

Summary

  • Context: The ScheduleInterval component (whose constants live in ScheduleInterval.constants.ts) renders the On Demand / Schedule toggle and the cron frequency editor used by ingestion pipelines, test-suite pipelines, and the AppSchedule edit modal.
  • Bug: Switching the card from On Demand back to Schedule always reseeds the cron from the period default instead of restoring the previously-saved schedule.
  • Actual vs. expected: In edit mode, toggling On Demand → Schedule then Save overwrites the user's existing cron (e.g. 30 8 * * 5, Friday 08:30) with the period default (e.g. 0 0 * * 1, Monday midnight); the previous behavior restored the saved cron.
  • Impact: Data loss of a previously-saved schedule on Save for three live edit surfaces — AppSchedule (the DataInsightsReportApplication and SearchIndexingApplication schedule-edit modals in AppDetails), the ingestion-pipeline edit screen (AddIngestion via ScheduleIntervalStep), and the test-suite pipeline edit screen (AddTestSuitePipeline). Three create-only consumers (TestCaseSchedulerSection, BundleSuiteFormBody, AppInstall) are unaffected because there is no pre-existing cron to lose. The overwrite is observable in the UI — the cronExpressionCard (ScheduleInterval.tsx:510) updates to the new human-readable cron on toggle — so an attentive user who re-reads the card before clicking Save can avoid it; a user who toggles back to Schedule and clicks Save without re-reading loses their schedule.

Code with Bug

openmetadata-ui/src/main/resources/ui/src/components/Settings/Services/AddIngestion/Steps/ScheduleInterval.tsx:

} else {
  // When switching to schedule, use default schedule
  const nonEmptyScheduleValue = getDefaultScheduleValue({
    includePeriodOptions,
    defaultSchedule,
  });
  const newState = getStateValue(nonEmptyScheduleValue); // <-- BUG 🔴 always reseeds from the period default, discarding any previously-saved cron
  setState(newState);
  emitChange(newState.cron);
}

Explanation

  • In edit flows, the saved cron is provided to ScheduleInterval via its controlled value prop (direct state in AppSchedule, RHF field in ScheduleIntervalStep, antd Form.Item field in AddTestSuitePipeline).
  • When a user clicks On Demand, the component emits undefined, and the parent/field updates its stored value to empty.
  • When the user clicks Schedule again, handleSelectedSchedular ignores any previously-saved cron and always derives nonEmptyScheduleValue from getDefaultScheduleValue(...) (often a period default like weekly 0 0 * * 1). It then emits that default, overwriting what was previously saved; clicking Save persists the default.
  • Pre-refactor behavior restored the saved cron on returning to Schedule via getStateValue(initialData?.cron ?? nonEmptyScheduleValue).

Codebase Inconsistency

Previous implementation (pre-refactor) explicitly restored the saved cron when returning to Schedule:

let newState = getStateValue(initialData?.cron ?? nonEmptyScheduleValue); // <-- preserved the existing saved cron, falling back to the default only when none was present
if (value === SchedularOptions.ON_DEMAND) {
  newState = { ...newState, cron: undefined };
}
setState(newState);
form.setFieldsValue(newState);

Failing Test

it('REPRO: toggling on-demand then schedule should restore saved cron, not the period default', async () => {
  const { onChange } = renderControlled('30 8 * * 5', {
    includePeriodOptions: ['week'],
  });

  await act(async () => {
    fireEvent.click(
      screen.getByTestId(`schedular-${SchedularOptions.ON_DEMAND}`)
    );
  });
  expect(onChange).toHaveBeenLastCalledWith(undefined);

  await act(async () => {
    fireEvent.click(
      screen.getByTestId(`schedular-${SchedularOptions.SCHEDULE}`)
    );
  });
  expect(onChange).toHaveBeenLastCalledWith('30 8 * * 5');
});

Test output:

Expected: "30 8 * * 5"
Received
       1: undefined
  ->    2: "0 0 * * 1"

Recommended Fix

Capture the last externally-provided non-empty cron (saved cron) and use it when switching back to Schedule, falling back to the default only if none exists.

const initialCronRef = useRef<string | undefined>(value || undefined);

useEffect(() => {
  const normalizedValue = value || undefined;
  if (normalizedValue === lastEmittedValueRef.current) return;
  lastEmittedValueRef.current = normalizedValue;
  if (normalizedValue) {
    initialCronRef.current = normalizedValue;
    setSelectedSchedular(SchedularOptions.SCHEDULE);
    setState(getStateValue(normalizedValue, initialDefaultSchedule));
  } else {
    setSelectedSchedular(SchedularOptions.ON_DEMAND);
    setState((prev) => ({ ...prev, cron: undefined }));
  }
}, [value]);

// in handleSelectedSchedular, Schedule branch:
} else {
  const existing = initialCronRef.current
    ?? getDefaultScheduleValue({ includePeriodOptions, defaultSchedule });
  const newState = getStateValue(existing);
  setState(newState);
  emitChange(newState.cron);
}

History

This bug was introduced in commit c63b171 ("refactor(ui): consolidate schedule interval (#32082)"). The change rewrote handleSelectedSchedular's Schedule branch from restoring the saved cron (initialData?.cron ?? nonEmptyScheduleValue) to always seeding from the default (nonEmptyScheduleValue), causing On-Demand → Schedule to discard the previously-saved schedule.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    • Status
      Done ✅

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions