From 2106078f55f21e1b0eb0d8f700be76ebbd1a4c6b Mon Sep 17 00:00:00 2001 From: Siddhant <86899184+Siddhanttimeline@users.noreply.github.com> Date: Fri, 22 Nov 2024 11:59:09 +0530 Subject: [PATCH] NoSchedule as ScheduleType for Slack App (#18715) * fix: NoSchedule as scheduleType for Slack App * fix: remove callback url from the config. * no render of schedule for noSchedule type * fix: Migrations and add ScheduleType.NoSchedule * fix: Migrations --------- Co-authored-by: karanh37 --- .../service/resources/apps/AppResource.java | 2 +- .../configuration/slackAppConfiguration.json | 14 +--- .../json/schema/entity/applications/app.json | 6 +- .../AppDetails/AppDetails.component.tsx | 56 ++++++++------- .../AppDetails/AppDetails.test.tsx | 26 +++++++ .../pages/AppInstall/AppInstall.component.tsx | 68 ++++++++++++------- .../src/pages/AppInstall/AppInstall.test.tsx | 33 +++++++++ 7 files changed, 144 insertions(+), 61 deletions(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/resources/apps/AppResource.java b/openmetadata-service/src/main/java/org/openmetadata/service/resources/apps/AppResource.java index a52a36bda939..f9066b215e0d 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/apps/AppResource.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/apps/AppResource.java @@ -103,7 +103,7 @@ public class AppResource extends EntityResource { static final String FIELDS = "owners"; private SearchRepository searchRepository; public static final List SCHEDULED_TYPES = - List.of(ScheduleType.Scheduled, ScheduleType.ScheduledOrManual); + List.of(ScheduleType.Scheduled, ScheduleType.ScheduledOrManual, ScheduleType.NoSchedule); public static final String SLACK_APPLICATION = "SlackApplication"; @Override diff --git a/openmetadata-spec/src/main/resources/json/schema/configuration/slackAppConfiguration.json b/openmetadata-spec/src/main/resources/json/schema/configuration/slackAppConfiguration.json index c3d4fdf6951c..de4e3de94f9e 100644 --- a/openmetadata-spec/src/main/resources/json/schema/configuration/slackAppConfiguration.json +++ b/openmetadata-spec/src/main/resources/json/schema/configuration/slackAppConfiguration.json @@ -17,20 +17,8 @@ "signingSecret": { "description": "Signing Secret of the Application. Confirm that each request comes from Slack by verifying its unique signature.", "type": "string" - }, - "scopes": { - "description": "Scopes to Request in OAuth", - "type": "string" - }, - "callbackUrl": { - "description": "The callback URL where temporary authorization code is exchanged for access tokens", - "type": "string" - }, - "callbackRedirectURL": { - "description": "The URL where the application redirects after handling the OAuth callback", - "type": "string" } }, - "required": ["clientId", "clientSecret","signingSecret","scopes","callbackUrl","callbackRedirectURL"], + "required": ["clientId", "clientSecret","signingSecret"], "additionalProperties": false } \ No newline at end of file diff --git a/openmetadata-spec/src/main/resources/json/schema/entity/applications/app.json b/openmetadata-spec/src/main/resources/json/schema/entity/applications/app.json index 830224c3f167..f4092bf15e27 100644 --- a/openmetadata-spec/src/main/resources/json/schema/entity/applications/app.json +++ b/openmetadata-spec/src/main/resources/json/schema/entity/applications/app.json @@ -14,7 +14,8 @@ "enum": [ "Live", "Scheduled", - "ScheduledOrManual" + "ScheduledOrManual", + "NoSchedule" ], "javaEnums": [ { @@ -25,6 +26,9 @@ }, { "name": "ScheduledOrManual" + }, + { + "name": "NoSchedule" } ] }, diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Settings/Applications/AppDetails/AppDetails.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Settings/Applications/AppDetails/AppDetails.component.tsx index ce61d90b8ea0..90d64e766489 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Settings/Applications/AppDetails/AppDetails.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Settings/Applications/AppDetails/AppDetails.component.tsx @@ -49,6 +49,7 @@ import { ServiceCategory } from '../../../../enums/service.enum'; import { App, ScheduleTimeline, + ScheduleType, } from '../../../../generated/entity/applications/app'; import { Include } from '../../../../generated/type/include'; import { useFqn } from '../../../../hooks/useFqn'; @@ -359,31 +360,40 @@ const AppDetails = () => { ] : []; + const showScheduleTab = appData?.scheduleType !== ScheduleType.NoSchedule; + return [ - { - label: ( - - ), - key: ApplicationTabs.SCHEDULE, - children: ( -
- {appData && ( - - )} -
- ), - }, + ...(showScheduleTab + ? [ + { + label: ( + + ), + key: ApplicationTabs.SCHEDULE, + children: ( +
+ {appData && ( + + )} +
+ ), + }, + ] + : []), ...tabConfiguration, - ...(!appData?.deleted + ...(!appData?.deleted && showScheduleTab ? [ { label: ( diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Settings/Applications/AppDetails/AppDetails.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Settings/Applications/AppDetails/AppDetails.test.tsx index 8956e35bbc85..0ab06f1c14d0 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Settings/Applications/AppDetails/AppDetails.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Settings/Applications/AppDetails/AppDetails.test.tsx @@ -15,6 +15,7 @@ import { render, screen, waitForElementToBeRemoved, + within, } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; @@ -218,6 +219,31 @@ describe('AppDetails component', () => { expect(mockPatchApplication).toHaveBeenCalled(); }); + it('Schedule and Recent Runs tab should not be visible for NoScheduleApps', async () => { + mockGetApplicationByName.mockReturnValueOnce({ + ...mockApplicationData, + scheduleType: 'NoSchedule', + deleted: true, + }); + + await renderAppDetails(); + + // Narrow the scope to the tablist within the container + const tabList = screen.getByTestId('tabs'); + + expect( + within(tabList).getByRole('tab', { name: 'label.configuration' }) + ).toBeInTheDocument(); + + expect( + within(tabList).queryByRole('tab', { name: 'label.schedule' }) + ).not.toBeInTheDocument(); + + expect( + within(tabList).queryByRole('tab', { name: 'label.recent-run-plural' }) + ).not.toBeInTheDocument(); + }); + it('Schedule tab Actions check', async () => { await renderAppDetails(); diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/AppInstall/AppInstall.component.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/AppInstall/AppInstall.component.tsx index b7e8b22247ab..7a62d40fac64 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/AppInstall/AppInstall.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/AppInstall/AppInstall.component.tsx @@ -40,7 +40,10 @@ import { CreateAppRequest, ScheduleTimeline, } from '../../generated/entity/applications/createAppRequest'; -import { AppMarketPlaceDefinition } from '../../generated/entity/applications/marketplace/appMarketPlaceDefinition'; +import { + AppMarketPlaceDefinition, + ScheduleType, +} from '../../generated/entity/applications/marketplace/appMarketPlaceDefinition'; import { useFqn } from '../../hooks/useFqn'; import { installApplication } from '../../rest/applicationAPI'; import { getMarketPlaceApplicationByFqn } from '../../rest/applicationMarketPlaceAPI'; @@ -72,13 +75,17 @@ const AppInstall = () => { (feature) => feature.name === 'app' ) ?? {}; - const stepperList = useMemo( - () => - !appData?.allowConfiguration - ? STEPS_FOR_APP_INSTALL.filter((item) => item.step !== 2) - : STEPS_FOR_APP_INSTALL, - [appData] - ); + const stepperList = useMemo(() => { + if (appData?.scheduleType === ScheduleType.NoSchedule) { + return STEPS_FOR_APP_INSTALL.filter((item) => item.step !== 3); + } + + if (!appData?.allowConfiguration) { + return STEPS_FOR_APP_INSTALL.filter((item) => item.step !== 2); + } + + return STEPS_FOR_APP_INSTALL; + }, [appData]); const { initialOptions, defaultValue } = useMemo(() => { if (!appData) { @@ -123,22 +130,10 @@ const AppInstall = () => { history.push(getSettingPath(GlobalSettingOptions.APPLICATIONS)); }; - const onSubmit = async (updatedValue: WorkflowExtraConfig) => { - const { cron } = updatedValue; + const installApp = async (data: CreateAppRequest) => { try { setIsSavingLoading(true); - const data: CreateAppRequest = { - appConfiguration: appConfiguration ?? appData?.appConfiguration, - appSchedule: { - scheduleTimeline: isEmpty(cron) - ? ScheduleTimeline.None - : ScheduleTimeline.Custom, - ...(cron ? { cronExpression: cron } : {}), - }, - name: fqn, - description: appData?.description, - displayName: appData?.displayName, - }; + await installApplication(data); showSuccessToast(t('message.app-installed-successfully')); @@ -154,10 +149,37 @@ const AppInstall = () => { } }; + const onSubmit = async (updatedValue: WorkflowExtraConfig) => { + const { cron } = updatedValue; + const data: CreateAppRequest = { + appConfiguration: appConfiguration ?? appData?.appConfiguration, + appSchedule: { + scheduleTimeline: isEmpty(cron) + ? ScheduleTimeline.None + : ScheduleTimeline.Custom, + ...(cron ? { cronExpression: cron } : {}), + }, + name: fqn, + description: appData?.description, + displayName: appData?.displayName, + }; + installApp(data); + }; + const onSaveConfiguration = (data: IChangeEvent) => { const updatedFormData = formatFormDataForSubmit(data.formData); setAppConfiguration(updatedFormData); - setActiveServiceStep(3); + if (appData?.scheduleType !== ScheduleType.NoSchedule) { + setActiveServiceStep(3); + } else { + const data: CreateAppRequest = { + appConfiguration: updatedFormData, + name: fqn, + description: appData?.description, + displayName: appData?.displayName, + }; + installApp(data); + } }; const RenderSelectedTab = useCallback(() => { diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/AppInstall/AppInstall.test.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/AppInstall/AppInstall.test.tsx index 2a224ee56181..f8ce26e0d499 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/AppInstall/AppInstall.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/AppInstall/AppInstall.test.tsx @@ -13,6 +13,7 @@ import { act, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; +import { ScheduleType } from '../../generated/entity/applications/app'; import { AppMarketPlaceDefinition } from '../../generated/entity/applications/marketplace/appMarketPlaceDefinition'; import AppInstall from './AppInstall.component'; @@ -29,6 +30,11 @@ const MARKETPLACE_DATA = { allowConfiguration: true, }; +const NO_SCHEDULE_DATA = { + scheduleType: ScheduleType.NoSchedule, + allowConfiguration: true, +}; + jest.mock('react-router-dom', () => ({ useHistory: jest.fn().mockImplementation(() => ({ push: mockPush, @@ -234,6 +240,33 @@ describe('AppInstall component', () => { expect(screen.getByText('AppInstallVerifyCard')).toBeInTheDocument(); }); + it('actions check with schedule type noSchedule', async () => { + mockGetMarketPlaceApplicationByFqn.mockResolvedValueOnce(NO_SCHEDULE_DATA); + + await act(async () => { + render(); + }); + + // change ActiveServiceStep to 2 + act(() => { + userEvent.click( + screen.getByRole('button', { name: 'Save AppInstallVerifyCard' }) + ); + }); + + expect(screen.getByText('FormBuilder')).toBeInTheDocument(); + expect(screen.queryByText('AppInstallVerifyCard')).not.toBeInTheDocument(); + + // submit the form here + act(() => { + userEvent.click( + screen.getByRole('button', { name: 'Submit FormBuilder' }) + ); + }); + + expect(mockInstallApplication).toHaveBeenCalled(); + }); + it('errors check in fetching application data', async () => { mockGetMarketPlaceApplicationByFqn.mockRejectedValueOnce(ERROR);