From f3c0673822ec9e939f5d6b270de2892c9b6ede43 Mon Sep 17 00:00:00 2001 From: Shrikant Sharat Kandula Date: Wed, 13 Mar 2024 20:44:53 +0530 Subject: [PATCH 1/9] fix: Incorrect field constants (#31760) --- .../java/com/appsmith/server/domains/ce/NewActionCE.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/NewActionCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/NewActionCE.java index 4faaeb019e..a814fd1036 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/NewActionCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/NewActionCE.java @@ -77,10 +77,9 @@ public class NewActionCE extends BranchAwareDomain { public static final String unpublishedAction_actionConfiguration_httpMethod = String.join( ".", unpublishedAction, ActionDTO.Fields.actionConfiguration, ActionConfiguration.Fields.httpMethod); - public static final String publishedAction_name = String.join(".", unpublishedAction, ActionDTO.Fields.name); - public static final String publishedAction_pageId = - String.join(".", unpublishedAction, ActionDTO.Fields.pageId); + public static final String publishedAction_name = String.join(".", publishedAction, ActionDTO.Fields.name); + public static final String publishedAction_pageId = String.join(".", publishedAction, ActionDTO.Fields.pageId); public static final String publishedAction_contextType = - String.join(".", unpublishedAction, ActionDTO.Fields.contextType); + String.join(".", publishedAction, ActionDTO.Fields.contextType); } } From 41a66a1168295caae9fe513ba48149c84c988c86 Mon Sep 17 00:00:00 2001 From: Shrikant Sharat Kandula Date: Thu, 14 Mar 2024 06:30:40 +0530 Subject: [PATCH 2/9] chore: Tiny NodeJS script as a linter for field name constants (#31766) The point is to prevent unfortunate field name problems like this: https://github.com/appsmithorg/appsmith/pull/31760/files. This NodeJS script does a very rudimentary analysis on all Java files, with a few regular expressions, and finds anomalies. As such, since it's not very smart, it's quite strict. I intend to make it a little more strict in the coming days, but it's a start. It's not hooked into any processes/CI yet, but that will also come in next. Since it's not very smart, it actually runs quite fast (.8s on EE). The script also doesn't exit with a non-zero exit code when it finds a problem. Also will be solved as part of integrating it into CI. --- .../appsmith/server/domains/Application.java | 11 ++-- .../server/domains/ce/ActionCollectionCE.java | 16 +++-- .../server/domains/ce/NewActionCE.java | 31 +++++---- .../appsmith/server/helpers/StringUtils.java | 9 +++ app/server/scripts/check-field-constants.mjs | 65 +++++++++++++++++++ 5 files changed, 104 insertions(+), 28 deletions(-) create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/StringUtils.java create mode 100644 app/server/scripts/check-field-constants.mjs diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Application.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Application.java index 0101dafc6e..95036505f4 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Application.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Application.java @@ -29,6 +29,7 @@ import java.util.Set; import static com.appsmith.server.constants.ResourceModes.EDIT; import static com.appsmith.server.constants.ResourceModes.VIEW; import static com.appsmith.server.helpers.DateUtils.ISO_FORMATTER; +import static com.appsmith.server.helpers.StringUtils.dotted; @Getter @Setter @@ -484,14 +485,14 @@ public class Application extends BaseDomain implements Artifact { public static class Fields extends BaseDomain.Fields { public static final String gitApplicationMetadata_gitAuth = - gitApplicationMetadata + "." + GitArtifactMetadata.Fields.gitAuth; + dotted(gitApplicationMetadata, GitArtifactMetadata.Fields.gitAuth); public static final String gitApplicationMetadata_defaultApplicationId = - gitApplicationMetadata + "." + GitArtifactMetadata.Fields.defaultApplicationId; + dotted(gitApplicationMetadata, GitArtifactMetadata.Fields.defaultApplicationId); public static final String gitApplicationMetadata_branchName = - gitApplicationMetadata + "." + GitArtifactMetadata.Fields.branchName; + dotted(gitApplicationMetadata, GitArtifactMetadata.Fields.branchName); public static final String gitApplicationMetadata_isRepoPrivate = - gitApplicationMetadata + "." + GitArtifactMetadata.Fields.isRepoPrivate; + dotted(gitApplicationMetadata, GitArtifactMetadata.Fields.isRepoPrivate); public static final String gitApplicationMetadata_isProtectedBranch = - gitApplicationMetadata + "." + GitArtifactMetadata.Fields.isProtectedBranch; + dotted(gitApplicationMetadata, GitArtifactMetadata.Fields.isProtectedBranch); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/ActionCollectionCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/ActionCollectionCE.java index e043ad6790..6c963393de 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/ActionCollectionCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/ActionCollectionCE.java @@ -10,6 +10,8 @@ import lombok.Setter; import lombok.ToString; import lombok.experimental.FieldNameConstants; +import static com.appsmith.server.helpers.StringUtils.dotted; + /** * This class represents a collection of actions that may or may not belong to the same plugin. * The logic for grouping is agnostic of the handling of this collection @@ -52,21 +54,21 @@ public class ActionCollectionCE extends BranchAwareDomain { public static class Fields extends BranchAwareDomain.Fields { public static final String publishedCollection_name = - publishedCollection + "." + ActionCollectionDTO.Fields.name; + dotted(publishedCollection, ActionCollectionDTO.Fields.name); public static final String unpublishedCollection_name = - unpublishedCollection + "." + ActionCollectionDTO.Fields.name; + dotted(unpublishedCollection, ActionCollectionDTO.Fields.name); public static final String publishedCollection_pageId = - publishedCollection + "." + ActionCollectionDTO.Fields.pageId; + dotted(publishedCollection, ActionCollectionDTO.Fields.pageId); public static final String unpublishedCollection_pageId = - unpublishedCollection + "." + ActionCollectionDTO.Fields.pageId; + dotted(unpublishedCollection, ActionCollectionDTO.Fields.pageId); public static final String publishedCollection_contextType = - publishedCollection + "." + ActionCollectionDTO.Fields.contextType; + dotted(publishedCollection, ActionCollectionDTO.Fields.contextType); public static final String unpublishedCollection_contextType = - unpublishedCollection + "." + ActionCollectionDTO.Fields.contextType; + dotted(unpublishedCollection, ActionCollectionDTO.Fields.contextType); public static final String unpublishedCollection_deletedAt = - unpublishedCollection + "." + ActionCollectionDTO.Fields.deletedAt; + dotted(unpublishedCollection, ActionCollectionDTO.Fields.deletedAt); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/NewActionCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/NewActionCE.java index a814fd1036..d8a0e9695b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/NewActionCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/NewActionCE.java @@ -13,6 +13,8 @@ import lombok.Setter; import lombok.ToString; import lombok.experimental.FieldNameConstants; +import static com.appsmith.server.helpers.StringUtils.dotted; + @Getter @Setter @ToString @@ -60,26 +62,23 @@ public class NewActionCE extends BranchAwareDomain { public static class Fields extends BranchAwareDomain.Fields { public static final String unpublishedAction_datasource_id = - String.join(".", unpublishedAction, ActionDTO.Fields.datasource, Datasource.Fields.id); - public static final String unpublishedAction_name = String.join(".", unpublishedAction, ActionDTO.Fields.name); - public static final String unpublishedAction_pageId = - String.join(".", unpublishedAction, ActionDTO.Fields.pageId); - public static final String unpublishedAction_deletedAt = - String.join(".", unpublishedAction, ActionDTO.Fields.deletedAt); + dotted(unpublishedAction, ActionDTO.Fields.datasource, Datasource.Fields.id); + public static final String unpublishedAction_name = dotted(unpublishedAction, ActionDTO.Fields.name); + public static final String unpublishedAction_pageId = dotted(unpublishedAction, ActionDTO.Fields.pageId); + public static final String unpublishedAction_deletedAt = dotted(unpublishedAction, ActionDTO.Fields.deletedAt); public static final String unpublishedAction_contextType = - String.join(".", unpublishedAction, ActionDTO.Fields.contextType); + dotted(unpublishedAction, ActionDTO.Fields.contextType); public static final String unpublishedAction_userSetOnLoad = - String.join(".", unpublishedAction, ActionDTO.Fields.userSetOnLoad); + dotted(unpublishedAction, ActionDTO.Fields.userSetOnLoad); public static final String unpublishedAction_executeOnLoad = - String.join(".", unpublishedAction, ActionDTO.Fields.executeOnLoad); + dotted(unpublishedAction, ActionDTO.Fields.executeOnLoad); public static final String unpublishedAction_fullyQualifiedName = - String.join(".", unpublishedAction, ActionDTO.Fields.fullyQualifiedName); - public static final String unpublishedAction_actionConfiguration_httpMethod = String.join( - ".", unpublishedAction, ActionDTO.Fields.actionConfiguration, ActionConfiguration.Fields.httpMethod); + dotted(unpublishedAction, ActionDTO.Fields.fullyQualifiedName); + public static final String unpublishedAction_actionConfiguration_httpMethod = + dotted(unpublishedAction, ActionDTO.Fields.actionConfiguration, ActionConfiguration.Fields.httpMethod); - public static final String publishedAction_name = String.join(".", publishedAction, ActionDTO.Fields.name); - public static final String publishedAction_pageId = String.join(".", publishedAction, ActionDTO.Fields.pageId); - public static final String publishedAction_contextType = - String.join(".", publishedAction, ActionDTO.Fields.contextType); + public static final String publishedAction_name = dotted(publishedAction, ActionDTO.Fields.name); + public static final String publishedAction_pageId = dotted(publishedAction, ActionDTO.Fields.pageId); + public static final String publishedAction_contextType = dotted(publishedAction, ActionDTO.Fields.contextType); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/StringUtils.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/StringUtils.java new file mode 100644 index 0000000000..98b656ace6 --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/StringUtils.java @@ -0,0 +1,9 @@ +package com.appsmith.server.helpers; + +public class StringUtils { + private StringUtils() {} + + public static String dotted(String... parts) { + return String.join(".", parts); + } +} diff --git a/app/server/scripts/check-field-constants.mjs b/app/server/scripts/check-field-constants.mjs new file mode 100644 index 0000000000..33877e873e --- /dev/null +++ b/app/server/scripts/check-field-constants.mjs @@ -0,0 +1,65 @@ +import {promises as fs} from "fs"; +import path from "path"; + +async function findInnerClassDefinitions(directory) { + try { + const files = await fs.readdir(directory); + + for (const file of files) { + const filePath = path.join(directory, file); + const stats = await fs.stat(filePath); + + if (stats.isDirectory()) { + await findInnerClassDefinitions(filePath); + } else if (path.extname(filePath) === '.java') { + await processJavaFile(filePath); + } + } + } catch (err) { + console.error(err); + } +} + +async function processJavaFile(filePath) { + try { + const contents = await fs.readFile(filePath, 'utf8'); + + const innerClassRegex = /^ {4}([\w ]+?)\s+class\s+Fields\s+(extends (\w+)\.Fields)?\s*{(.+?\n {4})?}$/gsm; + + for (const innerClassMatch of contents.matchAll(innerClassRegex)) { + const classQualifiers = innerClassMatch[1]; // we don't care much about this + const expectedParentClass = innerClassMatch[3]; + console.log(filePath, classQualifiers, expectedParentClass); + + for (const match of innerClassMatch[0].matchAll(/\bpublic\s+static\s+final\s+String\s+(\w+)\s+=\s+(.+?);/gs)) { + const key = match[1] + const valMatcherParts = [`^dotted\\(`]; + for (const [i, field] of key.split("_").entries()) { + if (i > 0) { + valMatcherParts.push(`\\s*,\\s+`); + } + valMatcherParts.push(`(\\w+\\.\\w+\\.)?${field}`); + } + valMatcherParts.push(`\\s*\\)$`); + const valMatcher = new RegExp(valMatcherParts.join('')); + if (!valMatcher.test(match[2])) { + console.log("key is", key); + console.log("val is", match[2]); + console.log("pattern", valMatcher); + console.error(`Field ${key} in ${filePath} is not looking right.`); + } + } + } + + // if (finds.length > 1) { + // console.error(`Found multiple inner class definitions in file: ${filePath}`); + // return; + // } + + } catch (err) { + console.error(err); + } +} + +const directoryPath = '.'; +findInnerClassDefinitions(directoryPath); From 89f31076210c0344b797d69458ecd4c3965bc448 Mon Sep 17 00:00:00 2001 From: Shrikant Sharat Kandula Date: Thu, 14 Mar 2024 11:56:59 +0530 Subject: [PATCH 3/9] Revert "chore: Add separate request/response views (#31640)" This reverts commit 4cdbe89586a88c56cbe76f801c8d2070e2d8e6de. --- .../external/models/ce/ActionCE_DTO.java | 17 +++++++++++------ .../appsmith/external/views/RequestOnly.java | 7 ------- .../appsmith/external/views/ResponseOnly.java | 8 -------- .../server/controllers/ActionController.java | 8 ++++++-- .../controllers/ce/ActionControllerCE.java | 18 +++++++++++++----- 5 files changed, 30 insertions(+), 28 deletions(-) delete mode 100644 app/server/appsmith-interfaces/src/main/java/com/appsmith/external/views/RequestOnly.java delete mode 100644 app/server/appsmith-interfaces/src/main/java/com/appsmith/external/views/ResponseOnly.java diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/ce/ActionCE_DTO.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/ce/ActionCE_DTO.java index e9887f3d24..1fc01c4c05 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/ce/ActionCE_DTO.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/ce/ActionCE_DTO.java @@ -16,9 +16,9 @@ import com.appsmith.external.models.Executable; import com.appsmith.external.models.PluginType; import com.appsmith.external.models.Policy; import com.appsmith.external.models.Property; -import com.appsmith.external.views.ResponseOnly; import com.appsmith.external.views.Views; import com.fasterxml.jackson.annotation.JsonFormat; +import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonView; import lombok.Getter; import lombok.NoArgsConstructor; @@ -88,8 +88,9 @@ public class ActionCE_DTO implements Identifiable, Executable { ActionConfiguration actionConfiguration; // this attribute carries error messages while processing the actionCollection + @JsonProperty(access = JsonProperty.Access.READ_ONLY) @Transient - @JsonView(ResponseOnly.class) + @JsonView(Views.Public.class) List errorReports; @JsonView(Views.Public.class) @@ -105,19 +106,23 @@ public class ActionCE_DTO implements Identifiable, Executable { @JsonView(Views.Public.class) List dynamicBindingPathList; - @JsonView(ResponseOnly.class) + @JsonProperty(access = JsonProperty.Access.READ_ONLY) + @JsonView(Views.Public.class) Boolean isValid; - @JsonView(ResponseOnly.class) + @JsonProperty(access = JsonProperty.Access.READ_ONLY) + @JsonView(Views.Public.class) Set invalids; @Transient - @JsonView(ResponseOnly.class) + @JsonProperty(access = JsonProperty.Access.READ_ONLY) + @JsonView(Views.Public.class) Set messages = new HashSet<>(); // This is a list of keys that the client whose values the client needs to send during action execution. // These are the Mustache keys that the server will replace before invoking the API - @JsonView(ResponseOnly.class) + @JsonProperty(access = JsonProperty.Access.READ_ONLY) + @JsonView(Views.Public.class) Set jsonPathKeys; @JsonView(Views.Internal.class) diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/views/RequestOnly.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/views/RequestOnly.java deleted file mode 100644 index 2e2fc0a716..0000000000 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/views/RequestOnly.java +++ /dev/null @@ -1,7 +0,0 @@ -package com.appsmith.external.views; - -/** - * Intended to annotate fields that can be set by HTTP request payloads, but should NOT be included - * in HTTP responses sent back to the client. - */ -public interface RequestOnly extends Views.Public {} diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/views/ResponseOnly.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/views/ResponseOnly.java deleted file mode 100644 index d498179131..0000000000 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/views/ResponseOnly.java +++ /dev/null @@ -1,8 +0,0 @@ -package com.appsmith.external.views; - -/** - * Intended to mark entity/DTO fields that should be included as part of HTTP responses, but should - * be ignored as part of HTTP requests. For example, if a field is marked with this annotation, in - * a class used with {@code @RequestBody}, it's value will NOT be deserialized. - */ -public interface ResponseOnly extends Views.Public {} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ActionController.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ActionController.java index ccff2b83be..62d9ec7d33 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ActionController.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ActionController.java @@ -6,19 +6,23 @@ import com.appsmith.server.newactions.base.NewActionService; import com.appsmith.server.refactors.applications.RefactoringService; import com.appsmith.server.services.LayoutActionService; import com.appsmith.server.solutions.ActionExecutionSolution; +import io.micrometer.observation.ObservationRegistry; +import lombok.extern.slf4j.Slf4j; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; @RestController @RequestMapping(Url.ACTION_URL) +@Slf4j public class ActionController extends ActionControllerCE { public ActionController( LayoutActionService layoutActionService, NewActionService newActionService, RefactoringService refactoringService, - ActionExecutionSolution actionExecutionSolution) { + ActionExecutionSolution actionExecutionSolution, + ObservationRegistry observationRegistry) { - super(layoutActionService, newActionService, refactoringService, actionExecutionSolution); + super(layoutActionService, newActionService, refactoringService, actionExecutionSolution, observationRegistry); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ActionControllerCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ActionControllerCE.java index 30d360c091..6dc5d69ae2 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ActionControllerCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ActionControllerCE.java @@ -2,7 +2,6 @@ package com.appsmith.server.controllers.ce; import com.appsmith.external.models.ActionDTO; import com.appsmith.external.models.ActionExecutionResult; -import com.appsmith.external.views.RequestOnly; import com.appsmith.external.views.Views; import com.appsmith.server.constants.FieldName; import com.appsmith.server.constants.Url; @@ -17,6 +16,7 @@ import com.appsmith.server.refactors.applications.RefactoringService; import com.appsmith.server.services.LayoutActionService; import com.appsmith.server.solutions.ActionExecutionSolution; import com.fasterxml.jackson.annotation.JsonView; +import io.micrometer.observation.ObservationRegistry; import jakarta.validation.Valid; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; @@ -48,25 +48,30 @@ public class ActionControllerCE { private final NewActionService newActionService; private final RefactoringService refactoringService; private final ActionExecutionSolution actionExecutionSolution; + private final ObservationRegistry observationRegistry; @Autowired public ActionControllerCE( LayoutActionService layoutActionService, NewActionService newActionService, RefactoringService refactoringService, - ActionExecutionSolution actionExecutionSolution) { + ActionExecutionSolution actionExecutionSolution, + ObservationRegistry observationRegistry) { this.layoutActionService = layoutActionService; this.newActionService = newActionService; this.refactoringService = refactoringService; this.actionExecutionSolution = actionExecutionSolution; + this.observationRegistry = observationRegistry; } @JsonView(Views.Public.class) @PostMapping @ResponseStatus(HttpStatus.CREATED) public Mono> createAction( - @Valid @RequestBody @JsonView(RequestOnly.class) ActionDTO resource, - @RequestHeader(name = FieldName.BRANCH_NAME, required = false) String branchName) { + @Valid @RequestBody ActionDTO resource, + @RequestHeader(name = FieldName.BRANCH_NAME, required = false) String branchName, + @RequestHeader(name = "Origin", required = false) String originHeader, + ServerWebExchange exchange) { log.debug("Going to create resource {}", resource.getClass().getName()); return layoutActionService .createSingleActionWithBranch(resource, branchName) @@ -77,7 +82,7 @@ public class ActionControllerCE { @PutMapping("/{defaultActionId}") public Mono> updateAction( @PathVariable String defaultActionId, - @Valid @RequestBody @JsonView(RequestOnly.class) ActionDTO resource, + @Valid @RequestBody ActionDTO resource, @RequestHeader(name = FieldName.BRANCH_NAME, required = false) String branchName) { log.debug("Going to update resource with defaultActionId: {}, branch: {}", defaultActionId, branchName); return layoutActionService @@ -173,6 +178,9 @@ public class ActionControllerCE { *

* The controller function is primarily used with param applicationId by the client to fetch the actions in edit * mode. + * + * @param params + * @return */ @JsonView(Views.Public.class) @GetMapping("") From 9574bd75878ebaa476c3705c027970bb20fb2eed Mon Sep 17 00:00:00 2001 From: albinAppsmith <87797149+albinAppsmith@users.noreply.github.com> Date: Thu, 14 Mar 2024 12:56:03 +0530 Subject: [PATCH 4/9] fix: updated add navigation hooks to solve EE issue (#31751) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Fixed add navigation issue when using query package. Fixes #31478 ## Automation /ok-to-test tags="@tag.IDE, @tag.Git" ### :mag: Cypress test results > [!IMPORTANT] > Workflow run: > Commit: `e5c12a51fe0e849f75a776755113ca08a55c89bd` > Cypress dashboard url: Click here! > All cypress tests have passed 🎉🎉🎉 ## Summary by CodeRabbit - **Refactor** - Enhanced flexibility in specifying URL functions for JavaScript and Query additions in the editor. --- app/client/src/ce/pages/Editor/IDE/EditorPane/JS/hooks.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/client/src/ce/pages/Editor/IDE/EditorPane/JS/hooks.ts b/app/client/src/ce/pages/Editor/IDE/EditorPane/JS/hooks.ts index aea677db80..fe816cf00b 100644 --- a/app/client/src/ce/pages/Editor/IDE/EditorPane/JS/hooks.ts +++ b/app/client/src/ce/pages/Editor/IDE/EditorPane/JS/hooks.ts @@ -20,8 +20,8 @@ import { BlankStateContainer } from "pages/Editor/IDE/EditorPane/JS/BlankStateCo import { useCurrentEditorState } from "pages/Editor/IDE/hooks"; import history from "utils/history"; import { FocusEntity, identifyEntityFromPath } from "navigation/FocusEntity"; -import { getJSUrl } from "./utils"; import { useModuleOptions } from "@appsmith/utils/moduleInstanceHelpers"; +import { getJSUrl } from "@appsmith/pages/Editor/IDE/EditorPane/JS/utils"; export const useJSAdd = () => { const pageId = useSelector(getCurrentPageId); From fbdb1d655c96625e84c57c696b6e94660e9e8655 Mon Sep 17 00:00:00 2001 From: Manish Kumar <107841575+sondermanish@users.noreply.github.com> Date: Thu, 14 Mar 2024 12:56:28 +0530 Subject: [PATCH 5/9] chore: ce split for permission scoping for CD (#31761) --- .../base/ActionCollectionServiceCE.java | 5 ++ .../base/ActionCollectionServiceCEImpl.java | 20 ++++- .../newactions/base/NewActionServiceCE.java | 3 + .../base/NewActionServiceCEImpl.java | 8 +- .../NewActionImportableServiceCEImpl.java | 3 +- .../NewPageImportableServiceCEImpl.java | 9 +- .../services/ce/ApplicationPageServiceCE.java | 12 ++- .../ce/ApplicationPageServiceCEImpl.java | 83 ++++++++++++++++--- .../ActionCollectionServiceImplTest.java | 2 +- 9 files changed, 123 insertions(+), 22 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/actioncollections/base/ActionCollectionServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/actioncollections/base/ActionCollectionServiceCE.java index 61a9de6e9e..8b0aa55c0c 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/actioncollections/base/ActionCollectionServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/actioncollections/base/ActionCollectionServiceCE.java @@ -45,6 +45,11 @@ public interface ActionCollectionServiceCE extends CrudService deleteUnpublishedActionCollection(String id); + Mono deleteUnpublishedActionCollectionWithOptionalPermission( + String id, + Optional deleteCollectionPermission, + Optional deleteActionPermission); + Mono deleteWithoutPermissionUnpublishedActionCollection(String id); Mono deleteUnpublishedActionCollection(String id, String branchName); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/actioncollections/base/ActionCollectionServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/actioncollections/base/ActionCollectionServiceCEImpl.java index 48542fa1b2..e226cfcfd9 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/actioncollections/base/ActionCollectionServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/actioncollections/base/ActionCollectionServiceCEImpl.java @@ -356,16 +356,28 @@ public class ActionCollectionServiceCEImpl extends BaseService deleteWithoutPermissionUnpublishedActionCollection(String id) { - return deleteUnpublishedActionCollectionEx(id, Optional.empty()); + return deleteUnpublishedActionCollectionEx( + id, Optional.empty(), Optional.of(actionPermission.getDeletePermission())); } @Override public Mono deleteUnpublishedActionCollection(String id) { - return deleteUnpublishedActionCollectionEx(id, Optional.of(actionPermission.getDeletePermission())); + return deleteUnpublishedActionCollectionEx( + id, + Optional.of(actionPermission.getDeletePermission()), + Optional.of(actionPermission.getDeletePermission())); + } + + @Override + public Mono deleteUnpublishedActionCollectionWithOptionalPermission( + String id, + Optional deleteCollectionPermission, + Optional deleteActionPermission) { + return deleteUnpublishedActionCollectionEx(id, deleteCollectionPermission, deleteActionPermission); } public Mono deleteUnpublishedActionCollectionEx( - String id, Optional permission) { + String id, Optional permission, Optional deleteActionPermission) { Mono actionCollectionMono = repository .findById(id, permission) .switchIfEmpty(Mono.error( @@ -381,7 +393,7 @@ public class ActionCollectionServiceCEImpl extends BaseService newActionService - .deleteUnpublishedAction(actionId) + .deleteUnpublishedActionWithOptionalPermission(actionId, deleteActionPermission) // return an empty action so that the filter can remove it from the list .onErrorResume(throwable -> { log.debug( diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCE.java index 768a422a41..16a4e30d75 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCE.java @@ -82,6 +82,9 @@ public interface NewActionServiceCE extends CrudService { Mono deleteUnpublishedAction(String id); + Mono deleteUnpublishedActionWithOptionalPermission( + String id, Optional newActionDeletePermission); + Flux getUnpublishedActions(MultiValueMap params, Boolean includeJsActions); Flux getUnpublishedActions( diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java index df9ac0dbde..232a645449 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java @@ -842,8 +842,14 @@ public class NewActionServiceCEImpl extends BaseService deleteUnpublishedAction(String id) { + return deleteUnpublishedActionWithOptionalPermission(id, Optional.of(actionPermission.getDeletePermission())); + } + + @Override + public Mono deleteUnpublishedActionWithOptionalPermission( + String id, Optional newActionDeletePermission) { Mono actionMono = repository - .findById(id, actionPermission.getDeletePermission()) + .findById(id, newActionDeletePermission) .switchIfEmpty( Mono.error(new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.ACTION, id))); return actionMono diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/importable/NewActionImportableServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/importable/NewActionImportableServiceCEImpl.java index 90eddc072d..f1da12cc83 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/importable/NewActionImportableServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/importable/NewActionImportableServiceCEImpl.java @@ -36,6 +36,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import static com.appsmith.external.helpers.AppsmithBeanUtils.copyNestedNonNullProperties; @@ -98,7 +99,7 @@ public class NewActionImportableServiceCEImpl implements ImportableServiceCE newActionService - .deleteUnpublishedAction(actionId) + .deleteUnpublishedActionWithOptionalPermission(actionId, Optional.empty()) // return an empty action so that the filter can remove it from the list .onErrorResume(throwable -> { log.debug("Failed to delete action with id {} during import", actionId); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/newpages/importable/NewPageImportableServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/newpages/importable/NewPageImportableServiceCEImpl.java index 771bc06077..d507243b92 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/newpages/importable/NewPageImportableServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/newpages/importable/NewPageImportableServiceCEImpl.java @@ -358,7 +358,14 @@ public class NewPageImportableServiceCEImpl implements ImportableServiceCE { + return applicationPageService.deleteUnpublishedPageWithOptionalPermission( + pageId, + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.empty()); + }) .flatMap(page -> newPageService .archiveWithoutPermissionById(page.getId()) .onErrorResume(e -> { diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationPageServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationPageServiceCE.java index d2af24340e..942320eab6 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationPageServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationPageServiceCE.java @@ -1,5 +1,6 @@ package com.appsmith.server.services.ce; +import com.appsmith.server.acl.AclPermission; import com.appsmith.server.domains.Application; import com.appsmith.server.domains.NewPage; import com.appsmith.server.domains.User; @@ -8,6 +9,8 @@ import com.appsmith.server.dtos.ClonePageMetaDTO; import com.appsmith.server.dtos.PageDTO; import reactor.core.publisher.Mono; +import java.util.Optional; + public interface ApplicationPageServiceCE { Mono createPage(PageDTO page); @@ -47,9 +50,14 @@ public interface ApplicationPageServiceCE { Mono deleteUnpublishedPageByBranchAndDefaultPageId(String defaultPageId, String branchName); - Mono deleteUnpublishedPage(String id); + Mono deleteUnpublishedPageWithOptionalPermission( + String id, + Optional deletePagePermission, + Optional readApplicationPermission, + Optional deleteCollectionPermission, + Optional deleteActionPermission); - Mono deleteWithoutPermissionUnpublishedPage(String id); + Mono deleteUnpublishedPage(String id); Mono publish(String applicationId, boolean isPublishedManually); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationPageServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationPageServiceCEImpl.java index 080ba3b0ce..18693fea13 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationPageServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationPageServiceCEImpl.java @@ -640,6 +640,10 @@ public class ApplicationPageServiceCEImpl implements ApplicationPageServiceCE { return page; })); + final Flux sourceActionCollectionsFlux = getCloneableActionCollections(pageId); + + Flux sourceActionFlux = getCloneableActions(pageId); + return sourcePageMono .flatMap(page -> { clonePageMetaDTO.setBranchedSourcePageId(page.getId()); @@ -738,6 +742,26 @@ public class ApplicationPageServiceCEImpl implements ApplicationPageServiceCE { })); } + protected Flux getCloneableActionCollections(String pageId) { + final Flux sourceActionCollectionsFlux = actionCollectionService.findByPageId(pageId); + return sourceActionCollectionsFlux; + } + + protected Flux getCloneableActions(String pageId) { + Flux sourceActionFlux = newActionService + .findByPageId(pageId, actionPermission.getEditPermission()) + // Set collection reference in actions to null to reset to the new application's collections later + .map(newAction -> { + if (newAction.getUnpublishedAction() != null) { + newAction.getUnpublishedAction().setCollectionId(null); + } + return newAction; + }) + // In case there are no actions in the page being cloned, return empty + .switchIfEmpty(Flux.empty()); + return sourceActionFlux; + } + private Mono clonePageGivenApplicationId(String pageId, String applicationId) { final ClonePageMetaDTO clonePageMetaDTO = new ClonePageMetaDTO(); return clonePageGivenApplicationId(pageId, applicationId, null, clonePageMetaDTO); @@ -901,17 +925,38 @@ public class ApplicationPageServiceCEImpl implements ApplicationPageServiceCE { * In this scenario, if we were to delete all actions associated with the page, we would end up deleting an action * which is currently in published state and is being used. * - * @param id The pageId which needs to be archived. + * @param id The pageId which needs to be archived. + * @param deletePagePermission * @return */ @Override - public Mono deleteWithoutPermissionUnpublishedPage(String id) { - return deleteUnpublishedPageEx(id, Optional.empty()); + public Mono deleteUnpublishedPageWithOptionalPermission( + String id, + Optional deletePagePermission, + Optional readApplicationPermission, + Optional deleteCollectionPermission, + Optional deleteActionPermission) { + return deleteUnpublishedPageEx( + id, + deletePagePermission, + readApplicationPermission, + deleteCollectionPermission, + deleteActionPermission); } @Override public Mono deleteUnpublishedPage(String id) { - return deleteUnpublishedPageEx(id, Optional.of(pagePermission.getDeletePermission())); + + Optional deletePagePermission = Optional.of(pagePermission.getDeletePermission()); + Optional readApplicationPermission = Optional.of(applicationPermission.getReadPermission()); + Optional deleteCollectionPermission = Optional.of(actionPermission.getDeletePermission()); + Optional deleteActionPermission = Optional.of(actionPermission.getDeletePermission()); + return deleteUnpublishedPageEx( + id, + deletePagePermission, + readApplicationPermission, + deleteCollectionPermission, + deleteActionPermission); } /** @@ -923,23 +968,36 @@ public class ApplicationPageServiceCEImpl implements ApplicationPageServiceCE { * In this scenario, if we were to delete all actions associated with the page, we would end up deleting an action * which is currently in published state and is being used. * - * @param id The pageId which needs to be archived. + * @param id The pageId which needs to be archived. + * @param readApplicationPermission + * @param deleteCollectionPermission + * @param deleteActionPermission * @return */ - private Mono deleteUnpublishedPageEx(String id, Optional permission) { + private Mono deleteUnpublishedPageEx( + String id, + Optional deletePagePermission, + Optional readApplicationPermission, + Optional deleteCollectionPermission, + Optional deleteActionPermission) { return newPageService - .findById(id, permission) + .findById(id, deletePagePermission) .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.PAGE, id))) .flatMap(page -> { log.debug( "Going to archive pageId: {} for applicationId: {}", page.getId(), page.getApplicationId()); + // Application is accessed without any application permission over here. + // previously it was getting accessed only with read permission. Mono applicationMono = applicationService - .getById(page.getApplicationId()) + .findById(page.getApplicationId(), readApplicationPermission) + .switchIfEmpty(Mono.error( + new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.APPLICATION, id))) .flatMap(application -> { application.getPages().removeIf(p -> p.getId().equals(page.getId())); return applicationService.save(application); }); + Mono newPageMono; if (page.getPublishedPage() != null) { PageDTO unpublishedPage = page.getUnpublishedPage(); @@ -966,12 +1024,13 @@ public class ApplicationPageServiceCEImpl implements ApplicationPageServiceCE { * condition for delete action */ Mono> archivedActionsMono = newActionService - .findByPageId(page.getId(), actionPermission.getDeletePermission()) + .findByPageId(page.getId(), deleteActionPermission) .filter(newAction -> !StringUtils.hasLength( newAction.getUnpublishedAction().getCollectionId())) .flatMap(action -> { log.debug("Going to archive actionId: {} for applicationId: {}", action.getId(), id); - return newActionService.deleteUnpublishedAction(action.getId()); + return newActionService.deleteUnpublishedActionWithOptionalPermission( + action.getId(), deleteActionPermission); }) .collectList(); @@ -985,8 +1044,8 @@ public class ApplicationPageServiceCEImpl implements ApplicationPageServiceCE { "Going to archive actionCollectionId: {} for applicationId: {}", actionCollection.getId(), id); - return actionCollectionService.deleteUnpublishedActionCollection( - actionCollection.getId()); + return actionCollectionService.deleteUnpublishedActionCollectionWithOptionalPermission( + actionCollection.getId(), deleteCollectionPermission, deleteActionPermission); }) .collectList(); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceImplTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceImplTest.java index 97d4f5fcca..df17a54fa2 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceImplTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceImplTest.java @@ -683,7 +683,7 @@ public class ActionCollectionServiceImplTest { Mockito.when(actionCollectionRepository.findById(Mockito.any(), Mockito.>any())) .thenReturn(Mono.just(actionCollection)); - Mockito.when(newActionService.deleteUnpublishedAction(Mockito.any())) + Mockito.when(newActionService.deleteUnpublishedActionWithOptionalPermission(Mockito.any(), Mockito.any())) .thenReturn(Mono.just( actionCollection.getUnpublishedCollection().getActions().get(0))); From 0ac0bd8b7753d178d8917530101bc1a436837bb9 Mon Sep 17 00:00:00 2001 From: albinAppsmith <87797149+albinAppsmith@users.noreply.github.com> Date: Thu, 14 Mar 2024 13:52:03 +0530 Subject: [PATCH 6/9] fix: Split pane JS setting overflow (#31782) --- .../Editor/JSEditor/JSFunctionSettings.tsx | 67 ++++++++++++------- 1 file changed, 44 insertions(+), 23 deletions(-) diff --git a/app/client/src/pages/Editor/JSEditor/JSFunctionSettings.tsx b/app/client/src/pages/Editor/JSEditor/JSFunctionSettings.tsx index ce5fd94290..86eda6b161 100644 --- a/app/client/src/pages/Editor/JSEditor/JSFunctionSettings.tsx +++ b/app/client/src/pages/Editor/JSEditor/JSFunctionSettings.tsx @@ -15,6 +15,7 @@ interface SettingsHeadingProps { hasInfo?: boolean; info?: string; grow: boolean; + headingCount: number; } export interface OnUpdateSettingsProps { @@ -24,10 +25,14 @@ export interface OnUpdateSettingsProps { } interface SettingsItemProps { + headingCount: number; action: JSAction; disabled?: boolean; onUpdateSettings?: (props: OnUpdateSettingsProps) => void; - renderAdditionalColumns?: (action: JSAction) => React.ReactNode; + renderAdditionalColumns?: ( + action: JSAction, + headingCount: number, + ) => React.ReactNode; } export interface JSFunctionSettingsProps { @@ -61,6 +66,7 @@ const StyledIcon = styled(Icon)` `; export const SettingColumn = styled.div<{ + headingCount: number; grow?: boolean; isHeading?: boolean; }>` @@ -68,13 +74,13 @@ export const SettingColumn = styled.div<{ align-items: center; flex-grow: ${(props) => (props.grow ? 1 : 0)}; padding: 5px 12px; - min-width: 250px; + width: ${({ headingCount }) => `calc(100% / ${headingCount})`}; ${(props) => props.isHeading && ` font-weight: ${props.theme.fontWeights[2]}; - font-size: ${props.theme.fontSizes[2]}px + font-size: ${props.theme.fontSizes[2]}px; margin-right: 9px; `} @@ -91,8 +97,7 @@ const JSFunctionSettingsWrapper = styled.div` const SettingsContainer = styled.div` display: flex; flex-direction: column; - width: max-content; - min-width: 700px; + width: 100%; height: 100%; & > h3 { margin: 20px 0; @@ -116,9 +121,15 @@ const SettingsBodyWrapper = styled.div` const SwitchWrapper = styled.div` margin-left: 6ch; `; -function SettingsHeading({ grow, hasInfo, info, text }: SettingsHeadingProps) { +function SettingsHeading({ + grow, + hasInfo, + headingCount, + info, + text, +}: SettingsHeadingProps) { return ( - + {text} {hasInfo && info && ( info)}> @@ -132,6 +143,7 @@ function SettingsHeading({ grow, hasInfo, info, text }: SettingsHeadingProps) { function SettingsItem({ action, disabled, + headingCount, onUpdateSettings, renderAdditionalColumns, }: SettingsItemProps) { @@ -174,10 +186,13 @@ function SettingsItem({ className="t--async-js-function-settings" id={`${action.name}-settings`} > - + {action.name} - + {RADIO_OPTIONS.length > 2 ? ( )} - + {RADIO_OPTIONS.length > 2 ? ( )} - {renderAdditionalColumns?.(action)} + {renderAdditionalColumns?.(action, headingCount)} ); } @@ -250,6 +268,7 @@ function JSFunctionSettingsView({ onUpdateSettings, renderAdditionalColumns, }: JSFunctionSettingsProps) { + const headings = [...SETTINGS_HEADINGS, ...additionalHeadings]; return ( @@ -257,17 +276,16 @@ function JSFunctionSettingsView({ - {[...SETTINGS_HEADINGS, ...additionalHeadings].map( - (setting, index) => ( - - ), - )} + {headings.map((setting, index) => ( + + ))} @@ -276,6 +294,7 @@ function JSFunctionSettingsView({ - {createMessage(NO_JS_FUNCTIONS)} + + {createMessage(NO_JS_FUNCTIONS)} + )} From 7b19b3d351dee27c489c67cbc87c25ee2f22d976 Mon Sep 17 00:00:00 2001 From: Ankita Kinger Date: Thu, 14 Mar 2024 14:31:44 +0530 Subject: [PATCH 7/9] fix: Refactoring fetching of icon for module instances (#31765) --- .../ErrorLogs/getLogIconForEntity.tsx | 12 +++---- .../ActionCreator/helpers.tsx | 23 ++++++------- .../ErrorLogs/components/LogEntityLink.tsx | 9 ++--- .../useSource/useConnectToOptions.tsx | 17 ++-------- .../ActionSelectorControl.tsx | 4 +-- .../src/pages/Editor/APIEditor/index.tsx | 10 +++++- .../pages/Editor/Explorer/ExplorerIcons.tsx | 8 +++++ .../src/pages/Editor/QueryEditor/index.tsx | 10 +++++- app/client/src/pages/Editor/utils.tsx | 34 +++++++++++++++++++ 9 files changed, 82 insertions(+), 45 deletions(-) diff --git a/app/client/src/ce/components/editorComponents/Debugger/ErrorLogs/getLogIconForEntity.tsx b/app/client/src/ce/components/editorComponents/Debugger/ErrorLogs/getLogIconForEntity.tsx index 0e02f7ae03..fafb6abbb0 100644 --- a/app/client/src/ce/components/editorComponents/Debugger/ErrorLogs/getLogIconForEntity.tsx +++ b/app/client/src/ce/components/editorComponents/Debugger/ErrorLogs/getLogIconForEntity.tsx @@ -1,5 +1,4 @@ import React from "react"; -import type { Plugin } from "api/PluginApi"; import type { LogItemProps } from "components/editorComponents/Debugger/ErrorLogs/ErrorLogItem"; import { PluginType } from "entities/Action"; import WidgetIcon from "pages/Editor/Explorer/Widgets/WidgetIcon"; @@ -13,7 +12,7 @@ import { ENTITY_TYPE } from "entities/DataTree/dataTreeFactory"; export const getIconForEntity: Record< string, - (props: LogItemProps, pluginGroups: Record) => any + (props: LogItemProps, pluginImages: Record) => any > = { [ENTITY_TYPE.WIDGET]: (props) => { if (props.source?.pluginType) { @@ -25,19 +24,16 @@ export const getIconForEntity: Record< [ENTITY_TYPE.JSACTION]: () => { return JsFileIconV2(16, 16, true, true); }, - [ENTITY_TYPE.ACTION]: (props, pluginGroups) => { + [ENTITY_TYPE.ACTION]: (props, pluginImages) => { const { iconId, source } = props; if (source?.pluginType === PluginType.API && source.httpMethod) { // If the source is an API action. return ApiMethodIcon(source.httpMethod, "16px", "32px", 50); - } else if (iconId && pluginGroups[iconId]) { + } else if (iconId && pluginImages[iconId]) { // If the source is a Datasource action. return ( - entityIcon + entityIcon ); } diff --git a/app/client/src/components/editorComponents/ActionCreator/helpers.tsx b/app/client/src/components/editorComponents/ActionCreator/helpers.tsx index 6b96ebc666..38b9cb3a2b 100644 --- a/app/client/src/components/editorComponents/ActionCreator/helpers.tsx +++ b/app/client/src/components/editorComponents/ActionCreator/helpers.tsx @@ -20,7 +20,6 @@ import type { JSAction, Variable } from "entities/JSCollection"; import keyBy from "lodash/keyBy"; import { getActionConfig } from "pages/Editor/Explorer/Actions/helpers"; import { JsFileIconV2 } from "pages/Editor/Explorer/ExplorerIcons"; -import { useMemo } from "react"; import { useDispatch, useSelector } from "react-redux"; import type { ActionData, @@ -69,9 +68,10 @@ import { setShowCreateNewModal } from "actions/propertyPaneActions"; import { setIdeEditorViewMode } from "actions/ideActions"; import { EditorViewMode } from "@appsmith/entities/IDE/constants"; import { getIsSideBySideEnabled } from "selectors/ideSelectors"; -import { resolveIcon } from "pages/Editor/utils"; +import { getModuleIcon, getPluginImagesFromPlugins } from "pages/Editor/utils"; import { getAllModules } from "@appsmith/selectors/modulesSelector"; import type { Module } from "@appsmith/constants/ModuleConstants"; +import type { Plugin } from "api/PluginApi"; const actionList: { label: string; @@ -396,7 +396,7 @@ export function useModalDropdownList(handleClose: () => void) { export function getApiQueriesAndJSActionOptionsWithChildren( pageId: string, - plugins: any, + plugins: Plugin[], actions: ActionDataState, jsActions: Array, dispatch: any, @@ -422,7 +422,7 @@ export function getApiQueriesAndJSActionOptionsWithChildren( } function getApiAndQueryOptions( - plugins: any, + plugins: Plugin[], actions: ActionDataState, dispatch: any, handleClose: () => void, @@ -431,6 +431,8 @@ function getApiAndQueryOptions( ) { const state = store.getState(); const isSideBySideEnabled = getIsSideBySideEnabled(state); + const pluginImages = getPluginImagesFromPlugins(plugins); + const pluginGroups: any = keyBy(plugins, "id"); const createQueryObject: TreeDropdownOption = { label: "New query", @@ -474,7 +476,7 @@ function getApiAndQueryOptions( type: queryOptions.value, icon: getActionConfig(api.config.pluginType)?.getIcon( api.config, - plugins[(api as any).config.datasource.pluginId], + pluginGroups[(api as any).config.datasource.pluginId], api.config.pluginType === PluginType.API, ), } as TreeDropdownOption); @@ -488,7 +490,7 @@ function getApiAndQueryOptions( type: queryOptions.value, icon: getActionConfig(query.config.pluginType)?.getIcon( query.config, - plugins[(query as any).config.datasource.pluginId], + pluginGroups[(query as any).config.datasource.pluginId], ), } as TreeDropdownOption); }); @@ -499,11 +501,7 @@ function getApiAndQueryOptions( id: instance.config.id, value: instance.config.name, type: queryOptions.value, - icon: resolveIcon({ - iconLocation: plugins[module.pluginId]?.iconLocation || "", - pluginType: module.pluginType, - moduleType: module.type, - }), + icon: getModuleIcon(module, pluginImages), } as TreeDropdownOption); }); } @@ -629,7 +627,6 @@ export function useApisQueriesAndJsActionOptions(handleClose: () => void) { const plugins = useSelector((state: AppState) => { return state.entities.plugins.list; }); - const pluginGroups: any = useMemo(() => keyBy(plugins, "id"), [plugins]); const actions = useSelector(getCurrentActions); const jsActions = useSelector(getCurrentJSCollections); const queryModuleInstances = useSelector( @@ -641,7 +638,7 @@ export function useApisQueriesAndJsActionOptions(handleClose: () => void) { // this function gets all the Queries/API's/JS Objects and attaches it to actionList return getApiQueriesAndJSActionOptionsWithChildren( pageId, - pluginGroups, + plugins, actions, jsActions, dispatch, diff --git a/app/client/src/components/editorComponents/Debugger/ErrorLogs/components/LogEntityLink.tsx b/app/client/src/components/editorComponents/Debugger/ErrorLogs/components/LogEntityLink.tsx index 2415a8f297..9e074a36e7 100644 --- a/app/client/src/components/editorComponents/Debugger/ErrorLogs/components/LogEntityLink.tsx +++ b/app/client/src/components/editorComponents/Debugger/ErrorLogs/components/LogEntityLink.tsx @@ -8,7 +8,7 @@ import { getPlugins } from "@appsmith/selectors/entitiesSelector"; import EntityLink from "../../EntityLink"; import { DebuggerLinkUI } from "components/editorComponents/Debugger/DebuggerEntityLink"; import { getIconForEntity } from "@appsmith/components/editorComponents/Debugger/ErrorLogs/getLogIconForEntity"; -import type { Plugin } from "api/PluginApi"; +import { getPluginImagesFromPlugins } from "pages/Editor/utils"; const EntityLinkWrapper = styled.div` display: flex; @@ -30,11 +30,11 @@ const IconWrapper = styled.span` `; // This function is used to fetch the icon component for the entity link. -const getIcon = (props: LogItemProps, pluginGroups: Record) => { +const getIcon = (props: LogItemProps, pluginImages: Record) => { const entityType = props.source?.type; let icon = null; if (entityType) { - icon = getIconForEntity[entityType](props, pluginGroups); + icon = getIconForEntity[entityType](props, pluginImages); } return icon || icon; }; @@ -43,6 +43,7 @@ const getIcon = (props: LogItemProps, pluginGroups: Record) => { export default function LogEntityLink(props: LogItemProps) { const plugins = useSelector(getPlugins); const pluginGroups = useMemo(() => keyBy(plugins, "id"), [plugins]); + const pluginImages = getPluginImagesFromPlugins(plugins); const plugin = props.iconId ? pluginGroups[props.iconId] : undefined; return ( @@ -55,7 +56,7 @@ export default function LogEntityLink(props: LogItemProps) { lineHeight: "14px", }} > - {getIcon(props, pluginGroups)} + {getIcon(props, pluginImages)} - - - ) - ); + return getModuleIcon(module, pluginImages); } else { const action = query as ActionData; return ( diff --git a/app/client/src/components/propertyControls/ActionSelectorControl.tsx b/app/client/src/components/propertyControls/ActionSelectorControl.tsx index ef4a73e6ac..53efb66192 100644 --- a/app/client/src/components/propertyControls/ActionSelectorControl.tsx +++ b/app/client/src/components/propertyControls/ActionSelectorControl.tsx @@ -22,7 +22,6 @@ import { getPlugins, } from "@appsmith/selectors/entitiesSelector"; import store from "store"; -import keyBy from "lodash/keyBy"; import { getCurrentPageId } from "selectors/editorSelectors"; import { getApiQueriesAndJSActionOptionsWithChildren } from "components/editorComponents/ActionCreator/helpers"; import { selectEvaluationVersion } from "@appsmith/selectors/applicationSelectors"; @@ -152,12 +151,11 @@ class ActionSelectorControl extends BaseControl { const pageId = getCurrentPageId(state); const plugins = getPlugins(state); - const pluginGroups: any = keyBy(plugins, "id"); // this function gets all the Queries/API's/JS Objects and attaches it to actionList const fieldOptions = getApiQueriesAndJSActionOptionsWithChildren( pageId, - pluginGroups, + plugins, actions, jsCollections, () => { diff --git a/app/client/src/pages/Editor/APIEditor/index.tsx b/app/client/src/pages/Editor/APIEditor/index.tsx index 90d7a519a3..7f1f5b7da2 100644 --- a/app/client/src/pages/Editor/APIEditor/index.tsx +++ b/app/client/src/pages/Editor/APIEditor/index.tsx @@ -40,6 +40,7 @@ import ConvertEntityNotification from "@appsmith/pages/common/ConvertEntityNotif import { useIsEditorPaneSegmentsEnabled } from "../IDE/hooks"; import { Icon } from "design-system"; import { resolveIcon } from "../utils"; +import { ENTITY_ICON_SIZE, EntityIcon } from "../Explorer/ExplorerIcons"; type ApiEditorWrapperProps = RouteComponentProps; @@ -72,7 +73,14 @@ function ApiEditorWrapper(props: ApiEditorWrapperProps) { iconLocation: pluginGroups[pluginId]?.iconLocation || "", pluginType: action?.pluginType || "", moduleType: action?.actionConfiguration?.body?.moduleType, - }) || ; + }) || ( + + + + ); const isChangePermitted = getHasManageActionPermission( isFeatureEnabled, diff --git a/app/client/src/pages/Editor/Explorer/ExplorerIcons.tsx b/app/client/src/pages/Editor/Explorer/ExplorerIcons.tsx index e1c760a333..7fd10420ba 100644 --- a/app/client/src/pages/Editor/Explorer/ExplorerIcons.tsx +++ b/app/client/src/pages/Editor/Explorer/ExplorerIcons.tsx @@ -337,3 +337,11 @@ export function AppsmithAIIcon() { export function ActionUrlIcon(url: string) { return ; } + +export function DefaultModuleIcon() { + return ( + + + + ); +} diff --git a/app/client/src/pages/Editor/QueryEditor/index.tsx b/app/client/src/pages/Editor/QueryEditor/index.tsx index e7c90460a7..5ca74efe6d 100644 --- a/app/client/src/pages/Editor/QueryEditor/index.tsx +++ b/app/client/src/pages/Editor/QueryEditor/index.tsx @@ -41,6 +41,7 @@ import { PluginType } from "entities/Action"; import { useIsEditorPaneSegmentsEnabled } from "../IDE/hooks"; import { Icon } from "design-system"; import { resolveIcon } from "../utils"; +import { ENTITY_ICON_SIZE, EntityIcon } from "../Explorer/ExplorerIcons"; type QueryEditorProps = RouteComponentProps; @@ -66,7 +67,14 @@ function QueryEditor(props: QueryEditorProps) { iconLocation: pluginImages[pluginId] || "", pluginType: action?.pluginType || "", moduleType: action?.actionConfiguration?.body?.moduleType, - }) || ; + }) || ( + + + + ); const isDeletePermitted = getHasDeleteActionPermission( isFeatureEnabled, diff --git a/app/client/src/pages/Editor/utils.tsx b/app/client/src/pages/Editor/utils.tsx index 03597d3b22..fab1158cc4 100644 --- a/app/client/src/pages/Editor/utils.tsx +++ b/app/client/src/pages/Editor/utils.tsx @@ -20,6 +20,7 @@ import { useSelector } from "react-redux"; import { getCurrentPageId } from "selectors/editorSelectors"; import type { WidgetCardProps } from "widgets/BaseWidget"; import type { ActionResponse } from "api/ActionAPI"; +import type { Module } from "@appsmith/constants/ModuleConstants"; import { MODULE_TYPE } from "@appsmith/constants/ModuleConstants"; import { ENTITY_ICON_SIZE, @@ -29,6 +30,9 @@ import { } from "pages/Editor/Explorer/ExplorerIcons"; import { PluginType } from "entities/Action"; import { getAssetUrl } from "@appsmith/utils/airgapHelpers"; +import type { Plugin } from "api/PluginApi"; +import ImageAlt from "assets/images/placeholder-image.svg"; +import { Icon } from "design-system"; export const draggableElement = ( id: string, @@ -381,3 +385,33 @@ export function resolveIcon({ return resolveQueryModuleIcon(iconLocation, pluginType, isLargeIcon); } } + +export function getModuleIcon( + module: Module | undefined, + pluginImages: Record, + isLargeIcon = false, +) { + return module ? ( + resolveIcon({ + iconLocation: pluginImages[module.pluginId] || "", + pluginType: module.pluginType, + moduleType: module.type, + isLargeIcon, + }) + ) : ( + + + + ); +} + +export function getPluginImagesFromPlugins(plugins: Plugin[]) { + const pluginImages: Record = {}; + plugins.forEach((plugin) => { + pluginImages[plugin.id] = plugin?.iconLocation ?? ImageAlt; + }); + return pluginImages; +} From 3c59066b893e8ac9a997f5659a019c5f2c2963f1 Mon Sep 17 00:00:00 2001 From: Hetu Nandu Date: Thu, 14 Mar 2024 15:21:54 +0530 Subject: [PATCH 8/9] fix: Query Sorting control width --- .../src/components/formControls/SortingControl.tsx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/app/client/src/components/formControls/SortingControl.tsx b/app/client/src/components/formControls/SortingControl.tsx index 100a20b87a..0335ae93bd 100644 --- a/app/client/src/components/formControls/SortingControl.tsx +++ b/app/client/src/components/formControls/SortingControl.tsx @@ -65,7 +65,7 @@ const SortingDropdownContainer = styled.div<{ size: string }>` gap: 5px; align-items: center; > div { - width: 270px; + width: 250px; } ${(props) => props.size === "small" && @@ -168,6 +168,9 @@ function SortingComponent(props: any) { From 05f9dcb9e65c0dc2f474c77f6a620839b4b502c4 Mon Sep 17 00:00:00 2001 From: Trisha Anand Date: Thu, 14 Mar 2024 16:18:44 +0530 Subject: [PATCH 9/9] fix: Making userTag a hidden field to not expose it as a user configurable option with Appsmith being the default value (#31792) This is to abide by the Databricks standard practice which expects the user agent tag to be set automatically and shouldnt be configurable by end user (in this case developers creating datasources on top of Databricks). The user agent tag would be set to Appsmith. --- .../databricksPlugin/src/main/resources/form.json | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/server/appsmith-plugins/databricksPlugin/src/main/resources/form.json b/app/server/appsmith-plugins/databricksPlugin/src/main/resources/form.json index c5610df152..0bc2b04c8f 100644 --- a/app/server/appsmith-plugins/databricksPlugin/src/main/resources/form.json +++ b/app/server/appsmith-plugins/databricksPlugin/src/main/resources/form.json @@ -92,11 +92,7 @@ "isRequired": false, "initialValue": "Appsmith", "placeholderText": "Appsmith", - "hidden": { - "path": "datasourceConfiguration.properties[0].value", - "comparison": "NOT_EQUALS", - "value": "FORM_PROPERTIES_CONFIGURATION" - } + "hidden": true }, { "label": "JDBC URL",