From 91bea3ea71007bff377b9e8cb9e3f441d177490e Mon Sep 17 00:00:00 2001 From: Manish Kumar <107841575+sondermanish@users.noreply.github.com> Date: Thu, 26 Sep 2024 13:38:00 +0530 Subject: [PATCH 01/13] fix: added flags back (#36561) --- .../java/com/appsmith/external/enums/FeatureFlagEnum.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/enums/FeatureFlagEnum.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/enums/FeatureFlagEnum.java index a654857ec80..e3e96745784 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/enums/FeatureFlagEnum.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/enums/FeatureFlagEnum.java @@ -15,5 +15,9 @@ public enum FeatureFlagEnum { release_embed_hide_share_settings_enabled, rollout_datasource_test_rate_limit_enabled, + // Deprecated CE flags over here + release_git_autocommit_feature_enabled, + release_git_autocommit_eligibility_enabled, + // Add EE flags below this line, to avoid conflicts. } From 347dba5aca831913c83c109aa20dc85a444182bb Mon Sep 17 00:00:00 2001 From: Aman Agarwal Date: Thu, 26 Sep 2024 16:58:39 +0530 Subject: [PATCH 02/13] fix: js library load functionality handling for default exports (#36539) --- .../workers/Evaluation/handlers/jsLibrary.ts | 82 ++++++++++++------- 1 file changed, 53 insertions(+), 29 deletions(-) diff --git a/app/client/src/workers/Evaluation/handlers/jsLibrary.ts b/app/client/src/workers/Evaluation/handlers/jsLibrary.ts index bdf704f2bab..ce81c2d7bf5 100644 --- a/app/client/src/workers/Evaluation/handlers/jsLibrary.ts +++ b/app/client/src/workers/Evaluation/handlers/jsLibrary.ts @@ -128,39 +128,31 @@ export async function installLibrary( // Find keys add that were installed to the global scope. const keysAfterInstallation = Object.keys(self); - const differentiatingKeys = difference( + let differentiatingKeys = difference( keysAfterInstallation, envKeysBeforeInstallation, ); - if ( - differentiatingKeys.length > 0 && - differentiatingKeys.includes("default") - ) { - // Changing default export to library specific name - const uniqueName = generateUniqueAccessor( - url, - takenAccessors, - takenNamesMap, - ); + // Changing default export to library specific name, if default exported + const uniqueName = generateUniqueAccessor( + url, + takenAccessors, + takenNamesMap, + ); - // mapping default functionality to library name accessor - self[uniqueName] = self["default"]; - // deleting the reference of default key from the self object - delete self["default"]; - // mapping all the references of differentiating keys from the self object to the self[uniqueName] key object - differentiatingKeys.map((key) => { - if (key !== "default") { - self[uniqueName][key] = self[key]; - // deleting the references from the self object - delete self[key]; - } - }); - // pushing the uniqueName to the accessor array - accessors.push(uniqueName); - } else { - accessors.push(...differentiatingKeys); - } + movetheDefaultExportedLibraryToAccessorKey( + differentiatingKeys, + uniqueName, + ); + + // Following the same process which was happening earlier + const keysAfterDefaultOperation = Object.keys(self); + + differentiatingKeys = difference( + keysAfterDefaultOperation, + envKeysBeforeInstallation, + ); + accessors.push(...differentiatingKeys); /** * Check the list of installed library to see if their values have changed. @@ -308,7 +300,18 @@ export async function loadLibraries( try { self.importScripts(url); const keysAfter = Object.keys(self); - const defaultAccessors = difference(keysAfter, keysBefore); + let defaultAccessors = difference(keysAfter, keysBefore); + + // Changing default export to library accessors name which was saved when it was installed, if default export present + movetheDefaultExportedLibraryToAccessorKey( + defaultAccessors, + accessors[0], + ); + + // Following the same process which was happening earlier + const keysAfterDefaultOperation = Object.keys(self); + + defaultAccessors = difference(keysAfterDefaultOperation, keysBefore); /** * Installing 2 different version of lodash tries to add the same accessor on the self object. Let take version a & b for example. @@ -447,3 +450,24 @@ export function flattenModule(module: Record) { return libModule; } + +// This function will update the self keys only when the diffAccessors has default included in it. +function movetheDefaultExportedLibraryToAccessorKey( + diffAccessors: string[], + uniqAccessor: string, +) { + if (diffAccessors.length > 0 && diffAccessors.includes("default")) { + // mapping default functionality to library name accessor + self[uniqAccessor] = self["default"]; + // deleting the reference of default key from the self object + delete self["default"]; + // mapping all the references of differentiating keys from the self object to the self[uniqAccessor] key object + diffAccessors.map((key) => { + if (key !== "default") { + self[uniqAccessor][key] = self[key]; + // deleting the references from the self object + delete self[key]; + } + }); + } +} From c2fcd512ad7b699dd9a9f4e46097a551021d41e5 Mon Sep 17 00:00:00 2001 From: saiprabhu-dandanayak Date: Thu, 26 Sep 2024 19:48:29 +0530 Subject: [PATCH 03/13] fix:icon align when datatype is of image (#35992) ### Description ### [Bug Link](https://github.com/appsmithorg/appsmith/issues/35549) I have raised this PR In-order to fix `alignment` of the `icon` in `input component` when `datatype` is selected as `number` and position is selected as `right` ### screenshot ### Before issue is resolved , icon dissapears ![image](https://github.com/user-attachments/assets/7d7f2865-5c88-458b-bf8f-cc4097f493ac) ### After the issue is resolved ![image](https://github.com/user-attachments/assets/9c4f1a65-e2c2-4725-bb94-723c512df70d) ### Cypress Testing ![image](https://github.com/user-attachments/assets/597cd565-4269-463f-bbf2-ba0c078add3c) ### unit Testcases ![image](https://github.com/user-attachments/assets/71c75575-0c1d-40a4-a2f3-7b2f129a1097) ### Cypress Testing video https://github.com/user-attachments/assets/3898144e-e84e-48b8-9ff1-9f449007cf41 ## Summary by CodeRabbit - **New Features** - Introduced a customizable `rightElement` prop in the `BaseInputComponent`, allowing users to display an icon on the right side of the input field based on specified properties. - **Tests** - Added a new test suite for the `BaseInputComponent` to validate the rendering and behavior of the component, including checks for icon visibility and alignment. --- .../BaseInputWidget/component/index.test.tsx | 53 +++++++++++++++++++ .../BaseInputWidget/component/index.tsx | 1 + 2 files changed, 54 insertions(+) create mode 100644 app/client/src/widgets/BaseInputWidget/component/index.test.tsx diff --git a/app/client/src/widgets/BaseInputWidget/component/index.test.tsx b/app/client/src/widgets/BaseInputWidget/component/index.test.tsx new file mode 100644 index 00000000000..cde15763bae --- /dev/null +++ b/app/client/src/widgets/BaseInputWidget/component/index.test.tsx @@ -0,0 +1,53 @@ +import React from "react"; +import { render } from "@testing-library/react"; +import "@testing-library/jest-dom/extend-expect"; +import BaseInputComponent, { type BaseInputComponentProps } from "./index"; +import { ThemeProvider } from "styled-components"; +import { lightTheme } from "selectors/themeSelectors"; + +const renderBaseInputComponent = ( + props: Partial = {}, +) => { + const defaultProps: BaseInputComponentProps = { + value: "", + inputType: "TEXT", + inputHTMLType: "TEXT", + disabled: false, + isLoading: false, + compactMode: false, + isInvalid: false, + label: "Salary", + showError: false, + onValueChange: jest.fn(), + onFocusChange: jest.fn(), + widgetId: "test-widget", + rtl: true, + }; + + return render( + + + , + ); +}; + +describe("BaseInputComponent TestCases", () => { + test("1. Icon should be visible and aligned to the right when the input type is a number", () => { + const { container } = renderBaseInputComponent({ + inputType: "NUMBER", + inputHTMLType: "NUMBER", + value: "123", + onStep: jest.fn(), + rtl: false, + shouldUseLocale: true, + iconName: "add", + iconAlign: "right", + }); + + const numericInputIcon = container.getElementsByClassName( + "bp3-icon bp3-icon-add", + )[0]; + + expect(numericInputIcon).toBeInTheDocument(); + }); +}); diff --git a/app/client/src/widgets/BaseInputWidget/component/index.tsx b/app/client/src/widgets/BaseInputWidget/component/index.tsx index d898b5b73e4..cbb7b21599a 100644 --- a/app/client/src/widgets/BaseInputWidget/component/index.tsx +++ b/app/client/src/widgets/BaseInputWidget/component/index.tsx @@ -565,6 +565,7 @@ class BaseInputComponent extends React.Component< onKeyUp={this.onKeyUp} onValueChange={this.onNumberChange} placeholder={this.props.placeholder} + rightElement={this.getRightIcon()} stepSize={this.props.stepSize} value={this.props.value} {...conditionalProps} From fc9a594a0a289de339fa83859612142c42d1872d Mon Sep 17 00:00:00 2001 From: NandanAnantharamu <67676905+NandanAnantharamu@users.noreply.github.com> Date: Fri, 27 Sep 2024 10:51:33 +0530 Subject: [PATCH 04/13] test: replace 3rd party API1 CE (#36518) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replacing the 3rd party API with TED API /ok-to-test tags="@tag.Datasource" > [!TIP] > 🟒 🟒 🟒 All cypress tests have passed! πŸŽ‰ πŸŽ‰ πŸŽ‰ > Workflow run: > Commit: 0446747b46e74c3ea16b86578cab06fe04346b94 > Cypress dashboard. > Tags: `@tag.Datasource` > Spec: >
Wed, 25 Sep 2024 07:37:49 UTC --------- Co-authored-by: β€œNandanAnantharamu” <β€œnandan@thinkify.io”> --- .../e2e/Regression/ServerSide/ApiTests/API_Edit_spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/client/cypress/e2e/Regression/ServerSide/ApiTests/API_Edit_spec.js b/app/client/cypress/e2e/Regression/ServerSide/ApiTests/API_Edit_spec.js index aa86b788a33..32f675a51fa 100644 --- a/app/client/cypress/e2e/Regression/ServerSide/ApiTests/API_Edit_spec.js +++ b/app/client/cypress/e2e/Regression/ServerSide/ApiTests/API_Edit_spec.js @@ -96,7 +96,7 @@ describe( force: true, }) .type( - "https://www.facebook.com/users/{{Button2.text}}?key=test&val={{Button2.text}}", + "http://host.docker.internal:5001/{{Button2.text}}?key=test&val={{Button2.text}}", { force: true, parseSpecialCharSequences: false }, ) .wait(3000) @@ -106,7 +106,7 @@ describe( .type("{enter}", { parseSpecialCharSequences: true }); cy.validateEvaluatedValue( - "https://www.facebook.com/users/Cancel?key=test&val=Cancel", + "http://host.docker.internal:5001/Cancel?key=test&val=Cancel", ); }); }, From 63682157ee8861248b3b36c6773d4facc2f94697 Mon Sep 17 00:00:00 2001 From: Hetu Nandu Date: Fri, 27 Sep 2024 11:41:15 +0530 Subject: [PATCH 05/13] fix: Quick fixes for current state of Plugin Action Editor (#36574) ## Description Fixes null values crashes in Plugin Action Editor ## Automation /ok-to-test tags="@tag.Datasource" ### :mag: Cypress test results > [!CAUTION] > If you modify the content in this section, you are likely to disrupt the CI result for your PR. ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No --- .../components/PluginActionForm/PluginActionForm.tsx | 7 ++++++- .../CommonEditorForm/hooks/useGetFormActionValues.ts | 2 +- .../PluginActionResponse/PluginActionResponse.tsx | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/client/src/PluginActionEditor/components/PluginActionForm/PluginActionForm.tsx b/app/client/src/PluginActionEditor/components/PluginActionForm/PluginActionForm.tsx index db91b29b3a2..66ab320a304 100644 --- a/app/client/src/PluginActionEditor/components/PluginActionForm/PluginActionForm.tsx +++ b/app/client/src/PluginActionEditor/components/PluginActionForm/PluginActionForm.tsx @@ -2,13 +2,18 @@ import React from "react"; import APIEditorForm from "./components/APIEditorForm"; import { Flex } from "@appsmith/ads"; import { useChangeActionCall } from "./hooks/useChangeActionCall"; +import { usePluginActionContext } from "../../PluginActionContext"; +import { UIComponentTypes } from "api/PluginApi"; const PluginActionForm = () => { useChangeActionCall(); + const { plugin } = usePluginActionContext(); return ( - + {plugin.uiComponent === UIComponentTypes.ApiEditorForm ? ( + + ) : null} ); }; diff --git a/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/hooks/useGetFormActionValues.ts b/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/hooks/useGetFormActionValues.ts index 1078d18282d..da75a1482a2 100644 --- a/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/hooks/useGetFormActionValues.ts +++ b/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/hooks/useGetFormActionValues.ts @@ -18,7 +18,7 @@ function useGetFormActionValues() { // In an unlikely scenario where form is not initialised, // return empty values to avoid form ui issues - if (!isAPIAction(formValues)) { + if (!formValues || !isAPIAction(formValues)) { return { actionHeaders: [], actionParams: [], diff --git a/app/client/src/PluginActionEditor/components/PluginActionResponse/PluginActionResponse.tsx b/app/client/src/PluginActionEditor/components/PluginActionResponse/PluginActionResponse.tsx index f20a395d4cb..9359c8130f3 100644 --- a/app/client/src/PluginActionEditor/components/PluginActionResponse/PluginActionResponse.tsx +++ b/app/client/src/PluginActionEditor/components/PluginActionResponse/PluginActionResponse.tsx @@ -57,7 +57,7 @@ function PluginActionResponse() { expandedHeight={`${ActionExecutionResizerHeight}px`} isCollapsed={!open} onSelect={updateSelectedResponseTab} - selectedTabKey={selectedTab || tabs[0].key} + selectedTabKey={selectedTab || tabs[0]?.key} tabs={tabs} /> From e6cd97318d294d85512e65a79c196e9e513001b7 Mon Sep 17 00:00:00 2001 From: Nilansh Bansal Date: Fri, 27 Sep 2024 14:00:01 +0530 Subject: [PATCH 06/13] feat: added LRU cache for mockDB connections (#36480) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description This PR implements an LRU (Least Recently Used) caching mechanism for the Mongo mockDB connections which get auto-cleaned up every 2 hours based on their access time. This is done to stop the overpopulation of the open dangling connections to the mockDB resulting in the max connection limit. The Caching Implementation used is [Google Guave Cache](https://github.com/google/guava/wiki/CachesExplained). Also refer - [Working of Guava cache ](https://medium.com/@alxkm/introduction-to-caching-with-google-guava-a-simple-and-flexible-solution-2c721427e72e) image Fixes #36474 ## Automation /ok-to-test tags="@tag.Sanity" ### :mag: Cypress test results > [!TIP] > 🟒 🟒 🟒 All cypress tests have passed! πŸŽ‰ πŸŽ‰ πŸŽ‰ > Workflow run: > Commit: 38fcf572b32f1b5d7544828ccf00f2b6fbaa180e > Cypress dashboard. > Tags: `@tag.Sanity` > Spec: >
Fri, 27 Sep 2024 08:01:28 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **New Features** - Introduced a new constant for the MongoDB plugin to enhance plugin identification. - Added a `DatasourcePluginContext` class to encapsulate datasource plugin context, including connection details and creation time. - Implemented a caching mechanism for datasource contexts to optimize connection management and reduce excessive database connections. - Added functionality to identify mock MongoDB datasources. - **Bug Fixes** - Enhanced cleanup processes for stale connections in the caching system. --- .../external/constants/PluginConstants.java | 1 + .../domains/DatasourcePluginContext.java | 20 +++ .../ce/DatasourceContextServiceCEImpl.java | 128 +++++++++++++++--- 3 files changed, 132 insertions(+), 17 deletions(-) create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/domains/DatasourcePluginContext.java diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/PluginConstants.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/PluginConstants.java index 7c52a719d02..960834f9064 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/PluginConstants.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/PluginConstants.java @@ -24,6 +24,7 @@ interface PackageName { String APPSMITH_AI_PLUGIN = "appsmithai-plugin"; String DATABRICKS_PLUGIN = "databricks-plugin"; String AWS_LAMBDA_PLUGIN = "aws-lambda-plugin"; + String MONGO_PLUGIN = "mongo-plugin"; } public static final String DEFAULT_REST_DATASOURCE = "DEFAULT_REST_DATASOURCE"; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/DatasourcePluginContext.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/DatasourcePluginContext.java new file mode 100644 index 00000000000..b3385964e2d --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/DatasourcePluginContext.java @@ -0,0 +1,20 @@ +package com.appsmith.server.domains; + +import lombok.Getter; +import lombok.Setter; +import lombok.ToString; + +import java.time.Instant; + +@Getter +@Setter +@ToString +public class DatasourcePluginContext { + private T connection; + private String pluginId; + private Instant creationTime; + + public DatasourcePluginContext() { + creationTime = Instant.now(); + } +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/DatasourceContextServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/DatasourceContextServiceCEImpl.java index 3693d1348c4..adc1d2ea82b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/DatasourceContextServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/DatasourceContextServiceCEImpl.java @@ -13,6 +13,7 @@ import com.appsmith.server.datasourcestorages.base.DatasourceStorageService; import com.appsmith.server.domains.DatasourceContext; import com.appsmith.server.domains.DatasourceContextIdentifier; +import com.appsmith.server.domains.DatasourcePluginContext; import com.appsmith.server.domains.Plugin; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; @@ -20,6 +21,10 @@ import com.appsmith.server.plugins.base.PluginService; import com.appsmith.server.services.ConfigService; import com.appsmith.server.solutions.DatasourcePermission; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.RemovalListener; +import com.google.common.cache.RemovalNotification; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Lazy; @@ -29,8 +34,12 @@ import java.time.Instant; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; import java.util.function.Function; +import static java.lang.Boolean.FALSE; +import static java.lang.Boolean.TRUE; + @Slf4j public class DatasourceContextServiceCEImpl implements DatasourceContextServiceCE { @@ -38,6 +47,21 @@ public class DatasourceContextServiceCEImpl implements DatasourceContextServiceC protected final Map>> datasourceContextMonoMap; protected final Map datasourceContextSynchronizationMonitorMap; protected final Map> datasourceContextMap; + + /** + * This cache is used to store the datasource context for a limited time and a limited max number of connections and + * then destroy the least recently used connection. The cleanup process is triggered when the cache is accessed and + * either the time limit or the max connections are reached. + * The purpose of this is to prevent the large number of open dangling connections to the movies mockDB. + * The removalListener method is called when the connection is removed from the cache. + */ + protected final Cache datasourcePluginContextMapLRUCache = + CacheBuilder.newBuilder() + .removalListener(createRemovalListener()) + .expireAfterAccess(2, TimeUnit.HOURS) + .maximumSize(300) // caches most recently used 300 mock connections per pod + .build(); + private final DatasourceService datasourceService; private final DatasourceStorageService datasourceStorageService; private final PluginService pluginService; @@ -67,6 +91,50 @@ public DatasourceContextServiceCEImpl( this.datasourcePermission = datasourcePermission; } + private RemovalListener createRemovalListener() { + return (RemovalNotification removalNotification) -> { + handleRemoval(removalNotification); + }; + } + + private Object getConnectionFromDatasourceContextMap(DatasourceContextIdentifier datasourceContextIdentifier) { + return this.datasourceContextMap.containsKey(datasourceContextIdentifier) + && this.datasourceContextMap.get(datasourceContextIdentifier) != null + ? this.datasourceContextMap.get(datasourceContextIdentifier).getConnection() + : null; + } + + private void handleRemoval( + RemovalNotification removalNotification) { + final DatasourceContextIdentifier datasourceContextIdentifier = removalNotification.getKey(); + final DatasourcePluginContext datasourcePluginContext = removalNotification.getValue(); + + log.debug( + "Removing Datasource Context from cache and closing the open connection for DatasourceId: {} and environmentId: {}", + datasourceContextIdentifier.getDatasourceId(), + datasourceContextIdentifier.getEnvironmentId()); + log.info("LRU Cache Size after eviction: {}", datasourcePluginContextMapLRUCache.size()); + + // Close connection and remove entry from both cache maps + final Object connection = getConnectionFromDatasourceContextMap(datasourceContextIdentifier); + + Mono pluginMono = + pluginService.findById(datasourcePluginContext.getPluginId()).cache(); + if (connection != null) { + pluginExecutorHelper + .getPluginExecutor(pluginMono) + .flatMap(pluginExecutor -> Mono.fromRunnable(() -> pluginExecutor.datasourceDestroy(connection))) + .onErrorResume(e -> { + log.error("Error destroying stale datasource connection", e); + return Mono.empty(); + }) + .subscribe(); // Trigger the execution + } + // Remove the entries from both maps + datasourceContextMonoMap.remove(datasourceContextIdentifier); + datasourceContextMap.remove(datasourceContextIdentifier); + } + /** * This method defines a critical section that can be executed only by one thread at a time per datasource id - i * .e. if two threads want to create datasource for different datasource ids then they would not be synchronized. @@ -115,6 +183,11 @@ public Mono> getCachedDatasourceContextMono( } datasourceContextMonoMap.remove(datasourceContextIdentifier); datasourceContextMap.remove(datasourceContextIdentifier); + log.info( + "Invalidating the LRU cache entry for datasource id {}, environment id {} as the connection is stale or in error state", + datasourceContextIdentifier.getDatasourceId(), + datasourceContextIdentifier.getEnvironmentId()); + datasourcePluginContextMapLRUCache.invalidate(datasourceContextIdentifier); } /* @@ -129,17 +202,13 @@ public Mono> getCachedDatasourceContextMono( + ": Cached resource context mono exists for datasource id {}, environment id {}. Returning the same.", datasourceContextIdentifier.getDatasourceId(), datasourceContextIdentifier.getEnvironmentId()); + // Accessing the LRU cache to update the last accessed time + datasourcePluginContextMapLRUCache.getIfPresent(datasourceContextIdentifier); return datasourceContextMonoMap.get(datasourceContextIdentifier); } /* Create a fresh datasource context */ DatasourceContext datasourceContext = new DatasourceContext<>(); - if (datasourceContextIdentifier.isKeyValid() && shouldCacheContextForThisPlugin(plugin)) { - /* For this datasource, either the context doesn't exist, or the context is stale. Replace (or add) with - the new connection in the context map. */ - datasourceContextMap.put(datasourceContextIdentifier, datasourceContext); - } - Mono connectionMonoCache = pluginExecutor .datasourceCreate(datasourceStorage.getDatasourceConfiguration()) .cache(); @@ -159,15 +228,34 @@ public Mono> getCachedDatasourceContextMono( datasourceContext) .cache(); /* Cache the value so that further evaluations don't result in new connections */ - if (datasourceContextIdentifier.isKeyValid() && shouldCacheContextForThisPlugin(plugin)) { - datasourceContextMonoMap.put(datasourceContextIdentifier, datasourceContextMonoCache); - } - log.debug( - Thread.currentThread().getName() - + ": Cached new datasource context for datasource id {}, environment id {}", - datasourceContextIdentifier.getDatasourceId(), - datasourceContextIdentifier.getEnvironmentId()); - return datasourceContextMonoCache; + return connectionMonoCache + .flatMap(connection -> { + datasourceContext.setConnection(connection); + if (datasourceContextIdentifier.isKeyValid() + && shouldCacheContextForThisPlugin(plugin)) { + datasourceContextMap.put(datasourceContextIdentifier, datasourceContext); + datasourceContextMonoMap.put( + datasourceContextIdentifier, datasourceContextMonoCache); + + if (TRUE.equals(datasourceStorage.getIsMock()) + && PluginConstants.PackageName.MONGO_PLUGIN.equals( + plugin.getPackageName())) { + log.info( + "Datasource is a mock mongo DB. Adding the connection to LRU cache!"); + DatasourcePluginContext datasourcePluginContext = + new DatasourcePluginContext<>(); + datasourcePluginContext.setConnection(datasourceContext.getConnection()); + datasourcePluginContext.setPluginId(plugin.getId()); + datasourcePluginContextMapLRUCache.put( + datasourceContextIdentifier, datasourcePluginContext); + log.info( + "LRU Cache Size after adding: {}", + datasourcePluginContextMapLRUCache.size()); + } + } + return datasourceContextMonoCache; + }) + .switchIfEmpty(datasourceContextMonoCache); } }) .flatMap(obj -> obj) @@ -195,7 +283,7 @@ public Mono updateDatasourceAndSetAuthentication(Object connection, Data .setAuthentication(updatableConnection.getAuthenticationDTO( datasourceStorage.getDatasourceConfiguration().getAuthentication())); datasourceStorageMono = datasourceStorageService.updateDatasourceStorage( - datasourceStorage, datasourceStorage.getEnvironmentId(), Boolean.FALSE, false); + datasourceStorage, datasourceStorage.getEnvironmentId(), FALSE, false); } return datasourceStorageMono.thenReturn(connection); } @@ -308,6 +396,8 @@ public Mono> getDatasourceContext(DatasourceStorage datasou } else { if (isValidDatasourceContextAvailable(datasourceStorage, datasourceContextIdentifier)) { log.debug("Resource context exists. Returning the same."); + // Accessing the LRU cache to update the last accessed time + datasourcePluginContextMapLRUCache.getIfPresent(datasourceContextIdentifier); return Mono.just(datasourceContextMap.get(datasourceContextIdentifier)); } } @@ -399,7 +489,11 @@ public Mono> deleteDatasourceContext(DatasourceStorage data log.info("Clearing datasource context for datasource storage ID {}.", datasourceStorage.getId()); pluginExecutor.datasourceDestroy(datasourceContext.getConnection()); datasourceContextMonoMap.remove(datasourceContextIdentifier); - + log.info( + "Invalidating the LRU cache entry for datasource id {}, environment id {} as delete datasource context is invoked", + datasourceContextIdentifier.getDatasourceId(), + datasourceContextIdentifier.getEnvironmentId()); + datasourcePluginContextMapLRUCache.invalidate(datasourceContextIdentifier); if (!datasourceContextMap.containsKey(datasourceContextIdentifier)) { log.info( "datasourceContextMap does not contain any entry for datasource storage with id: {} ", From b1ed82dbcf11f9cdd49814459368649f1b6518fd Mon Sep 17 00:00:00 2001 From: Rudraprasad Das Date: Fri, 27 Sep 2024 18:07:57 +0800 Subject: [PATCH 07/13] fix: fixed autocommit spec for airgap (#36579) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Changes autocommit test repo to a new airgap compatiable repo ## Automation ``` /test cypress/e2e/Regression/ClientSide/Git/GitAutocommit_spec.ts ``` ### :mag: Cypress test results > [!TIP] > 🟒 🟒 🟒 All cypress tests have passed! πŸŽ‰ πŸŽ‰ πŸŽ‰ > Workflow run: > Commit: ed27652b4fe386f943a937e0e7efdc059700d779 > Cypress dashboard. > Tags: `` > Spec: cypress/e2e/Regression/ClientSide/Git/GitAutocommit_spec.ts >
Fri, 27 Sep 2024 06:55:15 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **Tests** - Updated the repository name in the autocommit tests to better reflect current context. - Revised test suite tags for improved organization and focus. --- .../e2e/Regression/ClientSide/Git/GitAutocommit_spec.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/client/cypress/e2e/Regression/ClientSide/Git/GitAutocommit_spec.ts b/app/client/cypress/e2e/Regression/ClientSide/Git/GitAutocommit_spec.ts index 9d38aa357dc..c55b51711ce 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/Git/GitAutocommit_spec.ts +++ b/app/client/cypress/e2e/Regression/ClientSide/Git/GitAutocommit_spec.ts @@ -1,5 +1,4 @@ import ReconnectLocators from "../../../../locators/ReconnectLocators"; -import { featureFlagIntercept } from "../../../../support/Objects/FeatureFlags"; import { agHelper, gitSync, @@ -7,7 +6,7 @@ import { } from "../../../../support/Objects/ObjectsCore"; let wsName: string; -let repoName: string = "TED-testrepo1"; +let repoName: string = "TED-autocommit-test-1"; describe( "Git Autocommit", @@ -15,8 +14,8 @@ describe( tags: [ "@tag.Git", "@tag.GitAutocommit", - "@tag.excludeForAirgap", "@tag.Sanity", + "@tag.TedMigration", ], }, function () { From 5995e4292a3d8d3533c4d79a9cb1f82b06d20da6 Mon Sep 17 00:00:00 2001 From: Ankita Kinger Date: Fri, 27 Sep 2024 19:48:05 +0530 Subject: [PATCH 08/13] chore: Handling the updation of action name in the plugin action toolbar (#36560) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Handling the updation of action name in the plugin action toolbar in the new modularised flow. Fixes [#36498](https://github.com/appsmithorg/appsmith/issues/36498) ## Automation /ok-to-test tags="@tag.All" ### :mag: Cypress test results > [!TIP] > 🟒 🟒 🟒 All cypress tests have passed! πŸŽ‰ πŸŽ‰ πŸŽ‰ > Workflow run: > Commit: 73647e50cfeb6919b30c674f8f3a3a219f6f98c8 > Cypress dashboard. > Tags: `@tag.All` > Spec: >
Fri, 27 Sep 2024 14:15:24 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **New Features** - Introduced a new component for editing plugin action names, enhancing user experience in managing plugin actions. - Added optional icon size property to the editable text component for improved customization. - Enhanced the `CommonEditorForm` and `QueryEditorHeader` components to display plugin-specific information and saving status. - **Bug Fixes** - Streamlined action dispatching logic, improving reliability in saving actions. - **Documentation** - Updated interfaces and prop types for better clarity and type safety in the codebase. --- .../components/PluginActionNameEditor.tsx | 81 +++++++++++++++++++ app/client/src/PluginActionEditor/index.ts | 5 ++ .../editorComponents/ActionNameEditor.tsx | 71 +++++++--------- .../editorComponents/EditableText.tsx | 12 ++- .../components/utils/NameEditorComponent.tsx | 14 ++-- .../Editor/APIEditor/ApiEditorContext.tsx | 8 +- .../Editor/APIEditor/CommonEditorForm.tsx | 18 +++++ .../src/pages/Editor/APIEditor/index.tsx | 21 +++-- .../src/pages/Editor/Explorer/Entity/Name.tsx | 6 +- .../Editor/JSEditor/JSObjectNameEditor.tsx | 7 +- .../Editor/QueryEditor/QueryEditorContext.tsx | 8 +- .../Editor/QueryEditor/QueryEditorHeader.tsx | 19 +++++ .../src/pages/Editor/QueryEditor/index.tsx | 17 ++-- 13 files changed, 194 insertions(+), 93 deletions(-) create mode 100644 app/client/src/PluginActionEditor/components/PluginActionNameEditor.tsx diff --git a/app/client/src/PluginActionEditor/components/PluginActionNameEditor.tsx b/app/client/src/PluginActionEditor/components/PluginActionNameEditor.tsx new file mode 100644 index 00000000000..a51d6006dec --- /dev/null +++ b/app/client/src/PluginActionEditor/components/PluginActionNameEditor.tsx @@ -0,0 +1,81 @@ +import React from "react"; +import { useSelector } from "react-redux"; +import ActionNameEditor from "components/editorComponents/ActionNameEditor"; +import { usePluginActionContext } from "PluginActionEditor/PluginActionContext"; +import { useFeatureFlag } from "utils/hooks/useFeatureFlag"; +import { getHasManageActionPermission } from "ee/utils/BusinessFeatures/permissionPageHelpers"; +import { FEATURE_FLAG } from "ee/entities/FeatureFlag"; +import { PluginType } from "entities/Action"; +import type { ReduxAction } from "ee/constants/ReduxActionConstants"; +import styled from "styled-components"; +import { getSavingStatusForActionName } from "selectors/actionSelectors"; +import { getAssetUrl } from "ee/utils/airgapHelpers"; +import { ActionUrlIcon } from "pages/Editor/Explorer/ExplorerIcons"; + +export interface SaveActionNameParams { + id: string; + name: string; +} + +export interface PluginActionNameEditorProps { + saveActionName: ( + params: SaveActionNameParams, + ) => ReduxAction; +} + +const ActionNameEditorWrapper = styled.div` + & .ads-v2-box { + gap: var(--ads-v2-spaces-2); + } + + && .t--action-name-edit-field { + font-size: 12px; + + .bp3-editable-text-content { + height: unset !important; + line-height: unset !important; + } + } + + & .t--plugin-icon-box { + height: 12px; + width: 12px; + + img { + width: 12px; + height: auto; + } + } +`; + +const PluginActionNameEditor = (props: PluginActionNameEditorProps) => { + const { action, plugin } = usePluginActionContext(); + + const isFeatureEnabled = useFeatureFlag(FEATURE_FLAG.license_gac_enabled); + const isChangePermitted = getHasManageActionPermission( + isFeatureEnabled, + action?.userPermissions, + ); + + const saveStatus = useSelector((state) => + getSavingStatusForActionName(state, action?.id || ""), + ); + + const iconUrl = getAssetUrl(plugin?.iconLocation) || ""; + const icon = ActionUrlIcon(iconUrl); + + return ( + + + + ); +}; + +export default PluginActionNameEditor; diff --git a/app/client/src/PluginActionEditor/index.ts b/app/client/src/PluginActionEditor/index.ts index 20265c8bc5a..e8083e1b0f0 100644 --- a/app/client/src/PluginActionEditor/index.ts +++ b/app/client/src/PluginActionEditor/index.ts @@ -6,3 +6,8 @@ export { export { default as PluginActionToolbar } from "./components/PluginActionToolbar"; export { default as PluginActionForm } from "./components/PluginActionForm"; export { default as PluginActionResponse } from "./components/PluginActionResponse"; +export type { + SaveActionNameParams, + PluginActionNameEditorProps, +} from "./components/PluginActionNameEditor"; +export { default as PluginActionNameEditor } from "./components/PluginActionNameEditor"; diff --git a/app/client/src/components/editorComponents/ActionNameEditor.tsx b/app/client/src/components/editorComponents/ActionNameEditor.tsx index 8467855f479..1d9bf01aa7c 100644 --- a/app/client/src/components/editorComponents/ActionNameEditor.tsx +++ b/app/client/src/components/editorComponents/ActionNameEditor.tsx @@ -1,19 +1,13 @@ import React, { memo } from "react"; -import { useSelector } from "react-redux"; -import { useParams } from "react-router-dom"; import EditableText, { EditInteractionKind, } from "components/editorComponents/EditableText"; import { removeSpecialChars } from "utils/helpers"; -import type { AppState } from "ee/reducers"; -import { saveActionName } from "actions/pluginActionActions"; import { Flex } from "@appsmith/ads"; -import { getActionByBaseId, getPlugin } from "ee/selectors/entitiesSelector"; import NameEditorComponent, { IconBox, - IconWrapper, NameWrapper, } from "components/utils/NameEditorComponent"; import { @@ -21,14 +15,13 @@ import { ACTION_NAME_PLACEHOLDER, createMessage, } from "ee/constants/messages"; -import { getAssetUrl } from "ee/utils/airgapHelpers"; -import { getSavingStatusForActionName } from "selectors/actionSelectors"; import type { ReduxAction } from "ee/constants/ReduxActionConstants"; +import type { SaveActionNameParams } from "PluginActionEditor"; +import { useFeatureFlag } from "utils/hooks/useFeatureFlag"; +import { FEATURE_FLAG } from "ee/entities/FeatureFlag"; +import type { Action } from "entities/Action"; +import type { ModuleInstance } from "ee/constants/ModuleInstanceConstants"; -interface SaveActionNameParams { - id: string; - name: string; -} interface ActionNameEditorProps { /* This prop checks if page is API Pane or Query Pane or Curl Pane @@ -38,38 +31,34 @@ interface ActionNameEditorProps { */ enableFontStyling?: boolean; disabled?: boolean; - saveActionName?: ( + saveActionName: ( params: SaveActionNameParams, ) => ReduxAction; + actionConfig?: Action | ModuleInstance; + icon?: JSX.Element; + saveStatus: { isSaving: boolean; error: boolean }; } function ActionNameEditor(props: ActionNameEditorProps) { - const params = useParams<{ baseApiId?: string; baseQueryId?: string }>(); - - const currentActionConfig = useSelector((state: AppState) => - getActionByBaseId(state, params.baseApiId || params.baseQueryId || ""), - ); - - const currentPlugin = useSelector((state: AppState) => - getPlugin(state, currentActionConfig?.pluginId || ""), - ); + const { + actionConfig, + disabled = false, + enableFontStyling = false, + icon = "", + saveActionName, + saveStatus, + } = props; - const saveStatus = useSelector((state) => - getSavingStatusForActionName(state, currentActionConfig?.id || ""), + const isActionRedesignEnabled = useFeatureFlag( + FEATURE_FLAG.release_actions_redesign_enabled, ); return ( {({ @@ -85,28 +74,22 @@ function ActionNameEditor(props: ActionNameEditorProps) { isNew: boolean; saveStatus: { isSaving: boolean; error: boolean }; }) => ( - + - {currentPlugin && ( - - - - )} + {icon && {icon}} ))} diff --git a/app/client/src/components/utils/NameEditorComponent.tsx b/app/client/src/components/utils/NameEditorComponent.tsx index b7aa2e03ce4..876a9c9d1fd 100644 --- a/app/client/src/components/utils/NameEditorComponent.tsx +++ b/app/client/src/components/utils/NameEditorComponent.tsx @@ -11,6 +11,8 @@ import { } from "ee/constants/messages"; import styled from "styled-components"; import { Classes } from "@blueprintjs/core"; +import type { SaveActionNameParams } from "PluginActionEditor"; +import type { ReduxAction } from "ee/constants/ReduxActionConstants"; export const NameWrapper = styled.div<{ enableFontStyling?: boolean }>` min-width: 50%; @@ -71,9 +73,9 @@ interface NameEditorProps { children: (params: any) => JSX.Element; id?: string; name?: string; - // TODO: Fix this the next time the file is edited - // eslint-disable-next-line @typescript-eslint/no-explicit-any - dispatchAction: (a: any) => any; + onSaveName: ( + params: SaveActionNameParams, + ) => ReduxAction; // TODO: Fix this the next time the file is edited // eslint-disable-next-line @typescript-eslint/no-explicit-any suffixErrorMessage?: (params?: any) => string; @@ -90,10 +92,10 @@ interface NameEditorProps { function NameEditor(props: NameEditorProps) { const { - dispatchAction, id: entityId, idUndefinedErrorMessage, name: entityName, + onSaveName, saveStatus, suffixErrorMessage = ACTION_NAME_CONFLICT_ERROR, } = props; @@ -131,8 +133,8 @@ function NameEditor(props: NameEditorProps) { const handleNameChange = useCallback( (name: string) => { - if (name !== entityName && !isInvalidNameForEntity(name)) { - dispatch(dispatchAction({ id: entityId, name })); + if (name !== entityName && !isInvalidNameForEntity(name) && entityId) { + dispatch(onSaveName({ id: entityId, name })); } }, [dispatch, isInvalidNameForEntity, entityId, entityName], diff --git a/app/client/src/pages/Editor/APIEditor/ApiEditorContext.tsx b/app/client/src/pages/Editor/APIEditor/ApiEditorContext.tsx index a5d3aa5e235..1a2d9d51789 100644 --- a/app/client/src/pages/Editor/APIEditor/ApiEditorContext.tsx +++ b/app/client/src/pages/Editor/APIEditor/ApiEditorContext.tsx @@ -1,11 +1,7 @@ import type { ReduxAction } from "ee/constants/ReduxActionConstants"; import type { PaginationField } from "api/ActionAPI"; import React, { createContext, useMemo } from "react"; - -interface SaveActionNameParams { - id: string; - name: string; -} +import type { SaveActionNameParams } from "PluginActionEditor"; interface ApiEditorContextContextProps { moreActionsMenu?: React.ReactNode; @@ -15,7 +11,7 @@ interface ApiEditorContextContextProps { // TODO: Fix this the next time the file is edited // eslint-disable-next-line @typescript-eslint/no-explicit-any settingsConfig: any; - saveActionName?: ( + saveActionName: ( params: SaveActionNameParams, ) => ReduxAction; closeEditorLink?: React.ReactNode; diff --git a/app/client/src/pages/Editor/APIEditor/CommonEditorForm.tsx b/app/client/src/pages/Editor/APIEditor/CommonEditorForm.tsx index b542cdfbd04..979c4edf1cb 100644 --- a/app/client/src/pages/Editor/APIEditor/CommonEditorForm.tsx +++ b/app/client/src/pages/Editor/APIEditor/CommonEditorForm.tsx @@ -35,6 +35,9 @@ import { InfoFields, RequestTabs, } from "PluginActionEditor/components/PluginActionForm/components/CommonEditorForm"; +import { getSavingStatusForActionName } from "selectors/actionSelectors"; +import { getAssetUrl } from "ee/utils/airgapHelpers"; +import { ActionUrlIcon } from "../Explorer/ExplorerIcons"; const Form = styled.form` position: relative; @@ -245,6 +248,18 @@ function CommonEditorForm(props: CommonFormPropsWithExtraParams) { currentActionConfig?.userPermissions, ); + const currentPlugin = useSelector((state: AppState) => + getPlugin(state, currentActionConfig?.pluginId || ""), + ); + + const saveStatus = useSelector((state) => + getSavingStatusForActionName(state, currentActionConfig?.id || ""), + ); + + const iconUrl = getAssetUrl(currentPlugin?.iconLocation) || ""; + + const icon = ActionUrlIcon(iconUrl); + const plugin = useSelector((state: AppState) => getPlugin(state, pluginId ?? ""), ); @@ -281,9 +296,12 @@ function CommonEditorForm(props: CommonFormPropsWithExtraParams) { diff --git a/app/client/src/pages/Editor/APIEditor/index.tsx b/app/client/src/pages/Editor/APIEditor/index.tsx index 247c2cfd408..31328d25055 100644 --- a/app/client/src/pages/Editor/APIEditor/index.tsx +++ b/app/client/src/pages/Editor/APIEditor/index.tsx @@ -8,7 +8,11 @@ import { getPluginSettingConfigs, getPlugins, } from "ee/selectors/entitiesSelector"; -import { deleteAction, runAction } from "actions/pluginActionActions"; +import { + deleteAction, + runAction, + saveActionName, +} from "actions/pluginActionActions"; import AnalyticsUtil from "ee/utils/AnalyticsUtil"; import Editor from "./Editor"; import BackToCanvas from "components/common/BackToCanvas"; @@ -151,15 +155,7 @@ function ApiEditorWrapper(props: ApiEditorWrapperProps) { }); dispatch(runAction(action?.id ?? "", paginationField)); }, - [ - action?.id, - apiName, - pageName, - getPageName, - plugins, - pluginId, - datasourceId, - ], + [action?.id, apiName, pageName, plugins, pluginId, datasourceId, dispatch], ); const actionRightPaneBackLink = useMemo(() => { @@ -173,13 +169,13 @@ function ApiEditorWrapper(props: ApiEditorWrapperProps) { pageName, }); dispatch(deleteAction({ id: action?.id ?? "", name: apiName })); - }, [getPageName, pages, basePageId, apiName]); + }, [pages, basePageId, apiName, action?.id, dispatch, pageName]); const notification = useMemo(() => { if (!isConverting) return null; return ; - }, [action?.name, isConverting]); + }, [action?.name, isConverting, icon]); const isActionRedesignEnabled = useFeatureFlag( FEATURE_FLAG.release_actions_redesign_enabled, @@ -196,6 +192,7 @@ function ApiEditorWrapper(props: ApiEditorWrapperProps) { handleRunClick={handleRunClick} moreActionsMenu={moreActionsMenu} notification={notification} + saveActionName={saveActionName} settingsConfig={settingsConfig} > diff --git a/app/client/src/pages/Editor/Explorer/Entity/Name.tsx b/app/client/src/pages/Editor/Explorer/Entity/Name.tsx index de937dd86a1..e0fb005bc00 100644 --- a/app/client/src/pages/Editor/Explorer/Entity/Name.tsx +++ b/app/client/src/pages/Editor/Explorer/Entity/Name.tsx @@ -16,6 +16,8 @@ import { import { Tooltip } from "@appsmith/ads"; import { useSelector } from "react-redux"; import { getSavingStatusForActionName } from "selectors/actionSelectors"; +import type { ReduxAction } from "ee/constants/ReduxActionConstants"; +import type { SaveActionNameParams } from "PluginActionEditor"; export const searchHighlightSpanClassName = "token"; export const searchTokenizationDelimiter = "!!"; @@ -84,7 +86,7 @@ export interface EntityNameProps { name: string; isEditing?: boolean; onChange?: (name: string) => void; - updateEntityName: (name: string) => void; + updateEntityName: (name: string) => ReduxAction; entityId: string; searchKeyword?: string; className?: string; @@ -164,10 +166,10 @@ export const EntityName = React.memo( return ( diff --git a/app/client/src/pages/Editor/JSEditor/JSObjectNameEditor.tsx b/app/client/src/pages/Editor/JSEditor/JSObjectNameEditor.tsx index f82860f134b..ff1a598bc1b 100644 --- a/app/client/src/pages/Editor/JSEditor/JSObjectNameEditor.tsx +++ b/app/client/src/pages/Editor/JSEditor/JSObjectNameEditor.tsx @@ -25,11 +25,8 @@ import NameEditorComponent, { } from "components/utils/NameEditorComponent"; import { getSavingStatusForJSObjectName } from "selectors/actionSelectors"; import type { ReduxAction } from "ee/constants/ReduxActionConstants"; +import type { SaveActionNameParams } from "PluginActionEditor"; -export interface SaveActionNameParams { - id: string; - name: string; -} export interface JSObjectNameEditorProps { /* This prop checks if page is API Pane or Query Pane or Curl Pane @@ -64,10 +61,10 @@ export function JSObjectNameEditor(props: JSObjectNameEditorProps) { return ( {({ diff --git a/app/client/src/pages/Editor/QueryEditor/QueryEditorContext.tsx b/app/client/src/pages/Editor/QueryEditor/QueryEditorContext.tsx index 03c46a99d0d..c5089fc6bdf 100644 --- a/app/client/src/pages/Editor/QueryEditor/QueryEditorContext.tsx +++ b/app/client/src/pages/Editor/QueryEditor/QueryEditorContext.tsx @@ -1,18 +1,14 @@ import type { ReduxAction } from "ee/constants/ReduxActionConstants"; +import type { SaveActionNameParams } from "PluginActionEditor"; import React, { createContext, useMemo } from "react"; -interface SaveActionNameParams { - id: string; - name: string; -} - interface QueryEditorContextContextProps { moreActionsMenu?: React.ReactNode; onCreateDatasourceClick?: () => void; onEntityNotFoundBackClick?: () => void; changeQueryPage?: (baseQueryId: string) => void; actionRightPaneBackLink?: React.ReactNode; - saveActionName?: ( + saveActionName: ( params: SaveActionNameParams, ) => ReduxAction; closeEditorLink?: React.ReactNode; diff --git a/app/client/src/pages/Editor/QueryEditor/QueryEditorHeader.tsx b/app/client/src/pages/Editor/QueryEditor/QueryEditorHeader.tsx index af1d28c83a9..38f9a9b3d72 100644 --- a/app/client/src/pages/Editor/QueryEditor/QueryEditorHeader.tsx +++ b/app/client/src/pages/Editor/QueryEditor/QueryEditorHeader.tsx @@ -13,6 +13,7 @@ import { useActiveActionBaseId } from "ee/pages/Editor/Explorer/hooks"; import { useSelector } from "react-redux"; import { getActionByBaseId, + getPlugin, getPluginNameFromId, } from "ee/selectors/entitiesSelector"; import { QueryEditorContext } from "./QueryEditorContext"; @@ -21,6 +22,9 @@ import type { Datasource } from "entities/Datasource"; import type { AppState } from "ee/reducers"; import { SQL_DATASOURCES } from "constants/QueryEditorConstants"; import DatasourceSelector from "./DatasourceSelector"; +import { getSavingStatusForActionName } from "selectors/actionSelectors"; +import { getAssetUrl } from "ee/utils/airgapHelpers"; +import { ActionUrlIcon } from "../Explorer/ExplorerIcons"; const NameWrapper = styled.div` display: flex; @@ -79,6 +83,18 @@ const QueryEditorHeader = (props: Props) => { currentActionConfig?.userPermissions, ); + const currentPlugin = useSelector((state: AppState) => + getPlugin(state, currentActionConfig?.pluginId || ""), + ); + + const saveStatus = useSelector((state) => + getSavingStatusForActionName(state, currentActionConfig?.id || ""), + ); + + const iconUrl = getAssetUrl(currentPlugin?.iconLocation) || ""; + + const icon = ActionUrlIcon(iconUrl); + // get the current action's plugin name const currentActionPluginName = useSelector((state: AppState) => getPluginNameFromId(state, currentActionConfig?.pluginId || ""), @@ -106,8 +122,11 @@ const QueryEditorHeader = (props: Props) => { diff --git a/app/client/src/pages/Editor/QueryEditor/index.tsx b/app/client/src/pages/Editor/QueryEditor/index.tsx index f76cbffe1b5..03457a6228e 100644 --- a/app/client/src/pages/Editor/QueryEditor/index.tsx +++ b/app/client/src/pages/Editor/QueryEditor/index.tsx @@ -42,6 +42,7 @@ import { ENTITY_ICON_SIZE, EntityIcon } from "../Explorer/ExplorerIcons"; import { getIDEViewMode } from "selectors/ideSelectors"; import { EditorViewMode } from "ee/entities/IDE/constants"; import { AppPluginActionEditor } from "../AppPluginActionEditor"; +import { saveActionName } from "actions/pluginActionActions"; type QueryEditorProps = RouteComponentProps; @@ -126,6 +127,7 @@ function QueryEditor(props: QueryEditorProps) { }, [ action?.id, action?.name, + action?.pluginType, isChangePermitted, isDeletePermitted, basePageId, @@ -143,7 +145,7 @@ function QueryEditor(props: QueryEditorProps) { changeQuery({ baseQueryId: baseQueryId, basePageId, applicationId }), ); }, - [basePageId, applicationId], + [basePageId, applicationId, dispatch], ); const onCreateDatasourceClick = useCallback(() => { @@ -159,13 +161,7 @@ function QueryEditor(props: QueryEditorProps) { AnalyticsUtil.logEvent("NAVIGATE_TO_CREATE_NEW_DATASOURCE_PAGE", { entryPoint, }); - }, [ - basePageId, - history, - integrationEditorURL, - DatasourceCreateEntryPoints, - AnalyticsUtil, - ]); + }, [basePageId]); // custom function to return user to integrations page if action is not found const onEntityNotFoundBackClick = useCallback( @@ -176,7 +172,7 @@ function QueryEditor(props: QueryEditorProps) { selectedTab: INTEGRATION_TABS.ACTIVE, }), ), - [basePageId, history, integrationEditorURL], + [basePageId], ); const notification = useMemo(() => { @@ -189,7 +185,7 @@ function QueryEditor(props: QueryEditorProps) { withPadding /> ); - }, [action?.name, isConverting]); + }, [action?.name, isConverting, icon]); const isActionRedesignEnabled = useFeatureFlag( FEATURE_FLAG.release_actions_redesign_enabled, @@ -207,6 +203,7 @@ function QueryEditor(props: QueryEditorProps) { notification={notification} onCreateDatasourceClick={onCreateDatasourceClick} onEntityNotFoundBackClick={onEntityNotFoundBackClick} + saveActionName={saveActionName} > Date: Mon, 30 Sep 2024 12:45:35 +0800 Subject: [PATCH 09/13] fix: fixing modal height by removing min-content (#36589) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description min-content css property is behaving weirdly in the latest update of the chrome browser Fixes https://github.com/appsmithorg/appsmith/issues/36586 ## Automation /ok-to-test tags="@tag.Git" ### :mag: Cypress test results > [!TIP] > 🟒 🟒 🟒 All cypress tests have passed! πŸŽ‰ πŸŽ‰ πŸŽ‰ > Workflow run: > Commit: a888acf77730a014fe6d708bf8ed9b48e0cb8029 > Cypress dashboard. > Tags: `@tag.Git` > Spec: >
Sat, 28 Sep 2024 14:34:20 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **Style** - Removed the minimum height constraint from the modal body, allowing for more flexible vertical space allocation in the modal display. --- .../src/pages/Editor/gitSync/Tabs/GitConnectionV2/index.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/app/client/src/pages/Editor/gitSync/Tabs/GitConnectionV2/index.tsx b/app/client/src/pages/Editor/gitSync/Tabs/GitConnectionV2/index.tsx index 78d4c81889d..2212d561a66 100644 --- a/app/client/src/pages/Editor/gitSync/Tabs/GitConnectionV2/index.tsx +++ b/app/client/src/pages/Editor/gitSync/Tabs/GitConnectionV2/index.tsx @@ -32,7 +32,6 @@ const StyledModalBody = styled(ModalBody)` overflow-y: initial; display: flex; flex-direction: column; - min-height: min-content; max-height: calc( 100vh - 200px - 32px - 56px - 44px ); // 200px offset, 32px outer padding, 56px footer, 44px header From 73d3960121a3c79e958cc9fb1aa4978c79fddf45 Mon Sep 17 00:00:00 2001 From: Abhijeet <41686026+abhvsn@users.noreply.github.com> Date: Mon, 30 Sep 2024 10:29:20 +0530 Subject: [PATCH 10/13] chore: Exit the script if server artifact placer is not available (#36569) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description PR to fail fast at the build step only if the script to place the server artifacts for pg and mongo is not available. This PR also replaces pg tag with nightly tag for placing the server artifacts for pg to improve the stability of the image shipped to customers. Fixes: https://github.com/appsmithorg/appsmith/issues/36478 /test Sanity ### :mag: Cypress test results > [!TIP] > 🟒 🟒 🟒 All cypress tests have passed! πŸŽ‰ πŸŽ‰ πŸŽ‰ > Workflow run: > Commit: 40c56218e2cb487cc79bdaab92abc53ded89b4fb > Cypress dashboard. > Tags: `@tag.Sanity` > Spec: >
Thu, 26 Sep 2024 11:23:28 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No ## Summary by CodeRabbit - **Bug Fixes** - Improved error handling for missing scripts in the build and release workflows, ensuring clearer error messages and preventing process failures. - **Chores** - Introduced a new environment variable for using the nightly PostgreSQL Docker image in the release process. --- .github/workflows/github-release.yml | 6 +++++- .github/workflows/test-build-docker-image.yml | 7 +++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/.github/workflows/github-release.yml b/.github/workflows/github-release.yml index c4dd4001beb..cedac1f8481 100644 --- a/.github/workflows/github-release.yml +++ b/.github/workflows/github-release.yml @@ -247,10 +247,14 @@ jobs: run: | scripts/generate_info_json.sh + # As pg docker image is continuously updated for each scheduled cron on release, we are using the nightly tag while building the latest tag - name: Place server artifacts-es run: | if [[ -f scripts/prepare_server_artifacts.sh ]]; then - scripts/prepare_server_artifacts.sh + PG_TAG=nightly scripts/prepare_server_artifacts.sh + else + echo "No script found to prepare server artifacts" + exit 1 fi - name: Login to DockerHub diff --git a/.github/workflows/test-build-docker-image.yml b/.github/workflows/test-build-docker-image.yml index f14c565e210..e3c21798e6a 100644 --- a/.github/workflows/test-build-docker-image.yml +++ b/.github/workflows/test-build-docker-image.yml @@ -356,8 +356,12 @@ jobs: - name: Place server artifacts-es run: | + run: | if [[ -f scripts/prepare_server_artifacts.sh ]]; then scripts/prepare_server_artifacts.sh + else + echo "No script found to prepare server artifacts" + exit 1 fi - name: Set up Depot CLI @@ -439,6 +443,9 @@ jobs: run: | if [[ -f scripts/prepare_server_artifacts.sh ]]; then scripts/prepare_server_artifacts.sh + else + echo "No script found to prepare server artifacts" + exit 1 fi - name: Set up Depot CLI From ba1cc281e4771a437e321b967efa9a92a57fc8bc Mon Sep 17 00:00:00 2001 From: Nidhi Date: Mon, 30 Sep 2024 12:43:20 +0530 Subject: [PATCH 11/13] chore: Switch rate limit key to client_ip (#36606) --- deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs b/deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs index f265b2da71e..75d5e9296f9 100644 --- a/deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs +++ b/deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs @@ -146,7 +146,7 @@ parts.push(` ${isRateLimitingEnabled ? `rate_limit { zone dynamic_zone { - key {http.request.remote_ip} + key {http.request.client_ip} events ${RATE_LIMIT} window 1s } From 43b675eb6db3650a67a6ea93eab8df4fd6e18241 Mon Sep 17 00:00:00 2001 From: Abhijeet <41686026+abhvsn@users.noreply.github.com> Date: Mon, 30 Sep 2024 12:57:47 +0530 Subject: [PATCH 12/13] chore: Add postgres dependency for server to startup (#36585) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description As in the past we have seen the corruption of postgres DB which is being used for temporal we want to make sure we have a retry mechanism in place if: 1. `APPSMITH_DB_URL` is pointing to postgres url 2. Postgres is waiting in recovery mode As per local testing when the docker container is abruptly shutdown via `docker rm -f {container_name}` or `docker kill {container_name}` or even via docker desktop we end up in state where postgres goes into recovery state. logs: ``` 2024-09-27 08:02:49 backend stdout | SQL State : 57P03 2024-09-27 08:02:49 backend stdout | Error Code : 0 2024-09-27 08:02:49 backend stdout | Message : FATAL: the database system is starting up ``` Currently we have implemented polling mechanism, but we will keep looking for better alternative here if we can opt for. Note: 1. With release dump this is taking ~300sec to get out of that state and start accepting the connections. 2. With the existing implementation without retries server dies down within 60sec. ``` INFO exited: backend (exit status 1; not expected) INFO gave up: backend entered FATAL state, too many start retries too quickly ``` Reference doc: https://www.notion.so/appsmith/Postgres-critical-scenarios-668f49c96aef40308e24c2a8d6b1137c /test Sanity ### :mag: Cypress test results > [!TIP] > 🟒 🟒 🟒 All cypress tests have passed! πŸŽ‰ πŸŽ‰ πŸŽ‰ > Workflow run: > Commit: 9dbbe4b22ba12aa82c385ad0eef3cc3d4876f217 > Cypress dashboard. > Tags: `@tag.Sanity` > Spec: >
Mon, 30 Sep 2024 07:26:41 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Introduced new functions for enhanced handling of PostgreSQL database connections, including availability checks and parameter extraction. - Added a new utility script for managing PostgreSQL connections. - **Bug Fixes** - Implemented a retry mechanism for PostgreSQL server availability checks to ensure more reliable connections. - **Tests** - Added unit tests to validate the functionality of the PostgreSQL parameter extraction logic. --- deploy/docker/fs/opt/appsmith/pg-utils.sh | 92 +++++++++++++++++++++++ deploy/docker/fs/opt/appsmith/run-java.sh | 9 +++ deploy/docker/tests/test-pg-utils.sh | 68 +++++++++++++++++ 3 files changed, 169 insertions(+) create mode 100755 deploy/docker/fs/opt/appsmith/pg-utils.sh create mode 100755 deploy/docker/tests/test-pg-utils.sh diff --git a/deploy/docker/fs/opt/appsmith/pg-utils.sh b/deploy/docker/fs/opt/appsmith/pg-utils.sh new file mode 100755 index 00000000000..315446f552d --- /dev/null +++ b/deploy/docker/fs/opt/appsmith/pg-utils.sh @@ -0,0 +1,92 @@ +#!/bin/bash + +waitForPostgresAvailability() { + if [ -z "$PG_DB_HOST" ]; then + tlog "PostgreSQL host name is empty. Check env variables. Error. Exiting java setup" + exit 2 + else + + MAX_RETRIES=50 + RETRYSECONDS=10 + retry_count=0 + while true; do + su postgres -c "pg_isready -h '${PG_DB_HOST}' -p '${PG_DB_PORT}'" + status=$? + + case $status in + 0) + tlog "PostgreSQL host '$PG_DB_HOST' is ready." + break + ;; + 1) + tlog "PostgreSQL host '$PG_DB_HOST' is rejecting connections e.g. due to being in recovery mode or not accepting connections eg. connections maxed out." + ;; + 2) + tlog "PostgreSQL host '$PG_DB_HOST' is not responding or running." + ;; + 3) + tlog "The connection check failed e.g. due to network issues or incorrect parameters." + ;; + *) + tlog "pg_isready exited with unexpected status code: $status" + break + ;; + esac + + retry_count=$((retry_count + 1)) + if [ $retry_count -le $MAX_RETRIES ]; then + tlog "PostgreSQL connection failed. Retrying attempt $retry_count/$MAX_RETRIES in $RETRYSECONDS seconds..." + sleep $RETRYSECONDS + else + tlog "Exceeded maximum retry attempts ($MAX_RETRIES). Exiting." + # use exit code 2 to indicate that the script failed to connect to postgres and supervisor conf is set not to restart the program for 2. + exit 2 + fi + + done + fi +} + +# for PostgreSQL, we use APPSMITH_DB_URL=postgresql://username:password@postgresserver:5432/dbname +# Args: +# conn_string (string): PostgreSQL connection string +# Returns: +# None +# Example: +# postgres syntax +# "postgresql://user:password@localhost:5432/appsmith" +# "postgresql://user:password@localhost/appsmith" +# "postgresql://user@localhost:5432/appsmith" +# "postgresql://user@localhost/appsmith" +extract_postgres_db_params() { + local conn_string=$1 + + # Use node to parse the URI and extract components + IFS=' ' read -r USER PASSWORD HOST PORT DB <<<"$(node -e " + const connectionString = process.argv[1]; + const pgUri = connectionString.startsWith(\"postgresql://\") + ? connectionString + : 'http://' + connectionString; //Prepend a fake scheme for URL parsing + const url = require('url'); + const parsedUrl = new url.URL(pgUri); + + // Extract the pathname and remove the leading '/' + const db = parsedUrl.pathname.substring(1); + + // Default the port to 5432 if it's empty + const port = parsedUrl.port || '5432'; + + console.log(\`\${parsedUrl.username || '-'} \${parsedUrl.password || '-'} \${parsedUrl.hostname} \${port} \${db}\`); + " "$conn_string")" + + # Now, set the environment variables + export PG_DB_USER="$USER" + export PG_DB_PASSWORD="$PASSWORD" + export PG_DB_HOST="$HOST" + export PG_DB_PORT="$PORT" + export PG_DB_NAME="$DB" +} + +# Example usage of the functions +# waitForPostgresAvailability +# extract_postgres_db_params "postgresql://user:password@localhost:5432/dbname" \ No newline at end of file diff --git a/deploy/docker/fs/opt/appsmith/run-java.sh b/deploy/docker/fs/opt/appsmith/run-java.sh index 675c8e26511..ed88e26e119 100755 --- a/deploy/docker/fs/opt/appsmith/run-java.sh +++ b/deploy/docker/fs/opt/appsmith/run-java.sh @@ -1,5 +1,8 @@ #!/bin/bash +# Source the helper script +source pg-utils.sh + set -o errexit set -o pipefail set -o nounset @@ -29,6 +32,12 @@ match-proxy-url() { [[ -n $proxy_host ]] } +# Extract the database parameters from the APPSMITH_DB_URL and wait for the database to be available +if [[ "$mode" == "pg" ]]; then + extract_postgres_db_params "$APPSMITH_DB_URL" + waitForPostgresAvailability +fi + if match-proxy-url "${HTTP_PROXY-}"; then extra_args+=(-Dhttp.proxyHost="$proxy_host" -Dhttp.proxyPort="$proxy_port") if [[ -n $proxy_user ]]; then diff --git a/deploy/docker/tests/test-pg-utils.sh b/deploy/docker/tests/test-pg-utils.sh new file mode 100755 index 00000000000..56bec144560 --- /dev/null +++ b/deploy/docker/tests/test-pg-utils.sh @@ -0,0 +1,68 @@ +#!/usr/bin/env bash + +set -e + +# Include the script to be tested +source /Users/appsmith/Work/appsmith-ce/deploy/docker/fs/opt/appsmith/pg-utils.sh + +assert_equals() { + if [ "$1" != "$2" ]; then + echo "Assertion failed: expected '$2', but got '$1'" + return 1 + fi +} + +# Test extract_postgres_db_params function +test_extract_postgres_db_params_valid_db_string() { + local conn_string="postgresql://user:password@localhost:5432/dbname" + extract_postgres_db_params "$conn_string" + + if [ "$PG_DB_USER" != "user" ] || [ "$PG_DB_PASSWORD" != "password" ] || [ "$PG_DB_HOST" != "localhost" ] || [ "$PG_DB_PORT" != "5432" ] || [ "$PG_DB_NAME" != "dbname" ]; then + echo "Test failed: test_extract_postgres_db_params_valid_db_string did not extract parameters correctly" + echo_params + exit 1 + fi + + echo "Test passed: test_extract_postgres_db_params_valid_db_string" +} + +test_extract_postgres_db_params_empty_dbname() { + local conn_string="postgresql://user:password@localhost:5432" + extract_postgres_db_params "$conn_string" + + if [ "$PG_DB_USER" != "user" ] || [ "$PG_DB_PASSWORD" != "password" ] || [ "$PG_DB_HOST" != "localhost" ] || [ "$PG_DB_PORT" != "5432" ] || [ "$PG_DB_NAME" != "" ]; then + echo "Test failed: test_extract_postgres_db_params_empty_dbname did not extract parameters correctly" + echo_params + exit 1 + fi + + echo "Test passed: test_extract_postgres_db_params_empty_dbname" +} + +test_extract_postgres_db_params_with_spaces() { + local conn_string="postgresql://user:p a s s w o r d@localhost:5432/db_name" + extract_postgres_db_params "$conn_string" + + if [ "$PG_DB_USER" != "user" ] || [ "$PG_DB_PASSWORD" != "p%20a%20s%20s%20w%20o%20r%20d" ] || [ "$PG_DB_HOST" != "localhost" ] || [ "$PG_DB_PORT" != "5432" ] || [ "$PG_DB_NAME" != "db_name" ]; then + echo "Test failed: test_extract_postgres_db_params_with_spaces did not extract parameters correctly" + echo_params + exit 1 + fi + + echo "Test passed: test_extract_postgres_db_params_with_spaces" +} + +echo_params() { + echo "PG_DB_USER: $PG_DB_USER" + echo "PG_DB_PASSWORD: $PG_DB_PASSWORD" + echo "PG_DB_HOST: $PG_DB_HOST" + echo "PG_DB_PORT: $PG_DB_PORT" + echo "PG_DB_NAME: $PG_DB_NAME" +} + +# Run tests +test_extract_postgres_db_params_valid_db_string +test_extract_postgres_db_params_empty_dbname +test_extract_postgres_db_params_with_spaces + +echo "All Tests Pass!" \ No newline at end of file From 6f27959bce2beb779bccc4d72408121ad33d8b6b Mon Sep 17 00:00:00 2001 From: Jacques Ikot Date: Mon, 30 Sep 2024 10:02:42 +0100 Subject: [PATCH 13/13] fix: Invalid Date Display in Table Widget's Date Column When Using Unix Timestamp (ms) (#36455) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description **Problem** When populating a table widget with data that includes dates in Unix timestamp (milliseconds) format, setting the column type to "Date," the input format to "Unix timestamp (ms)," and selecting a display format leads to an issue during inline editing. While the date picker behaves correctly, selecting a new date results in the table cell showing an "Invalid Date" error. **Root Cause** The platform currently uses DateInputFormat.MILLISECONDS for Unix timestamp (ms) formatting. However, this value is not a valid option for the moment.format() function, which expects the input format to be 'x' for Unix timestamps in milliseconds. This mismatch leads to the "Invalid Date" error. **Solution** Modify the logic to map DateInputFormat.MILLISECONDS to the correct moment format string 'x'. Adjust the table's transformDataPureFn to correctly process and display dates in Unix timestamp (ms) format after inline edits, ensuring the moment library can handle the input properly. **Outcome** This fix ensures that when a user selects a date via the date picker in inline editing mode, the selected date is displayed correctly in the table without any errors. Fixes #35631, #25081 ## Automation /ok-to-test tags="@tag.Sanity, @tag.Binding, @tag.Table, @tag.Datepicker" ### :mag: Cypress test results > [!TIP] > 🟒 🟒 🟒 All cypress tests have passed! πŸŽ‰ πŸŽ‰ πŸŽ‰ > Workflow run: > Commit: 6a3cae774f3824bd2ee126b501bfa4b6d71ae0c8 > Cypress dashboard. > Tags: `@tag.Sanity, @tag.Binding, @tag.Table, @tag.Datepicker` > Spec: >
Mon, 30 Sep 2024 08:54:58 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Enhanced date column editing in table widgets to accept Unix timestamps in milliseconds without errors. - Introduced a new enumeration for improved date formatting options. - Added mock data structures for testing various date formats and transformations in the table widget. - New method for generating formatted date strings for tomorrow in both verbose and ISO formats. - **Bug Fixes** - Improved validation logic for date inputs in the table, ensuring proper handling of different date formats. - **Tests** - Added new test cases to validate date handling and input formats in the table widget. - Introduced a new test suite for transforming table data based on specified column metadata. --- .../Date_column_types_validation_spec.ts | 149 ++++++++++++++++++ .../cypress/fixtures/tableDateColumnTypes.ts | 26 +++ app/client/cypress/support/Pages/Table.ts | 34 ++++ .../component/cellComponents/DateCell.tsx | 30 +++- .../src/widgets/TableWidgetV2/constants.ts | 5 + .../TransformDataPureFn.test.ts | 54 +++++++ .../widget/reactTableUtils/fixtures.ts | 145 +++++++++++++++++ .../reactTableUtils/transformDataPureFn.tsx | 5 +- 8 files changed, 440 insertions(+), 8 deletions(-) create mode 100644 app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/Date_column_types_validation_spec.ts create mode 100644 app/client/cypress/fixtures/tableDateColumnTypes.ts create mode 100644 app/client/src/widgets/TableWidgetV2/widget/reactTableUtils/TransformDataPureFn.test.ts create mode 100644 app/client/src/widgets/TableWidgetV2/widget/reactTableUtils/fixtures.ts diff --git a/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/Date_column_types_validation_spec.ts b/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/Date_column_types_validation_spec.ts new file mode 100644 index 00000000000..0ce8bc321a3 --- /dev/null +++ b/app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/Date_column_types_validation_spec.ts @@ -0,0 +1,149 @@ +import { tableDateColumnTypes } from "../../../../../fixtures/tableDateColumnTypes"; +import { + agHelper, + entityExplorer, + propPane, + table, +} from "../../../../../support/Objects/ObjectsCore"; + +import EditorNavigation, { + EntityType, +} from "../../../../../support/Pages/EditorNavigation"; + +describe( + "Table widget date column type validation", + { tags: ["@tag.Widget", "@tag.Table"] }, + () => { + before(() => { + entityExplorer.DragNDropWidget("tablewidgetv2", 350, 500); + EditorNavigation.SelectEntityByName("Table1", EntityType.Widget); + propPane.ToggleJSMode("Table data", true); + propPane.UpdatePropertyFieldValue("Table data", tableDateColumnTypes); + table.EditColumn("unixs", "v2"); + }); + + beforeEach(() => { + propPane.NavigateBackToPropertyPane(false); + }); + + const setEditableDateFormats = (format: string) => { + // Update date format property + propPane.ToggleJSMode("Date format", true); + propPane.UpdatePropertyFieldValue("Date format", format); + + // Update display format property + propPane.ToggleJSMode("Display format", true); + propPane.UpdatePropertyFieldValue("Display format", "YYYY-MM-DD"); + + // Toggle editable + propPane.TogglePropertyState("Editable", "On"); + }; + + const clickAndValidateDateCell = (row: number, column: number) => { + // Click unix cell edit + table.ClickOnEditIcon(row, column); + + // Click on specific date within + agHelper.GetNClick( + `${table._dateInputPopover} [aria-label='${table.getFormattedTomorrowDates().verboseFormat}']`, + ); + + // Check that date is set in column + table + .ReadTableRowColumnData(row, column, "v2") + .then((val) => + expect(val).to.equal(table.getFormattedTomorrowDates().isoFormat), + ); + }; + + it("1. should allow inline editing of Unix Timestamp in seconds (unix/s)", () => { + table.ChangeColumnType("unixs", "Date"); + setEditableDateFormats("Epoch"); + clickAndValidateDateCell(0, 0); + }); + + it("2. should allow inline editing of Unix Timestamp in milliseconds (unix/ms)", () => { + table.ChangeColumnType("unixms", "Date"); + setEditableDateFormats("Milliseconds"); + clickAndValidateDateCell(0, 1); + }); + + it("3. should allow inline editing of date in YYYY-MM-DD format", () => { + table.EditColumn("yyyymmdd", "v2"); + setEditableDateFormats("YYYY-MM-DD"); + clickAndValidateDateCell(0, 2); + }); + + it("4. should allow inline editing of date in YYYY-MM-DD HH:mm format", () => { + table.EditColumn("yyyymmddhhmm", "v2"); + setEditableDateFormats("YYYY-MM-DD HH:mm"); + clickAndValidateDateCell(0, 3); + }); + + it("5. should allow inline editing of date in ISO 8601 format (YYYY-MM-DDTHH:mm:ss)", () => { + table.EditColumn("iso8601", "v2"); + setEditableDateFormats("YYYY-MM-DDTHH:mm:ss"); + clickAndValidateDateCell(0, 4); + }); + + it("6. should allow inline editing of date in YYYY-MM-DD HH:mm format", () => { + table.EditColumn("yyyymmddTHHmmss", "v2"); + setEditableDateFormats("YYYY-MM-DD HH:mm"); + clickAndValidateDateCell(0, 5); + }); + + it("7. should allow inline editing of date in 'do MMM yyyy' format", () => { + table.ChangeColumnType("yyyymmddhhmmss", "Date"); + setEditableDateFormats("YYYY-MM-DDTHH:mm:ss"); + clickAndValidateDateCell(0, 6); + }); + + it("8. should allow inline editing of date in DD/MM/YYYY format", () => { + table.ChangeColumnType("doMMMyyyy", "Date"); + setEditableDateFormats("Do MMM YYYY"); + clickAndValidateDateCell(0, 7); + }); + + it("9. should allow inline editing of date in DD/MM/YYYY HH:mm format", () => { + table.EditColumn("ddmmyyyy", "v2"); + setEditableDateFormats("DD/MM/YYYY"); + clickAndValidateDateCell(0, 8); + }); + + it("10. should allow inline editing of date in LLL (Month Day, Year Time) format", () => { + table.EditColumn("ddmmyyyyhhmm", "v2"); + setEditableDateFormats("DD/MM/YYYY HH:mm"); + clickAndValidateDateCell(0, 9); + }); + + it("11. should allow inline editing of date in LL (Month Day, Year) format", () => { + table.EditColumn("lll", "v2"); + setEditableDateFormats("LLL"); + clickAndValidateDateCell(0, 10); + }); + + it("12. should allow inline editing of date in 'D MMMM, YYYY' format", () => { + table.EditColumn("ll", "v2"); + setEditableDateFormats("LL"); + clickAndValidateDateCell(0, 11); + }); + + it("13. should allow inline editing of date in 'h:mm A D MMMM, YYYY' format", () => { + table.EditColumn("dmmmmyyyy", "v2"); + setEditableDateFormats("D MMMM, YYYY"); + clickAndValidateDateCell(0, 12); + }); + + it("14. should allow inline editing of date in MM-DD-YYYY format", () => { + table.EditColumn("hmmAdmmmmyyyy", "v2"); + setEditableDateFormats("H:mm A D MMMM, YYYY"); + clickAndValidateDateCell(0, 13); + }); + + it("15. should allow inline editing of date in DD-MM-YYYY format", () => { + table.EditColumn("mm1dd1yyyy", "v2"); + setEditableDateFormats("MM-DD-YYYY"); + clickAndValidateDateCell(0, 14); + }); + }, +); diff --git a/app/client/cypress/fixtures/tableDateColumnTypes.ts b/app/client/cypress/fixtures/tableDateColumnTypes.ts new file mode 100644 index 00000000000..614b36b5e11 --- /dev/null +++ b/app/client/cypress/fixtures/tableDateColumnTypes.ts @@ -0,0 +1,26 @@ +export const tableDateColumnTypes = ` +{{ + [ + { + "unixs": 1727212200, + "unixms": 1727212200000, + "yyyymmdd": "2024-09-25", + "yyyymmddhhmm": "2024-09-25 14:30", + iso8601: "2024-09-25T14:30:00.000Z", + "yyyymmddTHHmmss": "2024-09-25T14:30:00", + "yyyymmddhhmmss": "2024-09-25 02:30:00", + "doMMMyyyy": "25th Sep 2024", + "ddmmyyyy": "25/09/2024", + "ddmmyyyyhhmm": "25/09/2024 14:30", + lll: "September 25, 2024 2:30 PM", + ll: "September 25, 2024", + "dmmmmyyyy": "25 September, 2024", + "hmmAdmmmmyyyy": "2:30 PM 25 September, 2024", + "mm1dd1yyyy": "09-25-2024", + "dd1mm1yyyy": "25-09-2024", + "ddimmiyy": "25/09/24", + "mmddyy": "09/25/24", + }, + ] +}} +`; diff --git a/app/client/cypress/support/Pages/Table.ts b/app/client/cypress/support/Pages/Table.ts index d1439a58927..45dd2132ded 100644 --- a/app/client/cypress/support/Pages/Table.ts +++ b/app/client/cypress/support/Pages/Table.ts @@ -854,4 +854,38 @@ export class Table { this.agHelper.GetHoverNClick(selector, 1, true); verify && cy.get(selector).eq(1).should("be.disabled"); } + + /** + * Helper function to get formatted date strings for tomorrow's date. + * + * @returns {Object} An object containing: + * - verbose format (e.g., "Sat Sep 21 2024") + * - ISO date format (e.g., "2024-09-21") + */ + public getFormattedTomorrowDates() { + // Create a new Date object for today + const tomorrow = new Date(); + + // Set the date to tomorrow by adding 1 to today's date + tomorrow.setDate(tomorrow.getDate() + 1); + + // Format tomorrow's date in verbose form (e.g., "Sat Sep 21 2024") + const verboseFormat = tomorrow + .toLocaleDateString("en-US", { + weekday: "short", + year: "numeric", + month: "short", + day: "2-digit", + }) + .replace(/,/g, ""); // Remove commas from the formatted string + + // Format tomorrow's date in ISO form (e.g., "2024-09-21") + const isoFormat = tomorrow.toISOString().split("T")[0]; // Extract the date part only + + // Return both formatted date strings as an object + return { + verboseFormat, + isoFormat, + }; + } } diff --git a/app/client/src/widgets/TableWidgetV2/component/cellComponents/DateCell.tsx b/app/client/src/widgets/TableWidgetV2/component/cellComponents/DateCell.tsx index 17f5c679bf4..8e34d928d86 100644 --- a/app/client/src/widgets/TableWidgetV2/component/cellComponents/DateCell.tsx +++ b/app/client/src/widgets/TableWidgetV2/component/cellComponents/DateCell.tsx @@ -9,7 +9,11 @@ import DateComponent from "widgets/DatePickerWidget2/component"; import { TimePrecision } from "widgets/DatePickerWidget2/constants"; import type { RenderDefaultPropsType } from "./PlainTextCell"; import styled from "styled-components"; -import { EditableCellActions } from "widgets/TableWidgetV2/constants"; +import { + DateInputFormat, + EditableCellActions, + MomentDateInputFormat, +} from "widgets/TableWidgetV2/constants"; import { ISO_DATE_FORMAT } from "constants/WidgetValidation"; import moment from "moment"; import { BasicCell } from "./BasicCell"; @@ -196,6 +200,19 @@ export const DateCell = (props: DateComponentProps) => { const [isValid, setIsValid] = useState(true); const [showRequiredError, setShowRequiredError] = useState(false); const contentRef = useRef(null); + + const convertInputFormatToMomentFormat = (inputFormat: string) => { + let momentAdjustedInputFormat = inputFormat; + + if (inputFormat === DateInputFormat.MILLISECONDS) { + momentAdjustedInputFormat = MomentDateInputFormat.MILLISECONDS; + } else if (inputFormat === DateInputFormat.EPOCH) { + momentAdjustedInputFormat = MomentDateInputFormat.SECONDS; + } + + return momentAdjustedInputFormat; + }; + const isCellCompletelyValid = useMemo( () => isEditableCellValid && isValid, [isEditableCellValid, isValid], @@ -218,8 +235,15 @@ export const DateCell = (props: DateComponentProps) => { }, [value, props.outputFormat]); const onDateSelected = (date: string) => { + const momentAdjustedInputFormat = + convertInputFormatToMomentFormat(inputFormat); + + const formattedDate = date + ? moment(date).format(momentAdjustedInputFormat) + : ""; + if (isNewRow) { - updateNewRowValues(alias, date, date); + updateNewRowValues(alias, date, formattedDate); return; } @@ -235,8 +259,6 @@ export const DateCell = (props: DateComponentProps) => { setShowRequiredError(false); setHasFocus(false); - const formattedDate = date ? moment(date).format(inputFormat) : ""; - onDateSave(rowIndex, alias, formattedDate, onDateSelectedString); }; diff --git a/app/client/src/widgets/TableWidgetV2/constants.ts b/app/client/src/widgets/TableWidgetV2/constants.ts index 0e066c83f9a..8fb0e8605aa 100644 --- a/app/client/src/widgets/TableWidgetV2/constants.ts +++ b/app/client/src/widgets/TableWidgetV2/constants.ts @@ -219,6 +219,11 @@ export enum DateInputFormat { MILLISECONDS = "Milliseconds", } +export enum MomentDateInputFormat { + MILLISECONDS = "x", + SECONDS = "X", +} + export const defaultEditableCell: EditableCell = { column: "", index: -1, diff --git a/app/client/src/widgets/TableWidgetV2/widget/reactTableUtils/TransformDataPureFn.test.ts b/app/client/src/widgets/TableWidgetV2/widget/reactTableUtils/TransformDataPureFn.test.ts new file mode 100644 index 00000000000..d95d92df657 --- /dev/null +++ b/app/client/src/widgets/TableWidgetV2/widget/reactTableUtils/TransformDataPureFn.test.ts @@ -0,0 +1,54 @@ +import type { ReactTableColumnProps } from "widgets/TableWidgetV2/component/Constants"; +import { + columns, + columnsNonDate, + expectedDataNonDate, + tableDataNonDate, +} from "./fixtures"; +import { transformDataPureFn } from "./transformDataPureFn"; + +describe("transformDataPureFn", () => { + it("should handle invalid date values", () => { + const invalidTableData = [ + { + epoch: "invalid_epoch", + milliseconds: "invalid_milliseconds", + iso_8601: "invalid_iso_8601", + yyyy_mm_dd: "invalid_date", + lll: "invalid_date", + }, + ]; + + const expectedInvalidData = [ + { + epoch: "Invalid date", + milliseconds: "Invalid date", + iso_8601: "8601-01-01", + yyyy_mm_dd: "Invalid date", + lll: "Invalid date", + }, + ]; + + const result = transformDataPureFn( + invalidTableData, + columns as ReactTableColumnProps[], + ); + + expect(result).toEqual(expectedInvalidData); + }); + + it("should return an empty array when tableData is empty", () => { + const result = transformDataPureFn([], columns as ReactTableColumnProps[]); + + expect(result).toEqual([]); + }); + + it("should not transform non-date data", () => { + const result = transformDataPureFn( + tableDataNonDate, + columnsNonDate as ReactTableColumnProps[], + ); + + expect(result).toEqual(expectedDataNonDate); + }); +}); diff --git a/app/client/src/widgets/TableWidgetV2/widget/reactTableUtils/fixtures.ts b/app/client/src/widgets/TableWidgetV2/widget/reactTableUtils/fixtures.ts new file mode 100644 index 00000000000..75355f2f4b9 --- /dev/null +++ b/app/client/src/widgets/TableWidgetV2/widget/reactTableUtils/fixtures.ts @@ -0,0 +1,145 @@ +import { ColumnTypes, DateInputFormat } from "widgets/TableWidgetV2/constants"; + +// Mock columns data +export const columns = [ + { + alias: "epoch", + metaProperties: { + type: ColumnTypes.DATE, + inputFormat: DateInputFormat.EPOCH, + format: "YYYY-MM-DD", + }, + }, + { + alias: "milliseconds", + metaProperties: { + type: ColumnTypes.DATE, + inputFormat: DateInputFormat.MILLISECONDS, + format: "YYYY-MM-DD", + }, + }, + { + alias: "iso_8601", + metaProperties: { + type: ColumnTypes.DATE, + inputFormat: "YYYY-MM-DDTHH:mm:ss.SSSZ", + format: "YYYY-MM-DD", + }, + }, + { + alias: "yyyy_mm_dd", + metaProperties: { + type: ColumnTypes.DATE, + inputFormat: "YYYY-MM-DD", + format: "YYYY-MM-DD", + }, + }, + { + alias: "lll", + metaProperties: { + type: ColumnTypes.DATE, + inputFormat: "LLL", + format: "YYYY-MM-DD", + }, + }, +]; + +// Mock table data +export const tableData = [ + { + epoch: 1727132400, + milliseconds: 1727132400000, + iso_8601: "2024-09-24T00:00:00.000+01:00", + yyyy_mm_dd: "2024-09-24", + lll: "September 25, 2024 12:00 AM", + }, + { + epoch: 1726980974, + milliseconds: 1726980974328, + iso_8601: "2024-09-23T09:01:53.350627", + yyyy_mm_dd: "2024-09-23", + lll: "Sep 23, 2024 09:01", + }, +]; + +// Expected transformed data +export const expectedData = [ + { + epoch: "2024-09-24", // Converted from epoch to date + milliseconds: "2024-09-24", // Converted from milliseconds to date + iso_8601: "2024-09-24", // ISO 8601 to date + yyyy_mm_dd: "2024-09-24", // No transformation needed + lll: "2024-09-25", // LLL format to date + }, + { + epoch: "2024-09-22", // Converted from epoch to date + milliseconds: "2024-09-22", // Converted from milliseconds to date + iso_8601: "2024-09-23", // ISO 8601 to date + yyyy_mm_dd: "2024-09-23", // No transformation needed + lll: "2024-09-23", // LLL format to date + }, +]; + +// Mock columns for non-date data +export const columnsNonDate = [ + { + id: "role", + alias: "role", + metaProperties: { + type: ColumnTypes.NUMBER, + format: "", + inputFormat: "", + decimals: 0, + }, + }, + { + id: "id", + alias: "id", + metaProperties: { + type: ColumnTypes.NUMBER, + format: "", + inputFormat: "", + decimals: 0, + }, + }, + { + id: "name", + alias: "name", + metaProperties: { + type: ColumnTypes.TEXT, + format: "", + inputFormat: "", + decimals: 0, + }, + }, +]; + +// Mock table data for non-date transformation +export const tableDataNonDate = [ + { + role: 1, + id: 1, + name: "Alice Johnson", + __originalIndex__: 0, + }, + { + role: 2, + id: 2, + name: "Bob Smith", + __originalIndex__: 1, + }, +]; + +// Expected transformed data for non-date columns +export const expectedDataNonDate = [ + { + role: 1, + id: 1, + name: "Alice Johnson", + }, + { + role: 2, + id: 2, + name: "Bob Smith", + }, +]; diff --git a/app/client/src/widgets/TableWidgetV2/widget/reactTableUtils/transformDataPureFn.tsx b/app/client/src/widgets/TableWidgetV2/widget/reactTableUtils/transformDataPureFn.tsx index 4432568c620..1454694ce8b 100644 --- a/app/client/src/widgets/TableWidgetV2/widget/reactTableUtils/transformDataPureFn.tsx +++ b/app/client/src/widgets/TableWidgetV2/widget/reactTableUtils/transformDataPureFn.tsx @@ -1,7 +1,7 @@ import log from "loglevel"; import type { MomentInput } from "moment"; import moment from "moment"; -import _, { isNumber, isNil, isArray } from "lodash"; +import _, { isNil, isArray } from "lodash"; import type { EditableCell } from "../../constants"; import { ColumnTypes, DateInputFormat } from "../../constants"; import type { ReactTableColumnProps } from "../../component/Constants"; @@ -10,7 +10,6 @@ import shallowEqual from "shallowequal"; export type tableData = Array>; -//TODO: (Vamsi) need to unit test this function export const transformDataPureFn = ( tableData: Array>, columns: ReactTableColumnProps[], @@ -45,8 +44,6 @@ export const transformDataPureFn = ( ) { inputFormat = type; moment(value as MomentInput, inputFormat); - } else if (!isNumber(value)) { - isValidDate = false; } } catch (e) { isValidDate = false;