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 <karanh37@gmail.com>
open-metadata · Nov 24, 2024 · 2106078 · 2106078
1 parent d797e0c
commit 2106078
Show file tree

Hide file tree

Showing 7 changed files with 144 additions and 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
@@ -103,7 +103,7 @@ public class AppResource extends EntityResource<App, AppRepository> {
   static final String FIELDS = "owners";
   private SearchRepository searchRepository;
   public static final List<ScheduleType> 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
@@ -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
 }
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
@@ -14,7 +14,8 @@
       "enum": [
         "Live",
         "Scheduled",
-        "ScheduledOrManual"
+        "ScheduledOrManual",
+        "NoSchedule"
       ],
       "javaEnums": [
         {
@@ -25,6 +26,9 @@
         },
         {
           "name": "ScheduledOrManual"
+        },
+        {
+          "name": "NoSchedule"
         }
       ]
     },

diff --git a/...ain/resources/ui/src/components/Settings/Applications/AppDetails/AppDetails.component.tsx b/...ain/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: (
-          <TabsLabel id={ApplicationTabs.SCHEDULE} name={t('label.schedule')} />
-        ),
-        key: ApplicationTabs.SCHEDULE,
-        children: (
-          <div className="p-lg">
-            {appData && (
-              <AppSchedule
-                appData={appData}
-                loading={{
-                  isRunLoading: loadingState.isRunLoading,
-                  isDeployLoading: loadingState.isDeployLoading,
-                }}
-                onDemandTrigger={onDemandTrigger}
-                onDeployTrigger={onDeployTrigger}
-                onSave={onAppScheduleSave}
-              />
-            )}
-          </div>
-        ),
-      },
+      ...(showScheduleTab
+        ? [
+            {
+              label: (
+                <TabsLabel
+                  id={ApplicationTabs.SCHEDULE}
+                  name={t('label.schedule')}
+                />
+              ),
+              key: ApplicationTabs.SCHEDULE,
+              children: (
+                <div className="p-lg">
+                  {appData && (
+                    <AppSchedule
+                      appData={appData}
+                      loading={{
+                        isRunLoading: loadingState.isRunLoading,
+                        isDeployLoading: loadingState.isDeployLoading,
+                      }}
+                      onDemandTrigger={onDemandTrigger}
+                      onDeployTrigger={onDeployTrigger}
+                      onSave={onAppScheduleSave}
+                    />
+                  )}
+                </div>
+              ),
+            },
+          ]
+        : []),
       ...tabConfiguration,
-      ...(!appData?.deleted
+      ...(!appData?.deleted && showScheduleTab
         ? [
             {
               label: (

diff --git a/...src/main/resources/ui/src/components/Settings/Applications/AppDetails/AppDetails.test.tsx b/...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
@@ -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
@@ -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(<AppInstall />);
+    });
+
+    // 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);