Skip to content

Commit

Permalink
format cron from quartz to unix in applications (#15038)
Browse files Browse the repository at this point in the history
* fix initial value cron value in applications

* added unit test for cron utils functions

* remove quartz cron for applicaion from ui install and edit

* changes as per comments and fix unit test

* fix unit test

* fix user cypress failure on click
  • Loading branch information
Ashish8689 authored Feb 9, 2024
1 parent 8bea265 commit 847b2d8
Show file tree
Hide file tree
Showing 12 changed files with 91 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,7 @@ export const softDeleteUser = (username: string, displayName: string) => {
verifyResponseStatusCode('@searchUser', 200);

// Click on delete button
cy.get(`[data-testid="delete-user-btn-${username}"]`);
cy.get(':nth-child(4) > .ant-space > .ant-space-item > .ant-btn').click();
cy.get(`[data-testid="delete-user-btn-${username}"]`).click();
// Soft deleting the user
cy.get('[data-testid="soft-delete"]').click();
cy.get('[data-testid="confirmation-text-input"]').type('DELETE');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ export interface TestSuiteSchedulerProps {
initialData?: string;
onSubmit: (repeatFrequency: string) => void;
onCancel: () => void;
isQuartzCron?: boolean;
buttonProps?: {
okText: string;
cancelText: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
* limitations under the License.
*/

import { Button, Col, Row, Space } from 'antd';
import { Button, Col, Form, Row, Space } from 'antd';
import { t } from 'i18next';
import React, { useEffect, useState } from 'react';
import CronEditor from '../../common/CronEditor/CronEditor';
Expand All @@ -22,7 +22,6 @@ const TestSuiteScheduler: React.FC<TestSuiteSchedulerProps> = ({
buttonProps,
onCancel,
onSubmit,
isQuartzCron = false,
includePeriodOptions,
}) => {
const [repeatFrequency, setRepeatFrequency] = useState<string | undefined>(
Expand All @@ -38,12 +37,13 @@ const TestSuiteScheduler: React.FC<TestSuiteSchedulerProps> = ({
return (
<Row gutter={[16, 32]}>
<Col span={24}>
<CronEditor
includePeriodOptions={includePeriodOptions}
isQuartzCron={isQuartzCron}
value={repeatFrequency}
onChange={(value: string) => setRepeatFrequency(value)}
/>
<Form data-testid="schedule-container">
<CronEditor
includePeriodOptions={includePeriodOptions}
value={repeatFrequency}
onChange={(value: string) => setRepeatFrequency(value)}
/>
</Form>
</Col>
<Col span={24}>
<Space className="w-full justify-end" size={16}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,6 @@ const AppDetails = () => {
{appData && (
<AppSchedule
appData={appData}
onCancel={onBrowseAppsClick}
onDemandTrigger={onDemandTrigger}
onDeployTrigger={onDeployTrigger}
onSave={onAppScheduleSave}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,17 +124,14 @@ jest.mock('../AppRunsHistory/AppRunsHistory.component', () =>
jest.mock('../AppSchedule/AppSchedule.component', () =>
jest
.fn()
.mockImplementation(
({ onCancel, onSave, onDemandTrigger, onDeployTrigger }) => (
<>
AppSchedule
<button onClick={onCancel}>Cancel AppSchedule</button>
<button onClick={onSave}>Save AppSchedule</button>
<button onClick={onDemandTrigger}>DemandTrigger AppSchedule</button>
<button onClick={onDeployTrigger}>DeployTrigger AppSchedule</button>
</>
)
)
.mockImplementation(({ onSave, onDemandTrigger, onDeployTrigger }) => (
<>
AppSchedule
<button onClick={onSave}>Save AppSchedule</button>
<button onClick={onDemandTrigger}>DemandTrigger AppSchedule</button>
<button onClick={onDeployTrigger}>DeployTrigger AppSchedule</button>
</>
))
);

jest.mock('./ApplicationSchemaClassBase', () => ({
Expand Down Expand Up @@ -233,9 +230,5 @@ describe('AppDetails component', () => {

expect(mockDeployApp).toHaveBeenCalled();
expect(mockGetApplicationByName).toHaveBeenCalled();

userEvent.click(screen.getByRole('button', { name: 'Cancel AppSchedule' }));

expect(mockPush).toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ import {
AppScheduleClass,
AppType,
} from '../../../generated/entity/applications/app';
import { PipelineType } from '../../../generated/entity/services/ingestionPipelines/ingestionPipeline';
import { getIngestionPipelineByFqn } from '../../../rest/ingestionPipelineAPI';
import { getIngestionFrequency } from '../../../utils/CommonUtils';
import TestSuiteScheduler from '../../AddDataQualityTest/components/TestSuiteScheduler';
import Loader from '../../Loader/Loader';
import AppRunsHistory from '../AppRunsHistory/AppRunsHistory.component';
Expand Down Expand Up @@ -74,10 +72,6 @@ const AppSchedule = ({

return cronstrue.toString(cronExp, {
throwExceptionOnParseError: false,
// Quartz cron format accepts 1-7 or SUN-SAT so need to increment index by 1
// Ref: https://www.quartz-scheduler.org/api/2.1.7/org/quartz/CronExpression.html
dayOfWeekStartIndexZero: false,
monthStartIndexZero: false,
});
}

Expand Down Expand Up @@ -225,13 +219,14 @@ const AppSchedule = ({
open={showModal}
title={t('label.update-entity', { entity: t('label.schedule') })}>
<TestSuiteScheduler
isQuartzCron
buttonProps={{
cancelText: t('label.cancel'),
okText: t('label.save'),
}}
includePeriodOptions={initialOptions}
initialData={getIngestionFrequency(PipelineType.Application)}
initialData={
(appData.appSchedule as AppScheduleClass)?.cronExpression ?? ''
}
onCancel={onDialogCancel}
onSubmit={onDialogSave}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,6 @@ jest.mock('../../../rest/ingestionPipelineAPI', () => ({
.mockImplementation((...args) => mockGetIngestionPipelineByFqn(...args)),
}));

jest.mock('../../../utils/CommonUtils', () => ({
getIngestionFrequency: jest.fn(),
}));

jest.mock('../../AddDataQualityTest/components/TestSuiteScheduler', () =>
jest.fn().mockImplementation(({ onSubmit, onCancel }) => (
<div>
Expand Down Expand Up @@ -73,7 +69,6 @@ const mockProps1 = {
name: 'DataInsightsReportApplication',
},
onSave: mockOnSave,
onCancel: jest.fn(),
onDemandTrigger: mockOnDemandTrigger,
onDeployTrigger: mockOnDeployTrigger,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { App } from '../../../generated/entity/applications/app';

export interface AppScheduleProps {
appData: App;
onCancel: () => void;
onSave: (cron: string) => void;
onDemandTrigger: () => void;
onDeployTrigger: () => void;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,10 @@ const TeamHierarchy: FC<TeamHierarchyProps> = ({
render: (description: string) => (
<Typography.Paragraph
className="m-b-0"
style={{whiteSpace:"pre-wrap"}}
ellipsis={{
rows: 2,
}}
style={{ whiteSpace: 'pre-wrap' }}
title={description}>
{isEmpty(description) ? '--' : description}
</Typography.Paragraph>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,11 @@ import {
ScheduleTimeline,
} from '../../generated/entity/applications/createAppRequest';
import { AppMarketPlaceDefinition } from '../../generated/entity/applications/marketplace/appMarketPlaceDefinition';
import { PipelineType } from '../../generated/entity/services/ingestionPipelines/ingestionPipeline';
import { useFqn } from '../../hooks/useFqn';
import { installApplication } from '../../rest/applicationAPI';
import { getMarketPlaceApplicationByFqn } from '../../rest/applicationMarketPlaceAPI';
import {
getEntityMissingError,
getIngestionFrequency,
} from '../../utils/CommonUtils';
import { getEntityMissingError } from '../../utils/CommonUtils';
import { getCronInitialValue } from '../../utils/CronUtils';
import { formatFormDataForSubmit } from '../../utils/JSONSchemaFormUtils';
import {
getMarketPlaceAppDetailsPath,
Expand All @@ -72,6 +69,24 @@ const AppInstall = () => {
[appData]
);

const { initialOptions, initialValue } = useMemo(() => {
let initialOptions;

if (appData?.name === 'DataInsightsReportApplication') {
initialOptions = ['Week'];
} else if (appData?.appType === AppType.External) {
initialOptions = ['Day'];
}

return {
initialOptions,
initialValue: getCronInitialValue(
appData?.appType ?? AppType.Internal,
appData?.name ?? ''
),
};
}, [appData?.name, appData?.appType]);

const fetchAppDetails = useCallback(async () => {
setIsLoading(true);
try {
Expand Down Expand Up @@ -126,16 +141,6 @@ const AppInstall = () => {
setActiveServiceStep(3);
};

const initialOptions = useMemo(() => {
if (appData?.name === 'DataInsightsReportApplication') {
return ['Week'];
} else if (appData?.appType === AppType.External) {
return ['Day'];
}

return undefined;
}, [appData?.name, appData?.appType]);

const RenderSelectedTab = useCallback(() => {
if (!appData || !jsonSchema) {
return <></>;
Expand Down Expand Up @@ -180,9 +185,8 @@ const AppInstall = () => {
<div className="w-500 p-md border rounded-4">
<Typography.Title level={5}>{t('label.schedule')}</Typography.Title>
<TestSuiteScheduler
isQuartzCron
includePeriodOptions={initialOptions}
initialData={getIngestionFrequency(PipelineType.Application)}
initialData={initialValue}
onCancel={() =>
setActiveServiceStep(appData.allowConfiguration ? 2 : 1)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
* limitations under the License.
*/

import { getQuartzCronExpression } from './CronUtils';
import { AppType } from '../generated/entity/applications/app';
import { getCronInitialValue, getQuartzCronExpression } from './CronUtils';

describe('getQuartzCronExpression function', () => {
it('should generate cron expression for every minute', () => {
Expand Down Expand Up @@ -94,3 +95,32 @@ describe('getQuartzCronExpression function', () => {
expect(result).toEqual('0 30 10 ? * 4');
});
});

describe('getCronInitialValue function', () => {
it('should generate hour cron expression if appType is internal and appName is not DataInsightsReportApplication', () => {
const result = getCronInitialValue(
AppType.Internal,
'SearchIndexingApplication'
);

expect(result).toEqual('0 * * * *');
});

it('should generate week cron expression if appName is DataInsightsReportApplication', () => {
const result = getCronInitialValue(
AppType.Internal,
'DataInsightsReportApplication'
);

expect(result).toEqual('0 0 * * 0');
});

it('should generate day cron expression if appType is external', () => {
const result = getCronInitialValue(
AppType.External,
'DataInsightsApplication'
);

expect(result).toEqual('0 0 * * *');
});
});
18 changes: 18 additions & 0 deletions openmetadata-ui/src/main/resources/ui/src/utils/CronUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
StateValue,
ToDisplay,
} from '../components/common/CronEditor/CronEditor.interface';
import { AppType } from '../generated/entity/applications/app';

export const getQuartzCronExpression = (state: StateValue) => {
const {
Expand Down Expand Up @@ -172,3 +173,20 @@ export const getStateValue = (valueStr: string) => {

return stateVal;
};

export const getCronInitialValue = (appType: AppType, appName: string) => {
const value = {
min: 0,
hour: 0,
};

let initialValue = getHourCron(value);

if (appName === 'DataInsightsReportApplication') {
initialValue = getWeekCron({ ...value, dow: 0 });
} else if (appType === AppType.External) {
initialValue = getDayCron(value);
}

return initialValue;
};

0 comments on commit 847b2d8

Please sign in to comment.