From 1f72d758c06f4b87876511070c51f6b9a095885f Mon Sep 17 00:00:00 2001 From: Nidhi Date: Mon, 21 Oct 2024 17:45:12 +0530 Subject: [PATCH 1/9] chore: Allow all file I/Os for git in parallel (#36872) --- .../appsmith/git/files/FileUtilsCEImpl.java | 88 ++++++++++--------- 1 file changed, 46 insertions(+), 42 deletions(-) diff --git a/app/server/appsmith-git/src/main/java/com/appsmith/git/files/FileUtilsCEImpl.java b/app/server/appsmith-git/src/main/java/com/appsmith/git/files/FileUtilsCEImpl.java index 23891664b741..d919338cef95 100644 --- a/app/server/appsmith-git/src/main/java/com/appsmith/git/files/FileUtilsCEImpl.java +++ b/app/server/appsmith-git/src/main/java/com/appsmith/git/files/FileUtilsCEImpl.java @@ -47,6 +47,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Pattern; import static com.appsmith.external.git.constants.GitConstants.ACTION_COLLECTION_LIST; @@ -243,7 +244,7 @@ protected Set updateEntitiesInRepo(ApplicationGitReference applicationGi Set validPages = new HashSet<>(); for (Map.Entry pageResource : pageEntries) { - Map validWidgetToParentMap = new HashMap<>(); + Map validWidgetToParentMap = new ConcurrentHashMap<>(); final String pageName = pageResource.getKey(); Path pageSpecificDirectory = pageDirectory.resolve(pageName); boolean isResourceUpdated = @@ -255,7 +256,8 @@ protected Set updateEntitiesInRepo(ApplicationGitReference applicationGi pageSpecificDirectory.resolve(pageName + CommonConstants.JSON_EXTENSION)); Map result = DSLTransformerHelper.flatten( new JSONObject(applicationGitReference.getPageDsl().get(pageName))); - result.forEach((key, jsonObject) -> { + result.keySet().parallelStream().forEach(key -> { + JSONObject jsonObject = result.get(key); String widgetName = key.substring(key.lastIndexOf(CommonConstants.DELIMITER_POINT) + 1); String childPath = DSLTransformerHelper.getPathToWidgetFile(key, jsonObject, widgetName); @@ -291,8 +293,8 @@ protected Set updateEntitiesInRepo(ApplicationGitReference applicationGi Path jsLibDirectory = baseRepo.resolve(JS_LIB_DIRECTORY); Set> jsLibEntries = applicationGitReference.getJsLibraries().entrySet(); - Set validJsLibs = new HashSet<>(); - jsLibEntries.forEach(jsLibEntry -> { + Set validJsLibs = ConcurrentHashMap.newKeySet(); + jsLibEntries.parallelStream().forEach(jsLibEntry -> { String uidString = jsLibEntry.getKey(); boolean isResourceUpdated = modifiedResources.isResourceUpdated(CUSTOM_JS_LIB_LIST, uidString); @@ -308,18 +310,17 @@ protected Set updateEntitiesInRepo(ApplicationGitReference applicationGi } // Create HashMap for valid actions and actionCollections - HashMap> validActionsMap = new HashMap<>(); - HashMap> validActionCollectionsMap = new HashMap<>(); + ConcurrentHashMap> validActionsMap = new ConcurrentHashMap<>(); + ConcurrentHashMap> validActionCollectionsMap = new ConcurrentHashMap<>(); validPages.forEach(validPage -> { - validActionsMap.put(validPage, new HashSet<>()); - validActionCollectionsMap.put(validPage, new HashSet<>()); + validActionsMap.put(validPage, ConcurrentHashMap.newKeySet()); + validActionCollectionsMap.put(validPage, ConcurrentHashMap.newKeySet()); }); // Save actions - for (Map.Entry resource : - applicationGitReference.getActions().entrySet()) { - // queryName_pageName => nomenclature for the keys - // TODO queryName => for app level queries, this is not implemented yet + // queryName_pageName => nomenclature for the keys + // TODO queryName => for app level queries, this is not implemented yet + applicationGitReference.getActions().entrySet().parallelStream().forEach(resource -> { String[] names = resource.getKey().split(NAME_SEPARATOR); if (names.length > 1 && StringUtils.hasLength(names[1])) { // For actions, we are referring to validNames to maintain unique file names as just name @@ -349,7 +350,7 @@ protected Set updateEntitiesInRepo(ApplicationGitReference applicationGi .resolve(queryName + CommonConstants.JSON_EXTENSION)); } } - } + }); validActionsMap.forEach((pageName, validActionNames) -> { Path pageSpecificDirectory = pageDirectory.resolve(pageName); @@ -358,35 +359,38 @@ protected Set updateEntitiesInRepo(ApplicationGitReference applicationGi }); // Save JSObjects - for (Map.Entry resource : - applicationGitReference.getActionCollections().entrySet()) { - // JSObjectName_pageName => nomenclature for the keys - // TODO JSObjectName => for app level JSObjects, this is not implemented yet - String[] names = resource.getKey().split(NAME_SEPARATOR); - if (names.length > 1 && StringUtils.hasLength(names[1])) { - final String actionCollectionName = names[0]; - final String pageName = names[1]; - Path pageSpecificDirectory = pageDirectory.resolve(pageName); - Path actionCollectionSpecificDirectory = pageSpecificDirectory.resolve(ACTION_COLLECTION_DIRECTORY); - - if (!validActionCollectionsMap.containsKey(pageName)) { - validActionCollectionsMap.put(pageName, new HashSet<>()); - } - validActionCollectionsMap.get(pageName).add(actionCollectionName); - boolean isResourceUpdated = modifiedResources != null - && modifiedResources.isResourceUpdated(ACTION_COLLECTION_LIST, resource.getKey()); - if (Boolean.TRUE.equals(isResourceUpdated)) { - saveActionCollection( - resource.getValue(), - applicationGitReference.getActionCollectionBody().get(resource.getKey()), - actionCollectionName, - actionCollectionSpecificDirectory.resolve(actionCollectionName)); - // Delete the resource from the old file structure v2 - fileOperations.deleteFile(actionCollectionSpecificDirectory.resolve( - actionCollectionName + CommonConstants.JSON_EXTENSION)); - } - } - } + // JSObjectName_pageName => nomenclature for the keys + // TODO JSObjectName => for app level JSObjects, this is not implemented yet + applicationGitReference.getActionCollections().entrySet().parallelStream() + .forEach(resource -> { + String[] names = resource.getKey().split(NAME_SEPARATOR); + if (names.length > 1 && StringUtils.hasLength(names[1])) { + final String actionCollectionName = names[0]; + final String pageName = names[1]; + Path pageSpecificDirectory = pageDirectory.resolve(pageName); + Path actionCollectionSpecificDirectory = + pageSpecificDirectory.resolve(ACTION_COLLECTION_DIRECTORY); + + if (!validActionCollectionsMap.containsKey(pageName)) { + validActionCollectionsMap.put(pageName, new HashSet<>()); + } + validActionCollectionsMap.get(pageName).add(actionCollectionName); + boolean isResourceUpdated = modifiedResources != null + && modifiedResources.isResourceUpdated(ACTION_COLLECTION_LIST, resource.getKey()); + if (Boolean.TRUE.equals(isResourceUpdated)) { + saveActionCollection( + resource.getValue(), + applicationGitReference + .getActionCollectionBody() + .get(resource.getKey()), + actionCollectionName, + actionCollectionSpecificDirectory.resolve(actionCollectionName)); + // Delete the resource from the old file structure v2 + fileOperations.deleteFile(actionCollectionSpecificDirectory.resolve( + actionCollectionName + CommonConstants.JSON_EXTENSION)); + } + } + }); // Verify if the old files are deleted validActionCollectionsMap.forEach((pageName, validActionCollectionNames) -> { From 67046f869320fd0a300046019481d585ee1c975d Mon Sep 17 00:00:00 2001 From: Ashit Rath Date: Tue, 22 Oct 2024 10:48:03 +0530 Subject: [PATCH 2/9] chore: Split library side pane for adding package control section (#36926) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Purpose of this PR is to split the Libraries section to extend it in EE and introduced a tab layout that has both js libraries and packages Changes: 1. Increased width of Libraries pane to 384px 2. Added a route `/package` which opens the same as library 3. Extracted the JSLibraries section to be reused as a tab panel body in EE 4. Created `SidePaneWrapper` common component to be reused in EE 5. Split the title of the Libraries section under a hook to return empty string if package feature flag is enabled. 6. Minor linting refactors. In CE there is only one visual change i.e the increase in width of the Libraries side pane from 250px to 384px [Figma](https://www.figma.com/design/Z0QsSdGOydURn6WIMQ3rHM/Appsmith-Reusability?node-id=6816-319511&node-type=frame&t=x0DzN9HIxjefe3J1-0) PR for https://github.com/appsmithorg/appsmith-ee/pull/5362 ## Automation /ok-to-test tags="@tag.All" ### :mag: Cypress test results > [!TIP] > 🟒 🟒 🟒 All cypress tests have passed! πŸŽ‰ πŸŽ‰ πŸŽ‰ > Workflow run: > Commit: ec76cc569ea90ba55e451f5ab18fa85244ac076f > Cypress dashboard. > Tags: `@tag.All` > Spec: >
Mon, 21 Oct 2024 08:54:31 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit ## Release Notes - **New Features** - Introduced new functions for constructing URLs for application libraries and packages. - Added new route constants and matching functions for improved routing of application packages and libraries. - Implemented a custom hook for dynamic header titles in the libraries section. - Added a new `LibrarySidePane` component to enhance the editor interface. - Introduced a new `SidePaneWrapper` component for better layout management. - **Bug Fixes** - Enhanced entity identification logic to recognize both libraries and packages. - **Chores** - Updated routing logic for improved maintainability. - Added constants for configurable UI component widths. These updates aim to enhance user experience and improve the overall functionality of the application. --- .../src/IDE/Components/SidePaneWrapper.tsx | 29 +++++++++++++++ app/client/src/IDE/index.ts | 5 +++ app/client/src/ce/RouteBuilder.ts | 9 +++++ .../src/ce/constants/routes/appRoutes.ts | 7 ++++ .../src/ce/entities/Engine/actionHelpers.ts | 1 + .../IDE/Header/useLibraryHeaderTitle.ts | 12 +++++++ .../Editor/IDE/LeftPane/LibrarySidePane.tsx | 13 +++++++ .../ce/pages/Editor/IDE/MainPane/useRoutes.ts | 2 ++ app/client/src/constants/AppConstants.ts | 1 + .../IDE/Header/useLibraryHeaderTitle.ts | 3 ++ .../Editor/IDE/LeftPane/LibrarySidePane.tsx | 3 ++ .../src/entities/Engine/AppEditorEngine.ts | 15 ++++++-- app/client/src/navigation/FocusEntity.ts | 5 ++- .../src/pages/Editor/IDE/Header/index.tsx | 10 +++--- .../IDE/Layout/hooks/useGridLayoutTemplate.ts | 5 +-- ...arySidePane.tsx => JSLibrariesSection.tsx} | 32 +++++++---------- .../src/pages/Editor/IDE/LeftPane/index.tsx | 36 ++++++++++++------- 17 files changed, 145 insertions(+), 43 deletions(-) create mode 100644 app/client/src/IDE/Components/SidePaneWrapper.tsx create mode 100644 app/client/src/ce/pages/Editor/IDE/Header/useLibraryHeaderTitle.ts create mode 100644 app/client/src/ce/pages/Editor/IDE/LeftPane/LibrarySidePane.tsx create mode 100644 app/client/src/ee/pages/Editor/IDE/Header/useLibraryHeaderTitle.ts create mode 100644 app/client/src/ee/pages/Editor/IDE/LeftPane/LibrarySidePane.tsx rename app/client/src/pages/Editor/IDE/LeftPane/{LibrarySidePane.tsx => JSLibrariesSection.tsx} (56%) diff --git a/app/client/src/IDE/Components/SidePaneWrapper.tsx b/app/client/src/IDE/Components/SidePaneWrapper.tsx new file mode 100644 index 000000000000..5c1eaf1d88f7 --- /dev/null +++ b/app/client/src/IDE/Components/SidePaneWrapper.tsx @@ -0,0 +1,29 @@ +import { Flex } from "@appsmith/ads"; +import React from "react"; +import type { ReactNode } from "react"; +import styled from "styled-components"; + +interface SidePaneContainerProps { + children?: ReactNode; + padded?: boolean; +} + +const StyledContainer = styled(Flex)>` + padding: ${({ padded }) => padded && "var(--ads-v2-spaces-2)"}; +`; + +function SidePaneWrapper({ children, padded = false }: SidePaneContainerProps) { + return ( + + {children} + + ); +} + +export default SidePaneWrapper; diff --git a/app/client/src/IDE/index.ts b/app/client/src/IDE/index.ts index 358b1f2da97b..02e8aaf7ed86 100644 --- a/app/client/src/IDE/index.ts +++ b/app/client/src/IDE/index.ts @@ -54,6 +54,11 @@ export { default as IDEBottomView } from "./Components/BottomView"; * It switches between different editor states */ export { default as IDESidebar } from "./Components/Sidebar"; +/** + * IDESidePaneWrapper is used as a wrapper for side panes, which provides a border and optional padding + * and enforces 100% width and height to the parent. + */ +export { default as IDESidePaneWrapper } from "./Components/SidePaneWrapper"; /** * ToolbarSettingsPopover is a popover attached to a settings toggle button in the toolbar diff --git a/app/client/src/ce/RouteBuilder.ts b/app/client/src/ce/RouteBuilder.ts index 406c9b42ee38..b8df54bcd135 100644 --- a/app/client/src/ce/RouteBuilder.ts +++ b/app/client/src/ce/RouteBuilder.ts @@ -204,3 +204,12 @@ export const queryAddURL = (props: URLBuilderParams): string => ...props, suffix: `queries/add`, }); + +export const appLibrariesURL = (): string => + urlBuilder.build({ + suffix: "libraries", + }); +export const appPackagesURL = (): string => + urlBuilder.build({ + suffix: "packages", + }); diff --git a/app/client/src/ce/constants/routes/appRoutes.ts b/app/client/src/ce/constants/routes/appRoutes.ts index 949360d6e944..8031209435be 100644 --- a/app/client/src/ce/constants/routes/appRoutes.ts +++ b/app/client/src/ce/constants/routes/appRoutes.ts @@ -67,6 +67,7 @@ export const JS_COLLECTION_ID_ADD_PATH = `${JS_COLLECTION_EDITOR_PATH}/:baseColl export const DATA_SOURCES_EDITOR_LIST_PATH = `/datasource`; export const DATA_SOURCES_EDITOR_ID_PATH = `/datasource/:datasourceId`; export const APP_LIBRARIES_EDITOR_PATH = `/libraries`; +export const APP_PACKAGES_EDITOR_PATH = `/packages`; export const APP_SETTINGS_EDITOR_PATH = `/settings`; export const SAAS_GSHEET_EDITOR_ID_PATH = `/saas/google-sheets-plugin/datasources/:datasourceId`; export const GEN_TEMPLATE_URL = "generate-page"; @@ -128,6 +129,12 @@ export const matchGeneratePagePath = (pathName: string) => match(`${BUILDER_CUSTOM_PATH}${GENERATE_TEMPLATE_FORM_PATH}`)(pathName) || match(`${BUILDER_PATH_DEPRECATED}${GENERATE_TEMPLATE_FORM_PATH}`)(pathName); +export const matchAppLibrariesPath = (pathName: string) => + match(`${BUILDER_PATH}${APP_LIBRARIES_EDITOR_PATH}`)(pathName); + +export const matchAppPackagesPath = (pathName: string) => + match(`${BUILDER_PATH}${APP_PACKAGES_EDITOR_PATH}`)(pathName); + export const addBranchParam = (branch: string) => { const url = new URL(window.location.href); diff --git a/app/client/src/ce/entities/Engine/actionHelpers.ts b/app/client/src/ce/entities/Engine/actionHelpers.ts index d27051602e57..96518343501e 100644 --- a/app/client/src/ce/entities/Engine/actionHelpers.ts +++ b/app/client/src/ce/entities/Engine/actionHelpers.ts @@ -30,6 +30,7 @@ export const getPageDependencyActions = ( currentWorkspaceId: string = "", featureFlags: DependentFeatureFlags = {}, allResponses: EditConsolidatedApi, + applicationId: string, ) => { const { datasources, pagesWithMigratedDsl, plugins } = allResponses || {}; const initActions = [ diff --git a/app/client/src/ce/pages/Editor/IDE/Header/useLibraryHeaderTitle.ts b/app/client/src/ce/pages/Editor/IDE/Header/useLibraryHeaderTitle.ts new file mode 100644 index 000000000000..489d1d74425b --- /dev/null +++ b/app/client/src/ce/pages/Editor/IDE/Header/useLibraryHeaderTitle.ts @@ -0,0 +1,12 @@ +import { createMessage, HEADER_TITLES } from "ee/constants/messages"; + +/** + * In CE this returns a simple text as title but this + * hook is extended in EE where based on feature flags + * the title changes + */ +function useLibraryHeaderTitle() { + return createMessage(HEADER_TITLES.LIBRARIES); +} + +export default useLibraryHeaderTitle; diff --git a/app/client/src/ce/pages/Editor/IDE/LeftPane/LibrarySidePane.tsx b/app/client/src/ce/pages/Editor/IDE/LeftPane/LibrarySidePane.tsx new file mode 100644 index 000000000000..3203c1b5b64e --- /dev/null +++ b/app/client/src/ce/pages/Editor/IDE/LeftPane/LibrarySidePane.tsx @@ -0,0 +1,13 @@ +import React from "react"; +import JSLibrariesSection from "pages/Editor/IDE/LeftPane/JSLibrariesSection"; +import { IDESidePaneWrapper } from "IDE"; + +const LibrarySidePane = () => { + return ( + + + + ); +}; + +export default LibrarySidePane; diff --git a/app/client/src/ce/pages/Editor/IDE/MainPane/useRoutes.ts b/app/client/src/ce/pages/Editor/IDE/MainPane/useRoutes.ts index e2efc6cafe78..c49f522c1c33 100644 --- a/app/client/src/ce/pages/Editor/IDE/MainPane/useRoutes.ts +++ b/app/client/src/ce/pages/Editor/IDE/MainPane/useRoutes.ts @@ -4,6 +4,7 @@ import { API_EDITOR_ID_ADD_PATH, API_EDITOR_ID_PATH, APP_LIBRARIES_EDITOR_PATH, + APP_PACKAGES_EDITOR_PATH, APP_SETTINGS_EDITOR_PATH, BUILDER_CHECKLIST_PATH, BUILDER_CUSTOM_PATH, @@ -82,6 +83,7 @@ function useRoutes(path: string): RouteReturnType[] { `${path}${SAAS_EDITOR_API_ID_PATH}`, `${path}${SAAS_EDITOR_API_ID_ADD_PATH}`, `${path}${APP_LIBRARIES_EDITOR_PATH}`, + `${path}${APP_PACKAGES_EDITOR_PATH}`, `${path}${APP_SETTINGS_EDITOR_PATH}`, ], }, diff --git a/app/client/src/constants/AppConstants.ts b/app/client/src/constants/AppConstants.ts index 1d8994a6f2d4..c5d8ede0ae8c 100644 --- a/app/client/src/constants/AppConstants.ts +++ b/app/client/src/constants/AppConstants.ts @@ -9,6 +9,7 @@ export const CANVAS_DEFAULT_MIN_ROWS = Math.ceil( export const DEFAULT_ENTITY_EXPLORER_WIDTH = 256; export const DEFAULT_PROPERTY_PANE_WIDTH = 288; export const APP_SETTINGS_PANE_WIDTH = 525; +export const APP_LIBRARIES_PANE_WIDTH = 384; export const DEFAULT_EXPLORER_PANE_WIDTH = 255; export const SPLIT_SCREEN_RATIO = 0.404; diff --git a/app/client/src/ee/pages/Editor/IDE/Header/useLibraryHeaderTitle.ts b/app/client/src/ee/pages/Editor/IDE/Header/useLibraryHeaderTitle.ts new file mode 100644 index 000000000000..0b81d5d9afea --- /dev/null +++ b/app/client/src/ee/pages/Editor/IDE/Header/useLibraryHeaderTitle.ts @@ -0,0 +1,3 @@ +export * from "ce/pages/Editor/IDE/Header/useLibraryHeaderTitle"; +import { default as CE_useLibraryHeaderTitle } from "ce/pages/Editor/IDE/Header/useLibraryHeaderTitle"; +export default CE_useLibraryHeaderTitle; diff --git a/app/client/src/ee/pages/Editor/IDE/LeftPane/LibrarySidePane.tsx b/app/client/src/ee/pages/Editor/IDE/LeftPane/LibrarySidePane.tsx new file mode 100644 index 000000000000..20fb8b49fe3d --- /dev/null +++ b/app/client/src/ee/pages/Editor/IDE/LeftPane/LibrarySidePane.tsx @@ -0,0 +1,3 @@ +export * from "ce/pages/Editor/IDE/LeftPane/LibrarySidePane"; +import { default as CE_LibrarySidePane } from "ce/pages/Editor/IDE/LeftPane/LibrarySidePane"; +export default CE_LibrarySidePane; diff --git a/app/client/src/entities/Engine/AppEditorEngine.ts b/app/client/src/entities/Engine/AppEditorEngine.ts index a7520a0d03ea..8b63d0f333bc 100644 --- a/app/client/src/entities/Engine/AppEditorEngine.ts +++ b/app/client/src/entities/Engine/AppEditorEngine.ts @@ -199,6 +199,7 @@ export default class AppEditorEngine extends AppEngine { private *loadPluginsAndDatasources( allResponses: EditConsolidatedApi, rootSpan: Span, + applicationId: string, ) { const loadPluginsAndDatasourcesSpan = startNestedSpan( "AppEditorEngine.loadPluginsAndDatasources", @@ -211,7 +212,12 @@ export default class AppEditorEngine extends AppEngine { getFeatureFlagsForEngine, ); const { errorActions, initActions, successActions } = - getPageDependencyActions(currentWorkspaceId, featureFlags, allResponses); + getPageDependencyActions( + currentWorkspaceId, + featureFlags, + allResponses, + applicationId, + ); if (!isAirgappedInstance) { initActions.push(fetchMockDatasources(mockDatasources)); @@ -259,7 +265,12 @@ export default class AppEditorEngine extends AppEngine { allResponses, rootSpan, ); - yield call(this.loadPluginsAndDatasources, allResponses, rootSpan); + yield call( + this.loadPluginsAndDatasources, + allResponses, + rootSpan, + applicationId, + ); } public *completeChore(rootSpan: Span) { diff --git a/app/client/src/navigation/FocusEntity.ts b/app/client/src/navigation/FocusEntity.ts index e85b0ab64662..fcccac7a79a9 100644 --- a/app/client/src/navigation/FocusEntity.ts +++ b/app/client/src/navigation/FocusEntity.ts @@ -278,7 +278,10 @@ export function identifyEntityFromPath(path: string): FocusEntityInfo { } if (match.params.entity) { - if (match.params.entity === "libraries") { + if ( + match.params.entity === "libraries" || + match.params.entity === "packages" + ) { return { entity: FocusEntity.LIBRARY, id: "", diff --git a/app/client/src/pages/Editor/IDE/Header/index.tsx b/app/client/src/pages/Editor/IDE/Header/index.tsx index 611d21227421..ee4365196a5c 100644 --- a/app/client/src/pages/Editor/IDE/Header/index.tsx +++ b/app/client/src/pages/Editor/IDE/Header/index.tsx @@ -77,6 +77,7 @@ import type { Page } from "entities/Page"; import { IDEHeader, IDEHeaderTitle } from "IDE"; import { APPLICATIONS_URL } from "constants/routes"; import { useNavigationMenuData } from "../../EditorName/useNavigationMenuData"; +import useLibraryHeaderTitle from "ee/pages/Editor/IDE/Header/useLibraryHeaderTitle"; const StyledDivider = styled(Divider)` height: 50%; @@ -92,6 +93,8 @@ interface HeaderTitleProps { } const HeaderTitleComponent = ({ appState, currentPage }: HeaderTitleProps) => { + const libraryHeaderTitle = useLibraryHeaderTitle(); + switch (appState) { case EditorState.DATA: return ( @@ -110,12 +113,7 @@ const HeaderTitleComponent = ({ appState, currentPage }: HeaderTitleProps) => { /> ); case EditorState.LIBRARIES: - return ( - - ); + return ; default: return ; } diff --git a/app/client/src/pages/Editor/IDE/Layout/hooks/useGridLayoutTemplate.ts b/app/client/src/pages/Editor/IDE/Layout/hooks/useGridLayoutTemplate.ts index 4d17db5824ea..789a3837ebfa 100644 --- a/app/client/src/pages/Editor/IDE/Layout/hooks/useGridLayoutTemplate.ts +++ b/app/client/src/pages/Editor/IDE/Layout/hooks/useGridLayoutTemplate.ts @@ -16,6 +16,7 @@ import { import { APP_SETTINGS_PANE_WIDTH, APP_SIDEBAR_WIDTH, + APP_LIBRARIES_PANE_WIDTH, } from "constants/AppConstants"; import { useEditorStateLeftPaneWidth } from "./useEditorStateLeftPaneWidth"; import { type Area, Areas, SIDEBAR_WIDTH } from "../constants"; @@ -97,10 +98,10 @@ function useGridLayoutTemplate(): ReturnValue { } else { setColumns([ SIDEBAR_WIDTH, - "255px", + `${APP_LIBRARIES_PANE_WIDTH}px`, (windowWidth - APP_SIDEBAR_WIDTH - - 255 + + APP_LIBRARIES_PANE_WIDTH + "px") as AnimatedGridUnit, "0px", ]); diff --git a/app/client/src/pages/Editor/IDE/LeftPane/LibrarySidePane.tsx b/app/client/src/pages/Editor/IDE/LeftPane/JSLibrariesSection.tsx similarity index 56% rename from app/client/src/pages/Editor/IDE/LeftPane/LibrarySidePane.tsx rename to app/client/src/pages/Editor/IDE/LeftPane/JSLibrariesSection.tsx index b5444a9a0400..23c238595413 100644 --- a/app/client/src/pages/Editor/IDE/LeftPane/LibrarySidePane.tsx +++ b/app/client/src/pages/Editor/IDE/LeftPane/JSLibrariesSection.tsx @@ -1,13 +1,13 @@ -import React from "react"; -import AddLibraryPopover from "./AddLibraryPopover"; -import PaneHeader from "./PaneHeader"; -import { useSelector } from "react-redux"; +import React, { useMemo } from "react"; + +import PaneHeader from "pages/Editor/IDE/LeftPane/PaneHeader"; +import AddLibraryPopover from "pages/Editor/IDE/LeftPane/AddLibraryPopover"; import { selectLibrariesForExplorer } from "ee/selectors/entitiesSelector"; +import { useSelector } from "react-redux"; import { animated, useTransition } from "react-spring"; import { LibraryEntity } from "pages/Editor/Explorer/Libraries"; -import { Flex } from "@appsmith/ads"; -const LibrarySidePane = () => { +function JSLibrariesSection() { const libraries = useSelector(selectLibrariesForExplorer); const transitions = useTransition(libraries, { keys: (lib) => lib.name, @@ -16,24 +16,18 @@ const LibrarySidePane = () => { leave: { opacity: 1 }, }); + const rightIcon = useMemo(() => , []); + return ( - - } - title="Installed Libraries" - /> + <> + {transitions((style, lib) => ( ))} - + ); -}; +} -export default LibrarySidePane; +export default JSLibrariesSection; diff --git a/app/client/src/pages/Editor/IDE/LeftPane/index.tsx b/app/client/src/pages/Editor/IDE/LeftPane/index.tsx index 585632bc81e6..60a8eb743cb2 100644 --- a/app/client/src/pages/Editor/IDE/LeftPane/index.tsx +++ b/app/client/src/pages/Editor/IDE/LeftPane/index.tsx @@ -1,9 +1,10 @@ -import React from "react"; +import React, { useMemo } from "react"; import styled from "styled-components"; import { Switch, useRouteMatch } from "react-router"; import { SentryRoute } from "ee/AppRouter"; import { APP_LIBRARIES_EDITOR_PATH, + APP_PACKAGES_EDITOR_PATH, APP_SETTINGS_EDITOR_PATH, DATA_SOURCES_EDITOR_ID_PATH, DATA_SOURCES_EDITOR_LIST_PATH, @@ -12,8 +13,8 @@ import { } from "constants/routes"; import AppSettingsPane from "./AppSettings"; import DataSidePane from "./DataSidePane"; -import LibrarySidePane from "./LibrarySidePane"; import EditorPane from "../EditorPane"; +import LibrarySidePane from "ee/pages/Editor/IDE/LeftPane/LibrarySidePane"; export const LeftPaneContainer = styled.div<{ showRightBorder?: boolean }>` height: 100%; @@ -26,23 +27,32 @@ export const LeftPaneContainer = styled.div<{ showRightBorder?: boolean }>` const LeftPane = () => { const { path } = useRouteMatch(); + const dataSidePanePaths = useMemo( + () => [ + `${path}${DATA_SOURCES_EDITOR_LIST_PATH}`, + `${path}${DATA_SOURCES_EDITOR_ID_PATH}`, + `${path}${INTEGRATION_EDITOR_PATH}`, + `${path}${SAAS_GSHEET_EDITOR_ID_PATH}`, + ], + [path], + ); + + const librarySidePanePaths = useMemo( + () => [ + `${path}${APP_LIBRARIES_EDITOR_PATH}`, + `${path}${APP_PACKAGES_EDITOR_PATH}`, + ], + [path], + ); + return ( - + Date: Tue, 22 Oct 2024 11:28:10 +0530 Subject: [PATCH 3/9] chore: added instrumentation for js object update (#36999) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description > This PR adds the granular level instrumentation for the JS Object update calls. Fixes #36996 ## Automation /ok-to-test tags="@tag.Sanity" ### :mag: Cypress test results > [!TIP] > 🟒 🟒 🟒 All cypress tests have passed! πŸŽ‰ πŸŽ‰ πŸŽ‰ > Workflow run: > Commit: 2e31a47133ff22c6593e4274a648013ac83e4845 > Cypress dashboard. > Tags: `@tag.Sanity` > Spec: >
Mon, 21 Oct 2024 17:07:30 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **New Features** - Introduced new constants for action spans, application spans, layout spans, and page spans to enhance tracking capabilities. - Enhanced observability features in services related to page load and executable management. - **Bug Fixes** - Improved error handling for page and layout ID retrieval. - **Refactor** - Updated constructors and method signatures to incorporate new observability dependencies. These changes aim to improve tracking, error handling, and overall performance of the application. --- .../constants/spans/ce/ActionSpanCE.java | 2 + .../constants/spans/ce/ApplicationSpanCE.java | 1 + .../constants/spans/ce/LayoutSpanCE.java | 10 +++++ .../constants/spans/ce/PageSpanCE.java | 1 + .../ExecutableOnPageLoadServiceCEImpl.java | 13 ++++++ .../ExecutableOnPageLoadServiceImpl.java | 12 ++++- .../internal/OnLoadExecutablesUtilCEImpl.java | 44 +++++++++++++++++-- .../internal/OnLoadExecutablesUtilImpl.java | 8 +++- 8 files changed, 84 insertions(+), 7 deletions(-) diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/ActionSpanCE.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/ActionSpanCE.java index 1cbec44a3a75..b800f3a97707 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/ActionSpanCE.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/ActionSpanCE.java @@ -37,4 +37,6 @@ public class ActionSpanCE { public static final String CREATE_ACTION = APPSMITH_SPAN_PREFIX + "create.action"; public static final String UPDATE_ACTION = APPSMITH_SPAN_PREFIX + "update.action"; public static final String DELETE_ACTION = APPSMITH_SPAN_PREFIX + "delete.action"; + public static final String FILL_SELF_REFERENCING_PATHS_ACTION = + APPSMITH_SPAN_PREFIX + "action.fillSelfReferencingPaths"; } diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/ApplicationSpanCE.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/ApplicationSpanCE.java index 57a8982e0e30..dec0ed2aaadf 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/ApplicationSpanCE.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/ApplicationSpanCE.java @@ -7,4 +7,5 @@ public class ApplicationSpanCE { public static final String APPLICATION_FETCH_FROM_DB = CONSOLIDATED_API_PREFIX + "app_db"; public static final String APPLICATION_ID_FETCH_REDIS_SPAN = APPLICATION_ID_SPAN + ".read_redis"; public static final String APPLICATION_ID_UPDATE_REDIS_SPAN = APPLICATION_ID_SPAN + ".update_redis"; + public static final String APPLICATION_SAVE_LAST_EDIT_INFO_SPAN = APPLICATION_ID_SPAN + ".save_last_edit_info"; } diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/LayoutSpanCE.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/LayoutSpanCE.java index 98c972bd1285..be0660b5f544 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/LayoutSpanCE.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/LayoutSpanCE.java @@ -15,4 +15,14 @@ public class LayoutSpanCE { APPSMITH_SPAN_PREFIX + "onLoadExecutablesUtil.updateExecutablesExecuteOnLoad"; public static final String FIND_AND_UPDATE_LAYOUT = APPSMITH_SPAN_PREFIX + "onLoadExecutablesUtil.findAndUpdateLayout"; + + public static final String EXTRACT_AND_SET_EXECUTABLE_BINDINGS_IN_GRAPH_EDGES = + APPSMITH_SPAN_PREFIX + "extractAndSetExecutableBindingsInGraphEdges"; + public static final String RECURSIVELY_ADD_EXECUTABLES_AND_THEIR_DEPENDENTS_TO_GRAPH_FROM_BINDINGS = + APPSMITH_SPAN_PREFIX + "recursivelyAddExecutablesAndTheirDependentsToGraphFromBindings"; + public static final String ADD_WIDGET_RELATIONSHIP_TO_GRAPH = APPSMITH_SPAN_PREFIX + "addWidgetRelationshipToGraph"; + public static final String COMPUTE_ON_PAGE_LOAD_EXECUTABLES_SCHEDULING_ORDER = + APPSMITH_SPAN_PREFIX + "computeOnPageLoadExecutablesSchedulingOrder"; + public static final String FILTER_AND_TRANSFORM_SCHEDULING_ORDER_TO_DTO = + APPSMITH_SPAN_PREFIX + "filterAndTransformSchedulingOrderToDTO"; } diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/PageSpanCE.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/PageSpanCE.java index 8f3e8f78ed16..bf56b5bd37fb 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/PageSpanCE.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/PageSpanCE.java @@ -15,4 +15,5 @@ public class PageSpanCE { public static final String MIGRATE_DSL = PAGES + "migrate_dsl"; public static final String GET_PAGE_BY_ID = APPSMITH_SPAN_PREFIX + "get.page.unpublished"; + public static final String GET_PAGE_BY_ID_AND_LAYOUTS_ID = APPSMITH_SPAN_PREFIX + "getPageByIdAndLayoutsId"; } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/newpages/onload/ExecutableOnPageLoadServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/newpages/onload/ExecutableOnPageLoadServiceCEImpl.java index ae70869d13b6..d33cddf85137 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/newpages/onload/ExecutableOnPageLoadServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/newpages/onload/ExecutableOnPageLoadServiceCEImpl.java @@ -14,15 +14,21 @@ import com.appsmith.server.onload.executables.ExecutableOnLoadServiceCE; import com.appsmith.server.solutions.ActionPermission; import com.appsmith.server.solutions.PagePermission; +import io.micrometer.observation.ObservationRegistry; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.BeanUtils; import org.springframework.stereotype.Service; +import reactor.core.observability.micrometer.Micrometer; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import java.util.List; +import static com.appsmith.external.constants.spans.ActionSpan.FILL_SELF_REFERENCING_PATHS_ACTION; +import static com.appsmith.external.constants.spans.ApplicationSpan.APPLICATION_SAVE_LAST_EDIT_INFO_SPAN; +import static com.appsmith.external.constants.spans.PageSpan.GET_PAGE_BY_ID_AND_LAYOUTS_ID; + @Slf4j @RequiredArgsConstructor @Service @@ -34,6 +40,7 @@ public class ExecutableOnPageLoadServiceCEImpl implements ExecutableOnLoadServic private final ActionPermission actionPermission; private final PagePermission pagePermission; + private final ObservationRegistry observationRegistry; @Override public Flux getAllExecutablesByCreatorIdFlux(String creatorId) { @@ -48,6 +55,8 @@ public Flux getAllExecutablesByCreatorIdFlux(String creatorId) { public Mono fillSelfReferencingPaths(Executable executable) { return newActionService .fillSelfReferencingDataPaths((ActionDTO) executable) + .name(FILL_SELF_REFERENCING_PATHS_ACTION) + .tap(Micrometer.observation(observationRegistry)) .map(actionDTO -> actionDTO); } @@ -69,6 +78,8 @@ public Mono updateUnpublishedExecutable(String id, Executable execut public Mono findAndUpdateLayout(String creatorId, String layoutId, Layout layout) { Mono pageDTOMono = newPageService .findByIdAndLayoutsId(creatorId, layoutId, pagePermission.getEditPermission(), false) + .name(GET_PAGE_BY_ID_AND_LAYOUTS_ID) + .tap(Micrometer.observation(observationRegistry)) .switchIfEmpty(Mono.error(new AppsmithException( AppsmithError.ACL_NO_RESOURCE_FOUND, FieldName.PAGE_ID + " or " + FieldName.LAYOUT_ID, @@ -93,6 +104,8 @@ public Mono findAndUpdateLayout(String creatorId, String layoutId, Layou page.setLayouts(layoutList); return applicationService .saveLastEditInformation(page.getApplicationId()) + .name(APPLICATION_SAVE_LAST_EDIT_INFO_SPAN) + .tap(Micrometer.observation(observationRegistry)) .then(newPageService.saveUnpublishedPage(page)); }) .flatMap(page -> { diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/newpages/onload/ExecutableOnPageLoadServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/newpages/onload/ExecutableOnPageLoadServiceImpl.java index d83de5800519..f684b4c8889d 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/newpages/onload/ExecutableOnPageLoadServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/newpages/onload/ExecutableOnPageLoadServiceImpl.java @@ -7,6 +7,7 @@ import com.appsmith.server.onload.executables.ExecutableOnLoadService; import com.appsmith.server.solutions.ActionPermission; import com.appsmith.server.solutions.PagePermission; +import io.micrometer.observation.ObservationRegistry; import lombok.extern.slf4j.Slf4j; import org.springframework.stereotype.Service; @@ -19,7 +20,14 @@ public ExecutableOnPageLoadServiceImpl( NewPageService newPageService, ApplicationService applicationService, ActionPermission actionPermission, - PagePermission pagePermission) { - super(newActionService, newPageService, applicationService, actionPermission, pagePermission); + PagePermission pagePermission, + ObservationRegistry observationRegistry) { + super( + newActionService, + newPageService, + applicationService, + actionPermission, + pagePermission, + observationRegistry); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java index a998a695ffe2..bb3d599bb147 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java @@ -14,16 +14,20 @@ import com.appsmith.server.domains.NewPage; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; +import com.appsmith.server.helpers.ObservationHelperImpl; import com.appsmith.server.onload.executables.ExecutableOnLoadService; import com.appsmith.server.services.AstService; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; +import io.micrometer.observation.ObservationRegistry; +import io.micrometer.tracing.Span; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import net.minidev.json.JSONObject; import org.jgrapht.graph.DefaultEdge; import org.jgrapht.graph.DirectedAcyclicGraph; import org.jgrapht.traverse.BreadthFirstIterator; +import reactor.core.observability.micrometer.Micrometer; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.util.function.Tuple2; @@ -43,6 +47,11 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; +import static com.appsmith.external.constants.spans.LayoutSpan.ADD_WIDGET_RELATIONSHIP_TO_GRAPH; +import static com.appsmith.external.constants.spans.LayoutSpan.COMPUTE_ON_PAGE_LOAD_EXECUTABLES_SCHEDULING_ORDER; +import static com.appsmith.external.constants.spans.LayoutSpan.EXTRACT_AND_SET_EXECUTABLE_BINDINGS_IN_GRAPH_EDGES; +import static com.appsmith.external.constants.spans.LayoutSpan.FILTER_AND_TRANSFORM_SCHEDULING_ORDER_TO_DTO; +import static com.appsmith.external.constants.spans.LayoutSpan.RECURSIVELY_ADD_EXECUTABLES_AND_THEIR_DEPENDENTS_TO_GRAPH_FROM_BINDINGS; import static com.appsmith.external.helpers.MustacheHelper.EXECUTABLE_ENTITY_REFERENCES; import static com.appsmith.external.helpers.MustacheHelper.WIDGET_ENTITY_REFERENCES; import static com.appsmith.external.helpers.MustacheHelper.getPossibleParents; @@ -71,6 +80,8 @@ public class OnLoadExecutablesUtilCEImpl implements OnLoadExecutablesUtilCE { // TODO : This should contain all the static global variables present on the page like `appsmith`, etc. // TODO : Add all the global variables exposed on the client side. private final Set APPSMITH_GLOBAL_VARIABLES = Set.of(); + private final ObservationRegistry observationRegistry; + private final ObservationHelperImpl observationHelper; /** * This function computes the sequenced on page load executables. @@ -174,11 +185,15 @@ public Mono>> findAllOnLoadExecutables( bindingsFromExecutablesRef, executableNameToExecutableMapMono, evaluatedVersion)) + .name(RECURSIVELY_ADD_EXECUTABLES_AND_THEIR_DEPENDENTS_TO_GRAPH_FROM_BINDINGS) + .tap(Micrometer.observation(observationRegistry)) // At last, add all the widget relationships to the graph as well. .zipWith(executablesInCreatorContextMono) .flatMap(tuple -> { Set updatedEdges = tuple.getT1(); - return addWidgetRelationshipToGraph(updatedEdges, widgetDynamicBindingsMap, evaluatedVersion); + return addWidgetRelationshipToGraph(updatedEdges, widgetDynamicBindingsMap, evaluatedVersion) + .name(ADD_WIDGET_RELATIONSHIP_TO_GRAPH) + .tap(Micrometer.observation(observationRegistry)); }); // Create a graph given edges @@ -198,11 +213,20 @@ public Mono>> findAllOnLoadExecutables( Map executableNameToExecutableMap = tuple.getT1(); DirectedAcyclicGraph graph = tuple.getT2(); - return computeOnPageLoadExecutablesSchedulingOrder( + Span computeOnPageLoadExecutablesSchedulingOrderSpan = + observationHelper.createSpan(COMPUTE_ON_PAGE_LOAD_EXECUTABLES_SCHEDULING_ORDER); + + observationHelper.startSpan(computeOnPageLoadExecutablesSchedulingOrderSpan, true); + + List> executablesList = computeOnPageLoadExecutablesSchedulingOrder( graph, onLoadExecutableSetRef, executableNameToExecutableMap, explicitUserSetOnLoadExecutablesRef); + + observationHelper.endSpan(computeOnPageLoadExecutablesSchedulingOrderSpan, true); + + return executablesList; }) .map(onPageLoadExecutablesSchedulingOrder -> { // Find all explicitly turned on executables which haven't found their way into the scheduling order @@ -237,6 +261,8 @@ public Mono>> findAllOnLoadExecutables( onLoadExecutableSetRef, executableNameToExecutableMapMono, computeOnPageLoadScheduleNamesMono) + .name(FILTER_AND_TRANSFORM_SCHEDULING_ORDER_TO_DTO) + .tap(Micrometer.observation(observationRegistry)) .cache(); // With the final on page load scheduling order, also set the on page load executables which would be updated @@ -620,6 +646,8 @@ private Mono> addDirectlyReferencedExecutablesToGr executablesFoundDuringWalkRef, null, evalVersion)) + .name(EXTRACT_AND_SET_EXECUTABLE_BINDINGS_IN_GRAPH_EDGES) + .tap(Micrometer.observation(observationRegistry)) .thenReturn(possibleEntity); } return Mono.just(possibleEntity); @@ -899,6 +927,8 @@ private Mono> recursivelyAddExecutablesAndTheirDep executablesFoundDuringWalk, null, evalVersion)) + .name(EXTRACT_AND_SET_EXECUTABLE_BINDINGS_IN_GRAPH_EDGES) + .tap(Micrometer.observation(observationRegistry)) .thenReturn(possibleEntity); } else { return Mono.empty(); @@ -910,7 +940,13 @@ private Mono> recursivelyAddExecutablesAndTheirDep // Now that the next set of bindings have been found, find recursively all executables by these names // and their bindings return recursivelyAddExecutablesAndTheirDependentsToGraphFromBindings( - edges, executablesFoundDuringWalk, newBindings, executableNameToExecutableMapMono, evalVersion); + edges, + executablesFoundDuringWalk, + newBindings, + executableNameToExecutableMapMono, + evalVersion) + .name(RECURSIVELY_ADD_EXECUTABLES_AND_THEIR_DEPENDENTS_TO_GRAPH_FROM_BINDINGS) + .tap(Micrometer.observation(observationRegistry)); }); } @@ -963,6 +999,8 @@ private Mono> addExplicitUserSetOnLoadExecutablesT executablesFoundDuringWalkRef, executableBindingsInDsl, evalVersion) + .name(EXTRACT_AND_SET_EXECUTABLE_BINDINGS_IN_GRAPH_EDGES) + .tap(Micrometer.observation(observationRegistry)) .thenReturn(executable); }) .collectList() diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilImpl.java index fd04d515198a..acb26e326cbe 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilImpl.java @@ -1,9 +1,11 @@ package com.appsmith.server.onload.internal; import com.appsmith.server.domains.NewPage; +import com.appsmith.server.helpers.ObservationHelperImpl; import com.appsmith.server.onload.executables.ExecutableOnLoadService; import com.appsmith.server.services.AstService; import com.fasterxml.jackson.databind.ObjectMapper; +import io.micrometer.observation.ObservationRegistry; import lombok.extern.slf4j.Slf4j; import org.springframework.stereotype.Component; @@ -14,7 +16,9 @@ public class OnLoadExecutablesUtilImpl extends OnLoadExecutablesUtilCEImpl imple public OnLoadExecutablesUtilImpl( AstService astService, ObjectMapper objectMapper, - ExecutableOnLoadService pageExecutableOnLoadService) { - super(astService, objectMapper, pageExecutableOnLoadService); + ExecutableOnLoadService pageExecutableOnLoadService, + ObservationRegistry observationRegistry, + ObservationHelperImpl observationHelper) { + super(astService, objectMapper, pageExecutableOnLoadService, observationRegistry, observationHelper); } } From dc5fbed66b25120c4aa74b461eee063186d30cc4 Mon Sep 17 00:00:00 2001 From: Rishabh Rathod Date: Tue, 22 Oct 2024 14:22:29 +0530 Subject: [PATCH 4/9] chore: Add spans attributes for no of lines and action count (#37001) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Fixes #36995 ## Automation /ok-to-test tags="@tag.Sanity" ### :mag: Cypress test results > [!TIP] > 🟒 🟒 🟒 All cypress tests have passed! πŸŽ‰ πŸŽ‰ πŸŽ‰ > Workflow run: > Commit: 7285f3f04c37b0a78cf7fa8b9106c0fb5175bc29 > Cypress dashboard. > Tags: `@tag.Sanity` > Spec: >
Tue, 22 Oct 2024 08:35:48 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **New Features** - Added new constants for layout operations and data extraction to improve functionality. - Introduced observability tracking for various methods to enhance monitoring and performance insights. - **Bug Fixes** - Enhanced error handling and logging for action updates and layout modifications. - **Refactor** - Improved clarity and maintainability of methods related to action collections and layout updates. - **Chores** - Updated method signatures to include new observability parameters for better tracking. - Added metrics for line and action counts in action collection updates. --- .../external/constants/spans/OnLoadSpan.java | 5 ++ .../constants/spans/ce/LayoutSpanCE.java | 4 +- .../constants/spans/ce/OnLoadSpanCE.java | 22 ++++++ .../layouts/UpdateLayoutServiceCEImpl.java | 35 +++++++-- .../layouts/UpdateLayoutServiceImpl.java | 7 +- .../internal/OnLoadExecutablesUtilCEImpl.java | 75 ++++++++++++++----- .../ce/LayoutActionServiceCEImpl.java | 10 +++ .../ce/LayoutCollectionServiceCEImpl.java | 13 ++++ 8 files changed, 142 insertions(+), 29 deletions(-) create mode 100644 app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/OnLoadSpan.java create mode 100644 app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/OnLoadSpanCE.java diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/OnLoadSpan.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/OnLoadSpan.java new file mode 100644 index 000000000000..a77fbef0797f --- /dev/null +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/OnLoadSpan.java @@ -0,0 +1,5 @@ +package com.appsmith.external.constants.spans; + +import com.appsmith.external.constants.spans.ce.OnLoadSpanCE; + +public class OnLoadSpan extends OnLoadSpanCE {} diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/LayoutSpanCE.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/LayoutSpanCE.java index be0660b5f544..f8fb95a6a2d0 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/LayoutSpanCE.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/LayoutSpanCE.java @@ -15,7 +15,9 @@ public class LayoutSpanCE { APPSMITH_SPAN_PREFIX + "onLoadExecutablesUtil.updateExecutablesExecuteOnLoad"; public static final String FIND_AND_UPDATE_LAYOUT = APPSMITH_SPAN_PREFIX + "onLoadExecutablesUtil.findAndUpdateLayout"; - + public static final String UNESCAPE_MONGO_SPECIAL_CHARS = APPSMITH_SPAN_PREFIX + "unescapeMongoSpecialCharacters"; + public static final String EXTRACT_ALL_WIDGET_NAMES_AND_DYNAMIC_BINDINGS_FROM_DSL = + APPSMITH_SPAN_PREFIX + "extractAllWidgetNamesAndDynamicBindingsFromDSL"; public static final String EXTRACT_AND_SET_EXECUTABLE_BINDINGS_IN_GRAPH_EDGES = APPSMITH_SPAN_PREFIX + "extractAndSetExecutableBindingsInGraphEdges"; public static final String RECURSIVELY_ADD_EXECUTABLES_AND_THEIR_DEPENDENTS_TO_GRAPH_FROM_BINDINGS = diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/OnLoadSpanCE.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/OnLoadSpanCE.java new file mode 100644 index 000000000000..5b7dcc7ea20d --- /dev/null +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/OnLoadSpanCE.java @@ -0,0 +1,22 @@ +package com.appsmith.external.constants.spans.ce; + +import static com.appsmith.external.constants.spans.BaseSpan.APPSMITH_SPAN_PREFIX; + +public class OnLoadSpanCE { + + public static final String GET_ALL_EXECUTABLES_BY_CREATOR_ID = + APPSMITH_SPAN_PREFIX + "getAllExecutablesByCreatorIdFlux"; + public static final String EXECUTABLE_NAME_TO_EXECUTABLE_MAP = + APPSMITH_SPAN_PREFIX + "executableNameToExecutableMap"; + public static final String EXECUTABLE_IN_CREATOR_CONTEXT = APPSMITH_SPAN_PREFIX + "executablesInCreatorContext"; + public static final String ADD_DIRECTLY_REFERENCED_EXECUTABLES_TO_GRAPH = + APPSMITH_SPAN_PREFIX + "addDirectlyReferencedExecutablesToGraph"; + public static final String GET_POSSIBLE_ENTITY_REFERENCES = APPSMITH_SPAN_PREFIX + "getPossibleEntityReferences"; + public static final String UPDATE_EXECUTABLE_SELF_REFERENCING_PATHS = + APPSMITH_SPAN_PREFIX + "updateExecutableSelfReferencingPaths"; + public static final String GET_POSSIBLE_ENTITY_PARENTS_MAP = APPSMITH_SPAN_PREFIX + "getPossibleEntityParentsMap"; + public static final String ADD_EXPLICIT_USER_SET_ON_LOAD_EXECUTABLES_TO_GRAPH = + APPSMITH_SPAN_PREFIX + "addExplicitUserSetOnLoadExecutablesToGraph"; + public static final String GET_UNPUBLISHED_ON_LOAD_EXECUTABLES_EXPLICIT_SET_BY_USER_IN_CREATOR_CONTEXT = + APPSMITH_SPAN_PREFIX + "getUnpublishedOnLoadExecutablesExplicitSetByUserInCreatorContext"; +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/layouts/UpdateLayoutServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/layouts/UpdateLayoutServiceCEImpl.java index d9d44d439ff9..b4d159e67b7e 100755 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/layouts/UpdateLayoutServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/layouts/UpdateLayoutServiceCEImpl.java @@ -17,6 +17,7 @@ import com.appsmith.server.dtos.UpdateMultiplePageLayoutDTO; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; +import com.appsmith.server.helpers.ObservationHelperImpl; import com.appsmith.server.helpers.WidgetSpecificUtils; import com.appsmith.server.newpages.base.NewPageService; import com.appsmith.server.onload.internal.OnLoadExecutablesUtil; @@ -26,6 +27,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import io.micrometer.observation.ObservationRegistry; +import io.micrometer.tracing.Span; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import net.minidev.json.JSONObject; @@ -48,6 +50,7 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; +import static com.appsmith.external.constants.spans.LayoutSpan.EXTRACT_ALL_WIDGET_NAMES_AND_DYNAMIC_BINDINGS_FROM_DSL; import static com.appsmith.external.constants.spans.LayoutSpan.FIND_ALL_ON_LOAD_EXECUTABLES; import static com.appsmith.external.constants.spans.LayoutSpan.FIND_AND_UPDATE_LAYOUT; import static com.appsmith.external.constants.spans.LayoutSpan.UPDATE_EXECUTABLES_EXECUTE_ONLOAD; @@ -70,6 +73,7 @@ public class UpdateLayoutServiceCEImpl implements UpdateLayoutServiceCE { private final ApplicationService applicationService; private final ObjectMapper objectMapper; private final ObservationRegistry observationRegistry; + private final ObservationHelperImpl observationHelper; private final String layoutOnLoadActionErrorToastMessage = "A cyclic dependency error has been encountered on current page, \nqueries on page load will not run. \n Please check debugger and Appsmith documentation for more information"; @@ -127,6 +131,12 @@ private Mono updateLayoutDsl( Set widgetNames = new HashSet<>(); Map> widgetDynamicBindingsMap = new HashMap<>(); Set escapedWidgetNames = new HashSet<>(); + + Span extractAllWidgetNamesAndDynamicBindingsFromDSLSpan = + observationHelper.createSpan(EXTRACT_ALL_WIDGET_NAMES_AND_DYNAMIC_BINDINGS_FROM_DSL); + + observationHelper.startSpan(extractAllWidgetNamesAndDynamicBindingsFromDSLSpan, true); + try { dsl = extractAllWidgetNamesAndDynamicBindingsFromDSL( dsl, widgetNames, widgetDynamicBindingsMap, creatorId, layoutId, escapedWidgetNames, creatorType); @@ -136,6 +146,8 @@ private Mono updateLayoutDsl( .then(Mono.error(t)); } + observationHelper.endSpan(extractAllWidgetNamesAndDynamicBindingsFromDSLSpan, true); + layout.setWidgetNames(widgetNames); if (!escapedWidgetNames.isEmpty()) { @@ -151,7 +163,8 @@ private Mono updateLayoutDsl( AtomicReference validOnLoadExecutables = new AtomicReference<>(Boolean.TRUE); - // setting the layoutOnLoadActionActionErrors to empty to remove the existing errors before new DAG calculation. + // setting the layoutOnLoadActionActionErrors to empty to remove the existing + // errors before new DAG calculation. layout.setLayoutOnLoadActionErrors(new ArrayList<>()); Mono>> allOnLoadExecutablesMono = onLoadExecutablesUtil @@ -180,14 +193,18 @@ private Mono updateLayoutDsl( // First update the actions and set execute on load to true JSONObject finalDsl = dsl; - return allOnLoadExecutablesMono + + Mono layoutDTOMono = allOnLoadExecutablesMono .flatMap(allOnLoadExecutables -> { - // If there has been an error (e.g. cyclical dependency), then don't update any actions. - // This is so that unnecessary updates don't happen to actions while the page is in invalid state. + // If there has been an error (e.g. cyclical dependency), then don't update any + // actions. + // This is so that unnecessary updates don't happen to actions while the page is + // in invalid state. if (!validOnLoadExecutables.get()) { return Mono.just(allOnLoadExecutables); } - // Update these executables to be executed on load, unless the user has touched the executeOnLoad + // Update these executables to be executed on load, unless the user has touched + // the executeOnLoad // setting for this return onLoadExecutablesUtil .updateExecutablesExecuteOnLoad( @@ -201,12 +218,15 @@ private Mono updateLayoutDsl( layout.setLayoutOnLoadActions(onLoadExecutables); layout.setAllOnPageLoadActionNames(executableNames); layout.setActionsUsedInDynamicBindings(executablesUsedInDSL); - // The below field is to ensure that we record if the page load actions computation was + // The below field is to ensure that we record if the page load actions + // computation was // valid when last stored in the database. layout.setValidOnPageLoadActions(validOnLoadExecutables.get()); return onLoadExecutablesUtil .findAndUpdateLayout(creatorId, creatorType, layoutId, layout) + .tag("no_of_widgets", String.valueOf(widgetNames.size())) + .tag("no_of_executables", String.valueOf(executableNames.size())) .name(FIND_AND_UPDATE_LAYOUT) .tap(Micrometer.observation(observationRegistry)); }) @@ -222,6 +242,8 @@ private Mono updateLayoutDsl( return sendUpdateLayoutAnalyticsEvent(creatorId, layoutId, finalDsl, true, null, creatorType) .thenReturn(layoutDTO); }); + + return layoutDTOMono; } @Override @@ -326,6 +348,7 @@ public Mono>> getOnPageLoadActions( Set widgetNames = new HashSet<>(); Map> widgetDynamicBindingsMap = new HashMap<>(); Set escapedWidgetNames = new HashSet<>(); + // observationHelper.createSpan() try { dsl = extractAllWidgetNamesAndDynamicBindingsFromDSL( dsl, widgetNames, widgetDynamicBindingsMap, creatorId, layoutId, escapedWidgetNames, creatorType); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/layouts/UpdateLayoutServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/layouts/UpdateLayoutServiceImpl.java index 4d1689947366..a50e3e7c66fe 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/layouts/UpdateLayoutServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/layouts/UpdateLayoutServiceImpl.java @@ -1,6 +1,7 @@ package com.appsmith.server.layouts; import com.appsmith.server.applications.base.ApplicationService; +import com.appsmith.server.helpers.ObservationHelperImpl; import com.appsmith.server.newpages.base.NewPageService; import com.appsmith.server.onload.internal.OnLoadExecutablesUtil; import com.appsmith.server.services.AnalyticsService; @@ -21,7 +22,8 @@ public UpdateLayoutServiceImpl( PagePermission pagePermission, ApplicationService applicationService, ObjectMapper objectMapper, - ObservationRegistry observationRegistry) { + ObservationRegistry observationRegistry, + ObservationHelperImpl observationHelper) { super( onLoadExecutablesUtil, sessionUserService, @@ -30,6 +32,7 @@ public UpdateLayoutServiceImpl( pagePermission, applicationService, objectMapper, - observationRegistry); + observationRegistry, + observationHelper); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java index bb3d599bb147..7f8cc405cfae 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java @@ -52,6 +52,14 @@ import static com.appsmith.external.constants.spans.LayoutSpan.EXTRACT_AND_SET_EXECUTABLE_BINDINGS_IN_GRAPH_EDGES; import static com.appsmith.external.constants.spans.LayoutSpan.FILTER_AND_TRANSFORM_SCHEDULING_ORDER_TO_DTO; import static com.appsmith.external.constants.spans.LayoutSpan.RECURSIVELY_ADD_EXECUTABLES_AND_THEIR_DEPENDENTS_TO_GRAPH_FROM_BINDINGS; +import static com.appsmith.external.constants.spans.OnLoadSpan.ADD_DIRECTLY_REFERENCED_EXECUTABLES_TO_GRAPH; +import static com.appsmith.external.constants.spans.OnLoadSpan.ADD_EXPLICIT_USER_SET_ON_LOAD_EXECUTABLES_TO_GRAPH; +import static com.appsmith.external.constants.spans.OnLoadSpan.EXECUTABLE_NAME_TO_EXECUTABLE_MAP; +import static com.appsmith.external.constants.spans.OnLoadSpan.GET_ALL_EXECUTABLES_BY_CREATOR_ID; +import static com.appsmith.external.constants.spans.OnLoadSpan.GET_POSSIBLE_ENTITY_PARENTS_MAP; +import static com.appsmith.external.constants.spans.OnLoadSpan.GET_POSSIBLE_ENTITY_REFERENCES; +import static com.appsmith.external.constants.spans.OnLoadSpan.GET_UNPUBLISHED_ON_LOAD_EXECUTABLES_EXPLICIT_SET_BY_USER_IN_CREATOR_CONTEXT; +import static com.appsmith.external.constants.spans.OnLoadSpan.UPDATE_EXECUTABLE_SELF_REFERENCING_PATHS; import static com.appsmith.external.helpers.MustacheHelper.EXECUTABLE_ENTITY_REFERENCES; import static com.appsmith.external.helpers.MustacheHelper.WIDGET_ENTITY_REFERENCES; import static com.appsmith.external.helpers.MustacheHelper.getPossibleParents; @@ -140,6 +148,8 @@ public Mono>> findAllOnLoadExecutables( .toList(); }) .collectMap(Tuple2::getT1, Tuple2::getT2) + .name(EXECUTABLE_NAME_TO_EXECUTABLE_MAP) + .tap(Micrometer.observation(observationRegistry)) .cache(); Mono> executablesInCreatorContextMono = allExecutablesByCreatorIdFlux @@ -151,14 +161,16 @@ public Mono>> findAllOnLoadExecutables( Mono> directlyReferencedExecutablesToGraphMono = addDirectlyReferencedExecutablesToGraph( - edgesRef, - executablesUsedInDSLRef, - bindingsFromExecutablesRef, - executablesFoundDuringWalkRef, - widgetDynamicBindingsMap, - executableNameToExecutableMapMono, - executableBindingsInDslRef, - evaluatedVersion); + edgesRef, + executablesUsedInDSLRef, + bindingsFromExecutablesRef, + executablesFoundDuringWalkRef, + widgetDynamicBindingsMap, + executableNameToExecutableMapMono, + executableBindingsInDslRef, + evaluatedVersion) + .name(ADD_DIRECTLY_REFERENCED_EXECUTABLES_TO_GRAPH) + .tap(Micrometer.observation(observationRegistry)); // This following `createAllEdgesForPageMono` publisher traverses the executables and widgets to add all // possible edges between all possible entity paths @@ -168,15 +180,17 @@ public Mono>> findAllOnLoadExecutables( Mono> createAllEdgesForPageMono = directlyReferencedExecutablesToGraphMono // Add dependencies of all on page load executables set by the user in the graph .flatMap(updatedEdges -> addExplicitUserSetOnLoadExecutablesToGraph( - creatorId, - updatedEdges, - explicitUserSetOnLoadExecutablesRef, - executablesFoundDuringWalkRef, - bindingsFromExecutablesRef, - executableNameToExecutableMapMono, - executableBindingsInDslRef, - evaluatedVersion, - creatorType)) + creatorId, + updatedEdges, + explicitUserSetOnLoadExecutablesRef, + executablesFoundDuringWalkRef, + bindingsFromExecutablesRef, + executableNameToExecutableMapMono, + executableBindingsInDslRef, + evaluatedVersion, + creatorType) + .name(ADD_EXPLICIT_USER_SET_ON_LOAD_EXECUTABLES_TO_GRAPH) + .tap(Micrometer.observation(observationRegistry))) // For all the executables found so far, recursively walk the dynamic bindings of the executables to // find more relationships with other executables (& widgets) .flatMap(updatedEdges -> recursivelyAddExecutablesAndTheirDependentsToGraphFromBindings( @@ -424,7 +438,10 @@ private List addExecutableUpdatesForExecutableNames( } protected Flux getAllExecutablesByCreatorIdFlux(String creatorId, CreatorContextType creatorType) { - return pageExecutableOnLoadService.getAllExecutablesByCreatorIdFlux(creatorId); + return pageExecutableOnLoadService + .getAllExecutablesByCreatorIdFlux(creatorId) + .name(GET_ALL_EXECUTABLES_BY_CREATOR_ID) + .tap(Micrometer.observation(observationRegistry)); } /** @@ -488,7 +505,9 @@ private Mono>> filterAndTransformSchedulingOrderToDTO */ private Mono> getPossibleEntityReferences( Mono> executableNameToExecutableMapMono, Set bindings, int evalVersion) { - return getPossibleEntityReferences(executableNameToExecutableMapMono, bindings, evalVersion, null); + return getPossibleEntityReferences(executableNameToExecutableMapMono, bindings, evalVersion, null) + .name(GET_POSSIBLE_ENTITY_REFERENCES) + .tap(Micrometer.observation(observationRegistry)); } /** @@ -511,7 +530,9 @@ private Mono> getPossibleEntityReferences( final int entityTypes = EXECUTABLE_ENTITY_REFERENCES | WIDGET_ENTITY_REFERENCES; return executableNameToExecutableMono - .zipWith(getPossibleEntityParentsMap(bindings, entityTypes, evalVersion)) + .zipWith(getPossibleEntityParentsMap(bindings, entityTypes, evalVersion) + .name(GET_POSSIBLE_ENTITY_PARENTS_MAP) + .tap(Micrometer.observation(observationRegistry))) .map(tuple -> { Map executableMap = tuple.getT1(); // For each binding, here we receive a set of possible references to global entities @@ -626,6 +647,8 @@ private Mono> addDirectlyReferencedExecutablesToGr bindingsInWidget, evalVersion, executableBindingsInDslRef) + .name(GET_POSSIBLE_ENTITY_REFERENCES) + .tap(Micrometer.observation(observationRegistry)) .flatMapMany(Flux::fromIterable) // Add dependencies of the executables found in the DSL in the graph // We are ignoring the widget references at this point @@ -638,6 +661,8 @@ private Mono> addDirectlyReferencedExecutablesToGr // for on page load executablesUsedInDSLRef.add(possibleEntity.getValidEntityName()); return updateExecutableSelfReferencingPaths(possibleEntity) + .name(UPDATE_EXECUTABLE_SELF_REFERENCING_PATHS) + .tap(Micrometer.observation(observationRegistry)) .flatMap(executable -> extractAndSetExecutableBindingsInGraphEdges( possibleEntity, edgesRef, @@ -914,11 +939,15 @@ private Mono> recursivelyAddExecutablesAndTheirDep // First fetch all the executables in the page whose name matches the words found in all the dynamic bindings Mono> findAndAddExecutablesInBindingsMono = getPossibleEntityReferences( executableNameToExecutableMapMono, dynamicBindings, evalVersion) + .name(GET_POSSIBLE_ENTITY_REFERENCES) + .tap(Micrometer.observation(observationRegistry)) .flatMapMany(Flux::fromIterable) // Add dependencies of the executables found in the DSL in the graph. .flatMap(possibleEntity -> { if (getExecutableTypes().contains(possibleEntity.getEntityReferenceType())) { return updateExecutableSelfReferencingPaths(possibleEntity) + .name(UPDATE_EXECUTABLE_SELF_REFERENCING_PATHS) + .tap(Micrometer.observation(observationRegistry)) .then(extractAndSetExecutableBindingsInGraphEdges( possibleEntity, edges, @@ -981,6 +1010,8 @@ private Mono> addExplicitUserSetOnLoadExecutablesT // First fetch all the executables which have been tagged as on load by the user explicitly. return getUnpublishedOnLoadExecutablesExplicitSetByUserInCreatorContextFlux(creatorId, creatorType) + .name(GET_UNPUBLISHED_ON_LOAD_EXECUTABLES_EXPLICIT_SET_BY_USER_IN_CREATOR_CONTEXT) + .tap(Micrometer.observation(observationRegistry)) .flatMap(this::fillSelfReferencingPaths) // Add the vertices and edges to the graph for these executables .flatMap(executable -> { @@ -1084,6 +1115,8 @@ private Mono extractAndSetExecutableBindingsInGraphEdges( executableBindingsMap.get(bindingPath), evalVersion, bindingsInDsl) + .name(GET_POSSIBLE_ENTITY_REFERENCES) + .tap(Micrometer.observation(observationRegistry)) .flatMapMany(Flux::fromIterable) .map(relatedDependencyNode -> { bindingsFromExecutables.add(relatedDependencyNode.getReferenceString()); @@ -1114,6 +1147,8 @@ private Mono> addWidgetRelationshipToGraph( return Flux.fromIterable(widgetBindingMap.entrySet()) .flatMap(widgetBindingEntries -> getPossibleEntityParentsMap( widgetBindingEntries.getValue(), entityTypes, evalVersion) + .name(GET_POSSIBLE_ENTITY_PARENTS_MAP) + .tap(Micrometer.observation(observationRegistry)) .map(possibleParentsMap -> { possibleParentsMap.entrySet().stream().forEach(entry -> { if (entry.getValue() == null || entry.getValue().isEmpty()) { diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java index 43a29dcca185..45fe4a882efc 100755 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java @@ -79,6 +79,8 @@ public Mono updateAction(String id, ActionDTO actionDTO) { if (actionDTO.getCollectionId() == null) { return this.updateSingleAction(id, actionDTO).flatMap(updatedAction -> updateLayoutService .updatePageLayoutsByPageId(updatedAction.getPageId()) + .name(UPDATE_PAGE_LAYOUT_BY_PAGE_ID) + .tap(Micrometer.observation(observationRegistry)) .thenReturn(updatedAction)); } else if (actionDTO.getCollectionId().length() == 0) { // The Action has been removed from existing collection. @@ -91,6 +93,8 @@ public Mono updateAction(String id, ActionDTO actionDTO) { actionDTO.setCollectionId(null); return this.updateSingleAction(id, actionDTO).flatMap(updatedAction -> updateLayoutService .updatePageLayoutsByPageId(updatedAction.getPageId()) + .name(UPDATE_PAGE_LAYOUT_BY_PAGE_ID) + .tap(Micrometer.observation(observationRegistry)) .thenReturn(updatedAction)); }); } else { @@ -120,6 +124,8 @@ public Mono updateAction(String id, ActionDTO actionDTO) { action1.getId()); return this.updateSingleAction(id, actionDTO).flatMap(updatedAction -> updateLayoutService .updatePageLayoutsByPageId(updatedAction.getPageId()) + .name(UPDATE_PAGE_LAYOUT_BY_PAGE_ID) + .tap(Micrometer.observation(observationRegistry)) .thenReturn(updatedAction)); }); } @@ -276,6 +282,8 @@ public Mono setExecuteOnLoad(String id, Boolean isExecuteOnLoad) { return newActionService.save(newAction).flatMap(savedAction -> updateLayoutService .updatePageLayoutsByPageId( savedAction.getUnpublishedAction().getPageId()) + .name(UPDATE_PAGE_LAYOUT_BY_PAGE_ID) + .tap(Micrometer.observation(observationRegistry)) .thenReturn(newActionService.generateActionByViewMode(savedAction, false))); }); } @@ -289,6 +297,8 @@ public Mono deleteUnpublishedAction(String id) { .deleteUnpublishedAction(id) .flatMap(actionDTO -> Mono.zip( Mono.just(actionDTO), updateLayoutService.updatePageLayoutsByPageId(actionDTO.getPageId()))) + .name(UPDATE_PAGE_LAYOUT_BY_PAGE_ID) + .tap(Micrometer.observation(observationRegistry)) .flatMap(tuple -> { ActionDTO actionDTO = tuple.getT1(); return Mono.just(actionDTO); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutCollectionServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutCollectionServiceCEImpl.java index d4cc9fcbdf65..aa0d73de132e 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutCollectionServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutCollectionServiceCEImpl.java @@ -375,6 +375,17 @@ public Mono updateUnpublishedActionCollection( .name(DELETE_ACTION) .tap(Micrometer.observation(observationRegistry)); + String body = actionCollectionDTO.getBody(); + Number lineCount = 0; + if (body != null && !body.isEmpty()) { + lineCount = body.split("\n").length; + } + Number actionCount = 0; + if (actionCollectionDTO.getActions() != null + && !actionCollectionDTO.getActions().isEmpty()) { + actionCount = actionCollectionDTO.getActions().size(); + } + return deleteNonExistingActionMono .then(newValidActionIdsMono) .flatMap(tuple -> { @@ -392,6 +403,8 @@ public Mono updateUnpublishedActionCollection( }); }) .flatMap(actionCollection -> actionCollectionService.update(actionCollection.getId(), actionCollection)) + .tag("lineCount", lineCount.toString()) + .tag("actionCount", actionCount.toString()) .name(ACTION_COLLECTION_UPDATE) .tap(Micrometer.observation(observationRegistry)) .flatMap(actionCollectionRepository::setUserPermissionsInObject) From 554ec58d403fd4660f340ce43aa36a0acb7be5d3 Mon Sep 17 00:00:00 2001 From: Rahul Barwal Date: Tue, 22 Oct 2024 14:51:45 +0530 Subject: [PATCH 5/9] feat: Update TableWidgetV2 to include customIsLoading property (#36857) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Problem There are many problems with table loader logic, for which many users try to implement a modal for loader. These problems stem from dependency and delay on eval, discussed comprehensively in #12308 Solution This PR updates the TableWidgetV2 component to include a new property called `customIsLoading`. This property controls the loading state of the widget and is added to the TableWidgetProps interface. Additionally, the component's state is updated to include the `customIsLoading` property. The `contentConfig` file for the TableWidgetV2 is also modified to include the `customIsLoading` property with its corresponding label, control type, help text, and validation. These changes improve the flexibility and customization options of the TableWidgetV2 component. Fixes #12308 _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.Table" ### :mag: Cypress test results > [!TIP] > 🟒 🟒 🟒 All cypress tests have passed! πŸŽ‰ πŸŽ‰ πŸŽ‰ > Workflow run: > Commit: 1c6f4f9caabc3aa45ec3916e5ccb465d946ab0a1 > Cypress dashboard. > Tags: `@tag.Table` > Spec: >
Tue, 22 Oct 2024 09:17:37 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No ## Summary by CodeRabbit - **New Features** - Introduced a new feature flag for custom loading states in the table widget. - Added properties for managing custom loading behavior in the `TableWidgetV2`. - **Bug Fixes** - Enhanced loading state management to ensure accurate representation based on new properties. - **Tests** - Added unit tests for loading behavior in the `TableWidgetV2` component, covering default and custom loading scenarios. - **Documentation** - Updated help text for properties related to loading states to improve clarity. --- app/client/src/ce/entities/FeatureFlag.ts | 3 + .../src/widgets/TableWidgetV2/constants.ts | 5 + .../widget/TableRendered.test.ts | 181 ++++++++++++++++++ .../widgets/TableWidgetV2/widget/index.tsx | 10 +- .../widget/propertyConfig/contentConfig.ts | 29 ++- 5 files changed, 225 insertions(+), 3 deletions(-) create mode 100644 app/client/src/widgets/TableWidgetV2/widget/TableRendered.test.ts diff --git a/app/client/src/ce/entities/FeatureFlag.ts b/app/client/src/ce/entities/FeatureFlag.ts index 5b3919ce5dba..1f59947f117f 100644 --- a/app/client/src/ce/entities/FeatureFlag.ts +++ b/app/client/src/ce/entities/FeatureFlag.ts @@ -39,6 +39,8 @@ export const FEATURE_FLAG = { release_anvil_toggle_enabled: "release_anvil_toggle_enabled", release_git_persist_branch_enabled: "release_git_persist_branch_enabled", release_ide_animations_enabled: "release_ide_animations_enabled", + release_table_custom_loading_state_enabled: + "release_table_custom_loading_state_enabled", } as const; export type FeatureFlag = keyof typeof FEATURE_FLAG; @@ -74,6 +76,7 @@ export const DEFAULT_FEATURE_FLAG_VALUE: FeatureFlags = { release_anvil_toggle_enabled: false, release_git_persist_branch_enabled: false, release_ide_animations_enabled: false, + release_table_custom_loading_state_enabled: false, }; export const AB_TESTING_EVENT_KEYS = { diff --git a/app/client/src/widgets/TableWidgetV2/constants.ts b/app/client/src/widgets/TableWidgetV2/constants.ts index 8fb0e8605aad..c88992218216 100644 --- a/app/client/src/widgets/TableWidgetV2/constants.ts +++ b/app/client/src/widgets/TableWidgetV2/constants.ts @@ -109,6 +109,8 @@ export interface TableWidgetProps firstEditableColumnIdByOrder: string; enableServerSideFiltering: boolean; onTableFilterUpdate: string; + customIsLoading: boolean; + customIsLoadingValue: boolean; } export enum TableVariantTypes { @@ -237,3 +239,6 @@ export const DEFAULT_COLUMN_NAME = "Table Column"; export const ALLOW_TABLE_WIDGET_SERVER_SIDE_FILTERING = FEATURE_FLAG["release_table_serverside_filtering_enabled"]; + +export const CUSTOM_LOADING_STATE_ENABLED = + FEATURE_FLAG["release_table_custom_loading_state_enabled"]; diff --git a/app/client/src/widgets/TableWidgetV2/widget/TableRendered.test.ts b/app/client/src/widgets/TableWidgetV2/widget/TableRendered.test.ts new file mode 100644 index 000000000000..fcd4e4a1dfe0 --- /dev/null +++ b/app/client/src/widgets/TableWidgetV2/widget/TableRendered.test.ts @@ -0,0 +1,181 @@ +import { ResponsiveBehavior } from "layoutSystems/common/utils/constants"; +import TableWidgetV2 from "."; +import type { TableWidgetProps } from "../constants"; + +describe("TableWidgetV2 getWidgetView", () => { + const tableWidgetProps: TableWidgetProps = { + customIsLoading: false, + customIsLoadingValue: false, + delimiter: ",", + filteredTableData: [], + isVisibleDownload: true, + isVisibleFilters: true, + isVisiblePagination: true, + isVisibleSearch: true, + pageSize: 10, + primaryColumns: {}, + totalRecordsCount: 100, + accentColor: "#000000", + borderColor: "#000000", + borderRadius: "40px", + borderWidth: "1px", + boxShadow: "none", + canFreezeColumn: true, + columnWidthMap: {}, + compactMode: "DEFAULT", + filters: [], + isAddRowInProgress: false, + isEditableCellsValid: {}, + isLoading: false, + isSortable: true, + multiRowSelection: false, + pageNo: 1, + renderMode: "CANVAS", + searchText: "", + selectedRowIndex: -1, + selectedRowIndices: [], + serverSidePaginationEnabled: false, + tableData: [], + widgetId: "widgetId", + widgetName: "TableWidget", + componentWidth: 800, + componentHeight: 600, + onPageChange: "", + onSearchTextChanged: "", + onSort: "", + onRowSelected: "", + onAddNewRowSave: "", + onAddNewRowDiscard: "", + onBulkSave: "", + onBulkDiscard: "", + onPageSizeChange: "", + commitBatchMetaUpdates: jest.fn(), + pushBatchMetaUpdates: jest.fn(), + updateWidgetMetaProperty: jest.fn(), + updateWidgetProperty: "", + updateOneClickBindingOptionsVisibility: "", + // Added missing properties + primaryColumnId: "", + columnOrder: [], + derivedColumns: {}, + dynamicPropertyPathList: [], + dynamicTriggerPathList: [], + dynamicBindingPathList: [], + childStylesheet: {}, + isVisible: true, + version: 1, + parentColumnSpace: 1, + parentRowSpace: 1, + leftColumn: 0, + rightColumn: 0, + topRow: 0, + bottomRow: 0, + parentId: "", + responsiveBehavior: ResponsiveBehavior.Hug, + minWidth: 0, + minHeight: 0, + isDisabled: false, + animateLoading: false, + primaryColor: "", + backgroundColor: "", + textColor: "", + fontFamily: "", + fontSize: "", + fontStyle: "", + textAlign: "", + textDecoration: "", + textTransform: "", + letterSpacing: "", + lineHeight: "", + whiteSpace: "", + overflow: "", + textOverflow: "", + wordBreak: "", + wordWrap: "", + cursor: "", + zIndex: 0, + pristine: true, + label: "TableWidget", + defaultSearchText: "", + sortOrder: { column: "", order: null }, + transientTableData: { data: { name: "name" } }, + newRow: {}, + firstEditableColumnIdByOrder: "", + enableServerSideFiltering: false, + onTableFilterUpdate: "", + type: "", + allowAddNewRow: false, + defaultNewRow: {}, + frozenColumnIndices: { a: 1 }, + }; + + describe("TableWidgetV2 loading checks", () => { + describe("When custom loading logic is not provided", () => { + it("Should not be loading with built-in property isLoading is set to false", () => { + const tableWidget = new TableWidgetV2(tableWidgetProps); + const widgetView = tableWidget.getWidgetView(); + + expect(widgetView.props.children.props.isLoading).toBe(false); + }); + it("Should be loading with built-in property isLoading is set to true", () => { + const tableWidget = new TableWidgetV2({ + ...tableWidgetProps, + isLoading: true, + }); + const widgetView = tableWidget.getWidgetView(); + + expect(widgetView.props.children.props.isLoading).toBe(true); + }); + }); + describe("When custom loading logic is provided", () => { + describe("When isLoading is false", () => { + it("Should not be loading with isLoading: false, customIsLoading: true and customIsLoadingTrue: true", () => { + const tableWidget = new TableWidgetV2({ + ...tableWidgetProps, + customIsLoading: true, + customIsLoadingValue: false, + isLoading: false, + }); + const widgetView = tableWidget.getWidgetView(); + + expect(widgetView.props.children.props.isLoading).toBe(false); + }); + it("Should be loading with customIsLoading set to true and customIsLoadingTrue set to true", () => { + const tableWidget = new TableWidgetV2({ + ...tableWidgetProps, + customIsLoading: true, + customIsLoadingValue: true, + isLoading: false, + }); + const widgetView = tableWidget.getWidgetView(); + + expect(widgetView.props.children.props.isLoading).toBe(true); + }); + }); + describe("When isLoading is true", () => { + it("Should be loading with customIsLoading set to true and customIsLoadingTrue set to false", () => { + const tableWidget = new TableWidgetV2({ + ...tableWidgetProps, + customIsLoading: true, + customIsLoadingValue: false, + isLoading: true, + }); + const widgetView = tableWidget.getWidgetView(); + + expect(widgetView.props.children.props.isLoading).toBe(true); + }); + it("Should be loading with customIsLoading set to true and customIsLoadingTrue set to true, even if in built loading is false", () => { + const tableWidget = new TableWidgetV2({ + ...tableWidgetProps, + customIsLoading: true, + customIsLoadingValue: true, + isLoading: true, + }); + const widgetView = tableWidget.getWidgetView(); + + expect(widgetView.props.children.props.isLoading).toBe(true); + }); + }); + }); + }); +}); diff --git a/app/client/src/widgets/TableWidgetV2/widget/index.tsx b/app/client/src/widgets/TableWidgetV2/widget/index.tsx index b83902333947..4d8e31b3e57b 100644 --- a/app/client/src/widgets/TableWidgetV2/widget/index.tsx +++ b/app/client/src/widgets/TableWidgetV2/widget/index.tsx @@ -226,6 +226,8 @@ class TableWidgetV2 extends BaseWidget { ) ? false : undefined, + customIsLoading: false, + customIsLoadingValue: "", }; } @@ -1211,6 +1213,8 @@ class TableWidgetV2 extends BaseWidget { getWidgetView() { const { + customIsLoading, + customIsLoadingValue, delimiter, filteredTableData = [], isVisibleDownload, @@ -1266,7 +1270,11 @@ class TableWidgetV2 extends BaseWidget { height={componentHeight} isAddRowInProgress={this.props.isAddRowInProgress} isEditableCellsValid={this.props.isEditableCellsValid} - isLoading={this.props.isLoading} + isLoading={ + customIsLoading + ? customIsLoadingValue || this.props.isLoading + : this.props.isLoading + } isSortable={this.props.isSortable ?? true} isVisibleDownload={isVisibleDownload} isVisibleFilters={isVisibleFilters} diff --git a/app/client/src/widgets/TableWidgetV2/widget/propertyConfig/contentConfig.ts b/app/client/src/widgets/TableWidgetV2/widget/propertyConfig/contentConfig.ts index a3570b99228a..580daa70f83f 100644 --- a/app/client/src/widgets/TableWidgetV2/widget/propertyConfig/contentConfig.ts +++ b/app/client/src/widgets/TableWidgetV2/widget/propertyConfig/contentConfig.ts @@ -7,7 +7,10 @@ import { ValidationTypes } from "constants/WidgetValidation"; import { EvaluationSubstitutionType } from "entities/DataTree/dataTreeFactory"; import { AutocompleteDataType } from "utils/autocomplete/AutocompleteDataType"; import type { TableWidgetProps } from "widgets/TableWidgetV2/constants"; -import { ALLOW_TABLE_WIDGET_SERVER_SIDE_FILTERING } from "../../constants"; +import { + ALLOW_TABLE_WIDGET_SERVER_SIDE_FILTERING, + CUSTOM_LOADING_STATE_ENABLED, +} from "../../constants"; import { InlineEditingSaveOptions } from "widgets/TableWidgetV2/constants"; import { composePropertyUpdateHook } from "widgets/WidgetUtils"; import { @@ -494,13 +497,35 @@ export default [ propertyName: "animateLoading", label: "Animate loading", controlType: "SWITCH", - helpText: "Controls the loading of the widget", + helpText: "Controls the animation loading of the widget", defaultValue: true, isJSConvertible: true, isBindProperty: true, isTriggerProperty: false, validation: { type: ValidationTypes.BOOLEAN }, }, + { + propertyName: "customIsLoading", + label: `Custom loading state`, + controlType: "SWITCH", + helpText: "Defines a custom value for the loading state", + defaultValue: false, + isBindProperty: true, + isTriggerProperty: false, + validation: { type: ValidationTypes.BOOLEAN }, + hidden: () => !Widget.getFeatureFlag(CUSTOM_LOADING_STATE_ENABLED), + }, + { + propertyName: "customIsLoadingValue", + label: "isLoading value", + controlType: "INPUT_TEXT", + defaultValue: "", + isBindProperty: true, + isTriggerProperty: false, + validation: { type: ValidationTypes.BOOLEAN }, + hidden: (props: TableWidgetProps) => !props.customIsLoading, + dependencies: ["customIsLoading"], + }, { propertyName: "isVisibleDownload", helpText: "Toggle visibility of the data download", From a8da0e1e7232403665defdcd528aa93070b04b99 Mon Sep 17 00:00:00 2001 From: NandanAnantharamu <67676905+NandanAnantharamu@users.noreply.github.com> Date: Tue, 22 Oct 2024 17:44:54 +0530 Subject: [PATCH 6/9] test: udpated locator for GenetateCrud test (#37000) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Airgap test was failing because it was not able to click a particular dropdown. EE: https://github.com/appsmithorg/appsmith-ee/pull/5412 /ok-to-test tags="@tag.Sanity" > [!TIP] > 🟒 🟒 🟒 All cypress tests have passed! πŸŽ‰ πŸŽ‰ πŸŽ‰ > Workflow run: > Commit: 8ca5c3643a4a77bbd71797dc59488ae7897a93b3 > Cypress dashboard. > Tags: `@tag.Sanity` > Spec: >
Tue, 22 Oct 2024 10:45:27 UTC --------- Co-authored-by: β€œNandanAnantharamu” <β€œnandan@thinkify.io”> --- app/client/cypress/support/Pages/DataSources.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/client/cypress/support/Pages/DataSources.ts b/app/client/cypress/support/Pages/DataSources.ts index 4770826380c2..4e035fa5db1b 100644 --- a/app/client/cypress/support/Pages/DataSources.ts +++ b/app/client/cypress/support/Pages/DataSources.ts @@ -325,9 +325,7 @@ export class DataSources { datasourceName, ); this.agHelper.GetNClick(this._selectTableDropdown, 0, true); - cy.get( - `div[role="listbox"] p[kind="span"]:contains("${tableName}")`, - ).click(); + cy.get(`div[role="listbox"] p:contains("${tableName}")`).click(); this.agHelper.GetNClick(this._generatePageBtn); this.assertHelper.AssertNetworkStatus("@replaceLayoutWithCRUDPage", 201); this.agHelper.ClickButton("Got it"); From 203b3222c7e40d03a2c2ea9454185a4c7736fe4c Mon Sep 17 00:00:00 2001 From: Hetu Nandu Date: Wed, 23 Oct 2024 12:17:20 +0530 Subject: [PATCH 7/9] chore: Update JS Editor Run designs (#36998) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description - Update the icons in ADS - Override the Menu Title text styles ## Automation /ok-to-test tags="@tag.JS" ### :mag: Cypress test results > [!TIP] > 🟒 🟒 🟒 All cypress tests have passed! πŸŽ‰ πŸŽ‰ πŸŽ‰ > Workflow run: > Commit: 1df24c69c2996e42a435ccd8fbd650425dd0b020 > Cypress dashboard. > Tags: `@tag.JS` > Spec: >
Mon, 21 Oct 2024 13:43:16 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **New Features** - Introduced a new styled component, `MenuTitle`, for improved dropdown menu item labeling. - Enhanced the `JSFunctionRun` component with a new button icon and improved function selection handling. - **Bug Fixes** - Updated the callback function to ensure it correctly responds to changes in the `onSelect` prop. --- .../ads/src/Icon/Icon.provider.tsx | 6 ++++ .../IDE/Components/ToolbarSettingsPopover.tsx | 2 ++ .../PluginActionForm/PluginActionForm.tsx | 2 +- .../PluginActionSettings/SettingsPopover.tsx | 2 +- .../src/assets/icons/menu/js-function.svg | 12 ++++++- .../JSEditorToolbar/JSEditorToolbar.tsx | 24 +++++++++---- .../components/JSFunctionItem.tsx | 22 ++++++++++++ .../components/JSFunctionRun.tsx | 29 ++++++++------- .../components/JSFunctionSettings.test.tsx | 8 +---- .../components/JSFunctionSettings.tsx | 36 +++++++------------ .../JSEditorToolbar/components/styles.ts | 9 +++++ 11 files changed, 99 insertions(+), 53 deletions(-) create mode 100644 app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionItem.tsx create mode 100644 app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/styles.ts diff --git a/app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx b/app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx index 964058836cda..62662323becd 100644 --- a/app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx +++ b/app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx @@ -1,6 +1,7 @@ /* eslint-disable no-console */ import React from "react"; import { importRemixIcon, importSvg } from "./loadables"; + const AddMoreIcon = importRemixIcon( async () => import("remixicon-react/AddCircleLineIcon"), ); @@ -1063,6 +1064,10 @@ const MaximizeV3Icon = importSvg( async () => import("../__assets__/icons/ads/maximize-v3-icon.svg"), ); +const ExternalLinkIcon = importRemixIcon( + async () => import("remixicon-react/ExternalLinkLineIcon"), +); + import PlayIconPNG from "../__assets__/icons/control/play-icon.png"; function PlayIconPNGWrapper() { @@ -1211,6 +1216,7 @@ const ICON_LOOKUP = { "link-2": Link2, "link-unlink": LinkUnlinkIcon, "links-line": LinksLineIcon, + "external-link-line": ExternalLinkIcon, "lock-2-line": Lock2LineIcon, "lock-password-line": LockPasswordLineIcon, "lock-unlock-line": LockUnlockLineIcon, diff --git a/app/client/src/IDE/Components/ToolbarSettingsPopover.tsx b/app/client/src/IDE/Components/ToolbarSettingsPopover.tsx index 4783e6c1f3ee..859839018f80 100644 --- a/app/client/src/IDE/Components/ToolbarSettingsPopover.tsx +++ b/app/client/src/IDE/Components/ToolbarSettingsPopover.tsx @@ -15,6 +15,7 @@ interface Props { title: string; children: React.ReactNode; dataTestId?: string; + disabled?: boolean; } const Variables = css` @@ -43,6 +44,7 @@ export const ToolbarSettingsPopover = (props: Props) => { { const { plugin } = usePluginActionContext(); return ( - + {plugin.uiComponent === UIComponentTypes.ApiEditorForm && ( )} diff --git a/app/client/src/PluginActionEditor/components/PluginActionSettings/SettingsPopover.tsx b/app/client/src/PluginActionEditor/components/PluginActionSettings/SettingsPopover.tsx index 77edc839d8cf..f5a3f0395d81 100644 --- a/app/client/src/PluginActionEditor/components/PluginActionSettings/SettingsPopover.tsx +++ b/app/client/src/PluginActionEditor/components/PluginActionSettings/SettingsPopover.tsx @@ -88,7 +88,7 @@ const PluginActionSettingsPopover = (props: SettingsProps) => { {props.docsLink && ( diff --git a/app/client/src/assets/icons/menu/js-function.svg b/app/client/src/assets/icons/menu/js-function.svg index 765ab10a37c7..692d9b426c7f 100644 --- a/app/client/src/assets/icons/menu/js-function.svg +++ b/app/client/src/assets/icons/menu/js-function.svg @@ -1 +1,11 @@ - \ No newline at end of file + + + + + diff --git a/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSEditorToolbar.tsx b/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSEditorToolbar.tsx index f4455a59cad9..06f3df58cd2f 100644 --- a/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSEditorToolbar.tsx +++ b/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSEditorToolbar.tsx @@ -1,5 +1,5 @@ -import React from "react"; -import { IDEToolbar } from "IDE"; +import React, { useState } from "react"; +import { IDEToolbar, ToolbarSettingsPopover } from "IDE"; import { JSFunctionRun } from "./components/JSFunctionRun"; import type { JSActionDropdownOption } from "./types"; import type { SaveActionNameParams } from "PluginActionEditor"; @@ -8,6 +8,7 @@ import type { JSAction, JSCollection } from "entities/JSCollection"; import type { DropdownOnSelect } from "@appsmith/ads-old"; import { useFeatureFlag } from "utils/hooks/useFeatureFlag"; import { FEATURE_FLAG } from "ee/entities/FeatureFlag"; +import { createMessage, JS_EDITOR_SETTINGS } from "ee/constants/messages"; import { JSHeader } from "./JSHeader"; import { JSFunctionSettings } from "./components/JSFunctionSettings"; import type { JSFunctionSettingsProps } from "./components/old/JSFunctionSettings"; @@ -48,6 +49,8 @@ export const JSEditorToolbar = (props: Props) => { FEATURE_FLAG.release_actions_redesign_enabled, ); + const [isOpen, setIsOpen] = useState(false); + // If the action redesign is not enabled, render the JSHeader component if (!isActionRedesignEnabled) { return ; @@ -71,11 +74,18 @@ export const JSEditorToolbar = (props: Props) => { /> {props.showSettings ? ( - + + + ) : null} {props.hideContextMenuOnEditor ? null : props.contextMenu} diff --git a/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionItem.tsx b/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionItem.tsx new file mode 100644 index 000000000000..4b2913086726 --- /dev/null +++ b/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionItem.tsx @@ -0,0 +1,22 @@ +import React, { useCallback } from "react"; +import { MenuItem } from "@appsmith/ads"; +import type { JSActionDropdownOption } from "../types"; +import * as Styled from "./styles"; + +export const JSFunctionItem = ({ + onSelect, + option, +}: { + option: JSActionDropdownOption; + onSelect: (option: JSActionDropdownOption) => void; +}) => { + const onFunctionSelect = useCallback(() => { + onSelect(option); + }, [onSelect, option]); + + return ( + + {option.label} + + ); +}; diff --git a/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx b/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx index 6f5f1c14c963..f4b9ff413a75 100644 --- a/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx +++ b/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx @@ -8,13 +8,13 @@ import { Flex, Menu, MenuContent, - MenuItem, MenuTrigger, Tooltip, } from "@appsmith/ads"; import type { JSActionDropdownOption } from "../types"; import { RUN_BUTTON_DEFAULTS, testLocators } from "../constants"; import { createMessage, NO_JS_FUNCTION_TO_RUN } from "ee/constants/messages"; +import { JSFunctionItem } from "./JSFunctionItem"; interface Props { disabled: boolean; @@ -33,16 +33,21 @@ interface Props { * */ export const JSFunctionRun = (props: Props) => { + const { onSelect } = props; + const isActionRedesignEnabled = useFeatureFlag( FEATURE_FLAG.release_actions_redesign_enabled, ); // Callback function to handle function selection from the dropdown menu - const onFunctionSelect = useCallback((option: JSActionDropdownOption) => { - if (props.onSelect) { - props.onSelect(option.value); - } - }, []); + const onFunctionSelect = useCallback( + (option: JSActionDropdownOption) => { + if (onSelect) { + onSelect(option.value); + } + }, + [onSelect], + ); if (!isActionRedesignEnabled) { return ; @@ -58,20 +63,18 @@ export const JSFunctionRun = (props: Props) => { isDisabled={props.disabled} kind="tertiary" size="sm" - startIcon="" + startIcon="js-function" > {props.selected.label} {props.options.map((option) => ( - onFunctionSelect(option)} - size="sm" - > - {option.label} - + onSelect={onFunctionSelect} + option={option} + /> ))} diff --git a/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.test.tsx b/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.test.tsx index f5a54149d5b2..3662a47e06c4 100644 --- a/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.test.tsx +++ b/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.test.tsx @@ -1,6 +1,6 @@ import React from "react"; import "@testing-library/jest-dom"; -import { render, screen, fireEvent } from "test/testUtils"; +import { render, screen } from "test/testUtils"; import { JSFunctionSettings } from "./JSFunctionSettings"; import { useFeatureFlag } from "utils/hooks/useFeatureFlag"; import { JSObjectFactory } from "test/factories/Actions/JSObject"; @@ -31,8 +31,6 @@ describe("JSFunctionSettings", () => { />, ); - fireEvent.click(screen.getByRole("button")); - expect(screen.getByLabelText(actions[0].name)).toBeDisabled(); }); @@ -47,8 +45,6 @@ describe("JSFunctionSettings", () => { />, ); - fireEvent.click(screen.getByRole("button")); - expect(screen.getAllByRole("switch")).toHaveLength(actions.length); }); @@ -74,8 +70,6 @@ describe("JSFunctionSettings", () => { />, ); - fireEvent.click(screen.getByRole("button")); - const switchElement1 = screen.getByLabelText(updatedJSActions[0].name); const switchElement2 = screen.getByLabelText(updatedJSActions[1].name); diff --git a/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.tsx b/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.tsx index da28d4a826c4..6251e8b43a1e 100644 --- a/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.tsx +++ b/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.tsx @@ -8,7 +8,6 @@ import { useFeatureFlag } from "utils/hooks/useFeatureFlag"; import { FEATURE_FLAG } from "ee/entities/FeatureFlag"; import { createMessage, JS_EDITOR_SETTINGS } from "ee/constants/messages"; import AnalyticsUtil from "ee/utils/AnalyticsUtil"; -import { ToolbarSettingsPopover } from "IDE"; interface Props { disabled: boolean; @@ -73,8 +72,6 @@ export const JSFunctionSettings = (props: Props) => { FEATURE_FLAG.release_actions_redesign_enabled, ); - const [isOpen, setIsOpen] = useState(false); - // If the feature flag is disabled, render the old version of the component if (!isActionRedesignEnabled) { return ( @@ -88,25 +85,18 @@ export const JSFunctionSettings = (props: Props) => { // Render the new version of the component return ( - - - - {createMessage(JS_EDITOR_SETTINGS.ON_LOAD_TITLE)} - - {props.actions.map((action) => ( - - ))} - - + + + {createMessage(JS_EDITOR_SETTINGS.ON_LOAD_TITLE)} + + {props.actions.map((action) => ( + + ))} + ); }; diff --git a/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/styles.ts b/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/styles.ts new file mode 100644 index 000000000000..2671337911d3 --- /dev/null +++ b/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/styles.ts @@ -0,0 +1,9 @@ +import styled from "styled-components"; +import { Text } from "@appsmith/ads"; + +export const MenuTitle = styled(Text)` + --button-font-weight: bold; + overflow: hidden; + text-overflow: ellipsis; + max-width: 30ch; +`; From ccb0c9c32a621a48b99379f382bdf6c8d1ed66f6 Mon Sep 17 00:00:00 2001 From: Rahul Barwal Date: Wed, 23 Oct 2024 14:03:46 +0530 Subject: [PATCH 8/9] fix: Refactor handling of empty chart data in ChartWidget (#37009) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description * This pull request handles empty chart dAata more efficiently. * lso updates tests to ensure this is not overlooked again. These changes ensure that the ChartWidget can handle scenarios where the chart data is null or undefined, and that the tests accurately reflect this behavior. Fixes #37008 _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.Chart" ### :mag: Cypress test results > [!TIP] > 🟒 🟒 🟒 All cypress tests have passed! πŸŽ‰ πŸŽ‰ πŸŽ‰ > Workflow run: > Commit: 08602d3a0658b1753d4d377ce9673379ab234d3f > Cypress dashboard. > Tags: `@tag.Chart` > Spec: >
Wed, 23 Oct 2024 04:52:48 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No ## Summary by CodeRabbit - **Bug Fixes** - Improved data handling in the ChartWidget to prevent runtime errors when chart data is undefined. - **Tests** - Added a new test case to verify the behavior of the emptyChartData function when series data is null or undefined, enhancing test coverage. - **Documentation** - Updated interface to include an optional property for handling data point clicks in the ChartWidget. --- app/client/src/widgets/ChartWidget/widget/index.test.ts | 9 +++++++++ app/client/src/widgets/ChartWidget/widget/index.tsx | 5 ++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/client/src/widgets/ChartWidget/widget/index.test.ts b/app/client/src/widgets/ChartWidget/widget/index.test.ts index 083e92f32d5f..040cc61a89e5 100644 --- a/app/client/src/widgets/ChartWidget/widget/index.test.ts +++ b/app/client/src/widgets/ChartWidget/widget/index.test.ts @@ -72,6 +72,15 @@ describe("emptyChartData", () => { expect(emptyChartData(props)).toEqual(true); }); + it("returns true if each series is null or undefined", () => { + const props = JSON.parse(JSON.stringify(basicEChartProps)); + + props.chartData.seriesID1 = { data: undefined }; + props.chartData.seriesID2 = { data: null }; + + expect(emptyChartData(props)).toEqual(true); + }); + it("returns true if no series is present", () => { const props = JSON.parse(JSON.stringify(basicEChartProps)); diff --git a/app/client/src/widgets/ChartWidget/widget/index.tsx b/app/client/src/widgets/ChartWidget/widget/index.tsx index 9b95ed665182..65498ad61110 100644 --- a/app/client/src/widgets/ChartWidget/widget/index.tsx +++ b/app/client/src/widgets/ChartWidget/widget/index.tsx @@ -51,7 +51,10 @@ export const emptyChartData = (props: ChartWidgetProps) => { const builder = new EChartsDatasetBuilder(props.chartType, props.chartData); for (const seriesID in builder.filteredChartData) { - if (Object.keys(props.chartData[seriesID].data).length > 0) { + if ( + Array.isArray(props.chartData[seriesID].data) && + props.chartData[seriesID].data.length > 0 + ) { return false; } } From 371b523cab07465c798932aadd28e02d39e7c458 Mon Sep 17 00:00:00 2001 From: Diljit Date: Wed, 23 Oct 2024 14:48:29 +0530 Subject: [PATCH 9/9] chore: fix errors related to service worker in dev mode (#37012) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description In dev mode the service worker was throwing the following error. This fix moves all the app constants import from the ce file. Screenshot 2024-10-22 at 8 31 24 PM Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.All" ### :mag: Cypress test results > [!TIP] > 🟒 🟒 🟒 All cypress tests have passed! πŸŽ‰ πŸŽ‰ πŸŽ‰ > Workflow run: > Commit: ec42a0d66f76e26048a39a7623e829054fae4845 > Cypress dashboard. > Tags: `@tag.All` > Spec: >
Wed, 23 Oct 2024 09:04:03 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No ## Summary by CodeRabbit - **Chores** - Updated import paths for routing constants to improve organization. - No changes to functionality or public interfaces. --- app/client/src/ce/utils/serviceWorkerUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/client/src/ce/utils/serviceWorkerUtils.ts b/app/client/src/ce/utils/serviceWorkerUtils.ts index e32455dbcd07..1d359270188b 100644 --- a/app/client/src/ce/utils/serviceWorkerUtils.ts +++ b/app/client/src/ce/utils/serviceWorkerUtils.ts @@ -9,7 +9,7 @@ import { VIEWER_CUSTOM_PATH, BUILDER_PATH_DEPRECATED, VIEWER_PATH_DEPRECATED, -} from "ee/constants/routes/appRoutes"; +} from "../constants/routes/appRoutes"; interface TMatchResult { basePageId?: string;