From 36163ee71008caf368b7000864a95c41f8d3c95e Mon Sep 17 00:00:00 2001 From: Aishwarya-U-R <91450662+Aishwarya-U-R@users.noreply.github.com> Date: Wed, 20 Sep 2023 14:28:22 +0530 Subject: [PATCH 01/32] test: Cypress | CodeScanner1_spec from js to ts (#27487) ## Description - This PR updates the CodeScanner1_spec from js to ts to comply rule --- .../{CodeScanner1_spec.js => CodeScanner1_spec.ts} | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) rename app/client/cypress/e2e/Regression/ClientSide/Widgets/CodeScanner/{CodeScanner1_spec.js => CodeScanner1_spec.ts} (96%) diff --git a/app/client/cypress/e2e/Regression/ClientSide/Widgets/CodeScanner/CodeScanner1_spec.js b/app/client/cypress/e2e/Regression/ClientSide/Widgets/CodeScanner/CodeScanner1_spec.ts similarity index 96% rename from app/client/cypress/e2e/Regression/ClientSide/Widgets/CodeScanner/CodeScanner1_spec.js rename to app/client/cypress/e2e/Regression/ClientSide/Widgets/CodeScanner/CodeScanner1_spec.ts index 9ce99a7ab9..c39d2a2c17 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/Widgets/CodeScanner/CodeScanner1_spec.js +++ b/app/client/cypress/e2e/Regression/ClientSide/Widgets/CodeScanner/CodeScanner1_spec.ts @@ -1,7 +1,7 @@ -const explorer = require("../../../../../locators/explorerlocators.json"); -const widgetsPage = require("../../../../../locators/Widgets.json"); -const commonlocators = require("../../../../../locators/commonlocators.json"); -const publish = require("../../../../../locators/publishWidgetspage.json"); +import explorer from "../../../../../locators/explorerlocators.json"; +import publish from "../../../../../locators/publishWidgetspage.json"; +import commonlocators from "../../../../../locators/commonlocators.json"; +import widgetsPage from "../../../../../locators/Widgets.json"; import * as _ from "../../../../../support/Objects/ObjectsCore"; const widgetName = "codescannerwidget"; const codeScannerVideoOnPublishPage = `${publish.codescannerwidget} ${commonlocators.codeScannerVideo}`; From f2ae7695d84c2d707e7c3901e5fb8b70fa280437 Mon Sep 17 00:00:00 2001 From: Vemparala Surya Vamsi <121419957+vsvamsi1@users.noreply.github.com> Date: Wed, 20 Sep 2023 21:07:02 +0530 Subject: [PATCH 02/32] fix: revert skip klona (#27503) ## Description Reverting change for skip klona changes, this commit reverts the fix for https://github.com/appsmithorg/appsmith/issues/27048 #### PR fixes following issue(s) Fixes #27048 #### Type of change - Bug fix (non-breaking change which fixes an issue) ## Testing #### How Has This Been Tested? - [ ] Manual - [ ] JUnit - [ ] Jest - [x] Cypress > > #### Test Plan > Add Testsmith test cases links that relate to this PR > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --- .../workers/common/DataTreeEvaluator/index.ts | 18 ++++++++---------- .../workers/common/DataTreeEvaluator/test.ts | 7 +++---- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/app/client/src/workers/common/DataTreeEvaluator/index.ts b/app/client/src/workers/common/DataTreeEvaluator/index.ts index cf1fb653d5..a87a3fb5da 100644 --- a/app/client/src/workers/common/DataTreeEvaluator/index.ts +++ b/app/client/src/workers/common/DataTreeEvaluator/index.ts @@ -344,8 +344,7 @@ export default class DataTreeEvaluator { const evaluationOrder = this.sortedDependencies; // Evaluate const { evalMetaUpdates, evaluatedTree, staleMetaIds } = this.evaluateTree( - //we need to deep clone oldUnEvalTree because evaluateTree will mutate it - klona(this.oldUnEvalTree), + this.oldUnEvalTree, evaluationOrder, undefined, this.oldConfigTree, @@ -782,7 +781,6 @@ export default class DataTreeEvaluator { evaluatedTree: newEvalTree, staleMetaIds, } = this.evaluateTree( - // should not clone evalTree unnessarily because it is anyways being overwritten in the subsequent statement this.evalTree, evaluationOrder, { @@ -924,7 +922,7 @@ export default class DataTreeEvaluator { } evaluateTree( - dataTree: DataTree, + oldUnevalTree: DataTree, evaluationOrder: Array, options: { isFirstTree: boolean; @@ -941,8 +939,10 @@ export default class DataTreeEvaluator { evalMetaUpdates: EvalMetaUpdates; staleMetaIds: string[]; } { + const tree = klona(oldUnevalTree); + errorModifier.updateAsyncFunctions( - dataTree, + tree, this.getConfigTree(), this.dependencyMap, ); @@ -1168,7 +1168,7 @@ export default class DataTreeEvaluator { return set(currentTree, fullPropertyPath, evalPropertyValue); } }, - dataTree, + tree, ); return { @@ -1181,7 +1181,7 @@ export default class DataTreeEvaluator { type: EvalErrorTypes.EVAL_TREE_ERROR, message: (error as Error).message, }); - return { evaluatedTree: dataTree, evalMetaUpdates, staleMetaIds: [] }; + return { evaluatedTree: tree, evalMetaUpdates, staleMetaIds: [] }; } } @@ -1472,9 +1472,7 @@ export default class DataTreeEvaluator { // setting parseValue in dataTree set(currentTree, fullPropertyPath, parsedValue); // setting evalPropertyValue in unParsedEvalTree - // cloning evalPropertyValue because parsedValue and evalPropertyValue could be equal, they both could share the same reference - //hence we are cloning evalPropertyValue to seperate them - set(this.getUnParsedEvalTree(), fullPropertyPath, klona(evalPropertyValue)); + set(this.getUnParsedEvalTree(), fullPropertyPath, evalPropertyValue); } reValidateWidgetDependentProperty({ diff --git a/app/client/src/workers/common/DataTreeEvaluator/test.ts b/app/client/src/workers/common/DataTreeEvaluator/test.ts index 1c4dc3ae3c..7b78bbe691 100644 --- a/app/client/src/workers/common/DataTreeEvaluator/test.ts +++ b/app/client/src/workers/common/DataTreeEvaluator/test.ts @@ -16,7 +16,6 @@ import { replaceThisDotParams } from "./utils"; import { isDataField } from "./utils"; import widgets from "widgets"; import type { WidgetConfiguration } from "WidgetProvider/constants"; -import { klona } from "klona"; const widgetConfigMap: Record< string, @@ -360,7 +359,7 @@ describe("DataTreeEvaluator", () => { nonDynamicFieldValidationOrder: nonDynamicFieldValidationOrder5, unEvalUpdates, } = dataTreeEvaluator.setupUpdateTree( - klona(nestedArrayAccessorCyclicDependency.apiSuccessUnEvalTree), + nestedArrayAccessorCyclicDependency.apiSuccessUnEvalTree, nestedArrayAccessorCyclicDependencyConfig.apiSuccessConfigTree, ); dataTreeEvaluator.evalAndValidateSubTree( @@ -424,7 +423,7 @@ describe("DataTreeEvaluator", () => { nonDynamicFieldValidationOrder, unEvalUpdates, } = dataTreeEvaluator.setupUpdateTree( - klona(nestedArrayAccessorCyclicDependency.apiSuccessUnEvalTree), + nestedArrayAccessorCyclicDependency.apiSuccessUnEvalTree, nestedArrayAccessorCyclicDependencyConfig.apiSuccessConfigTree, ); dataTreeEvaluator.evalAndValidateSubTree( @@ -471,7 +470,7 @@ describe("DataTreeEvaluator", () => { nonDynamicFieldValidationOrder: nonDynamicFieldValidationOrder2, unEvalUpdates, } = dataTreeEvaluator.setupUpdateTree( - klona(nestedArrayAccessorCyclicDependency.apiSuccessUnEvalTree), + nestedArrayAccessorCyclicDependency.apiSuccessUnEvalTree, nestedArrayAccessorCyclicDependencyConfig.apiSuccessConfigTree, ); dataTreeEvaluator.evalAndValidateSubTree( From 39c62826ee96a169302c4253e0ae63eb10dc4685 Mon Sep 17 00:00:00 2001 From: Trisha Anand Date: Fri, 22 Sep 2023 13:34:26 +0530 Subject: [PATCH 03/32] Revert "fix: fixed failing queries using aggregation pipeline (#26132)" (#27562) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 66d5027126523fb73cb14ce1336b171a85f8db37. > Pull Request Template > > Use this template to quickly create a well written pull request. Delete all quotes before creating the pull request. > ## Description > Add a TL;DR when description is extra long (helps content team) > > Please include a summary of the changes and which issue has been fixed. Please also include relevant motivation > and context. List any dependencies that are required for this change > > Links to Notion, Figma or any other documents that might be relevant to the PR > > #### PR fixes following issue(s) Fixes # (issue number) > if no issue exists, please create an issue and ask the maintainers about this first > > #### Media > A video or a GIF is preferred. when using Loom, don’t embed because it looks like it’s a GIF. instead, just link to the video > > #### Type of change > Please delete options that are not relevant. - Bug fix (non-breaking change which fixes an issue) - New feature (non-breaking change which adds functionality) - Breaking change (fix or feature that would cause existing functionality to not work as expected) - Chore (housekeeping or task changes that don't impact user perception) - This change requires a documentation update > > > ## Testing > #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [ ] Manual - [ ] JUnit - [ ] Jest - [ ] Cypress > > #### Test Plan > Add Testsmith test cases links that relate to this PR > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --- .../repositories/BaseRepositoryImpl.java | 4 +- .../CustomNewActionRepositoryImpl.java | 6 +- .../CustomNewPageRepositoryImpl.java | 6 +- .../ce/BaseAppsmithRepositoryCEImpl.java | 6 +- .../ce/CustomNewActionRepositoryCE.java | 2 +- .../ce/CustomNewActionRepositoryCEImpl.java | 42 +++------- .../ce/CustomNewPageRepositoryCE.java | 6 +- .../ce/CustomNewPageRepositoryCEImpl.java | 77 +++---------------- .../ce/NewActionRepositoryCE.java | 5 -- .../ce/ApplicationPageServiceCEImpl.java | 5 +- .../services/ce/NewActionServiceCE.java | 4 +- .../services/ce/NewActionServiceCEImpl.java | 4 +- .../server/services/ce/NewPageServiceCE.java | 4 +- .../services/ce/NewPageServiceCEImpl.java | 4 +- .../services/ce/NewActionServiceUnitTest.java | 11 +-- 15 files changed, 45 insertions(+), 141 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/BaseRepositoryImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/BaseRepositoryImpl.java index a4090d8f70..7bae25049d 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/BaseRepositoryImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/BaseRepositoryImpl.java @@ -161,9 +161,7 @@ public class BaseRepositoryImpl .flatMapMany(principal -> { Query query = new Query(notDeleted()); return mongoOperations.find( - query.cursorBatchSize(10000), - entityInformation.getJavaType(), - entityInformation.getCollectionName()); + query, entityInformation.getJavaType(), entityInformation.getCollectionName()); }); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomNewActionRepositoryImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomNewActionRepositoryImpl.java index b3eb05cb8d..e45c7fd224 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomNewActionRepositoryImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomNewActionRepositoryImpl.java @@ -2,7 +2,6 @@ package com.appsmith.server.repositories; import com.appsmith.server.repositories.ce.CustomNewActionRepositoryCEImpl; import lombok.extern.slf4j.Slf4j; -import org.springframework.data.mongodb.core.MongoTemplate; import org.springframework.data.mongodb.core.ReactiveMongoOperations; import org.springframework.data.mongodb.core.convert.MongoConverter; import org.springframework.stereotype.Component; @@ -15,8 +14,7 @@ public class CustomNewActionRepositoryImpl extends CustomNewActionRepositoryCEIm public CustomNewActionRepositoryImpl( ReactiveMongoOperations mongoOperations, MongoConverter mongoConverter, - CacheableRepositoryHelper cacheableRepositoryHelper, - MongoTemplate mongoTemplate) { - super(mongoOperations, mongoConverter, cacheableRepositoryHelper, mongoTemplate); + CacheableRepositoryHelper cacheableRepositoryHelper) { + super(mongoOperations, mongoConverter, cacheableRepositoryHelper); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomNewPageRepositoryImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomNewPageRepositoryImpl.java index 690b54d4c1..370e5202b2 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomNewPageRepositoryImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomNewPageRepositoryImpl.java @@ -2,7 +2,6 @@ package com.appsmith.server.repositories; import com.appsmith.server.repositories.ce.CustomNewPageRepositoryCEImpl; import lombok.extern.slf4j.Slf4j; -import org.springframework.data.mongodb.core.MongoTemplate; import org.springframework.data.mongodb.core.ReactiveMongoOperations; import org.springframework.data.mongodb.core.convert.MongoConverter; import org.springframework.stereotype.Component; @@ -14,8 +13,7 @@ public class CustomNewPageRepositoryImpl extends CustomNewPageRepositoryCEImpl i public CustomNewPageRepositoryImpl( ReactiveMongoOperations mongoOperations, MongoConverter mongoConverter, - CacheableRepositoryHelper cacheableRepositoryHelper, - MongoTemplate mongoTemplate) { - super(mongoOperations, mongoConverter, cacheableRepositoryHelper, mongoTemplate); + CacheableRepositoryHelper cacheableRepositoryHelper) { + super(mongoOperations, mongoConverter, cacheableRepositoryHelper); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/BaseAppsmithRepositoryCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/BaseAppsmithRepositoryCEImpl.java index 28d0012b85..c2d9b0634b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/BaseAppsmithRepositoryCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/BaseAppsmithRepositoryCEImpl.java @@ -22,7 +22,6 @@ import org.springframework.data.mongodb.core.query.Criteria; import org.springframework.data.mongodb.core.query.Query; import org.springframework.data.mongodb.core.query.Update; import org.springframework.data.mongodb.core.query.UpdateDefinition; -import org.springframework.data.mongodb.repository.Meta; import org.springframework.security.core.context.ReactiveSecurityContextHolder; import org.springframework.util.CollectionUtils; import reactor.core.publisher.Flux; @@ -167,7 +166,7 @@ public abstract class BaseAppsmithRepositoryCEImpl { return mongoOperations .query(this.genericDomain) - .matching(query.cursorBatchSize(10000)) + .matching(query) .one() .flatMap(obj -> setUserPermissionsInObject(obj, permissionGroups)); }); @@ -332,7 +331,6 @@ public abstract class BaseAppsmithRepositoryCEImpl { }); } - @Meta(cursorBatchSize = 10000) protected Mono queryOne( List criterias, List projectionFieldNames, Optional permission) { Mono> permissionGroupsMono = getCurrentUserPermissionGroupsIfRequired(permission); @@ -541,7 +539,7 @@ public abstract class BaseAppsmithRepositoryCEImpl { sortOptional.ifPresent(sort -> query.with(sort)); return mongoOperations .query(this.genericDomain) - .matching(query.cursorBatchSize(10000)) + .matching(query) .all() .flatMap(obj -> setUserPermissionsInObject(obj, permissionGroups)); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCE.java index 11a6cb6af1..007ff409f8 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCE.java @@ -78,7 +78,7 @@ public interface CustomNewActionRepositoryCE extends AppsmithRepository> bulkUpdate(List newActions); - Mono> publishActions(String applicationId, AclPermission permission); + Mono publishActions(String applicationId, AclPermission permission); Mono archiveDeletedUnpublishedActions(String applicationId, AclPermission permission); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCEImpl.java index c45cf40bfa..5a90b20dce 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCEImpl.java @@ -20,12 +20,9 @@ import lombok.extern.slf4j.Slf4j; import org.bson.Document; import org.bson.types.ObjectId; import org.springframework.data.domain.Sort; -import org.springframework.data.mongodb.core.MongoTemplate; import org.springframework.data.mongodb.core.ReactiveMongoOperations; import org.springframework.data.mongodb.core.aggregation.Aggregation; -import org.springframework.data.mongodb.core.aggregation.AggregationOperation; -import org.springframework.data.mongodb.core.aggregation.AggregationResults; -import org.springframework.data.mongodb.core.aggregation.Fields; +import org.springframework.data.mongodb.core.aggregation.AggregationUpdate; import org.springframework.data.mongodb.core.aggregation.GroupOperation; import org.springframework.data.mongodb.core.aggregation.MatchOperation; import org.springframework.data.mongodb.core.aggregation.ProjectionOperation; @@ -55,15 +52,11 @@ import static org.springframework.data.mongodb.core.query.Criteria.where; public class CustomNewActionRepositoryCEImpl extends BaseAppsmithRepositoryImpl implements CustomNewActionRepositoryCE { - private final MongoTemplate mongoTemplate; - public CustomNewActionRepositoryCEImpl( ReactiveMongoOperations mongoOperations, MongoConverter mongoConverter, - CacheableRepositoryHelper cacheableRepositoryHelper, - MongoTemplate mongoTemplate) { + CacheableRepositoryHelper cacheableRepositoryHelper) { super(mongoOperations, mongoConverter, cacheableRepositoryHelper); - this.mongoTemplate = mongoTemplate; } @Override @@ -578,33 +571,16 @@ public class CustomNewActionRepositoryCEImpl extends BaseAppsmithRepositoryImpl< } @Override - public Mono> publishActions(String applicationId, AclPermission permission) { + public Mono publishActions(String applicationId, AclPermission permission) { Criteria applicationIdCriteria = where(fieldName(QNewAction.newAction.applicationId)).is(applicationId); + // using aggregation update instead of regular update here + // it's required to set a field to a value of another field from the same domain + AggregationUpdate aggregationUpdate = AggregationUpdate.update() + .set(fieldName(QNewAction.newAction.publishedAction)) + .toValue("$" + fieldName(QNewAction.newAction.unpublishedAction)); - Mono> permissionGroupsMono = - getCurrentUserPermissionGroupsIfRequired(Optional.ofNullable(permission)); - - return permissionGroupsMono.flatMap(permissionGroups -> { - AggregationOperation matchAggregationWithPermission = null; - if (permission == null) { - matchAggregationWithPermission = Aggregation.match(new Criteria().andOperator(notDeleted())); - } else { - matchAggregationWithPermission = Aggregation.match( - new Criteria().andOperator(notDeleted(), userAcl(permissionGroups, permission))); - } - AggregationOperation matchAggregation = Aggregation.match(applicationIdCriteria); - AggregationOperation wholeProjection = Aggregation.project(NewAction.class); - AggregationOperation addFieldsOperation = Aggregation.addFields() - .addField(fieldName(QNewAction.newAction.publishedAction)) - .withValueOf(Fields.field(fieldName(QNewAction.newAction.unpublishedAction))) - .build(); - Aggregation combinedAggregation = Aggregation.newAggregation( - matchAggregation, matchAggregationWithPermission, wholeProjection, addFieldsOperation); - AggregationResults updatedResults = - mongoTemplate.aggregate(combinedAggregation, NewAction.class, NewAction.class); - return bulkUpdate(updatedResults.getMappedResults()); - }); + return updateByCriteria(List.of(applicationIdCriteria), aggregationUpdate, permission); } @Override diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCE.java index ec6775c2cb..dc940ee25b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCE.java @@ -3,7 +3,7 @@ package com.appsmith.server.repositories.ce; import com.appsmith.server.acl.AclPermission; import com.appsmith.server.domains.NewPage; import com.appsmith.server.repositories.AppsmithRepository; -import com.mongodb.bulk.BulkWriteResult; +import com.mongodb.client.result.UpdateResult; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -43,7 +43,5 @@ public interface CustomNewPageRepositoryCE extends AppsmithRepository { Mono findByGitSyncIdAndDefaultApplicationId( String defaultApplicationId, String gitSyncId, Optional permission); - Mono> publishPages(Collection pageIds, AclPermission permission); - - Mono> bulkUpdate(List newPages); + Mono publishPages(Collection pageIds, AclPermission permission); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCEImpl.java index acd1a16e0f..92a93f963e 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCEImpl.java @@ -9,32 +9,20 @@ import com.appsmith.server.domains.QNewPage; import com.appsmith.server.dtos.PageDTO; import com.appsmith.server.repositories.BaseAppsmithRepositoryImpl; import com.appsmith.server.repositories.CacheableRepositoryHelper; -import com.mongodb.bulk.BulkWriteResult; -import com.mongodb.client.model.UpdateOneModel; -import com.mongodb.client.model.WriteModel; +import com.mongodb.client.result.UpdateResult; import lombok.extern.slf4j.Slf4j; -import org.bson.Document; -import org.bson.types.ObjectId; -import org.springframework.data.mongodb.core.MongoTemplate; import org.springframework.data.mongodb.core.ReactiveMongoOperations; -import org.springframework.data.mongodb.core.aggregation.Aggregation; -import org.springframework.data.mongodb.core.aggregation.AggregationOperation; -import org.springframework.data.mongodb.core.aggregation.AggregationResults; -import org.springframework.data.mongodb.core.aggregation.Fields; +import org.springframework.data.mongodb.core.aggregation.AggregationUpdate; import org.springframework.data.mongodb.core.convert.MongoConverter; import org.springframework.data.mongodb.core.query.Criteria; import org.springframework.data.mongodb.core.query.Query; -import org.springframework.util.CollectionUtils; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.List; import java.util.Optional; -import java.util.Set; -import java.util.stream.Collectors; import static org.springframework.data.mongodb.core.query.Criteria.where; @@ -42,15 +30,11 @@ import static org.springframework.data.mongodb.core.query.Criteria.where; public class CustomNewPageRepositoryCEImpl extends BaseAppsmithRepositoryImpl implements CustomNewPageRepositoryCE { - private final MongoTemplate mongoTemplate; - public CustomNewPageRepositoryCEImpl( ReactiveMongoOperations mongoOperations, MongoConverter mongoConverter, - CacheableRepositoryHelper cacheableRepositoryHelper, - MongoTemplate mongoTemplate) { + CacheableRepositoryHelper cacheableRepositoryHelper) { super(mongoOperations, mongoConverter, cacheableRepositoryHelper); - this.mongoTemplate = mongoTemplate; } @Override @@ -267,55 +251,14 @@ public class CustomNewPageRepositoryCEImpl extends BaseAppsmithRepositoryImpl> publishPages(Collection pageIds, AclPermission permission) { + public Mono publishPages(Collection pageIds, AclPermission permission) { Criteria applicationIdCriteria = where(fieldName(QNewPage.newPage.id)).in(pageIds); + // using aggregation update instead of regular update here + // it's required to set a field to a value of another field from the same domain + AggregationUpdate aggregationUpdate = AggregationUpdate.update() + .set(fieldName(QNewPage.newPage.publishedPage)) + .toValue("$" + fieldName(QNewPage.newPage.unpublishedPage)); - Mono> permissionGroupsMono = - getCurrentUserPermissionGroupsIfRequired(Optional.ofNullable(permission)); - - return permissionGroupsMono.flatMap(permissionGroups -> { - AggregationOperation matchAggregationWithPermission = null; - if (permission == null) { - matchAggregationWithPermission = Aggregation.match(new Criteria().andOperator(notDeleted())); - } else { - matchAggregationWithPermission = Aggregation.match( - new Criteria().andOperator(notDeleted(), userAcl(permissionGroups, permission))); - } - AggregationOperation matchAggregation = Aggregation.match(applicationIdCriteria); - AggregationOperation wholeProjection = Aggregation.project(NewPage.class); - AggregationOperation addFieldsOperation = Aggregation.addFields() - .addField(fieldName(QNewPage.newPage.publishedPage)) - .withValueOf(Fields.field(fieldName(QNewPage.newPage.unpublishedPage))) - .build(); - Aggregation combinedAggregation = Aggregation.newAggregation( - matchAggregation, matchAggregationWithPermission, wholeProjection, addFieldsOperation); - AggregationResults updatedResults = - mongoTemplate.aggregate(combinedAggregation, NewPage.class, NewPage.class); - return bulkUpdate(updatedResults.getMappedResults()); - }); - } - - @Override - public Mono> bulkUpdate(List newPages) { - if (CollectionUtils.isEmpty(newPages)) { - return Mono.just(Collections.emptyList()); - } - - // convert the list of new pages to a list of DBObjects - List> dbObjects = newPages.stream() - .map(newPage -> { - assert newPage.getId() != null; - Document document = new Document(); - mongoOperations.getConverter().write(newPage, document); - document.remove("_id"); - return (WriteModel) new UpdateOneModel( - new Document("_id", new ObjectId(newPage.getId())), new Document("$set", document)); - }) - .collect(Collectors.toList()); - - return mongoOperations - .getCollection(mongoOperations.getCollectionName(NewPage.class)) - .flatMapMany(documentMongoCollection -> documentMongoCollection.bulkWrite(dbObjects)) - .collectList(); + return updateByCriteria(List.of(applicationIdCriteria), aggregationUpdate, permission); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/NewActionRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/NewActionRepositoryCE.java index 505981ef1d..e74852d3b5 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/NewActionRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/NewActionRepositoryCE.java @@ -3,17 +3,12 @@ package com.appsmith.server.repositories.ce; import com.appsmith.server.domains.NewAction; import com.appsmith.server.repositories.BaseRepository; import com.appsmith.server.repositories.CustomNewActionRepository; -import org.springframework.data.mongodb.repository.Meta; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; public interface NewActionRepositoryCE extends BaseRepository, CustomNewActionRepository { - @Meta(cursorBatchSize = 10000) Flux findByApplicationId(String applicationId); - @Meta(cursorBatchSize = 10000) - Flux findAllByIdIn(Iterable ids); - Mono countByDeletedAtNull(); } 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 000b4bf90c..7269060239 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 @@ -50,7 +50,6 @@ import com.appsmith.server.solutions.ApplicationPermission; import com.appsmith.server.solutions.PagePermission; import com.appsmith.server.solutions.WorkspacePermission; import com.google.common.base.Strings; -import com.mongodb.bulk.BulkWriteResult; import com.mongodb.client.result.UpdateResult; import jakarta.annotation.Nullable; import lombok.RequiredArgsConstructor; @@ -1135,7 +1134,7 @@ public class ApplicationPageServiceCEImpl implements ApplicationPageServiceCE { if (isPublishedManually) { application.setLastDeployedAt(Instant.now()); } - Mono> publishPagesMono = + Mono publishPagesMono = newPageService.publishPages(editedPageIds, pagePermission.getEditPermission()); // Archive the deleted pages and save the application changes and then return the pages so that @@ -1145,7 +1144,7 @@ public class ApplicationPageServiceCEImpl implements ApplicationPageServiceCE { }) .cache(); // caching as we'll need this to send analytics attributes after publishing the app - Mono> publishActionsMono = + Mono publishActionsMono = newActionService.publishActions(applicationId, actionPermission.getEditPermission()); // this is a map of pluginType to count of actions for that pluginType, required for analytics diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCE.java index 733b08c21d..9bf8e71f8e 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCE.java @@ -14,7 +14,7 @@ import com.appsmith.server.dtos.ce.ImportActionResultDTO; import com.appsmith.server.dtos.ce.ImportedActionAndCollectionMapsDTO; import com.appsmith.server.helpers.ce.ImportApplicationPermissionProvider; import com.appsmith.server.services.CrudService; -import com.mongodb.bulk.BulkWriteResult; +import com.mongodb.client.result.UpdateResult; import org.springframework.data.domain.Sort; import org.springframework.util.MultiValueMap; import reactor.core.publisher.Flux; @@ -134,7 +134,7 @@ public interface NewActionServiceCE extends CrudService { ImportActionCollectionResultDTO importActionCollectionResultDTO, ImportActionResultDTO importActionResultDTO); - Mono> publishActions(String applicationId, AclPermission permission); + Mono publishActions(String applicationId, AclPermission permission); Flux countActionsByPluginType(String applicationId); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java index 276d2ce24b..fb4dd3057b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java @@ -53,7 +53,7 @@ import com.appsmith.server.solutions.ApplicationPermission; import com.appsmith.server.solutions.DatasourcePermission; import com.appsmith.server.solutions.PagePermission; import com.appsmith.server.solutions.PolicySolution; -import com.mongodb.bulk.BulkWriteResult; +import com.mongodb.client.result.UpdateResult; import io.micrometer.observation.ObservationRegistry; import jakarta.validation.Validator; import lombok.extern.slf4j.Slf4j; @@ -1965,7 +1965,7 @@ public class NewActionServiceCEImpl extends BaseService> publishActions(String applicationId, AclPermission permission) { + public Mono publishActions(String applicationId, AclPermission permission) { // delete the actions that were deleted in edit mode return repository .archiveDeletedUnpublishedActions(applicationId, permission) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewPageServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewPageServiceCE.java index b42c05c5fd..cb480d23f3 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewPageServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewPageServiceCE.java @@ -7,7 +7,7 @@ import com.appsmith.server.domains.NewPage; import com.appsmith.server.dtos.ApplicationPagesDTO; import com.appsmith.server.dtos.PageDTO; import com.appsmith.server.services.CrudService; -import com.mongodb.bulk.BulkWriteResult; +import com.mongodb.client.result.UpdateResult; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -93,5 +93,5 @@ public interface NewPageServiceCE extends CrudService { Flux findPageSlugsByApplicationIds(List applicationIds, AclPermission aclPermission); - Mono> publishPages(Collection pageIds, AclPermission permission); + Mono publishPages(Collection pageIds, AclPermission permission); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewPageServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewPageServiceCEImpl.java index e6fa147a0f..9430d3bfa2 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewPageServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewPageServiceCEImpl.java @@ -23,7 +23,7 @@ import com.appsmith.server.services.BaseService; import com.appsmith.server.services.UserDataService; import com.appsmith.server.solutions.ApplicationPermission; import com.appsmith.server.solutions.PagePermission; -import com.mongodb.bulk.BulkWriteResult; +import com.mongodb.client.result.UpdateResult; import jakarta.validation.Validator; import lombok.extern.slf4j.Slf4j; import net.minidev.json.JSONObject; @@ -692,7 +692,7 @@ public class NewPageServiceCEImpl extends BaseService> publishPages(Collection pageIds, AclPermission permission) { + public Mono publishPages(Collection pageIds, AclPermission permission) { return repository.publishPages(pageIds, permission); } } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/NewActionServiceUnitTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/NewActionServiceUnitTest.java index 36f9d9aeef..21141119fb 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/NewActionServiceUnitTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/NewActionServiceUnitTest.java @@ -23,6 +23,7 @@ import com.appsmith.server.solutions.ApplicationPermission; import com.appsmith.server.solutions.DatasourcePermission; import com.appsmith.server.solutions.PagePermission; import com.appsmith.server.solutions.PolicySolution; +import com.mongodb.client.result.UpdateResult; import io.micrometer.observation.ObservationRegistry; import jakarta.validation.Validator; import lombok.extern.slf4j.Slf4j; @@ -38,8 +39,6 @@ import reactor.core.publisher.Mono; import reactor.core.scheduler.Scheduler; import reactor.test.StepVerifier; -import java.util.List; - import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.anyString; @@ -200,8 +199,9 @@ public class NewActionServiceUnitTest { @Test public void testPublishActionArchivesAndPublishesActions() { String applicationId = "dummy-application-id"; - List updateResult = Mockito.mock(List.class); - Mockito.when(updateResult.size()).thenReturn(10); + UpdateResult updateResult = Mockito.mock(UpdateResult.class); + Mockito.when(updateResult.getModifiedCount()).thenReturn(10L); + Mockito.when(updateResult.getMatchedCount()).thenReturn(5L); Mockito.when(newActionRepository.archiveDeletedUnpublishedActions( applicationId, actionPermission.getEditPermission())) @@ -212,7 +212,8 @@ public class NewActionServiceUnitTest { StepVerifier.create(newActionService.publishActions(applicationId, actionPermission.getEditPermission())) .assertNext(updateResult1 -> { - assertEquals(10, updateResult1.size()); + assertEquals(10L, updateResult1.getModifiedCount()); + assertEquals(5L, updateResult1.getMatchedCount()); }) .verifyComplete(); } From 7941ec25a65cef92265ac14c48c14af3c408496a Mon Sep 17 00:00:00 2001 From: Trisha Anand Date: Fri, 22 Sep 2023 14:55:52 +0530 Subject: [PATCH 04/32] Fixed compilation issue --- .../appsmith/server/services/ce/NewActionServiceCEImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java index fb4dd3057b..3088b833a0 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java @@ -663,7 +663,7 @@ public class NewActionServiceCEImpl extends BaseService findAllById(Iterable id) { - return repository.findAllByIdIn(id).flatMap(this::sanitizeAction); + return repository.findAllById(id).flatMap(this::sanitizeAction); } @Override @@ -1907,7 +1907,7 @@ public class NewActionServiceCEImpl extends BaseService { // Update collectionId and defaultCollectionIds in actionDTOs ActionDTO unpublishedAction = newAction.getUnpublishedAction(); From 3c118af42c98e41431601144f54f07b9a900a995 Mon Sep 17 00:00:00 2001 From: Diljit Date: Wed, 20 Sep 2023 14:48:35 +0530 Subject: [PATCH 05/32] feat: Add placeholder component for AskAI Button in Code Editor (#27441) --- .../editorComponents/GPT/AskAIButton.tsx | 15 +++++++++++++++ .../editorComponents/CodeEditor/index.tsx | 11 +++++++++++ .../editorComponents/GPT/AskAIButton.tsx | 1 + 3 files changed, 27 insertions(+) create mode 100644 app/client/src/ce/components/editorComponents/GPT/AskAIButton.tsx create mode 100644 app/client/src/ee/components/editorComponents/GPT/AskAIButton.tsx diff --git a/app/client/src/ce/components/editorComponents/GPT/AskAIButton.tsx b/app/client/src/ce/components/editorComponents/GPT/AskAIButton.tsx new file mode 100644 index 0000000000..1bc3a41200 --- /dev/null +++ b/app/client/src/ce/components/editorComponents/GPT/AskAIButton.tsx @@ -0,0 +1,15 @@ +import type { + FieldEntityInformation, + TEditorModes, +} from "components/editorComponents/CodeEditor/EditorConfig"; + +type AskAIButtonProps = { + mode: TEditorModes; + onClick: () => void; + entity: FieldEntityInformation; +}; + +// eslint-disable-next-line @typescript-eslint/no-unused-vars +export function AskAIButton(props: AskAIButtonProps) { + return null; +} diff --git a/app/client/src/components/editorComponents/CodeEditor/index.tsx b/app/client/src/components/editorComponents/CodeEditor/index.tsx index 09291f81cf..5db148358e 100644 --- a/app/client/src/components/editorComponents/CodeEditor/index.tsx +++ b/app/client/src/components/editorComponents/CodeEditor/index.tsx @@ -144,6 +144,7 @@ import { import { getAssetUrl } from "@appsmith/utils/airgapHelpers"; import { selectFeatureFlags } from "@appsmith/selectors/featureFlagsSelectors"; import { AIWindow } from "@appsmith/components/editorComponents/GPT"; +import { AskAIButton } from "@appsmith/components/editorComponents/GPT/AskAIButton"; import classNames from "classnames"; import { APPSMITH_AI, @@ -1594,6 +1595,16 @@ class CodeEditor extends Component { +
+ { + this.setState({ showAIWindow: true }); + }} + /> +
+ Date: Thu, 21 Sep 2023 15:37:41 +0530 Subject: [PATCH 06/32] fix: Active color of primary buttons broken after ADS 2.0 migration (#26958) ## Description When admin has configured the branding color, on click of primary button, still appsmith orange color was being rendered. This PR fixes that issue and also added a fallback to handle the old customers who already have a branding color. This fallback is required since the new fix work only for people who choose brand color after this PR is merged. Corresponding EE PR: https://github.com/appsmithorg/appsmith-ee/pull/2182 #### PR fixes following issue(s) Fixes #25446 Fixes #27501 #### Type of change - Bug fix (non-breaking change which fixes an issue) ## Testing > #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [x] Manual - [ ] JUnit - [ ] Jest - [ ] Cypress > > #### Test Plan > Add Testsmith test cases links that relate to this PR > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [x] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --------- Co-authored-by: Pawan Kumar --- .../theming/src/color/src/LightModeTheme.ts | 2 +- .../AdminSettings/Branding/SettingsForm.tsx | 2 +- .../AdminSettings/FormGroup/ColorInput.tsx | 11 ++++++++-- app/client/src/utils/BrandingUtils.ts | 20 +++++++++++++++---- .../src/utils/hooks/useBrandingTheme.ts | 18 +++++++++++++++++ 5 files changed, 45 insertions(+), 8 deletions(-) diff --git a/app/client/packages/design-system/theming/src/color/src/LightModeTheme.ts b/app/client/packages/design-system/theming/src/color/src/LightModeTheme.ts index 521a3ea6ba..fd63f74b4e 100644 --- a/app/client/packages/design-system/theming/src/color/src/LightModeTheme.ts +++ b/app/client/packages/design-system/theming/src/color/src/LightModeTheme.ts @@ -217,7 +217,7 @@ export class LightModeTheme implements ColorModeTheme { return color; } - private get bgAccentActive() { + public get bgAccentActive() { // Active state of bgAccent. Slightly darker than the resting state to produce the effect of moving further from the viewer / being pushed down. const color = this.bgAccent.clone(); diff --git a/app/client/src/pages/AdminSettings/Branding/SettingsForm.tsx b/app/client/src/pages/AdminSettings/Branding/SettingsForm.tsx index d911507cd7..bb3ef50180 100644 --- a/app/client/src/pages/AdminSettings/Branding/SettingsForm.tsx +++ b/app/client/src/pages/AdminSettings/Branding/SettingsForm.tsx @@ -155,7 +155,7 @@ function SettingsForm(props: SettingsFormProps) { !["disabled", "hover"].includes(key)} + filter={(key) => !["disabled", "hover", "active"].includes(key)} logEvent={(property: string) => { AnalyticsUtil.logEvent("BRANDING_PROPERTY_UPDATE", { propertyName: property, diff --git a/app/client/src/pages/AdminSettings/FormGroup/ColorInput.tsx b/app/client/src/pages/AdminSettings/FormGroup/ColorInput.tsx index 0e9128f520..1dd4ea0c68 100644 --- a/app/client/src/pages/AdminSettings/FormGroup/ColorInput.tsx +++ b/app/client/src/pages/AdminSettings/FormGroup/ColorInput.tsx @@ -6,6 +6,7 @@ import tinycolor from "tinycolor2"; import styled from "styled-components"; import { Icon, Text, Tooltip } from "design-system"; import { InputGroup, Classes } from "@blueprintjs/core"; +import Color from "colorjs.io"; import type { SettingComponentProps } from "./Common"; import { FormGroup } from "./Common"; @@ -116,10 +117,16 @@ export const ColorInput = (props: ColorInputProps) => { shades[selectedIndex] = e.target.value; logEvent && logEvent(selectedIndex); - + let color = undefined; + try { + color = Color.parse(e.target.value); + } catch (error) { + // eslint-disable-next-line no-console + console.error(error); + } // if user is touching the primary color // then we need to update the shades - if (selectedIndex === "primary") { + if (selectedIndex === "primary" && color) { shades = createBrandColorsFromPrimaryColor(e.target.value); } diff --git a/app/client/src/utils/BrandingUtils.ts b/app/client/src/utils/BrandingUtils.ts index 9ce51f6ab2..eef24cdd4d 100644 --- a/app/client/src/utils/BrandingUtils.ts +++ b/app/client/src/utils/BrandingUtils.ts @@ -11,13 +11,15 @@ import { import { toast } from "design-system"; import { ASSETS_CDN_URL } from "constants/ThirdPartyConstants"; import { getAssetUrl } from "@appsmith/utils/airgapHelpers"; +import { LightModeTheme } from "@design-system/theming"; const FAVICON_MAX_WIDTH = 32; const FAVICON_MAX_HEIGHT = 32; -const DEFAULT_BRANDING_PRIMARY_COLOR = "#D7D7D7"; -export const APPSMITH_BRAND_PRIMARY_COLOR = getComputedStyle( - document.documentElement, -).getPropertyValue("--ads-v2-color-bg-brand"); +const DEFAULT_BRANDING_PRIMARY_COLOR = "#E15615"; +export const APPSMITH_BRAND_PRIMARY_COLOR = + getComputedStyle(document.documentElement).getPropertyValue( + "--ads-v2-color-bg-brand", + ) || DEFAULT_BRANDING_PRIMARY_COLOR; export const APPSMITH_BRAND_BG_COLOR = "#F1F5F9"; export const APPSMITH_BRAND_FAVICON_URL = getAssetUrl( `${ASSETS_CDN_URL}/appsmith-favicon-orange.ico`, @@ -37,6 +39,8 @@ export function createBrandColorsFromPrimaryColor( const hsl = tinycolor(brand).toHsl(); const hue = hsl.h; const saturation = hsl.s; + // initialize light theme + const lightTheme = new LightModeTheme(brand); let textColor = "#000"; const isReadable = tinycolor.isReadable(brand, "#000", { @@ -66,10 +70,18 @@ export function createBrandColorsFromPrimaryColor( ) : darkenColor(brand); + // get active color from color algorithm + const activeColor = + brand === APPSMITH_BRAND_PRIMARY_COLOR + ? getComputedStyle(document.documentElement).getPropertyValue( + "--ads-v2-color-bg-brand-emphasis-plus", + ) + : lightTheme.bgAccentActive.toString({ format: "hex" }); return { primary: brand, background: bgColor, hover: hoverColor, + active: activeColor, font: textColor, disabled: disabledColor, }; diff --git a/app/client/src/utils/hooks/useBrandingTheme.ts b/app/client/src/utils/hooks/useBrandingTheme.ts index 220a9783d7..b763b00069 100644 --- a/app/client/src/utils/hooks/useBrandingTheme.ts +++ b/app/client/src/utils/hooks/useBrandingTheme.ts @@ -3,9 +3,25 @@ import { useSelector } from "react-redux"; import { getTenantConfig } from "@appsmith/selectors/tenantSelectors"; import { useLayoutEffect } from "react"; import { getAssetUrl } from "@appsmith/utils/airgapHelpers"; +import { APPSMITH_BRAND_PRIMARY_COLOR } from "utils/BrandingUtils"; +import { LightModeTheme } from "@design-system/theming"; const useBrandingTheme = () => { const config = useSelector(getTenantConfig); + let activeColor: string | undefined = undefined; + if ( + config.brandColors.primary !== undefined && + (config.brandColors.active === undefined || + config.brandColors.active === "") + ) { + const lightTheme = new LightModeTheme(config.brandColors.primary); + activeColor = + config.brandColors.primary === APPSMITH_BRAND_PRIMARY_COLOR + ? getComputedStyle(document.documentElement).getPropertyValue( + "--ads-v2-color-bg-brand-emphasis-plus", + ) + : lightTheme.bgAccentActive.toString({ format: "hex" }); + } useLayoutEffect(() => { const cssVariables: Record = { @@ -15,8 +31,10 @@ const useBrandingTheme = () => { // TODO:(Albin) Remove this once branding and new DS is fully integrated "background-secondary": config.brandColors.background, "bg-brand-emphasis": config.brandColors.hover, + "bg-brand-emphasis-plus": activeColor || config.brandColors.active, "fg-brand-emphasis": config.brandColors.hover, "border-brand-emphasis": config.brandColors.hover, + "border-brand-emphasis-plus": activeColor || config.brandColors.active, "fg-on-brand": config.brandColors.font, }; From 228025ed6ecd0cbb3e7646bd5220e9e8425ea8fe Mon Sep 17 00:00:00 2001 From: Shubham Saxena <136057998+shubham7saxena7@users.noreply.github.com> Date: Thu, 21 Sep 2023 18:43:39 +0530 Subject: [PATCH 07/32] fix: apply brand changes (#27536) ## Description apply branding changes to email verification template Fixes #27528 Fixes #27431 Fixes #27475 #### Type of change > Please delete options that are not relevant. - Bug fix (non-breaking change which fixes an issue) ## Testing > #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [x] Manual - [ ] JUnit - [ ] Jest - [ ] Cypress > > #### Test Plan > Add Testsmith test cases links that relate to this PR > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --- .../server/constants/ce/EmailConstantsCE.java | 2 +- .../server/services/ce/EmailServiceCE.java | 2 +- .../services/ce/EmailServiceCEImpl.java | 17 +- .../server/services/ce/UserServiceCEImpl.java | 3 +- .../email/ce/emailVerificationTemplate.html | 540 +++++++++ .../email/emailVerificationTemplate.html | 1000 ----------------- 6 files changed, 555 insertions(+), 1009 deletions(-) create mode 100644 app/server/appsmith-server/src/main/resources/email/ce/emailVerificationTemplate.html delete mode 100644 app/server/appsmith-server/src/main/resources/email/emailVerificationTemplate.html diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/ce/EmailConstantsCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/ce/EmailConstantsCE.java index f0774c0ec0..6d2abde6d6 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/ce/EmailConstantsCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/ce/EmailConstantsCE.java @@ -24,7 +24,7 @@ public class EmailConstantsCE { public static final String INVITE_WORKSPACE_TEMPLATE_NEW_USER_CE = "email/ce/inviteWorkspaceNewUserTemplate.html"; public static final String FORGOT_PASSWORD_TEMPLATE_CE = "email/ce/forgotPasswordTemplate.html"; public static final String INSTANCE_ADMIN_INVITE_EMAIL_TEMPLATE = "email/ce/instanceAdminInviteTemplate.html"; - public static final String EMAIL_VERIFICATION_EMAIL_TEMPLATE = "email/emailVerificationTemplate.html"; + public static final String EMAIL_VERIFICATION_EMAIL_TEMPLATE_CE = "email/ce/emailVerificationTemplate.html"; public static final String WORKSPACE_URL = "%s/applications#%s"; public static final String INVITER_FIRST_NAME = "inviterFirstName"; public static final String INVITER_WORKSPACE_NAME = "inviterWorkspaceName"; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/EmailServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/EmailServiceCE.java index 4bbbbf0d1d..31d37ae847 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/EmailServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/EmailServiceCE.java @@ -16,7 +16,7 @@ public interface EmailServiceCE { String originHeader, boolean isNewUser); - Mono sendEmailVerificationEmail(User user, String verificationUrl); + Mono sendEmailVerificationEmail(User user, String verificationUrl, String originHeader); Mono sendInstanceAdminInviteEmail( User invitedUser, User invitingUser, String originHeader, boolean isNewUser); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/EmailServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/EmailServiceCEImpl.java index 573faeb50a..bab662d65a 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/EmailServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/EmailServiceCEImpl.java @@ -63,15 +63,16 @@ public class EmailServiceCEImpl implements EmailServiceCE { } @Override - public Mono sendEmailVerificationEmail(User user, String verificationURL) { + public Mono sendEmailVerificationEmail(User user, String verificationURL, String originHeader) { Map params = new HashMap<>(); params.put(EMAIL_VERIFICATION_URL, verificationURL); return this.enrichParams(params) - .flatMap(updatedParams -> emailSender.sendMail( - user.getEmail(), - EMAIL_VERIFICATION_EMAIL_SUBJECT, - EMAIL_VERIFICATION_EMAIL_TEMPLATE, - updatedParams)); + .flatMap(enrichedParams -> this.enrichWithBrandParams(enrichedParams, originHeader) + .flatMap(updatedParams -> emailSender.sendMail( + user.getEmail(), + EMAIL_VERIFICATION_EMAIL_SUBJECT, + getEmailVerificationTemplate(), + updatedParams))); } @Override @@ -123,6 +124,10 @@ public class EmailServiceCEImpl implements EmailServiceCE { return INVITE_WORKSPACE_TEMPLATE_EXISTING_USER_CE; } + protected String getEmailVerificationTemplate() { + return EMAIL_VERIFICATION_EMAIL_TEMPLATE_CE; + } + protected String getAdminInstanceInviteTemplate() { return INSTANCE_ADMIN_INVITE_EMAIL_TEMPLATE; } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java index 1c8c21294d..36558c5363 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java @@ -850,7 +850,8 @@ public class UserServiceCEImpl extends BaseService URLEncoder.encode(emailVerificationToken.getEmail(), StandardCharsets.UTF_8), redirectUrlCopy); - return emailService.sendEmailVerificationEmail(user, verificationUrl); + return emailService.sendEmailVerificationEmail( + user, verificationUrl, resendEmailVerificationDTO.getBaseUrl()); }) .thenReturn(true); } diff --git a/app/server/appsmith-server/src/main/resources/email/ce/emailVerificationTemplate.html b/app/server/appsmith-server/src/main/resources/email/ce/emailVerificationTemplate.html new file mode 100644 index 0000000000..17530b1f16 --- /dev/null +++ b/app/server/appsmith-server/src/main/resources/email/ce/emailVerificationTemplate.html @@ -0,0 +1,540 @@ + + + + Verify your email for {{instanceName}} + + + + + + + + + + + + + + + + + + + + + +
+ +
+ + + + + + +
+
+ + + + + + +
+ + + + + + + + + + + + + + + + + + + + + +
+ + + + + + +
+ +
+
+
+

+ + Hey, + +

+
+
+
+

+ Thank you for signing up! Before we get started, we just need to confirm that this is you. Click below to verify your email address. +

+
+
+
+

+ The link expires in two days’ + time. Better not wait. +

+
+
+ + + + + + +
+ + Verify +
+
+
+

+ 1390, Market Street, Suite 200, San + Francisco, California 94102, United + States +

+
+
+
+
+ +
+
+
+ + diff --git a/app/server/appsmith-server/src/main/resources/email/emailVerificationTemplate.html b/app/server/appsmith-server/src/main/resources/email/emailVerificationTemplate.html deleted file mode 100644 index 17e9cc2a5c..0000000000 --- a/app/server/appsmith-server/src/main/resources/email/emailVerificationTemplate.html +++ /dev/null @@ -1,1000 +0,0 @@ - - - - Verify email for your Appsmith account - - - - - - - - - - - - - - - - - - - - - -
- -
- - - - - - -
- -
- - - - - - -
-

- -
-
- -
- - - - - - -
- - -
-
-
- -
- - - - - - -
- - - - - - - - - - - - - - - - - - - - - - - - -
- - - - - - -
- -
-
-
-   -
-
-
-

- Verify your email address -

-
-
-
-

- Hello,
-
- Thank you for signing up! Before we get - started, we just need to confirm that - this is you. Click below to verify your - email address: - -

-
-
-
-   -
-
- - - - - - -
- - Verify - -
-
-
-

- The link expires in two days’ time. - Better not wait. -
- P.S.: You will see a welcome e-mail - soon after you join to help get started - with Appsmith. -

-
-
-
-
- -
-
- -
- - - - - - -
- -
- - - - - - -
-

- -
-
- -
- - - - - - -
- - -
-
-
- -
- - - - - - -
- - - - - - - - - -
- - - - - - -
- -
-
-
-

- Appsmith is an open source framework - that helps developer and companies to - build powerful internal tools. -

-
-
-
-
- -
-
- -
- - From 6775323cbd9ead8feedb79e8d5630e992352cabb Mon Sep 17 00:00:00 2001 From: Anagh Hegde Date: Fri, 22 Sep 2023 13:16:04 +0530 Subject: [PATCH 08/32] chore: fix the jgit version updates in other places (#27558) Should resolve the following CVE reports: 1. https://github.com/appsmithorg/appsmith/security/dependabot/263 1. https://github.com/appsmithorg/appsmith/security/dependabot/264 --- app/server/appsmith-git/pom.xml | 10 +--------- .../git/helpers/SshTransportConfigCallback.java | 4 ++-- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/app/server/appsmith-git/pom.xml b/app/server/appsmith-git/pom.xml index a0b89e9a40..60def1cd47 100644 --- a/app/server/appsmith-git/pom.xml +++ b/app/server/appsmith-git/pom.xml @@ -29,15 +29,7 @@ org.eclipse.jgit org.eclipse.jgit.ssh.jsch - 5.13.0.202109080827-r - - - - - org.eclipse.jgit - org.eclipse.jgit.junit.ssh - 6.4.0.202211300538-r - test + 6.6.1.202309021850-r diff --git a/app/server/appsmith-git/src/main/java/com/appsmith/git/helpers/SshTransportConfigCallback.java b/app/server/appsmith-git/src/main/java/com/appsmith/git/helpers/SshTransportConfigCallback.java index a210ddb5ce..bb74d5ad6e 100644 --- a/app/server/appsmith-git/src/main/java/com/appsmith/git/helpers/SshTransportConfigCallback.java +++ b/app/server/appsmith-git/src/main/java/com/appsmith/git/helpers/SshTransportConfigCallback.java @@ -4,11 +4,11 @@ import com.jcraft.jsch.JSch; import com.jcraft.jsch.JSchException; import com.jcraft.jsch.Session; import org.eclipse.jgit.api.TransportConfigCallback; -import org.eclipse.jgit.transport.JschConfigSessionFactory; -import org.eclipse.jgit.transport.OpenSshConfig; import org.eclipse.jgit.transport.SshSessionFactory; import org.eclipse.jgit.transport.SshTransport; import org.eclipse.jgit.transport.Transport; +import org.eclipse.jgit.transport.ssh.jsch.JschConfigSessionFactory; +import org.eclipse.jgit.transport.ssh.jsch.OpenSshConfig; import org.eclipse.jgit.util.FS; /** From 4fcb639225612769050f1bbcbc10f7dbcf89dd3a Mon Sep 17 00:00:00 2001 From: Nayan Date: Mon, 25 Sep 2023 12:03:28 +0600 Subject: [PATCH 09/32] fix: Queries are missing from git after renaming a page (#27569) ## Description In git connected applications, if user renames a page, their queries are not pushed to git for that page. #### PR fixes following issue(s) Fixes #27508 --- .../server/helpers/ImportExportUtils.java | 16 +++ .../ImportExportApplicationServiceCEImpl.java | 12 ++ .../server/helpers/ImportExportUtilsTest.java | 25 ++++ .../ImportExportApplicationServiceTests.java | 124 ++++++++++++++++++ 4 files changed, 177 insertions(+) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ImportExportUtils.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ImportExportUtils.java index 6de6563aa9..d3d2807556 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ImportExportUtils.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ImportExportUtils.java @@ -2,13 +2,17 @@ package com.appsmith.server.helpers; import com.appsmith.external.models.ActionDTO; import com.appsmith.external.models.Datasource; +import com.appsmith.server.constants.FieldName; import com.appsmith.server.domains.Application; +import com.appsmith.server.dtos.ApplicationJson; import lombok.extern.slf4j.Slf4j; import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.data.mongodb.MongoTransactionException; import org.springframework.transaction.TransactionException; +import org.springframework.util.CollectionUtils; import java.util.Map; +import java.util.Set; import static com.appsmith.external.helpers.AppsmithBeanUtils.copyNestedNonNullProperties; @@ -115,4 +119,16 @@ public class ImportExportUtils { importedApplication.setPublishedApplicationDetail(importedApplication.getUnpublishedApplicationDetail()); importedApplication.setPublishedAppLayout(importedApplication.getUnpublishedAppLayout()); } + + public static boolean isPageNameInUpdatedList(ApplicationJson applicationJson, String pageName) { + Map> updatedResources = applicationJson.getUpdatedResources(); + if (updatedResources == null) { + return false; + } + Set updatedPageNames = updatedResources.get(FieldName.PAGE_LIST); + if (CollectionUtils.isEmpty(updatedPageNames)) { + return false; + } + return pageName != null && updatedPageNames.contains(pageName); + } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ImportExportApplicationServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ImportExportApplicationServiceCEImpl.java index d32b737610..428a1da88f 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ImportExportApplicationServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ImportExportApplicationServiceCEImpl.java @@ -477,6 +477,12 @@ public class ImportExportApplicationServiceCEImpl implements ImportExportApplica ActionCollectionDTO actionCollectionDTO = unpublishedActionCollectionDTO != null ? unpublishedActionCollectionDTO : publishedActionCollectionDTO; + + // TODO: check whether resource updated after last commit - move to a function + // we've replaced page id with page name in previous step + String pageName = actionCollectionDTO.getPageId(); + boolean isPageUpdated = + ImportExportUtils.isPageNameInUpdatedList(applicationJson, pageName); String actionCollectionName = actionCollectionDTO != null ? actionCollectionDTO.getName() + NAME_SEPARATOR @@ -485,6 +491,7 @@ public class ImportExportApplicationServiceCEImpl implements ImportExportApplica Instant actionCollectionUpdatedAt = actionCollection.getUpdatedAt(); boolean isActionCollectionUpdated = isClientSchemaMigrated || isServerSchemaMigrated + || isPageUpdated || applicationLastCommittedAt == null || actionCollectionUpdatedAt == null || applicationLastCommittedAt.isBefore(actionCollectionUpdatedAt); @@ -569,10 +576,15 @@ public class ImportExportApplicationServiceCEImpl implements ImportExportApplica String newActionName = actionDTO != null ? actionDTO.getValidName() + NAME_SEPARATOR + actionDTO.getPageId() : null; + // TODO: check whether resource updated after last commit - move to a function + String pageName = actionDTO.getPageId(); + boolean isPageUpdated = + ImportExportUtils.isPageNameInUpdatedList(applicationJson, pageName); Instant newActionUpdatedAt = newAction.getUpdatedAt(); boolean isNewActionUpdated = isClientSchemaMigrated || isServerSchemaMigrated || applicationLastCommittedAt == null + || isPageUpdated || newActionUpdatedAt == null || applicationLastCommittedAt.isBefore(newActionUpdatedAt); if (isNewActionUpdated && newActionName != null) { diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ImportExportUtilsTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ImportExportUtilsTest.java index 45003e0d83..b2a7924917 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ImportExportUtilsTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ImportExportUtilsTest.java @@ -1,11 +1,16 @@ package com.appsmith.server.helpers; +import com.appsmith.server.constants.FieldName; +import com.appsmith.server.dtos.ApplicationJson; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.springframework.data.mongodb.MongoTransactionException; +import java.util.Map; +import java.util.Set; + class ImportExportUtilsTest { @Test @@ -22,4 +27,24 @@ class ImportExportUtilsTest { String errorMessage = ImportExportUtils.getErrorMessage(throwable); Assertions.assertEquals(errorMessage, "Error: " + throwable.getMessage()); } + + @Test + void isPageNameInUpdatedList() { + String pageName = "Page1"; + Set updatedPageNames = Set.of("Page1", "Page2"); + ApplicationJson applicationJson = new ApplicationJson(); + Assertions.assertFalse(ImportExportUtils.isPageNameInUpdatedList(applicationJson, pageName)); + + applicationJson.setUpdatedResources(Map.of()); + Assertions.assertFalse(ImportExportUtils.isPageNameInUpdatedList(applicationJson, pageName)); + + Map> stringSetMap = Map.of(FieldName.PAGE_LIST, Set.of("Page1", "Page2")); + applicationJson.setUpdatedResources(stringSetMap); + Assertions.assertTrue(ImportExportUtils.isPageNameInUpdatedList(applicationJson, pageName)); + + Assertions.assertFalse(ImportExportUtils.isPageNameInUpdatedList(applicationJson, pageName.toLowerCase())); + Assertions.assertFalse(ImportExportUtils.isPageNameInUpdatedList(applicationJson, "test")); + Assertions.assertFalse(ImportExportUtils.isPageNameInUpdatedList(applicationJson, "")); + Assertions.assertFalse(ImportExportUtils.isPageNameInUpdatedList(applicationJson, null)); + } } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ImportExportApplicationServiceTests.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ImportExportApplicationServiceTests.java index 338a62e224..07e25034da 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ImportExportApplicationServiceTests.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ImportExportApplicationServiceTests.java @@ -108,6 +108,7 @@ import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; +import static com.appsmith.external.constants.GitConstants.NAME_SEPARATOR; import static com.appsmith.server.acl.AclPermission.MANAGE_ACTIONS; import static com.appsmith.server.acl.AclPermission.MANAGE_APPLICATIONS; import static com.appsmith.server.acl.AclPermission.MANAGE_DATASOURCES; @@ -4738,4 +4739,127 @@ public class ImportExportApplicationServiceTests { .expectError(AppsmithException.class) .verify(); } + + private Mono createActionToPage(String actionName, String pageId) { + ActionDTO action = new ActionDTO(); + ActionConfiguration actionConfiguration = new ActionConfiguration(); + actionConfiguration.setHttpMethod(HttpMethod.GET); + action.setActionConfiguration(actionConfiguration); + action.setDatasource(datasourceMap.get("DS1")); + action.setName(actionName); + action.setPageId(pageId); + return layoutActionService.createAction(action); + } + + private Mono createActionCollectionToPage(Application application, int pageIndex) { + ActionCollectionDTO actionCollectionDTO1 = new ActionCollectionDTO(); + actionCollectionDTO1.setName("TestJsObject"); + actionCollectionDTO1.setPageId(application.getPages().get(pageIndex).getId()); + actionCollectionDTO1.setApplicationId(application.getId()); + actionCollectionDTO1.setWorkspaceId(application.getWorkspaceId()); + actionCollectionDTO1.setPluginId(jsDatasource.getPluginId()); + ActionDTO action1 = new ActionDTO(); + action1.setName("testMethod"); + action1.setActionConfiguration(new ActionConfiguration()); + action1.getActionConfiguration().setBody("mockBody"); + actionCollectionDTO1.setActions(List.of(action1)); + actionCollectionDTO1.setPluginType(PluginType.JS); + return layoutCollectionService.createCollection(actionCollectionDTO1); + } + + @Test + @WithUserDetails("api_user") + public void exportApplicationByWhen_WhenGitConnectedAndPageRenamed_QueriesAreInUpdatedResources() { + String renamedPageName = "Renamed Page"; + // create an application + Application testApplication = new Application(); + final String appName = UUID.randomUUID().toString(); + testApplication.setName(appName); + testApplication.setWorkspaceId(workspaceId); + testApplication.setUpdatedAt(Instant.now()); + testApplication.setLastDeployedAt(Instant.now()); + testApplication.setClientSchemaVersion(JsonSchemaVersions.clientVersion); + testApplication.setServerSchemaVersion(JsonSchemaVersions.serverVersion); + + Mono applicationJsonMono = applicationPageService + .createApplication(testApplication, workspaceId) + .flatMap(application -> { + // add another page to the application + PageDTO pageDTO = new PageDTO(); + pageDTO.setName("second_page"); + pageDTO.setApplicationId(application.getId()); + return applicationPageService + .createPage(pageDTO) // get the updated application + .then(applicationService.findById(application.getId())); + }) + .flatMap(application -> { + assert application.getPages().size() == 2; + // add one action to each of the pages + return createActionToPage( + "first_page_action", + application.getPages().get(0).getId()) + .then(createActionToPage( + "second_page_action", + application.getPages().get(1).getId())) + .thenReturn(application); + }) + .flatMap(application -> { + // add one action collection to each of the pages + return createActionCollectionToPage(application, 0) + .then(createActionCollectionToPage(application, 1)) + .thenReturn(application); + }) + .flatMap(application -> { + // set git meta data for the application and set a last commit date + GitApplicationMetadata gitApplicationMetadata = new GitApplicationMetadata(); + // add buffer of 5 seconds so that the last commit date is definitely after the last updated date + gitApplicationMetadata.setLastCommittedAt(Instant.now()); + application.setGitApplicationMetadata(gitApplicationMetadata); + return applicationRepository.save(application); + }) + .delayElement(Duration.ofMillis( + 100)) // to make sure the last commit date is definitely after the last updated date + .flatMap(application -> { + // rename the page + ApplicationPage applicationPage = application.getPages().get(0); + PageDTO pageDTO = new PageDTO(); + pageDTO.setName(renamedPageName); + return newPageService + .updatePage(applicationPage.getId(), pageDTO) + // export the application + .then(importExportApplicationService.exportApplicationById( + application.getId(), SerialiseApplicationObjective.VERSION_CONTROL)); + }); + + // verify that the exported json has the updated page name, and the queries are in the updated resources + StepVerifier.create(applicationJsonMono) + .assertNext(applicationJson -> { + Map> updatedResources = applicationJson.getUpdatedResources(); + assertThat(updatedResources).isNotNull(); + Set updatedPageNames = updatedResources.get(FieldName.PAGE_LIST); + Set updatedActionNames = updatedResources.get(FieldName.ACTION_LIST); + Set updatedActionCollectionNames = updatedResources.get(FieldName.ACTION_COLLECTION_LIST); + + assertThat(updatedPageNames).isNotNull(); + assertThat(updatedActionNames).isNotNull(); + assertThat(updatedActionCollectionNames).isNotNull(); + + // only the first page should be present in the updated resources + assertThat(updatedPageNames.size()).isEqualTo(1); + assertThat(updatedPageNames).contains(renamedPageName); + + // only actions from first page should be present in the updated resources + // 1 query + 1 method from action collection + assertThat(updatedActionNames.size()).isEqualTo(2); + assertThat(updatedActionNames).contains("first_page_action" + NAME_SEPARATOR + renamedPageName); + assertThat(updatedActionNames) + .contains("TestJsObject.testMethod" + NAME_SEPARATOR + renamedPageName); + + // only action collections from first page should be present in the updated resources + assertThat(updatedActionCollectionNames.size()).isEqualTo(1); + assertThat(updatedActionCollectionNames) + .contains("TestJsObject" + NAME_SEPARATOR + renamedPageName); + }) + .verifyComplete(); + } } From 6ce2c82fd08a0fa9cbce98b8b9ccf87928b5a859 Mon Sep 17 00:00:00 2001 From: balajisoundar Date: Mon, 25 Sep 2023 13:31:23 +0530 Subject: [PATCH 10/32] fix: [Airgapped] use local assets url in one click binding (#27584) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### PR fixes following issue(s) Fixes https://github.com/appsmithorg/appsmith/issues/27555 > if no issue exists, please create an issue and ask the maintainers about this first > > #### Media > A video or a GIF is preferred. when using Loom, don’t embed because it looks like it’s a GIF. instead, just link to the video > > #### Type of change > Please delete options that are not relevant. - Bug fix (non-breaking change which fixes an issue) - New feature (non-breaking change which adds functionality) - Breaking change (fix or feature that would cause existing functionality to not work as expected) - Chore (housekeeping or task changes that don't impact user perception) - This change requires a documentation update > > > ## Testing > #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [ ] Manual - [ ] JUnit - [ ] Jest - [ ] Cypress > > #### Test Plan > Add Testsmith test cases links that relate to this PR > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --- .../DatasourceDropdown/useDatasource.tsx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/CommonControls/DatasourceDropdown/useDatasource.tsx b/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/CommonControls/DatasourceDropdown/useDatasource.tsx index edb2067462..73f0914ea8 100644 --- a/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/CommonControls/DatasourceDropdown/useDatasource.tsx +++ b/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/CommonControls/DatasourceDropdown/useDatasource.tsx @@ -43,6 +43,7 @@ import { import type { ActionDataState } from "reducers/entityReducers/actionsReducer"; import { getDatatype } from "utils/AppsmithUtils"; import { getCurrentEnvironmentId } from "@appsmith/selectors/environmentSelectors"; +import { getAssetUrl } from "@appsmith/utils/airgapHelpers"; enum SortingWeights { alphabetical = 1, @@ -161,7 +162,7 @@ export function useDatasource(searchText: string) { ), @@ -253,13 +254,13 @@ export function useDatasource(searchText: string) { ), @@ -394,7 +395,7 @@ export function useDatasource(searchText: string) { ), From 97ad65054390f1df9e8a1d654df6ec1085c27020 Mon Sep 17 00:00:00 2001 From: Ankita Kinger Date: Mon, 25 Sep 2023 15:36:24 +0530 Subject: [PATCH 11/32] fix: Removing saas integrations section on datasources page for airgap (#27590) ## Description Removing saas integrations section on the datasources page for airgap, as it is not supported. #### PR fixes following issue(s) Fixes [#27544](https://github.com/appsmithorg/appsmith/issues/27544) #### Type of change - Bug fix (non-breaking change which fixes an issue) ## Testing #### How Has This Been Tested? - [x] Manual - [ ] JUnit - [ ] Jest - [ ] Cypress ## Checklist: #### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --- .../Editor/IntegrationEditor/IntegrationsHomeScreen.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/client/src/pages/Editor/IntegrationEditor/IntegrationsHomeScreen.tsx b/app/client/src/pages/Editor/IntegrationEditor/IntegrationsHomeScreen.tsx index 1367848ced..de30120d40 100644 --- a/app/client/src/pages/Editor/IntegrationEditor/IntegrationsHomeScreen.tsx +++ b/app/client/src/pages/Editor/IntegrationEditor/IntegrationsHomeScreen.tsx @@ -37,6 +37,7 @@ import Debugger, { import { showDebuggerFlag } from "selectors/debuggerSelectors"; import AnalyticsUtil from "utils/AnalyticsUtil"; import { DatasourceCreateEntryPoints } from "constants/Datasource"; +import { isAirgapped } from "@appsmith/utils/airgapHelpers"; const HeaderFlex = styled.div` font-size: 20px; @@ -219,6 +220,7 @@ function CreateNewSaasIntegration({ }: any) { const newSaasAPIRef = useRef(null); const isMounted = useRef(false); + const isAirgappedInstance = isAirgapped(); useEffect(() => { if (active && newSaasAPIRef.current) { @@ -233,7 +235,7 @@ function CreateNewSaasIntegration({ isMounted.current = true; } }, [active]); - return ( + return !isAirgappedInstance ? (
Saas Integrations
- ); + ) : null; } function CreateNewDatasource({ From a9635812228bd4370daf062852f47885d65e6a1e Mon Sep 17 00:00:00 2001 From: sneha122 Date: Mon, 25 Sep 2023 15:47:17 +0530 Subject: [PATCH 12/32] fix: gsheet issue with import and redirection fixed (#27588) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description This PR fixes issue with gsheet import and redirection. When an application containing a couple of datasources like mysql, postgres and gsheet are imported, we are shown reconnect datasource modal, suppose we first configure all datasources except gsheet, lastly we select gsheet datasource and save and authorise, we are not being redirected to authorisation. The issue occurred because after saving each datasource, that datasource was disappearing from the list of datasources shown on left panel in reconnect datasource modal, so if we configure gsheet datasource at last, it would remove datasource from the list and lose all the context. So the request sent to `/oauth` endpoint would be cancelled. This PR essentially solves 2 issues: - datasource disappearing from left panel in reconnect datasource modal - gsheet datasource not being redirected to authorisation #### PR fixes following issue(s) Fixes #27520 > if no issue exists, please create an issue and ask the maintainers about this first > > #### Media > A video or a GIF is preferred. when using Loom, don’t embed because it looks like it’s a GIF. instead, just link to the video > > #### Type of change - Bug fix (non-breaking change which fixes an issue) > > > ## Testing > #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [x] Manual - [ ] JUnit - [ ] Jest - [ ] Cypress > > #### Test Plan Covered the following areas manually: - workspace import with app containing multiple DS - fork of above imported app into another workspace - app level import with multiple DS - importing an app without any datasources > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [x] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [x] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed Co-authored-by: “sneha122” <“sneha@appsmith.com”> --- .../pages/Editor/gitSync/ReconnectDatasourceModal.tsx | 5 ++++- app/client/src/sagas/DatasourcesSagas.ts | 10 ---------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/app/client/src/pages/Editor/gitSync/ReconnectDatasourceModal.tsx b/app/client/src/pages/Editor/gitSync/ReconnectDatasourceModal.tsx index 3021995d05..df4fb44d0b 100644 --- a/app/client/src/pages/Editor/gitSync/ReconnectDatasourceModal.tsx +++ b/app/client/src/pages/Editor/gitSync/ReconnectDatasourceModal.tsx @@ -516,7 +516,10 @@ function ReconnectDatasourceModal() { JSON.stringify(appInfo), ); } - } else if (appURL && unconfiguredDatasources.length === 0) { + } + // When datasources are present and pending datasources are 0, + // then only we want to update status as success + else if (appURL && pending.length === 0 && datasources.length > 0) { // open application import successfule localStorage.setItem("importApplicationSuccess", "true"); localStorage.setItem("importedAppPendingInfo", "null"); diff --git a/app/client/src/sagas/DatasourcesSagas.ts b/app/client/src/sagas/DatasourcesSagas.ts index a49453c836..bc7e5b854d 100644 --- a/app/client/src/sagas/DatasourcesSagas.ts +++ b/app/client/src/sagas/DatasourcesSagas.ts @@ -45,12 +45,10 @@ import { getPluginByPackageName, getDatasourcesUsedInApplicationByActions, getEntityExplorerDatasources, - getUnconfiguredDatasources, } from "@appsmith/selectors/entitiesSelector"; import { addMockDatasourceToWorkspace, setDatasourceViewModeFlag, - setUnconfiguredDatasourcesDuringImport, } from "actions/datasourceActions"; import type { UpdateDatasourceSuccessAction, @@ -525,14 +523,6 @@ function* updateDatasourceSaga( kind: "success", }); - const unconfiguredDSList: Datasource[] = yield select( - getUnconfiguredDatasources, - ); - const updatedList = unconfiguredDSList.filter( - (d: Datasource) => d.id !== datasourcePayload?.id, - ); - yield put(setUnconfiguredDatasourcesDuringImport(updatedList)); - const expandDatasourceId = state.ui.datasourcePane.expandDatasourceId; // Dont redirect if action payload has an onSuccess From f225758efff82315b9b4b062867409543b95e904 Mon Sep 17 00:00:00 2001 From: Rudraprasad Das Date: Tue, 26 Sep 2023 16:07:26 +0530 Subject: [PATCH 13/32] fix: adding offline error for git failure (#27597) ## Description Error message were ignored when offline. Fixed it! #### PR fixes following issue(s) Fixes #27589 #### Media ![image](https://github.com/appsmithorg/appsmith/assets/8724051/eaad3879-49f6-4c96-9281-2ab464d35a0a) #### Type of change - Bug fix (non-breaking change which fixes an issue) ## Testing > #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [x] Manual - [ ] JUnit - [ ] Jest - [ ] Cypress > > #### Test Plan > Add Testsmith test cases links that relate to this PR > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --- .../pages/Editor/gitSync/Tabs/GitConnectionV2/index.tsx | 8 +++++--- .../src/pages/Editor/gitSync/hooks/useGitConnect.ts | 3 ++- app/client/src/sagas/GitSyncSagas.ts | 9 ++++++--- app/client/src/selectors/gitSyncSelectors.tsx | 2 +- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/app/client/src/pages/Editor/gitSync/Tabs/GitConnectionV2/index.tsx b/app/client/src/pages/Editor/gitSync/Tabs/GitConnectionV2/index.tsx index 0a09c500b2..f6380536a4 100644 --- a/app/client/src/pages/Editor/gitSync/Tabs/GitConnectionV2/index.tsx +++ b/app/client/src/pages/Editor/gitSync/Tabs/GitConnectionV2/index.tsx @@ -170,12 +170,13 @@ function GitConnectionV2({ isImport = false }: GitConnectionV2Props) { isDefaultProfile: true, }, { - onErrorCallback: (err: Error, response?: any) => { + onErrorCallback: (error: any, response?: any) => { // AE-GIT-4033 is repo not empty error if (response?.responseMeta?.error?.code === "AE-GIT-4033") { setActiveStep(GIT_CONNECT_STEPS.GENERATE_SSH_KEY); } - setErrorData(response); + const errorResponse = response || error?.response?.data; + setErrorData(errorResponse); }, }, ); @@ -192,7 +193,8 @@ function GitConnectionV2({ isImport = false }: GitConnectionV2Props) { isDefaultProfile: true, }, onErrorCallback(error, response) { - setErrorData(response); + const errorResponse = response || error?.response?.data; + setErrorData(errorResponse); }, }), ); diff --git a/app/client/src/pages/Editor/gitSync/hooks/useGitConnect.ts b/app/client/src/pages/Editor/gitSync/hooks/useGitConnect.ts index e9db1ff138..bff9580ab9 100644 --- a/app/client/src/pages/Editor/gitSync/hooks/useGitConnect.ts +++ b/app/client/src/pages/Editor/gitSync/hooks/useGitConnect.ts @@ -27,7 +27,8 @@ export const useGitConnect = () => { }, onErrorCallback: (err, response) => { onErrorCallback(err, response); - setErrResponse(response); + const errorResponse = response || err?.response?.data; + setErrResponse(errorResponse); setIsConnectingToGit(false); }, }), diff --git a/app/client/src/sagas/GitSyncSagas.ts b/app/client/src/sagas/GitSyncSagas.ts index 076b5d6b4d..9408ddc60f 100644 --- a/app/client/src/sagas/GitSyncSagas.ts +++ b/app/client/src/sagas/GitSyncSagas.ts @@ -235,7 +235,7 @@ function* connectToGitSaga(action: ConnectToGitReduxAction) { } /* commit effect END */ } - } catch (error) { + } catch (error: any) { if (action.onErrorCallback) { action.onErrorCallback(error as Error, response); } @@ -248,11 +248,14 @@ function* connectToGitSaga(action: ConnectToGitReduxAction) { // Api error // Display on the UI - if (response && !response?.responseMeta?.success) { + + const errorResponse = response || error?.response?.data; + + if (errorResponse && !errorResponse?.responseMeta?.success) { yield put({ type: ReduxActionErrorTypes.CONNECT_TO_GIT_ERROR, payload: { - error: response?.responseMeta.error, + error: errorResponse?.responseMeta.error, show: false, }, }); diff --git a/app/client/src/selectors/gitSyncSelectors.tsx b/app/client/src/selectors/gitSyncSelectors.tsx index c5f3d88c84..4aa28df34b 100644 --- a/app/client/src/selectors/gitSyncSelectors.tsx +++ b/app/client/src/selectors/gitSyncSelectors.tsx @@ -184,7 +184,7 @@ export const getDisconnectDocUrl = () => "https://docs.appsmith.com/advanced-concepts/version-control-with-git/disconnect-the-git-repository"; export const getConnectingErrorDocUrl = (state: AppState) => - state.ui.gitSync.connectError?.error.referenceDoc || + state.ui.gitSync.connectError?.error?.referenceDoc || FALLBACK_GIT_SYNC_DOCS_URL; export const getUpstreamErrorDocUrl = (state: AppState) => From f763e75b9c8437bc3862354ccf37d5b35cd07374 Mon Sep 17 00:00:00 2001 From: Nidhi Date: Tue, 26 Sep 2023 18:21:33 +0530 Subject: [PATCH 14/32] chore: Added checks to only return plugins when new updates are present (#27641) --- .../server/constants/ce/FieldNameCE.java | 2 ++ .../ce/PluginScheduledTaskUtilsCEImpl.java | 4 ++- .../server/services/ConfigServiceImpl.java | 10 ++---- .../services/ce/ConfigServiceCEImpl.java | 15 +-------- .../services/ce/PluginServiceCEImpl.java | 5 +++ .../solutions/PluginScheduledTaskImpl.java | 7 ++-- .../ce/PluginScheduledTaskCEImpl.java | 32 ++++++++++++++++--- 7 files changed, 43 insertions(+), 32 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/ce/FieldNameCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/ce/FieldNameCE.java index 6292e0b310..77e45838a9 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/ce/FieldNameCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/ce/FieldNameCE.java @@ -187,4 +187,6 @@ public class FieldNameCE { public static final String IS_MERGEABLE = "isMergeable"; public static final String FILE_LOCK_DURATION = "fileLockDuration"; + + public static final String REMOTE_PLUGINS = "remotePlugins"; } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/PluginScheduledTaskUtilsCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/PluginScheduledTaskUtilsCEImpl.java index 3bf23bf3f6..6c379e42e4 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/PluginScheduledTaskUtilsCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/PluginScheduledTaskUtilsCEImpl.java @@ -37,8 +37,10 @@ public class PluginScheduledTaskUtilsCEImpl implements PluginScheduledTaskUtilsC return Mono.empty(); } + String lastUpdatedAtParam = lastUpdatedAt != null ? "&lastUpdatedAt=" + lastUpdatedAt : ""; + return configService.getInstanceId().flatMap(instanceId -> WebClientUtils.create( - baseUrl + "/api/v1/plugins?instanceId=" + instanceId + "&lastUpdatedAt=" + lastUpdatedAt) + baseUrl + "/api/v1/plugins?instanceId=" + instanceId + lastUpdatedAtParam) .get() .exchangeToMono(clientResponse -> clientResponse.bodyToMono(new ParameterizedTypeReference>>() {})) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ConfigServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ConfigServiceImpl.java index e19ccad98d..5f96581892 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ConfigServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ConfigServiceImpl.java @@ -1,8 +1,6 @@ package com.appsmith.server.services; -import com.appsmith.server.repositories.ApplicationRepository; import com.appsmith.server.repositories.ConfigRepository; -import com.appsmith.server.repositories.DatasourceRepository; import com.appsmith.server.services.ce.ConfigServiceCEImpl; import lombok.extern.slf4j.Slf4j; import org.springframework.stereotype.Service; @@ -11,11 +9,7 @@ import org.springframework.stereotype.Service; @Service public class ConfigServiceImpl extends ConfigServiceCEImpl implements ConfigService { - public ConfigServiceImpl( - ConfigRepository repository, - ApplicationRepository applicationRepository, - DatasourceRepository datasourceRepository) { - - super(repository, applicationRepository, datasourceRepository); + public ConfigServiceImpl(ConfigRepository repository) { + super(repository); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ConfigServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ConfigServiceCEImpl.java index e20f0d75a4..ba8f13e235 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ConfigServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ConfigServiceCEImpl.java @@ -8,9 +8,7 @@ import com.appsmith.server.domains.Config; import com.appsmith.server.domains.User; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; -import com.appsmith.server.repositories.ApplicationRepository; import com.appsmith.server.repositories.ConfigRepository; -import com.appsmith.server.repositories.DatasourceRepository; import lombok.extern.slf4j.Slf4j; import net.minidev.json.JSONObject; import reactor.core.publisher.Flux; @@ -24,23 +22,12 @@ import static org.apache.commons.lang3.ObjectUtils.defaultIfNull; @Slf4j public class ConfigServiceCEImpl implements ConfigServiceCE { - - private static final String TEMPLATE_WORKSPACE_CONFIG_NAME = "template-workspace"; - - private final ApplicationRepository applicationRepository; - private final DatasourceRepository datasourceRepository; private final ConfigRepository repository; // This is permanently cached through the life of the JVM process as this is not intended to change at runtime ever. private String instanceId = null; - public ConfigServiceCEImpl( - ConfigRepository repository, - ApplicationRepository applicationRepository, - DatasourceRepository datasourceRepository) { - - this.applicationRepository = applicationRepository; - this.datasourceRepository = datasourceRepository; + public ConfigServiceCEImpl(ConfigRepository repository) { this.repository = repository; } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/PluginServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/PluginServiceCEImpl.java index ff3390e385..8756c48f61 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/PluginServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/PluginServiceCEImpl.java @@ -196,6 +196,11 @@ public class PluginServiceCEImpl extends BaseService { // Only perform a DB op if plugins associated to this org have changed if (workspace.getPlugins().containsAll(newWorkspacePlugins)) { diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/PluginScheduledTaskImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/PluginScheduledTaskImpl.java index 73a555b2a2..67ed080bc6 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/PluginScheduledTaskImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/PluginScheduledTaskImpl.java @@ -1,6 +1,7 @@ package com.appsmith.server.solutions; import com.appsmith.server.helpers.PluginScheduledTaskUtils; +import com.appsmith.server.services.ConfigService; import com.appsmith.server.solutions.ce.PluginScheduledTaskCEImpl; import lombok.extern.slf4j.Slf4j; import org.springframework.stereotype.Component; @@ -8,9 +9,7 @@ import org.springframework.stereotype.Component; @Slf4j @Component public class PluginScheduledTaskImpl extends PluginScheduledTaskCEImpl implements PluginScheduledTask { - - public PluginScheduledTaskImpl(PluginScheduledTaskUtils pluginScheduledTaskUtils) { - - super(pluginScheduledTaskUtils); + public PluginScheduledTaskImpl(PluginScheduledTaskUtils pluginScheduledTaskUtils, ConfigService configService) { + super(pluginScheduledTaskUtils, configService); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/PluginScheduledTaskCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/PluginScheduledTaskCEImpl.java index d699a4c1a7..b0ca1d5858 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/PluginScheduledTaskCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/PluginScheduledTaskCEImpl.java @@ -1,15 +1,21 @@ package com.appsmith.server.solutions.ce; +import com.appsmith.server.constants.FieldName; +import com.appsmith.server.domains.Config; import com.appsmith.server.helpers.PluginScheduledTaskUtils; +import com.appsmith.server.services.ConfigService; import lombok.AllArgsConstructor; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import net.minidev.json.JSONObject; import org.springframework.scheduling.annotation.Scheduled; import reactor.core.scheduler.Schedulers; import java.time.Instant; +import java.util.Date; +import java.util.Map; /** * This class represents a scheduled task that pings cloud services for any updates in available plugins. @@ -19,18 +25,34 @@ import java.time.Instant; public class PluginScheduledTaskCEImpl implements PluginScheduledTaskCE { private final PluginScheduledTaskUtils pluginScheduledTaskUtils; - - private Instant lastUpdatedAt = null; + private final ConfigService configService; // Number of milliseconds between the start of each scheduled calls to this method. @Scheduled(initialDelay = 30 * 1000 /* 30 seconds */, fixedRate = 2 * 60 * 60 * 1000 /* two hours */) public void updateRemotePlugins() { // Moving the fetch and update remote plugins to helper classes to have custom implementation for business // edition - pluginScheduledTaskUtils - .fetchAndUpdateRemotePlugins(lastUpdatedAt) + configService + .getByName(FieldName.REMOTE_PLUGINS) + .onErrorReturn(new Config()) + .map(config -> { + JSONObject config1 = config.getConfig(); + Instant lastUpdatedAt = null; + + if (config1 != null) { + Object tempUpdatedAt = config1.getOrDefault(FieldName.UPDATED_AT, null); + if (tempUpdatedAt != null) { + lastUpdatedAt = ((Date) tempUpdatedAt).toInstant(); + } + } + return pluginScheduledTaskUtils.fetchAndUpdateRemotePlugins(lastUpdatedAt); + }) // Set new updated time - .doOnSuccess(success -> this.lastUpdatedAt = Instant.now()) + .flatMap(success -> { + Config config = new Config( + new JSONObject(Map.of(FieldName.UPDATED_AT, Instant.now())), FieldName.REMOTE_PLUGINS); + return configService.save(config); + }) .subscribeOn(Schedulers.single()) .subscribe(); } From e3d08140d4abc124d3f51556cf621ab197790bcd Mon Sep 17 00:00:00 2001 From: Nidhi Nair Date: Tue, 26 Sep 2023 18:43:16 +0530 Subject: [PATCH 15/32] fix: Added checks to only return plugins when new updates are present --- .../appsmith/server/services/ConfigServiceImpl.java | 10 ++++++++-- .../server/services/ce/ConfigServiceCEImpl.java | 13 ++++++++++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ConfigServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ConfigServiceImpl.java index 5f96581892..e19ccad98d 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ConfigServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ConfigServiceImpl.java @@ -1,6 +1,8 @@ package com.appsmith.server.services; +import com.appsmith.server.repositories.ApplicationRepository; import com.appsmith.server.repositories.ConfigRepository; +import com.appsmith.server.repositories.DatasourceRepository; import com.appsmith.server.services.ce.ConfigServiceCEImpl; import lombok.extern.slf4j.Slf4j; import org.springframework.stereotype.Service; @@ -9,7 +11,11 @@ import org.springframework.stereotype.Service; @Service public class ConfigServiceImpl extends ConfigServiceCEImpl implements ConfigService { - public ConfigServiceImpl(ConfigRepository repository) { - super(repository); + public ConfigServiceImpl( + ConfigRepository repository, + ApplicationRepository applicationRepository, + DatasourceRepository datasourceRepository) { + + super(repository, applicationRepository, datasourceRepository); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ConfigServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ConfigServiceCEImpl.java index ba8f13e235..f28c96e663 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ConfigServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ConfigServiceCEImpl.java @@ -8,7 +8,9 @@ import com.appsmith.server.domains.Config; import com.appsmith.server.domains.User; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; +import com.appsmith.server.repositories.ApplicationRepository; import com.appsmith.server.repositories.ConfigRepository; +import com.appsmith.server.repositories.DatasourceRepository; import lombok.extern.slf4j.Slf4j; import net.minidev.json.JSONObject; import reactor.core.publisher.Flux; @@ -22,12 +24,21 @@ import static org.apache.commons.lang3.ObjectUtils.defaultIfNull; @Slf4j public class ConfigServiceCEImpl implements ConfigServiceCE { + private static final String TEMPLATE_WORKSPACE_CONFIG_NAME = "template-workspace"; private final ConfigRepository repository; + private final ApplicationRepository applicationRepository; + private final DatasourceRepository datasourceRepository; // This is permanently cached through the life of the JVM process as this is not intended to change at runtime ever. private String instanceId = null; - public ConfigServiceCEImpl(ConfigRepository repository) { + public ConfigServiceCEImpl( + ConfigRepository repository, + ApplicationRepository applicationRepository, + DatasourceRepository datasourceRepository) { + + this.applicationRepository = applicationRepository; + this.datasourceRepository = datasourceRepository; this.repository = repository; } From c85b454750fba566161dbc1a753ba60d9f27658c Mon Sep 17 00:00:00 2001 From: Nidhi Date: Wed, 27 Sep 2023 10:32:35 +0530 Subject: [PATCH 16/32] chore: Fixed flatmapping cs req (#27649) --- .../appsmith/server/solutions/ce/PluginScheduledTaskCEImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/PluginScheduledTaskCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/PluginScheduledTaskCEImpl.java index b0ca1d5858..d705705cdf 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/PluginScheduledTaskCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/PluginScheduledTaskCEImpl.java @@ -35,7 +35,7 @@ public class PluginScheduledTaskCEImpl implements PluginScheduledTaskCE { configService .getByName(FieldName.REMOTE_PLUGINS) .onErrorReturn(new Config()) - .map(config -> { + .flatMap(config -> { JSONObject config1 = config.getConfig(); Instant lastUpdatedAt = null; From 61ef8f7775e87b7aa8593bc608a3fcc4a9632013 Mon Sep 17 00:00:00 2001 From: tkAppsmith <131347120+tkAppsmith@users.noreply.github.com> Date: Mon, 11 Sep 2023 11:22:52 +0530 Subject: [PATCH 17/32] fix: fixed failing queries using aggregation pipeline (#26132) ## Description > Queries using aggregation update failing. hence added a fallback. #### PR fixes following issue(s) Fixes #26090 Fixes https://github.com/appsmithorg/appsmith-ee/issues/1659 #### Type of change - Bug fix (non-breaking change which fixes an issue) ## Testing > #### How Has This Been Tested? - [x] Manual - [ ] Jest - [ ] Cypress > > #### Test Plan > Add Testsmith test cases links that relate to this PR > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --- .../repositories/BaseRepositoryImpl.java | 4 +- .../CustomNewActionRepositoryImpl.java | 6 +- .../CustomNewPageRepositoryImpl.java | 6 +- .../ce/BaseAppsmithRepositoryCEImpl.java | 6 +- .../ce/CustomNewActionRepositoryCE.java | 2 +- .../ce/CustomNewActionRepositoryCEImpl.java | 42 +++++++--- .../ce/CustomNewPageRepositoryCE.java | 6 +- .../ce/CustomNewPageRepositoryCEImpl.java | 77 ++++++++++++++++--- .../ce/NewActionRepositoryCE.java | 5 ++ .../ce/ApplicationPageServiceCEImpl.java | 5 +- .../services/ce/NewActionServiceCE.java | 4 +- .../services/ce/NewActionServiceCEImpl.java | 8 +- .../server/services/ce/NewPageServiceCE.java | 4 +- .../services/ce/NewPageServiceCEImpl.java | 4 +- .../services/ce/NewActionServiceUnitTest.java | 11 ++- 15 files changed, 143 insertions(+), 47 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/BaseRepositoryImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/BaseRepositoryImpl.java index 7bae25049d..a4090d8f70 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/BaseRepositoryImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/BaseRepositoryImpl.java @@ -161,7 +161,9 @@ public class BaseRepositoryImpl .flatMapMany(principal -> { Query query = new Query(notDeleted()); return mongoOperations.find( - query, entityInformation.getJavaType(), entityInformation.getCollectionName()); + query.cursorBatchSize(10000), + entityInformation.getJavaType(), + entityInformation.getCollectionName()); }); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomNewActionRepositoryImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomNewActionRepositoryImpl.java index e45c7fd224..b3eb05cb8d 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomNewActionRepositoryImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomNewActionRepositoryImpl.java @@ -2,6 +2,7 @@ package com.appsmith.server.repositories; import com.appsmith.server.repositories.ce.CustomNewActionRepositoryCEImpl; import lombok.extern.slf4j.Slf4j; +import org.springframework.data.mongodb.core.MongoTemplate; import org.springframework.data.mongodb.core.ReactiveMongoOperations; import org.springframework.data.mongodb.core.convert.MongoConverter; import org.springframework.stereotype.Component; @@ -14,7 +15,8 @@ public class CustomNewActionRepositoryImpl extends CustomNewActionRepositoryCEIm public CustomNewActionRepositoryImpl( ReactiveMongoOperations mongoOperations, MongoConverter mongoConverter, - CacheableRepositoryHelper cacheableRepositoryHelper) { - super(mongoOperations, mongoConverter, cacheableRepositoryHelper); + CacheableRepositoryHelper cacheableRepositoryHelper, + MongoTemplate mongoTemplate) { + super(mongoOperations, mongoConverter, cacheableRepositoryHelper, mongoTemplate); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomNewPageRepositoryImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomNewPageRepositoryImpl.java index 370e5202b2..690b54d4c1 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomNewPageRepositoryImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomNewPageRepositoryImpl.java @@ -2,6 +2,7 @@ package com.appsmith.server.repositories; import com.appsmith.server.repositories.ce.CustomNewPageRepositoryCEImpl; import lombok.extern.slf4j.Slf4j; +import org.springframework.data.mongodb.core.MongoTemplate; import org.springframework.data.mongodb.core.ReactiveMongoOperations; import org.springframework.data.mongodb.core.convert.MongoConverter; import org.springframework.stereotype.Component; @@ -13,7 +14,8 @@ public class CustomNewPageRepositoryImpl extends CustomNewPageRepositoryCEImpl i public CustomNewPageRepositoryImpl( ReactiveMongoOperations mongoOperations, MongoConverter mongoConverter, - CacheableRepositoryHelper cacheableRepositoryHelper) { - super(mongoOperations, mongoConverter, cacheableRepositoryHelper); + CacheableRepositoryHelper cacheableRepositoryHelper, + MongoTemplate mongoTemplate) { + super(mongoOperations, mongoConverter, cacheableRepositoryHelper, mongoTemplate); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/BaseAppsmithRepositoryCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/BaseAppsmithRepositoryCEImpl.java index c2d9b0634b..28d0012b85 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/BaseAppsmithRepositoryCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/BaseAppsmithRepositoryCEImpl.java @@ -22,6 +22,7 @@ import org.springframework.data.mongodb.core.query.Criteria; import org.springframework.data.mongodb.core.query.Query; import org.springframework.data.mongodb.core.query.Update; import org.springframework.data.mongodb.core.query.UpdateDefinition; +import org.springframework.data.mongodb.repository.Meta; import org.springframework.security.core.context.ReactiveSecurityContextHolder; import org.springframework.util.CollectionUtils; import reactor.core.publisher.Flux; @@ -166,7 +167,7 @@ public abstract class BaseAppsmithRepositoryCEImpl { return mongoOperations .query(this.genericDomain) - .matching(query) + .matching(query.cursorBatchSize(10000)) .one() .flatMap(obj -> setUserPermissionsInObject(obj, permissionGroups)); }); @@ -331,6 +332,7 @@ public abstract class BaseAppsmithRepositoryCEImpl { }); } + @Meta(cursorBatchSize = 10000) protected Mono queryOne( List criterias, List projectionFieldNames, Optional permission) { Mono> permissionGroupsMono = getCurrentUserPermissionGroupsIfRequired(permission); @@ -539,7 +541,7 @@ public abstract class BaseAppsmithRepositoryCEImpl { sortOptional.ifPresent(sort -> query.with(sort)); return mongoOperations .query(this.genericDomain) - .matching(query) + .matching(query.cursorBatchSize(10000)) .all() .flatMap(obj -> setUserPermissionsInObject(obj, permissionGroups)); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCE.java index 007ff409f8..11a6cb6af1 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCE.java @@ -78,7 +78,7 @@ public interface CustomNewActionRepositoryCE extends AppsmithRepository> bulkUpdate(List newActions); - Mono publishActions(String applicationId, AclPermission permission); + Mono> publishActions(String applicationId, AclPermission permission); Mono archiveDeletedUnpublishedActions(String applicationId, AclPermission permission); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCEImpl.java index 5a90b20dce..c45cf40bfa 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCEImpl.java @@ -20,9 +20,12 @@ import lombok.extern.slf4j.Slf4j; import org.bson.Document; import org.bson.types.ObjectId; import org.springframework.data.domain.Sort; +import org.springframework.data.mongodb.core.MongoTemplate; import org.springframework.data.mongodb.core.ReactiveMongoOperations; import org.springframework.data.mongodb.core.aggregation.Aggregation; -import org.springframework.data.mongodb.core.aggregation.AggregationUpdate; +import org.springframework.data.mongodb.core.aggregation.AggregationOperation; +import org.springframework.data.mongodb.core.aggregation.AggregationResults; +import org.springframework.data.mongodb.core.aggregation.Fields; import org.springframework.data.mongodb.core.aggregation.GroupOperation; import org.springframework.data.mongodb.core.aggregation.MatchOperation; import org.springframework.data.mongodb.core.aggregation.ProjectionOperation; @@ -52,11 +55,15 @@ import static org.springframework.data.mongodb.core.query.Criteria.where; public class CustomNewActionRepositoryCEImpl extends BaseAppsmithRepositoryImpl implements CustomNewActionRepositoryCE { + private final MongoTemplate mongoTemplate; + public CustomNewActionRepositoryCEImpl( ReactiveMongoOperations mongoOperations, MongoConverter mongoConverter, - CacheableRepositoryHelper cacheableRepositoryHelper) { + CacheableRepositoryHelper cacheableRepositoryHelper, + MongoTemplate mongoTemplate) { super(mongoOperations, mongoConverter, cacheableRepositoryHelper); + this.mongoTemplate = mongoTemplate; } @Override @@ -571,16 +578,33 @@ public class CustomNewActionRepositoryCEImpl extends BaseAppsmithRepositoryImpl< } @Override - public Mono publishActions(String applicationId, AclPermission permission) { + public Mono> publishActions(String applicationId, AclPermission permission) { Criteria applicationIdCriteria = where(fieldName(QNewAction.newAction.applicationId)).is(applicationId); - // using aggregation update instead of regular update here - // it's required to set a field to a value of another field from the same domain - AggregationUpdate aggregationUpdate = AggregationUpdate.update() - .set(fieldName(QNewAction.newAction.publishedAction)) - .toValue("$" + fieldName(QNewAction.newAction.unpublishedAction)); - return updateByCriteria(List.of(applicationIdCriteria), aggregationUpdate, permission); + Mono> permissionGroupsMono = + getCurrentUserPermissionGroupsIfRequired(Optional.ofNullable(permission)); + + return permissionGroupsMono.flatMap(permissionGroups -> { + AggregationOperation matchAggregationWithPermission = null; + if (permission == null) { + matchAggregationWithPermission = Aggregation.match(new Criteria().andOperator(notDeleted())); + } else { + matchAggregationWithPermission = Aggregation.match( + new Criteria().andOperator(notDeleted(), userAcl(permissionGroups, permission))); + } + AggregationOperation matchAggregation = Aggregation.match(applicationIdCriteria); + AggregationOperation wholeProjection = Aggregation.project(NewAction.class); + AggregationOperation addFieldsOperation = Aggregation.addFields() + .addField(fieldName(QNewAction.newAction.publishedAction)) + .withValueOf(Fields.field(fieldName(QNewAction.newAction.unpublishedAction))) + .build(); + Aggregation combinedAggregation = Aggregation.newAggregation( + matchAggregation, matchAggregationWithPermission, wholeProjection, addFieldsOperation); + AggregationResults updatedResults = + mongoTemplate.aggregate(combinedAggregation, NewAction.class, NewAction.class); + return bulkUpdate(updatedResults.getMappedResults()); + }); } @Override diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCE.java index dc940ee25b..ec6775c2cb 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCE.java @@ -3,7 +3,7 @@ package com.appsmith.server.repositories.ce; import com.appsmith.server.acl.AclPermission; import com.appsmith.server.domains.NewPage; import com.appsmith.server.repositories.AppsmithRepository; -import com.mongodb.client.result.UpdateResult; +import com.mongodb.bulk.BulkWriteResult; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -43,5 +43,7 @@ public interface CustomNewPageRepositoryCE extends AppsmithRepository { Mono findByGitSyncIdAndDefaultApplicationId( String defaultApplicationId, String gitSyncId, Optional permission); - Mono publishPages(Collection pageIds, AclPermission permission); + Mono> publishPages(Collection pageIds, AclPermission permission); + + Mono> bulkUpdate(List newPages); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCEImpl.java index 92a93f963e..acd1a16e0f 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCEImpl.java @@ -9,20 +9,32 @@ import com.appsmith.server.domains.QNewPage; import com.appsmith.server.dtos.PageDTO; import com.appsmith.server.repositories.BaseAppsmithRepositoryImpl; import com.appsmith.server.repositories.CacheableRepositoryHelper; -import com.mongodb.client.result.UpdateResult; +import com.mongodb.bulk.BulkWriteResult; +import com.mongodb.client.model.UpdateOneModel; +import com.mongodb.client.model.WriteModel; import lombok.extern.slf4j.Slf4j; +import org.bson.Document; +import org.bson.types.ObjectId; +import org.springframework.data.mongodb.core.MongoTemplate; import org.springframework.data.mongodb.core.ReactiveMongoOperations; -import org.springframework.data.mongodb.core.aggregation.AggregationUpdate; +import org.springframework.data.mongodb.core.aggregation.Aggregation; +import org.springframework.data.mongodb.core.aggregation.AggregationOperation; +import org.springframework.data.mongodb.core.aggregation.AggregationResults; +import org.springframework.data.mongodb.core.aggregation.Fields; import org.springframework.data.mongodb.core.convert.MongoConverter; import org.springframework.data.mongodb.core.query.Criteria; import org.springframework.data.mongodb.core.query.Query; +import org.springframework.util.CollectionUtils; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; import static org.springframework.data.mongodb.core.query.Criteria.where; @@ -30,11 +42,15 @@ import static org.springframework.data.mongodb.core.query.Criteria.where; public class CustomNewPageRepositoryCEImpl extends BaseAppsmithRepositoryImpl implements CustomNewPageRepositoryCE { + private final MongoTemplate mongoTemplate; + public CustomNewPageRepositoryCEImpl( ReactiveMongoOperations mongoOperations, MongoConverter mongoConverter, - CacheableRepositoryHelper cacheableRepositoryHelper) { + CacheableRepositoryHelper cacheableRepositoryHelper, + MongoTemplate mongoTemplate) { super(mongoOperations, mongoConverter, cacheableRepositoryHelper); + this.mongoTemplate = mongoTemplate; } @Override @@ -251,14 +267,55 @@ public class CustomNewPageRepositoryCEImpl extends BaseAppsmithRepositoryImpl publishPages(Collection pageIds, AclPermission permission) { + public Mono> publishPages(Collection pageIds, AclPermission permission) { Criteria applicationIdCriteria = where(fieldName(QNewPage.newPage.id)).in(pageIds); - // using aggregation update instead of regular update here - // it's required to set a field to a value of another field from the same domain - AggregationUpdate aggregationUpdate = AggregationUpdate.update() - .set(fieldName(QNewPage.newPage.publishedPage)) - .toValue("$" + fieldName(QNewPage.newPage.unpublishedPage)); - return updateByCriteria(List.of(applicationIdCriteria), aggregationUpdate, permission); + Mono> permissionGroupsMono = + getCurrentUserPermissionGroupsIfRequired(Optional.ofNullable(permission)); + + return permissionGroupsMono.flatMap(permissionGroups -> { + AggregationOperation matchAggregationWithPermission = null; + if (permission == null) { + matchAggregationWithPermission = Aggregation.match(new Criteria().andOperator(notDeleted())); + } else { + matchAggregationWithPermission = Aggregation.match( + new Criteria().andOperator(notDeleted(), userAcl(permissionGroups, permission))); + } + AggregationOperation matchAggregation = Aggregation.match(applicationIdCriteria); + AggregationOperation wholeProjection = Aggregation.project(NewPage.class); + AggregationOperation addFieldsOperation = Aggregation.addFields() + .addField(fieldName(QNewPage.newPage.publishedPage)) + .withValueOf(Fields.field(fieldName(QNewPage.newPage.unpublishedPage))) + .build(); + Aggregation combinedAggregation = Aggregation.newAggregation( + matchAggregation, matchAggregationWithPermission, wholeProjection, addFieldsOperation); + AggregationResults updatedResults = + mongoTemplate.aggregate(combinedAggregation, NewPage.class, NewPage.class); + return bulkUpdate(updatedResults.getMappedResults()); + }); + } + + @Override + public Mono> bulkUpdate(List newPages) { + if (CollectionUtils.isEmpty(newPages)) { + return Mono.just(Collections.emptyList()); + } + + // convert the list of new pages to a list of DBObjects + List> dbObjects = newPages.stream() + .map(newPage -> { + assert newPage.getId() != null; + Document document = new Document(); + mongoOperations.getConverter().write(newPage, document); + document.remove("_id"); + return (WriteModel) new UpdateOneModel( + new Document("_id", new ObjectId(newPage.getId())), new Document("$set", document)); + }) + .collect(Collectors.toList()); + + return mongoOperations + .getCollection(mongoOperations.getCollectionName(NewPage.class)) + .flatMapMany(documentMongoCollection -> documentMongoCollection.bulkWrite(dbObjects)) + .collectList(); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/NewActionRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/NewActionRepositoryCE.java index e74852d3b5..505981ef1d 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/NewActionRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/NewActionRepositoryCE.java @@ -3,12 +3,17 @@ package com.appsmith.server.repositories.ce; import com.appsmith.server.domains.NewAction; import com.appsmith.server.repositories.BaseRepository; import com.appsmith.server.repositories.CustomNewActionRepository; +import org.springframework.data.mongodb.repository.Meta; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; public interface NewActionRepositoryCE extends BaseRepository, CustomNewActionRepository { + @Meta(cursorBatchSize = 10000) Flux findByApplicationId(String applicationId); + @Meta(cursorBatchSize = 10000) + Flux findAllByIdIn(Iterable ids); + Mono countByDeletedAtNull(); } 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 7269060239..000b4bf90c 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 @@ -50,6 +50,7 @@ import com.appsmith.server.solutions.ApplicationPermission; import com.appsmith.server.solutions.PagePermission; import com.appsmith.server.solutions.WorkspacePermission; import com.google.common.base.Strings; +import com.mongodb.bulk.BulkWriteResult; import com.mongodb.client.result.UpdateResult; import jakarta.annotation.Nullable; import lombok.RequiredArgsConstructor; @@ -1134,7 +1135,7 @@ public class ApplicationPageServiceCEImpl implements ApplicationPageServiceCE { if (isPublishedManually) { application.setLastDeployedAt(Instant.now()); } - Mono publishPagesMono = + Mono> publishPagesMono = newPageService.publishPages(editedPageIds, pagePermission.getEditPermission()); // Archive the deleted pages and save the application changes and then return the pages so that @@ -1144,7 +1145,7 @@ public class ApplicationPageServiceCEImpl implements ApplicationPageServiceCE { }) .cache(); // caching as we'll need this to send analytics attributes after publishing the app - Mono publishActionsMono = + Mono> publishActionsMono = newActionService.publishActions(applicationId, actionPermission.getEditPermission()); // this is a map of pluginType to count of actions for that pluginType, required for analytics diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCE.java index 9bf8e71f8e..733b08c21d 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCE.java @@ -14,7 +14,7 @@ import com.appsmith.server.dtos.ce.ImportActionResultDTO; import com.appsmith.server.dtos.ce.ImportedActionAndCollectionMapsDTO; import com.appsmith.server.helpers.ce.ImportApplicationPermissionProvider; import com.appsmith.server.services.CrudService; -import com.mongodb.client.result.UpdateResult; +import com.mongodb.bulk.BulkWriteResult; import org.springframework.data.domain.Sort; import org.springframework.util.MultiValueMap; import reactor.core.publisher.Flux; @@ -134,7 +134,7 @@ public interface NewActionServiceCE extends CrudService { ImportActionCollectionResultDTO importActionCollectionResultDTO, ImportActionResultDTO importActionResultDTO); - Mono publishActions(String applicationId, AclPermission permission); + Mono> publishActions(String applicationId, AclPermission permission); Flux countActionsByPluginType(String applicationId); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java index 3088b833a0..276d2ce24b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java @@ -53,7 +53,7 @@ import com.appsmith.server.solutions.ApplicationPermission; import com.appsmith.server.solutions.DatasourcePermission; import com.appsmith.server.solutions.PagePermission; import com.appsmith.server.solutions.PolicySolution; -import com.mongodb.client.result.UpdateResult; +import com.mongodb.bulk.BulkWriteResult; import io.micrometer.observation.ObservationRegistry; import jakarta.validation.Validator; import lombok.extern.slf4j.Slf4j; @@ -663,7 +663,7 @@ public class NewActionServiceCEImpl extends BaseService findAllById(Iterable id) { - return repository.findAllById(id).flatMap(this::sanitizeAction); + return repository.findAllByIdIn(id).flatMap(this::sanitizeAction); } @Override @@ -1907,7 +1907,7 @@ public class NewActionServiceCEImpl extends BaseService { // Update collectionId and defaultCollectionIds in actionDTOs ActionDTO unpublishedAction = newAction.getUnpublishedAction(); @@ -1965,7 +1965,7 @@ public class NewActionServiceCEImpl extends BaseService publishActions(String applicationId, AclPermission permission) { + public Mono> publishActions(String applicationId, AclPermission permission) { // delete the actions that were deleted in edit mode return repository .archiveDeletedUnpublishedActions(applicationId, permission) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewPageServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewPageServiceCE.java index cb480d23f3..b42c05c5fd 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewPageServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewPageServiceCE.java @@ -7,7 +7,7 @@ import com.appsmith.server.domains.NewPage; import com.appsmith.server.dtos.ApplicationPagesDTO; import com.appsmith.server.dtos.PageDTO; import com.appsmith.server.services.CrudService; -import com.mongodb.client.result.UpdateResult; +import com.mongodb.bulk.BulkWriteResult; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -93,5 +93,5 @@ public interface NewPageServiceCE extends CrudService { Flux findPageSlugsByApplicationIds(List applicationIds, AclPermission aclPermission); - Mono publishPages(Collection pageIds, AclPermission permission); + Mono> publishPages(Collection pageIds, AclPermission permission); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewPageServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewPageServiceCEImpl.java index 9430d3bfa2..e6fa147a0f 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewPageServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewPageServiceCEImpl.java @@ -23,7 +23,7 @@ import com.appsmith.server.services.BaseService; import com.appsmith.server.services.UserDataService; import com.appsmith.server.solutions.ApplicationPermission; import com.appsmith.server.solutions.PagePermission; -import com.mongodb.client.result.UpdateResult; +import com.mongodb.bulk.BulkWriteResult; import jakarta.validation.Validator; import lombok.extern.slf4j.Slf4j; import net.minidev.json.JSONObject; @@ -692,7 +692,7 @@ public class NewPageServiceCEImpl extends BaseService publishPages(Collection pageIds, AclPermission permission) { + public Mono> publishPages(Collection pageIds, AclPermission permission) { return repository.publishPages(pageIds, permission); } } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/NewActionServiceUnitTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/NewActionServiceUnitTest.java index 21141119fb..36f9d9aeef 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/NewActionServiceUnitTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/NewActionServiceUnitTest.java @@ -23,7 +23,6 @@ import com.appsmith.server.solutions.ApplicationPermission; import com.appsmith.server.solutions.DatasourcePermission; import com.appsmith.server.solutions.PagePermission; import com.appsmith.server.solutions.PolicySolution; -import com.mongodb.client.result.UpdateResult; import io.micrometer.observation.ObservationRegistry; import jakarta.validation.Validator; import lombok.extern.slf4j.Slf4j; @@ -39,6 +38,8 @@ import reactor.core.publisher.Mono; import reactor.core.scheduler.Scheduler; import reactor.test.StepVerifier; +import java.util.List; + import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.anyString; @@ -199,9 +200,8 @@ public class NewActionServiceUnitTest { @Test public void testPublishActionArchivesAndPublishesActions() { String applicationId = "dummy-application-id"; - UpdateResult updateResult = Mockito.mock(UpdateResult.class); - Mockito.when(updateResult.getModifiedCount()).thenReturn(10L); - Mockito.when(updateResult.getMatchedCount()).thenReturn(5L); + List updateResult = Mockito.mock(List.class); + Mockito.when(updateResult.size()).thenReturn(10); Mockito.when(newActionRepository.archiveDeletedUnpublishedActions( applicationId, actionPermission.getEditPermission())) @@ -212,8 +212,7 @@ public class NewActionServiceUnitTest { StepVerifier.create(newActionService.publishActions(applicationId, actionPermission.getEditPermission())) .assertNext(updateResult1 -> { - assertEquals(10L, updateResult1.getModifiedCount()); - assertEquals(5L, updateResult1.getMatchedCount()); + assertEquals(10, updateResult1.size()); }) .verifyComplete(); } From fc9f03d0175f3094eaccb93d724472d339c2b639 Mon Sep 17 00:00:00 2001 From: Aman Agarwal Date: Wed, 27 Sep 2023 17:01:14 +0530 Subject: [PATCH 18/32] fix: spacing issues on action page (#27656) --- app/client/src/pages/Editor/APIEditor/CommonEditorForm.tsx | 2 +- app/client/src/pages/Editor/QueryEditor/EditorJSONtoForm.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/client/src/pages/Editor/APIEditor/CommonEditorForm.tsx b/app/client/src/pages/Editor/APIEditor/CommonEditorForm.tsx index 50260f72ba..4b893ee375 100644 --- a/app/client/src/pages/Editor/APIEditor/CommonEditorForm.tsx +++ b/app/client/src/pages/Editor/APIEditor/CommonEditorForm.tsx @@ -86,7 +86,7 @@ const Form = styled.form` const MainConfiguration = styled.div` z-index: 7; - padding-left: var(--ads-v2-spaces-7); + padding: 0 var(--ads-v2-spaces-7); .api-info-row { padding-top: var(--ads-v2-spaces-5); .ads-v2-select > .rc-select-selector { diff --git a/app/client/src/pages/Editor/QueryEditor/EditorJSONtoForm.tsx b/app/client/src/pages/Editor/QueryEditor/EditorJSONtoForm.tsx index 26c698e7d4..e852b0d936 100644 --- a/app/client/src/pages/Editor/QueryEditor/EditorJSONtoForm.tsx +++ b/app/client/src/pages/Editor/QueryEditor/EditorJSONtoForm.tsx @@ -315,7 +315,7 @@ const DocumentationButton = styled(Button)` const SidebarWrapper = styled.div<{ show: boolean }>` border-left: 1px solid var(--ads-v2-color-border); - padding: 0 var(--ads-v2-spaces-4) var(--ads-v2-spaces-4); + padding: 0 var(--ads-v2-spaces-7) var(--ads-v2-spaces-4); overflow: hidden; border-bottom: 0; display: ${(props) => (props.show ? "flex" : "none")}; From a27576f74c142fcb863db078e8b163ebc4025389 Mon Sep 17 00:00:00 2001 From: Parthvi <80334441+Parthvi12@users.noreply.github.com> Date: Sun, 1 Oct 2023 16:35:28 +0530 Subject: [PATCH 19/32] test: Cypress| Fix omnibar test (#27728) ## Description Doc link got updated which was causing test to fail, fixed it. #### How Has This Been Tested? locally --- .../e2e/Regression/ClientSide/OtherUIFeatures/Omnibar_spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/Omnibar_spec.js b/app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/Omnibar_spec.js index 528a735422..cffad0e5ef 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/Omnibar_spec.js +++ b/app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/Omnibar_spec.js @@ -20,7 +20,7 @@ describe("Omnibar functionality test cases", () => { cy.dragAndDropToCanvas(draggableWidgets.AUDIO, { x: 300, y: 500 }); deployMode.StubWindowNAssert( '//span[text()="Learn more"]', - "connect-datasource", + "connect-to-a-database", "getWorkspace", ); }); From 86ccba382a2527710ec19d6bc7c9d813616a666e Mon Sep 17 00:00:00 2001 From: sharanya-appsmith <135708039+sharanya-appsmith@users.noreply.github.com> Date: Mon, 2 Oct 2023 16:11:09 +0530 Subject: [PATCH 20/32] test: Cypress - fix (#27743) --- .../ClientSide/Widgets/Datepicker/DatePicker3_spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker3_spec.ts b/app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker3_spec.ts index 52b4da1de8..bda5ac446f 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker3_spec.ts +++ b/app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker3_spec.ts @@ -53,7 +53,7 @@ describe("Date picker widget testcases", () => { .then((labelValue) => { const formattedDate = format( new Date(dateValueSet), - "dd MMMM, yyyy", + "d MMMM, yyyy", ); expect(labelValue).to.contain(formattedDate); }); From 645263e64180f953f402cb1a7bcae7de3344e4ba Mon Sep 17 00:00:00 2001 From: NandanAnantharamu <67676905+NandanAnantharamu@users.noreply.github.com> Date: Tue, 10 Oct 2023 13:51:33 +0530 Subject: [PATCH 21/32] test: cypress - commenting a test in Tab spec (#27916) Skipping a test in this Tab regression spec --- .../e2e/Regression/ClientSide/Widgets/Tab/Tabs_2_spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/client/cypress/e2e/Regression/ClientSide/Widgets/Tab/Tabs_2_spec.ts b/app/client/cypress/e2e/Regression/ClientSide/Widgets/Tab/Tabs_2_spec.ts index eeca97b3f6..57240928a3 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/Widgets/Tab/Tabs_2_spec.ts +++ b/app/client/cypress/e2e/Regression/ClientSide/Widgets/Tab/Tabs_2_spec.ts @@ -178,7 +178,8 @@ describe("Tabs widget Tests", function () { propPane.SelectPropertiesDropDown("height", "Auto Height"); }); - it("7. Verify colors, borders and shadows", () => { + // to work on redesign of the test, commenting for now + it.skip("7. Verify colors, borders and shadows", () => { // Verify font color picker opens up propPane.MoveToTab("Style"); agHelper.GetNClick(propPane._propertyControlColorPicker("accentcolor")); From bff76ca1bf05ad998bc05c3f9011102bd0d8a5fb Mon Sep 17 00:00:00 2001 From: albinAppsmith <87797149+albinAppsmith@users.noreply.github.com> Date: Thu, 12 Oct 2023 14:46:04 +0530 Subject: [PATCH 22/32] fix: Branding hover color issue (#27964) ## Description Primary buttons when hovered, turns the background to white. This was happening due to some recent changes in the branding related APIs and API calls. #### PR fixes following issue(s) Fixes https://github.com/appsmithorg/appsmith/issues/27940 #### Media #### Type of change - Bug fix (non-breaking change which fixes an issue) ## Testing #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [ ] Manual - [ ] JUnit - [ ] Jest - [ ] Cypress > > #### Test Plan > Add Testsmith test cases links that relate to this PR > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --------- Co-authored-by: Pawan Kumar (cherry picked from commit c218f9228fb18e0e866ae2447cefc0ef27d02ed3) --- app/client/src/ce/sagas/tenantSagas.tsx | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/app/client/src/ce/sagas/tenantSagas.tsx b/app/client/src/ce/sagas/tenantSagas.tsx index c197e3df13..4abb084117 100644 --- a/app/client/src/ce/sagas/tenantSagas.tsx +++ b/app/client/src/ce/sagas/tenantSagas.tsx @@ -22,12 +22,22 @@ export function* fetchCurrentTenantConfigSaga() { ); const isValidResponse: boolean = yield validateResponse(response); if (isValidResponse) { - const data: any = response.data; + const payload: any = response.data; yield put({ type: ReduxActionTypes.FETCH_CURRENT_TENANT_CONFIG_SUCCESS, - payload: data, + payload: { + ...payload, + tenantConfiguration: { + ...CE_defaultBrandingConfig, + ...payload.tenantConfiguration, + brandColors: { + ...CE_defaultBrandingConfig.brandColors, + ...payload.tenantConfiguration.brandColors, + }, + }, + }, }); - AnalyticsUtil.initInstanceId(data.instanceId); + AnalyticsUtil.initInstanceId(payload.instanceId); } } catch (error) { yield put({ From 83970b9770baabad8cbad832907350ec25b34160 Mon Sep 17 00:00:00 2001 From: Dipyaman Biswas Date: Wed, 11 Oct 2023 23:19:19 +0530 Subject: [PATCH 23/32] feat: make features call a blocking API call for page load (#27974) ## Description Make features call a blocking API call for page load #### PR fixes following issue(s) Fixes #27973 #### Media #### Type of change > Please delete options that are not relevant. - Chore (housekeeping or task changes that don't impact user perception) ## Testing > #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [x] Manual - [ ] JUnit - [ ] Jest - [ ] Cypress > > #### Test Plan > Add Testsmith test cases links that relate to this PR > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed (cherry picked from commit be7f6674f755b10a780e64280d4da1526482a021) --- app/client/src/ce/AppRouter.tsx | 16 ++++++++++++---- .../src/reducers/uiReducers/usersReducer.ts | 12 ++++++++++++ app/client/src/selectors/usersSelectors.tsx | 8 ++++++++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/app/client/src/ce/AppRouter.tsx b/app/client/src/ce/AppRouter.tsx index cbfd14f1ef..1d95ef88c6 100644 --- a/app/client/src/ce/AppRouter.tsx +++ b/app/client/src/ce/AppRouter.tsx @@ -45,7 +45,10 @@ import * as Sentry from "@sentry/react"; import { getSafeCrash, getSafeCrashCode } from "selectors/errorSelectors"; import UserProfile from "pages/UserProfile"; import { getCurrentUser } from "actions/authActions"; -import { getCurrentUserLoading } from "selectors/usersSelectors"; +import { + getCurrentUserLoading, + getFeatureFlagsFetching, +} from "selectors/usersSelectors"; import Setup from "pages/setup"; import SettingsLoader from "pages/AdminSettings/loader"; import SignupSuccess from "pages/setup/SignupSuccess"; @@ -153,6 +156,7 @@ function AppRouter(props: { } = props; const tenantIsLoading = useSelector(isTenantLoading); const currentUserIsLoading = useSelector(getCurrentUserLoading); + const featuresIsLoading = useSelector(getFeatureFlagsFetching); useEffect(() => { getCurrentUser(); @@ -166,7 +170,11 @@ function AppRouter(props: { // hide the top loader once the tenant is loaded useEffect(() => { - if (tenantIsLoading === false && currentUserIsLoading === false) { + if ( + tenantIsLoading === false && + currentUserIsLoading === false && + featuresIsLoading === false + ) { const loader = document.getElementById("loader") as HTMLDivElement; if (loader) { @@ -177,9 +185,9 @@ function AppRouter(props: { }); } } - }, [tenantIsLoading, currentUserIsLoading]); + }, [tenantIsLoading, currentUserIsLoading, featuresIsLoading]); - if (tenantIsLoading || currentUserIsLoading) return null; + if (tenantIsLoading || currentUserIsLoading || featuresIsLoading) return null; return ( diff --git a/app/client/src/reducers/uiReducers/usersReducer.ts b/app/client/src/reducers/uiReducers/usersReducer.ts index cfef428966..52c0b5f96c 100644 --- a/app/client/src/reducers/uiReducers/usersReducer.ts +++ b/app/client/src/reducers/uiReducers/usersReducer.ts @@ -24,6 +24,7 @@ const initialState: UsersReduxState = { featureFlag: { data: DEFAULT_FEATURE_FLAG_VALUE, isFetched: false, + isFetching: true, }, productAlert: { config: { @@ -173,6 +174,14 @@ const usersReducer = createReducer(initialState, { photoId: action.payload.photoId, }, }), + [ReduxActionTypes.FETCH_FEATURE_FLAGS_INIT]: (state: UsersReduxState) => ({ + ...state, + featureFlag: { + ...state.featureFlag, + isFetched: false, + isFetching: true, + }, + }), [ReduxActionTypes.FETCH_FEATURE_FLAGS_SUCCESS]: ( state: UsersReduxState, action: ReduxAction, @@ -181,6 +190,7 @@ const usersReducer = createReducer(initialState, { featureFlag: { data: action.payload, isFetched: true, + isFetching: false, }, }), [ReduxActionErrorTypes.FETCH_FEATURE_FLAGS_ERROR]: ( @@ -190,6 +200,7 @@ const usersReducer = createReducer(initialState, { featureFlag: { data: {}, isFetched: true, + isFetching: false, }, }), [ReduxActionTypes.FETCH_PRODUCT_ALERT_SUCCESS]: ( @@ -252,6 +263,7 @@ export interface UsersReduxState { featureFlag: { isFetched: boolean; data: FeatureFlags; + isFetching: boolean; }; productAlert: ProductAlertState; } diff --git a/app/client/src/selectors/usersSelectors.tsx b/app/client/src/selectors/usersSelectors.tsx index 53fcc78c02..e693ec0aa1 100644 --- a/app/client/src/selectors/usersSelectors.tsx +++ b/app/client/src/selectors/usersSelectors.tsx @@ -5,16 +5,24 @@ import { ANONYMOUS_USERNAME } from "constants/userConstants"; export const getCurrentUser = (state: AppState): User | undefined => state.ui?.users?.currentUser; + export const getCurrentUserLoading = (state: AppState): boolean => state.ui.users.loadingStates.fetchingUser; + export const getUserAuthError = (state: AppState): string => state.ui.users.error; + export const getUsers = (state: AppState): User[] => state.ui.users.users; + export const getProppanePreference = ( state: AppState, ): PropertyPanePositionConfig | undefined => state.ui.users.propPanePreferences; + export const getFeatureFlagsFetched = (state: AppState) => state.ui.users.featureFlag.isFetched; +export const getFeatureFlagsFetching = (state: AppState) => + state.ui.users.featureFlag.isFetching; + export const getIsUserLoggedIn = (state: AppState): boolean => state.ui.users.currentUser?.email !== ANONYMOUS_USERNAME; From 6abd5e2c0a8c57cfa4e8a001b3e8ba26472f6f47 Mon Sep 17 00:00:00 2001 From: Valera Melnikov Date: Fri, 13 Oct 2023 12:08:01 +0300 Subject: [PATCH 24/32] fix: auto label position (#28022) ## Description Fix auto label position Fixes #27975 #### Media https://github.com/appsmithorg/appsmith/assets/11555074/bf6e4c8c-97db-4eab-8986-b4cd2de5bd0a #### Type of change - Bug fix (non-breaking change which fixes an issue) ## Testing > #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [x] Manual - [ ] JUnit - [ ] Jest - [ ] Cypress > > #### Test Plan > Add Testsmith test cases links that relate to this PR > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed (cherry picked from commit b42caa030823f47625098305565b3c68d4725895) --- app/client/src/widgets/WidgetUtils.test.ts | 7 +++++++ app/client/src/widgets/WidgetUtils.ts | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/app/client/src/widgets/WidgetUtils.test.ts b/app/client/src/widgets/WidgetUtils.test.ts index d6a90203a8..aeec7138e3 100644 --- a/app/client/src/widgets/WidgetUtils.test.ts +++ b/app/client/src/widgets/WidgetUtils.test.ts @@ -26,6 +26,7 @@ import { isAutoHeightEnabledForWidgetWithLimits, getWidgetMaxAutoHeight, getWidgetMinAutoHeight, + isCompactMode, } from "./WidgetUtils"; import { getCustomTextColor, @@ -696,4 +697,10 @@ describe("Should Update Widget Height Automatically?", () => { const result = shouldUpdateWidgetHeightAutomatically(input, props); expect(result).toStrictEqual(expected); }); + it("should return correct value for isCompactMode", () => { + const compactHeight = 40; + const unCompactHeight = 41; + expect(isCompactMode(compactHeight)).toBeTruthy(); + expect(isCompactMode(unCompactHeight)).toBeFalsy(); + }); }); diff --git a/app/client/src/widgets/WidgetUtils.ts b/app/client/src/widgets/WidgetUtils.ts index 31761b438a..8f60f05e20 100644 --- a/app/client/src/widgets/WidgetUtils.ts +++ b/app/client/src/widgets/WidgetUtils.ts @@ -921,7 +921,7 @@ const findReactInstanceProps = (domElement: any) => { export function isCompactMode(componentHeight: number) { return ( - componentHeight < + componentHeight <= COMPACT_MODE_MIN_ROWS * GridDefaults.DEFAULT_GRID_ROW_HEIGHT ); } From ee37b5272ad1e46b1dc9d77033e2cf450b35bcae Mon Sep 17 00:00:00 2001 From: arunvjn <32433245+arunvjn@users.noreply.github.com> Date: Mon, 16 Oct 2023 13:32:27 +0530 Subject: [PATCH 25/32] fix: Race condition in JS object mutation (#28083) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes race condition in JS Object mutation where evaluation overrides the variables state. Root Cause: Execution context is shared between every evaluation request and trigger execution request. A trigger execution request could be paused when an asynchronous task is awaited. In the mean time, worker might pick up the task to perform and evaluation. Evaluation would end up replacing the entities in the execution context, there by resetting the actions performed by the trigger execution before it was paused. What the fix does? We are now caching the JS object for reuse which means that every execution/evaluation request reuses the same JS Object as long the JS Object isn't modified by a user action. Fixes #27978 > > > A video or a GIF is preferred. when using Loom, don’t embed because it looks like it’s a GIF. instead, just link to the video > > - Bug fix (non-breaking change which fixes an issue) > > > - [x] Manual - [x] Jest - [x] Cypress > > > Add Testsmith test cases links that relate to this PR > > > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag - [x] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [x] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [x] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [x] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed (cherry picked from commit 1afd223e10a61db916c3c91ddd73daf10ada02d8) --- .../JSObject/JSObjectMutation_spec.ts | 48 ++++++++++++++ .../src/ce/actions/evaluationActions.ts | 2 - .../workers/Evaluation/JSObject/Collection.ts | 62 +++++++++++++++++++ .../Evaluation/JSObject/JSVariableFactory.ts | 52 ---------------- .../Evaluation/JSObject/JSVariableUpdates.ts | 13 +--- .../__test__/JSVariableFactory.test.ts | 23 +++++-- .../src/workers/Evaluation/JSObject/index.ts | 1 + .../Evaluation/fns/utils/TriggerEmitter.ts | 6 +- .../workers/Evaluation/getEntityForContext.ts | 4 +- 9 files changed, 136 insertions(+), 75 deletions(-) delete mode 100644 app/client/src/workers/Evaluation/JSObject/JSVariableFactory.ts diff --git a/app/client/cypress/e2e/Regression/ClientSide/JSObject/JSObjectMutation_spec.ts b/app/client/cypress/e2e/Regression/ClientSide/JSObject/JSObjectMutation_spec.ts index 3488d7a6e5..52b60ccbf0 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/JSObject/JSObjectMutation_spec.ts +++ b/app/client/cypress/e2e/Regression/ClientSide/JSObject/JSObjectMutation_spec.ts @@ -90,4 +90,52 @@ describe("JSObject testing", () => { expect($label).contains("[]"); }); }); + + it("6. Bug 27978 Check assignment should not get overridden by evaluation", () => { + _.entityExplorer.DragNDropWidget(_.draggableWidgets.TEXT, 400, 400); + _.propPane.TypeTextIntoField( + "Text", + `{{JSObject1.data.length ? 'id-' + JSObject1.data[0].id : 'Not Set' }}`, + true, + false, + ); + _.entityExplorer.NavigateToSwitcher("Explorer"); + _.apiPage.CreateAndFillApi( + _.dataManager.dsValues[_.dataManager.defaultEnviorment].mockApiUrl, + ); + const JS_OBJECT_BODY = `export default { + data: [], + async getData() { + await Api1.run() + return Api1.data + }, + async myFun1() { + this.data = await this.getData(); + console.log(this.data); + }, + async myFun2() { + const data = await this.getData(); + data.push({ name: "test123" }) + this.data = data; + console.log(this.data); + }, + }`; + _.jsEditor.CreateJSObject(JS_OBJECT_BODY, { + paste: true, + completeReplace: true, + toRun: false, + shouldCreateNewJSObj: true, + }); + _.jsEditor.SelectFunctionDropdown("myFun1"); + _.jsEditor.RunJSObj(); + _.entityExplorer.SelectEntityByName("Text2", "Widgets"); + _.agHelper.AssertContains("id-1"); + cy.reload(); + _.agHelper.AssertContains("Not Set"); + _.entityExplorer.SelectEntityByName("JSObject1", "Queries/JS"); + _.jsEditor.SelectFunctionDropdown("myFun2"); + _.jsEditor.RunJSObj(); + _.entityExplorer.SelectEntityByName("Text2", "Widgets"); + _.agHelper.AssertContains("id-1"); + }); }); diff --git a/app/client/src/ce/actions/evaluationActions.ts b/app/client/src/ce/actions/evaluationActions.ts index 2504d7b1be..8bb299de54 100644 --- a/app/client/src/ce/actions/evaluationActions.ts +++ b/app/client/src/ce/actions/evaluationActions.ts @@ -66,8 +66,6 @@ export const EVALUATE_REDUX_ACTIONS = [ ReduxActionTypes.MOVE_ACTION_SUCCESS, ReduxActionTypes.RUN_ACTION_SUCCESS, ReduxActionErrorTypes.RUN_ACTION_ERROR, - ReduxActionTypes.EXECUTE_PLUGIN_ACTION_SUCCESS, - ReduxActionErrorTypes.EXECUTE_PLUGIN_ACTION_ERROR, ReduxActionTypes.CLEAR_ACTION_RESPONSE, // JS Actions ReduxActionTypes.CREATE_JS_ACTION_SUCCESS, diff --git a/app/client/src/workers/Evaluation/JSObject/Collection.ts b/app/client/src/workers/Evaluation/JSObject/Collection.ts index df62aedeb5..d2874a2f0b 100644 --- a/app/client/src/workers/Evaluation/JSObject/Collection.ts +++ b/app/client/src/workers/Evaluation/JSObject/Collection.ts @@ -1,6 +1,20 @@ import { getEntityNameAndPropertyPath } from "@appsmith/workers/Evaluation/evaluationUtils"; import { klona } from "klona/full"; import { get, set } from "lodash"; +import TriggerEmitter, { BatchKey } from "../fns/utils/TriggerEmitter"; +import ExecutionMetaData from "../fns/utils/ExecutionMetaData"; +import type { JSActionEntity } from "@appsmith/entities/DataTree/types"; + +export enum PatchType { + "SET" = "SET", + "GET" = "GET", +} + +export interface Patch { + path: string; + method: PatchType; + value?: unknown; +} export type VariableState = Record>; @@ -61,6 +75,7 @@ export default class JSObjectCollection { const newVarState = { ...this.variableState[entityName] }; newVarState[propertyPath] = variableValue; this.variableState[entityName] = newVarState; + JSObjectCollection.clearCachedVariablesForEvaluationContext(entityName); } static getVariableState( @@ -77,6 +92,53 @@ export default class JSObjectCollection { delete jsObject[propertyPath]; } + /**Map */ + static cachedJSVariablesByEntityName: Record = {}; + + /**Computes Map with getters & setters to track JS mutations + * We cache and reuse this map. We recreate only when the JSObject's content changes or when any of the variables + * gets evaluated + */ + static getVariablesForEvaluationContext(entityName: string) { + if (JSObjectCollection.cachedJSVariablesByEntityName[entityName]) + return JSObjectCollection.cachedJSVariablesByEntityName[entityName]; + const varState = JSObjectCollection.getVariableState(entityName); + const variables = Object.entries(varState); + const newJSObject = {} as JSActionEntity; + + for (const [varName, varValue] of variables) { + let variable = varValue; + Object.defineProperty(newJSObject, varName, { + enumerable: true, + configurable: true, + get() { + TriggerEmitter.emit(BatchKey.process_js_variable_updates, { + path: `${entityName}.${varName}`, + method: PatchType.GET, + }); + return variable; + }, + set(value) { + TriggerEmitter.emit(BatchKey.process_js_variable_updates, { + path: `${entityName}.${varName}`, + method: PatchType.SET, + value, + }); + variable = value; + }, + }); + } + ExecutionMetaData.setExecutionMetaData({ + enableJSVarUpdateTracking: true, + }); + JSObjectCollection.cachedJSVariablesByEntityName[entityName] = newJSObject; + return JSObjectCollection.cachedJSVariablesByEntityName[entityName]; + } + + static clearCachedVariablesForEvaluationContext(entityName: string) { + delete JSObjectCollection.cachedJSVariablesByEntityName[entityName]; + } + static clear() { this.variableState = {}; this.unEvalState = {}; diff --git a/app/client/src/workers/Evaluation/JSObject/JSVariableFactory.ts b/app/client/src/workers/Evaluation/JSObject/JSVariableFactory.ts deleted file mode 100644 index 1922f98a63..0000000000 --- a/app/client/src/workers/Evaluation/JSObject/JSVariableFactory.ts +++ /dev/null @@ -1,52 +0,0 @@ -import { PatchType } from "./JSVariableUpdates"; -import ExecutionMetaData from "../fns/utils/ExecutionMetaData"; -import type { JSActionEntity } from "@appsmith/entities/DataTree/types"; -import TriggerEmitter, { BatchKey } from "../fns/utils/TriggerEmitter"; - -class JSFactory { - static create( - jsObjectName: string, - varState: Record = {}, - ): JSActionEntity { - const newJSObject: any = {}; - - const variables = Object.entries(varState); - - for (const [varName, varValue] of variables) { - let variable = varValue; - Object.defineProperty(newJSObject, varName, { - enumerable: true, - configurable: true, - get() { - TriggerEmitter.emit(BatchKey.process_js_variable_updates, { - path: `${jsObjectName}.${varName}`, - method: PatchType.GET, - }); - return variable; - }, - set(value) { - TriggerEmitter.emit(BatchKey.process_js_variable_updates, { - path: `${jsObjectName}.${varName}`, - method: PatchType.SET, - value, - }); - variable = value; - }, - }); - - ExecutionMetaData.setExecutionMetaData({ - enableJSVarUpdateTracking: false, - }); - - newJSObject[varName] = varValue; - - ExecutionMetaData.setExecutionMetaData({ - enableJSVarUpdateTracking: true, - }); - } - - return newJSObject; - } -} - -export default JSFactory; diff --git a/app/client/src/workers/Evaluation/JSObject/JSVariableUpdates.ts b/app/client/src/workers/Evaluation/JSObject/JSVariableUpdates.ts index a7b117d1f0..3d2638abba 100644 --- a/app/client/src/workers/Evaluation/JSObject/JSVariableUpdates.ts +++ b/app/client/src/workers/Evaluation/JSObject/JSVariableUpdates.ts @@ -5,17 +5,8 @@ import { evalTreeWithChanges } from "../evalTreeWithChanges"; import { get } from "lodash"; import { isJSObjectVariable } from "./utils"; import isDeepEqualES6 from "fast-deep-equal/es6"; - -export enum PatchType { - "SET" = "SET", - "GET" = "GET", -} - -export type Patch = { - path: string; - method: PatchType; - value?: unknown; -}; +import type { Patch } from "./Collection"; +import { PatchType } from "./Collection"; export type UpdatedPathsMap = Record; diff --git a/app/client/src/workers/Evaluation/JSObject/__test__/JSVariableFactory.test.ts b/app/client/src/workers/Evaluation/JSObject/__test__/JSVariableFactory.test.ts index f7e0355b70..80191e1c6d 100644 --- a/app/client/src/workers/Evaluation/JSObject/__test__/JSVariableFactory.test.ts +++ b/app/client/src/workers/Evaluation/JSObject/__test__/JSVariableFactory.test.ts @@ -1,9 +1,9 @@ -import JSFactory from "../JSVariableFactory"; import ExecutionMetaData from "workers/Evaluation/fns/utils/ExecutionMetaData"; import type { JSActionEntity } from "@appsmith/entities/DataTree/types"; import TriggerEmitter, { jsVariableUpdatesHandlerWrapper, } from "workers/Evaluation/fns/utils/TriggerEmitter"; +import JSObjectCollection from "../Collection"; const applyJSVariableUpdatesToEvalTreeMock = jest.fn(); jest.mock("../JSVariableUpdates.ts", () => ({ @@ -36,7 +36,12 @@ describe("JSVariableFactory", () => { weakSet: new WeakSet(), } as unknown as JSActionEntity; - const proxiedJSObject = JSFactory.create("JSObject1", jsObject); + Object.entries(jsObject).forEach(([k, v]) => + JSObjectCollection.setVariableValue(v, `JSObject1.${k}`), + ); + + const proxiedJSObject = + JSObjectCollection.getVariablesForEvaluationContext("JSObject1"); ExecutionMetaData.setExecutionMetaData({ enableJSVarUpdateTracking: true, @@ -96,7 +101,12 @@ describe("JSVariableFactory", () => { weakSet: new WeakSet(), } as unknown as JSActionEntity; - const proxiedJSObject = JSFactory.create("JSObject1", jsObject); + Object.entries(jsObject).forEach(([k, v]) => + JSObjectCollection.setVariableValue(v, `JSObject1.${k}`), + ); + + const proxiedJSObject = + JSObjectCollection.getVariablesForEvaluationContext("JSObject1"); ExecutionMetaData.setExecutionMetaData({ enableJSVarUpdateTracking: false, @@ -125,7 +135,12 @@ describe("JSVariableFactory", () => { weakSet: new WeakSet(), } as unknown as JSActionEntity; - const proxiedJSObject = JSFactory.create("JSObject1", jsObject); + Object.entries(jsObject).forEach(([k, v]) => + JSObjectCollection.setVariableValue(v, `JSObject1.${k}`), + ); + + const proxiedJSObject = + JSObjectCollection.getVariablesForEvaluationContext("JSObject1"); ExecutionMetaData.setExecutionMetaData({ enableJSVarUpdateTracking: true, diff --git a/app/client/src/workers/Evaluation/JSObject/index.ts b/app/client/src/workers/Evaluation/JSObject/index.ts index 304b28009f..b4d5bceb4c 100644 --- a/app/client/src/workers/Evaluation/JSObject/index.ts +++ b/app/client/src/workers/Evaluation/JSObject/index.ts @@ -95,6 +95,7 @@ export function saveResolvedFunctionsAndJSUpdates( try { JSObjectCollection.deleteResolvedFunction(entityName); JSObjectCollection.deleteUnEvalState(entityName); + JSObjectCollection.clearCachedVariablesForEvaluationContext(entityName); const parseStartTime = performance.now(); const { parsedObject, success } = parseJSObject(entity.body); diff --git a/app/client/src/workers/Evaluation/fns/utils/TriggerEmitter.ts b/app/client/src/workers/Evaluation/fns/utils/TriggerEmitter.ts index 7d50754db0..95ba119c36 100644 --- a/app/client/src/workers/Evaluation/fns/utils/TriggerEmitter.ts +++ b/app/client/src/workers/Evaluation/fns/utils/TriggerEmitter.ts @@ -1,10 +1,7 @@ import { EventEmitter } from "events"; import { MAIN_THREAD_ACTION } from "@appsmith/workers/Evaluation/evalWorkerActions"; import { WorkerMessenger } from "workers/Evaluation/fns/utils/Messenger"; -import type { - Patch, - UpdatedPathsMap, -} from "workers/Evaluation/JSObject/JSVariableUpdates"; +import type { UpdatedPathsMap } from "workers/Evaluation/JSObject/JSVariableUpdates"; import { applyJSVariableUpdatesToEvalTree } from "workers/Evaluation/JSObject/JSVariableUpdates"; import ExecutionMetaData from "./ExecutionMetaData"; import { get } from "lodash"; @@ -18,6 +15,7 @@ import type { import type { UpdateActionProps } from "workers/Evaluation/handlers/updateActionData"; import { handleActionsDataUpdate } from "workers/Evaluation/handlers/updateActionData"; import { getEntityNameAndPropertyPath } from "@appsmith/workers/Evaluation/evaluationUtils"; +import type { Patch } from "workers/Evaluation/JSObject/Collection"; const _internalSetTimeout = self.setTimeout; const _internalClearTimeout = self.clearTimeout; diff --git a/app/client/src/workers/Evaluation/getEntityForContext.ts b/app/client/src/workers/Evaluation/getEntityForContext.ts index 06dd01c605..592d098b5a 100644 --- a/app/client/src/workers/Evaluation/getEntityForContext.ts +++ b/app/client/src/workers/Evaluation/getEntityForContext.ts @@ -4,7 +4,6 @@ import type { } from "@appsmith/entities/DataTree/types"; import { ENTITY_TYPE_VALUE } from "entities/DataTree/dataTreeFactory"; import JSObjectCollection from "./JSObject/Collection"; -import JSFactory from "./JSObject/JSVariableFactory"; import { jsObjectFunctionFactory } from "./fns/utils/jsObjectFnFactory"; import { isObject } from "lodash"; @@ -55,7 +54,8 @@ export function getEntityForEvalContext( return Object.assign({}, jsObject, fns); } - jsObjectForEval = JSFactory.create(entityName, jsObjectForEval); + jsObjectForEval = + JSObjectCollection.getVariablesForEvaluationContext(entityName); return Object.assign(jsObjectForEval, fns); } } From 1baa693f74d804b1229d85a4f8e5a9ed679813bc Mon Sep 17 00:00:00 2001 From: arunvjn <32433245+arunvjn@users.noreply.github.com> Date: Mon, 16 Oct 2023 13:32:27 +0530 Subject: [PATCH 26/32] fix: Race condition in JS object mutation (#28083) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Fixes race condition in JS Object mutation where evaluation overrides the variables state. Root Cause: Execution context is shared between every evaluation request and trigger execution request. A trigger execution request could be paused when an asynchronous task is awaited. In the mean time, worker might pick up the task to perform and evaluation. Evaluation would end up replacing the entities in the execution context, there by resetting the actions performed by the trigger execution before it was paused. What the fix does? We are now caching the JS object for reuse which means that every execution/evaluation request reuses the same JS Object as long the JS Object isn't modified by a user action. #### PR fixes following issue(s) Fixes #27978 > > #### Media > A video or a GIF is preferred. when using Loom, don’t embed because it looks like it’s a GIF. instead, just link to the video > > #### Type of change - Bug fix (non-breaking change which fixes an issue) > > ## Testing > #### How Has This Been Tested? - [x] Manual - [x] Jest - [x] Cypress > > #### Test Plan > Add Testsmith test cases links that relate to this PR > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [x] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [x] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [x] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [x] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed (cherry picked from commit 1afd223e10a61db916c3c91ddd73daf10ada02d8) --- .../JSObject/JSObjectMutation_spec.ts | 48 ++++++++++++++ .../src/ce/actions/evaluationActions.ts | 2 - .../workers/Evaluation/JSObject/Collection.ts | 62 +++++++++++++++++++ .../Evaluation/JSObject/JSVariableFactory.ts | 52 ---------------- .../Evaluation/JSObject/JSVariableUpdates.ts | 13 +--- .../__test__/JSVariableFactory.test.ts | 23 +++++-- .../src/workers/Evaluation/JSObject/index.ts | 1 + .../Evaluation/fns/utils/TriggerEmitter.ts | 6 +- .../workers/Evaluation/getEntityForContext.ts | 4 +- 9 files changed, 136 insertions(+), 75 deletions(-) delete mode 100644 app/client/src/workers/Evaluation/JSObject/JSVariableFactory.ts diff --git a/app/client/cypress/e2e/Regression/ClientSide/JSObject/JSObjectMutation_spec.ts b/app/client/cypress/e2e/Regression/ClientSide/JSObject/JSObjectMutation_spec.ts index 3488d7a6e5..52b60ccbf0 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/JSObject/JSObjectMutation_spec.ts +++ b/app/client/cypress/e2e/Regression/ClientSide/JSObject/JSObjectMutation_spec.ts @@ -90,4 +90,52 @@ describe("JSObject testing", () => { expect($label).contains("[]"); }); }); + + it("6. Bug 27978 Check assignment should not get overridden by evaluation", () => { + _.entityExplorer.DragNDropWidget(_.draggableWidgets.TEXT, 400, 400); + _.propPane.TypeTextIntoField( + "Text", + `{{JSObject1.data.length ? 'id-' + JSObject1.data[0].id : 'Not Set' }}`, + true, + false, + ); + _.entityExplorer.NavigateToSwitcher("Explorer"); + _.apiPage.CreateAndFillApi( + _.dataManager.dsValues[_.dataManager.defaultEnviorment].mockApiUrl, + ); + const JS_OBJECT_BODY = `export default { + data: [], + async getData() { + await Api1.run() + return Api1.data + }, + async myFun1() { + this.data = await this.getData(); + console.log(this.data); + }, + async myFun2() { + const data = await this.getData(); + data.push({ name: "test123" }) + this.data = data; + console.log(this.data); + }, + }`; + _.jsEditor.CreateJSObject(JS_OBJECT_BODY, { + paste: true, + completeReplace: true, + toRun: false, + shouldCreateNewJSObj: true, + }); + _.jsEditor.SelectFunctionDropdown("myFun1"); + _.jsEditor.RunJSObj(); + _.entityExplorer.SelectEntityByName("Text2", "Widgets"); + _.agHelper.AssertContains("id-1"); + cy.reload(); + _.agHelper.AssertContains("Not Set"); + _.entityExplorer.SelectEntityByName("JSObject1", "Queries/JS"); + _.jsEditor.SelectFunctionDropdown("myFun2"); + _.jsEditor.RunJSObj(); + _.entityExplorer.SelectEntityByName("Text2", "Widgets"); + _.agHelper.AssertContains("id-1"); + }); }); diff --git a/app/client/src/ce/actions/evaluationActions.ts b/app/client/src/ce/actions/evaluationActions.ts index 2504d7b1be..8bb299de54 100644 --- a/app/client/src/ce/actions/evaluationActions.ts +++ b/app/client/src/ce/actions/evaluationActions.ts @@ -66,8 +66,6 @@ export const EVALUATE_REDUX_ACTIONS = [ ReduxActionTypes.MOVE_ACTION_SUCCESS, ReduxActionTypes.RUN_ACTION_SUCCESS, ReduxActionErrorTypes.RUN_ACTION_ERROR, - ReduxActionTypes.EXECUTE_PLUGIN_ACTION_SUCCESS, - ReduxActionErrorTypes.EXECUTE_PLUGIN_ACTION_ERROR, ReduxActionTypes.CLEAR_ACTION_RESPONSE, // JS Actions ReduxActionTypes.CREATE_JS_ACTION_SUCCESS, diff --git a/app/client/src/workers/Evaluation/JSObject/Collection.ts b/app/client/src/workers/Evaluation/JSObject/Collection.ts index df62aedeb5..d2874a2f0b 100644 --- a/app/client/src/workers/Evaluation/JSObject/Collection.ts +++ b/app/client/src/workers/Evaluation/JSObject/Collection.ts @@ -1,6 +1,20 @@ import { getEntityNameAndPropertyPath } from "@appsmith/workers/Evaluation/evaluationUtils"; import { klona } from "klona/full"; import { get, set } from "lodash"; +import TriggerEmitter, { BatchKey } from "../fns/utils/TriggerEmitter"; +import ExecutionMetaData from "../fns/utils/ExecutionMetaData"; +import type { JSActionEntity } from "@appsmith/entities/DataTree/types"; + +export enum PatchType { + "SET" = "SET", + "GET" = "GET", +} + +export interface Patch { + path: string; + method: PatchType; + value?: unknown; +} export type VariableState = Record>; @@ -61,6 +75,7 @@ export default class JSObjectCollection { const newVarState = { ...this.variableState[entityName] }; newVarState[propertyPath] = variableValue; this.variableState[entityName] = newVarState; + JSObjectCollection.clearCachedVariablesForEvaluationContext(entityName); } static getVariableState( @@ -77,6 +92,53 @@ export default class JSObjectCollection { delete jsObject[propertyPath]; } + /**Map */ + static cachedJSVariablesByEntityName: Record = {}; + + /**Computes Map with getters & setters to track JS mutations + * We cache and reuse this map. We recreate only when the JSObject's content changes or when any of the variables + * gets evaluated + */ + static getVariablesForEvaluationContext(entityName: string) { + if (JSObjectCollection.cachedJSVariablesByEntityName[entityName]) + return JSObjectCollection.cachedJSVariablesByEntityName[entityName]; + const varState = JSObjectCollection.getVariableState(entityName); + const variables = Object.entries(varState); + const newJSObject = {} as JSActionEntity; + + for (const [varName, varValue] of variables) { + let variable = varValue; + Object.defineProperty(newJSObject, varName, { + enumerable: true, + configurable: true, + get() { + TriggerEmitter.emit(BatchKey.process_js_variable_updates, { + path: `${entityName}.${varName}`, + method: PatchType.GET, + }); + return variable; + }, + set(value) { + TriggerEmitter.emit(BatchKey.process_js_variable_updates, { + path: `${entityName}.${varName}`, + method: PatchType.SET, + value, + }); + variable = value; + }, + }); + } + ExecutionMetaData.setExecutionMetaData({ + enableJSVarUpdateTracking: true, + }); + JSObjectCollection.cachedJSVariablesByEntityName[entityName] = newJSObject; + return JSObjectCollection.cachedJSVariablesByEntityName[entityName]; + } + + static clearCachedVariablesForEvaluationContext(entityName: string) { + delete JSObjectCollection.cachedJSVariablesByEntityName[entityName]; + } + static clear() { this.variableState = {}; this.unEvalState = {}; diff --git a/app/client/src/workers/Evaluation/JSObject/JSVariableFactory.ts b/app/client/src/workers/Evaluation/JSObject/JSVariableFactory.ts deleted file mode 100644 index 1922f98a63..0000000000 --- a/app/client/src/workers/Evaluation/JSObject/JSVariableFactory.ts +++ /dev/null @@ -1,52 +0,0 @@ -import { PatchType } from "./JSVariableUpdates"; -import ExecutionMetaData from "../fns/utils/ExecutionMetaData"; -import type { JSActionEntity } from "@appsmith/entities/DataTree/types"; -import TriggerEmitter, { BatchKey } from "../fns/utils/TriggerEmitter"; - -class JSFactory { - static create( - jsObjectName: string, - varState: Record = {}, - ): JSActionEntity { - const newJSObject: any = {}; - - const variables = Object.entries(varState); - - for (const [varName, varValue] of variables) { - let variable = varValue; - Object.defineProperty(newJSObject, varName, { - enumerable: true, - configurable: true, - get() { - TriggerEmitter.emit(BatchKey.process_js_variable_updates, { - path: `${jsObjectName}.${varName}`, - method: PatchType.GET, - }); - return variable; - }, - set(value) { - TriggerEmitter.emit(BatchKey.process_js_variable_updates, { - path: `${jsObjectName}.${varName}`, - method: PatchType.SET, - value, - }); - variable = value; - }, - }); - - ExecutionMetaData.setExecutionMetaData({ - enableJSVarUpdateTracking: false, - }); - - newJSObject[varName] = varValue; - - ExecutionMetaData.setExecutionMetaData({ - enableJSVarUpdateTracking: true, - }); - } - - return newJSObject; - } -} - -export default JSFactory; diff --git a/app/client/src/workers/Evaluation/JSObject/JSVariableUpdates.ts b/app/client/src/workers/Evaluation/JSObject/JSVariableUpdates.ts index d15265aeaf..3d2638abba 100644 --- a/app/client/src/workers/Evaluation/JSObject/JSVariableUpdates.ts +++ b/app/client/src/workers/Evaluation/JSObject/JSVariableUpdates.ts @@ -5,17 +5,8 @@ import { evalTreeWithChanges } from "../evalTreeWithChanges"; import { get } from "lodash"; import { isJSObjectVariable } from "./utils"; import isDeepEqualES6 from "fast-deep-equal/es6"; - -export enum PatchType { - "SET" = "SET", - "GET" = "GET", -} - -export interface Patch { - path: string; - method: PatchType; - value?: unknown; -} +import type { Patch } from "./Collection"; +import { PatchType } from "./Collection"; export type UpdatedPathsMap = Record; diff --git a/app/client/src/workers/Evaluation/JSObject/__test__/JSVariableFactory.test.ts b/app/client/src/workers/Evaluation/JSObject/__test__/JSVariableFactory.test.ts index f7e0355b70..80191e1c6d 100644 --- a/app/client/src/workers/Evaluation/JSObject/__test__/JSVariableFactory.test.ts +++ b/app/client/src/workers/Evaluation/JSObject/__test__/JSVariableFactory.test.ts @@ -1,9 +1,9 @@ -import JSFactory from "../JSVariableFactory"; import ExecutionMetaData from "workers/Evaluation/fns/utils/ExecutionMetaData"; import type { JSActionEntity } from "@appsmith/entities/DataTree/types"; import TriggerEmitter, { jsVariableUpdatesHandlerWrapper, } from "workers/Evaluation/fns/utils/TriggerEmitter"; +import JSObjectCollection from "../Collection"; const applyJSVariableUpdatesToEvalTreeMock = jest.fn(); jest.mock("../JSVariableUpdates.ts", () => ({ @@ -36,7 +36,12 @@ describe("JSVariableFactory", () => { weakSet: new WeakSet(), } as unknown as JSActionEntity; - const proxiedJSObject = JSFactory.create("JSObject1", jsObject); + Object.entries(jsObject).forEach(([k, v]) => + JSObjectCollection.setVariableValue(v, `JSObject1.${k}`), + ); + + const proxiedJSObject = + JSObjectCollection.getVariablesForEvaluationContext("JSObject1"); ExecutionMetaData.setExecutionMetaData({ enableJSVarUpdateTracking: true, @@ -96,7 +101,12 @@ describe("JSVariableFactory", () => { weakSet: new WeakSet(), } as unknown as JSActionEntity; - const proxiedJSObject = JSFactory.create("JSObject1", jsObject); + Object.entries(jsObject).forEach(([k, v]) => + JSObjectCollection.setVariableValue(v, `JSObject1.${k}`), + ); + + const proxiedJSObject = + JSObjectCollection.getVariablesForEvaluationContext("JSObject1"); ExecutionMetaData.setExecutionMetaData({ enableJSVarUpdateTracking: false, @@ -125,7 +135,12 @@ describe("JSVariableFactory", () => { weakSet: new WeakSet(), } as unknown as JSActionEntity; - const proxiedJSObject = JSFactory.create("JSObject1", jsObject); + Object.entries(jsObject).forEach(([k, v]) => + JSObjectCollection.setVariableValue(v, `JSObject1.${k}`), + ); + + const proxiedJSObject = + JSObjectCollection.getVariablesForEvaluationContext("JSObject1"); ExecutionMetaData.setExecutionMetaData({ enableJSVarUpdateTracking: true, diff --git a/app/client/src/workers/Evaluation/JSObject/index.ts b/app/client/src/workers/Evaluation/JSObject/index.ts index a1af115b2a..56ac11224f 100644 --- a/app/client/src/workers/Evaluation/JSObject/index.ts +++ b/app/client/src/workers/Evaluation/JSObject/index.ts @@ -92,6 +92,7 @@ export function saveResolvedFunctionsAndJSUpdates( try { JSObjectCollection.deleteResolvedFunction(entityName); JSObjectCollection.deleteUnEvalState(entityName); + JSObjectCollection.clearCachedVariablesForEvaluationContext(entityName); const parseStartTime = performance.now(); const { parsedObject, success } = parseJSObject(entity.body); diff --git a/app/client/src/workers/Evaluation/fns/utils/TriggerEmitter.ts b/app/client/src/workers/Evaluation/fns/utils/TriggerEmitter.ts index 7d50754db0..95ba119c36 100644 --- a/app/client/src/workers/Evaluation/fns/utils/TriggerEmitter.ts +++ b/app/client/src/workers/Evaluation/fns/utils/TriggerEmitter.ts @@ -1,10 +1,7 @@ import { EventEmitter } from "events"; import { MAIN_THREAD_ACTION } from "@appsmith/workers/Evaluation/evalWorkerActions"; import { WorkerMessenger } from "workers/Evaluation/fns/utils/Messenger"; -import type { - Patch, - UpdatedPathsMap, -} from "workers/Evaluation/JSObject/JSVariableUpdates"; +import type { UpdatedPathsMap } from "workers/Evaluation/JSObject/JSVariableUpdates"; import { applyJSVariableUpdatesToEvalTree } from "workers/Evaluation/JSObject/JSVariableUpdates"; import ExecutionMetaData from "./ExecutionMetaData"; import { get } from "lodash"; @@ -18,6 +15,7 @@ import type { import type { UpdateActionProps } from "workers/Evaluation/handlers/updateActionData"; import { handleActionsDataUpdate } from "workers/Evaluation/handlers/updateActionData"; import { getEntityNameAndPropertyPath } from "@appsmith/workers/Evaluation/evaluationUtils"; +import type { Patch } from "workers/Evaluation/JSObject/Collection"; const _internalSetTimeout = self.setTimeout; const _internalClearTimeout = self.clearTimeout; diff --git a/app/client/src/workers/Evaluation/getEntityForContext.ts b/app/client/src/workers/Evaluation/getEntityForContext.ts index 35b5bc46db..8037878d76 100644 --- a/app/client/src/workers/Evaluation/getEntityForContext.ts +++ b/app/client/src/workers/Evaluation/getEntityForContext.ts @@ -2,7 +2,6 @@ import type { JSActionEntity } from "@appsmith/entities/DataTree/types"; import type { DataTreeEntity } from "entities/DataTree/dataTreeTypes"; import { ENTITY_TYPE_VALUE } from "entities/DataTree/dataTreeFactory"; import JSObjectCollection from "./JSObject/Collection"; -import JSFactory from "./JSObject/JSVariableFactory"; import { jsObjectFunctionFactory } from "./fns/utils/jsObjectFnFactory"; import { isObject } from "lodash"; @@ -53,7 +52,8 @@ export function getEntityForEvalContext( return Object.assign({}, jsObject, fns); } - jsObjectForEval = JSFactory.create(entityName, jsObjectForEval); + jsObjectForEval = + JSObjectCollection.getVariablesForEvaluationContext(entityName); return Object.assign(jsObjectForEval, fns); } } From 97166581d383da149ebd85b0327134bb18a743f7 Mon Sep 17 00:00:00 2001 From: Nilesh Sarupriya Date: Mon, 16 Oct 2023 14:42:38 +0530 Subject: [PATCH 27/32] chore: populate user management roles with default domain (#26803) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description > Populate user management roles with default domain type and id. #### PR fixes following issue(s) Fixes # (issue number) > if no issue exists, please create an issue and ask the maintainers about this first > > #### Media > A video or a GIF is preferred. when using Loom, don’t embed because it looks like it’s a GIF. instead, just link to the video > > #### Type of change - Chore (housekeeping or task changes that don't impact user perception) ## Testing > #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [ ] Manual - [ ] JUnit - [ ] Jest - [ ] Cypress > > #### Test Plan > Add Testsmith test cases links that relate to this PR > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --------- Co-authored-by: Nilesh Sarupriya <20905988+nsarupr@users.noreply.github.com> (cherry picked from commit 1305b1a2af3b4cae91e1f829c40bcba384faa8f5) --- .../server/exceptions/AppsmithError.java | 8 ++ .../server/exceptions/AppsmithErrorCode.java | 1 + ...entRolesWithoutDefaultDomainTypeAndId.java | 88 +++++++++++++ ...eDefaultDomainIdInUserManagementRoles.java | 117 ++++++++++++++++++ ...n030TagUsersWithNoUserManagementRoles.java | 70 +++++++++++ ...serManagementRolesForUsersTaggedIn030.java | 110 ++++++++++++++++ 6 files changed, 394 insertions(+) create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration028TagUserManagementRolesWithoutDefaultDomainTypeAndId.java create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration029PopulateDefaultDomainIdInUserManagementRoles.java create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration030TagUsersWithNoUserManagementRoles.java create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration031CreateUserManagementRolesForUsersTaggedIn030.java diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java index 9de15ba27e..8985b52e39 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java @@ -969,6 +969,14 @@ public enum AppsmithError { "Invalid usage for custom annotation", ErrorType.CONFIGURATION_ERROR, null), + MIGRATION_FAILED( + 500, + AppsmithErrorCode.MIGRATION_FAILED.getCode(), + "Migration {0} failed. Reason: {1}. Note: {2}", + AppsmithErrorAction.DEFAULT, + "Migration failed", + ErrorType.INTERNAL_ERROR, + null), FeatureFlagMigrationFailure( 500, diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithErrorCode.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithErrorCode.java index e47cb2ad6a..9cd8a99b38 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithErrorCode.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithErrorCode.java @@ -66,6 +66,7 @@ public enum AppsmithErrorCode { PLUGIN_EXECUTION_TIMEOUT("AE-APP-5040", "Plugin execution timeout"), MARKETPLACE_TIMEOUT("AE-APP-5041", "Marketplace timeout"), GOOGLE_RECAPTCHA_TIMEOUT("AE-APP-5042", "Google recaptcha timeout"), + MIGRATION_FAILED("AE-APP-5043", "Migration failed"), INVALID_PROPERTIES_CONFIGURATION("AE-APP-5044", "Property configuration is wrong or malformed"), NAME_CLASH_NOT_ALLOWED_IN_REFACTOR("AE-AST-4009", "Name clash not allowed in refactor"), GENERIC_BAD_REQUEST("AE-BAD-4000", "Generic bad request"), diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration028TagUserManagementRolesWithoutDefaultDomainTypeAndId.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration028TagUserManagementRolesWithoutDefaultDomainTypeAndId.java new file mode 100644 index 0000000000..49d163b95f --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration028TagUserManagementRolesWithoutDefaultDomainTypeAndId.java @@ -0,0 +1,88 @@ +package com.appsmith.server.migrations.db.ce; + +import com.appsmith.external.models.Policy; +import com.appsmith.server.domains.PermissionGroup; +import com.appsmith.server.domains.QPermissionGroup; +import com.appsmith.server.domains.QUser; +import com.appsmith.server.domains.User; +import com.appsmith.server.helpers.CollectionUtils; +import io.mongock.api.annotations.ChangeUnit; +import io.mongock.api.annotations.Execution; +import io.mongock.api.annotations.RollbackExecution; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.data.mongodb.core.MongoTemplate; +import org.springframework.data.mongodb.core.query.Criteria; +import org.springframework.data.mongodb.core.query.Query; +import org.springframework.data.mongodb.core.query.Update; + +import java.util.HashSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; + +import static com.appsmith.server.acl.AclPermission.RESET_PASSWORD_USERS; +import static com.appsmith.server.repositories.ce.BaseAppsmithRepositoryCEImpl.fieldName; +import static com.appsmith.server.repositories.ce.BaseAppsmithRepositoryCEImpl.notDeleted; + +@Slf4j +@RequiredArgsConstructor +@ChangeUnit(order = "028", id = "tag-user-management-roles-without-default-domain-type-and-id") +public class Migration028TagUserManagementRolesWithoutDefaultDomainTypeAndId { + private final MongoTemplate mongoTemplate; + + public static final String MIGRATION_FLAG_028_TAG_USER_MANAGEMENT_ROLE_WITHOUT_DEFAULT_DOMAIN_TYPE_AND_ID = + "tagUserManagementRoleWithoutDefaultDomainTypeAndId"; + + @RollbackExecution + public void rollbackExecution() {} + + @Execution + public void tagUserManagementRolesWithoutDefaultDomainTypeAndId() { + Criteria resetPasswordPolicyExistsAndNotDeleted = Criteria.where("policies.permission") + .is(RESET_PASSWORD_USERS.getValue()) + .andOperator(notDeleted()); + Query queryExistingUsersWithResetPasswordPolicy = new Query(resetPasswordPolicyExistsAndNotDeleted); + queryExistingUsersWithResetPasswordPolicy.fields().include(fieldName(QUser.user.policies)); + + List existingUsers = mongoTemplate.find(queryExistingUsersWithResetPasswordPolicy, User.class); + + /* + * At the moment, RESET_PASSWORD_USERS policy for the user resource only contains 1 permission group ID, i.e. + * the ID of user management role for that user resource. Hence, we pick the ID present at the 0th index below. + */ + Set userManagementRoleIds = new HashSet<>(); + existingUsers.forEach(existingUser -> { + Optional resetPasswordPolicyOptional = existingUser.getPolicies().stream() + .filter(policy1 -> RESET_PASSWORD_USERS.getValue().equals(policy1.getPermission())) + .findFirst(); + resetPasswordPolicyOptional.ifPresent(resetPasswordPolicy -> { + List roleIdList = + resetPasswordPolicy.getPermissionGroups().stream().toList(); + if (!CollectionUtils.isNullOrEmpty(roleIdList)) { + userManagementRoleIds.add(roleIdList.get(0)); + } + }); + }); + + Criteria criteriaUserManagementRoleIds = + Criteria.where(fieldName(QPermissionGroup.permissionGroup.id)).in(userManagementRoleIds); + Criteria criteriaDefaultDomainIdDoesNotExist = new Criteria() + .orOperator( + Criteria.where(fieldName(QPermissionGroup.permissionGroup.defaultDomainId)) + .isNull(), + Criteria.where(fieldName(QPermissionGroup.permissionGroup.defaultDomainId)) + .exists(false)); + Criteria criteriaUserManagementRolesWithDefaultDomainIdDoesNotExist = + new Criteria().andOperator(criteriaUserManagementRoleIds, criteriaDefaultDomainIdDoesNotExist); + Query queryUserManagementRolesWithDefaultDomainIdDoesNotExist = + new Query(criteriaUserManagementRolesWithDefaultDomainIdDoesNotExist); + + Update updateSetMigrationFlag = new Update(); + updateSetMigrationFlag.set( + MIGRATION_FLAG_028_TAG_USER_MANAGEMENT_ROLE_WITHOUT_DEFAULT_DOMAIN_TYPE_AND_ID, Boolean.TRUE); + + mongoTemplate.updateMulti( + queryUserManagementRolesWithDefaultDomainIdDoesNotExist, updateSetMigrationFlag, PermissionGroup.class); + } +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration029PopulateDefaultDomainIdInUserManagementRoles.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration029PopulateDefaultDomainIdInUserManagementRoles.java new file mode 100644 index 0000000000..332f76a529 --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration029PopulateDefaultDomainIdInUserManagementRoles.java @@ -0,0 +1,117 @@ +package com.appsmith.server.migrations.db.ce; + +import com.appsmith.external.models.Policy; +import com.appsmith.server.domains.PermissionGroup; +import com.appsmith.server.domains.QPermissionGroup; +import com.appsmith.server.domains.QUser; +import com.appsmith.server.domains.User; +import com.appsmith.server.exceptions.AppsmithError; +import com.appsmith.server.exceptions.AppsmithException; +import io.mongock.api.annotations.ChangeUnit; +import io.mongock.api.annotations.Execution; +import io.mongock.api.annotations.RollbackExecution; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.StringUtils; +import org.springframework.data.mongodb.core.MongoTemplate; +import org.springframework.data.mongodb.core.query.Criteria; +import org.springframework.data.mongodb.core.query.Query; +import org.springframework.data.mongodb.core.query.Update; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import static com.appsmith.server.acl.AclPermission.RESET_PASSWORD_USERS; +import static com.appsmith.server.repositories.ce.BaseAppsmithRepositoryCEImpl.fieldName; +import static com.appsmith.server.repositories.ce.BaseAppsmithRepositoryCEImpl.notDeleted; + +@Slf4j +@RequiredArgsConstructor +@ChangeUnit(order = "029", id = "populate-default-domain-id-in-user-management-roles") +public class Migration029PopulateDefaultDomainIdInUserManagementRoles { + + private final MongoTemplate mongoTemplate; + + private static final int migrationRetries = 5; + private static final String migrationId = "populate-default-domain-id-in-user-management-roles"; + private static final String migrationNote = "This will not have any adverse effects on the Data. Restarting the " + + "server will begin the migration from where it left."; + + @RollbackExecution + public void rollbackExecution() {} + + @Execution + public void populateDefaultDomainIdInUserManagementRoles() { + Criteria resetPasswordPolicyExistsAndNotDeleted = Criteria.where("policies.permission") + .is(RESET_PASSWORD_USERS.getValue()) + .andOperator(notDeleted()); + Query queryExistingUsersWithResetPasswordPolicy = new Query(resetPasswordPolicyExistsAndNotDeleted); + queryExistingUsersWithResetPasswordPolicy.fields().include(fieldName(QUser.user.policies)); + Map userManagementRoleIdToUserIdMap = new HashMap<>(); + mongoTemplate.stream(queryExistingUsersWithResetPasswordPolicy, User.class) + .forEach(existingUser -> { + Optional resetPasswordPolicyOptional = existingUser.getPolicies().stream() + .filter(policy1 -> RESET_PASSWORD_USERS.getValue().equals(policy1.getPermission())) + .findFirst(); + resetPasswordPolicyOptional.ifPresent(resetPasswordPolicy -> { + List roleIdList = resetPasswordPolicy.getPermissionGroups().stream() + .toList(); + roleIdList.forEach(roleId -> userManagementRoleIdToUserIdMap.put(roleId, existingUser.getId())); + }); + }); + + Criteria criteriaUserManagementRolesWithMigrationFlag028Set = Criteria.where( + Migration028TagUserManagementRolesWithoutDefaultDomainTypeAndId + .MIGRATION_FLAG_028_TAG_USER_MANAGEMENT_ROLE_WITHOUT_DEFAULT_DOMAIN_TYPE_AND_ID) + .exists(Boolean.TRUE); + Query queryUserManagementRolesWithWithMigrationFlag028Set = + new Query(criteriaUserManagementRolesWithMigrationFlag028Set); + + queryUserManagementRolesWithWithMigrationFlag028Set + .fields() + .include(fieldName(QPermissionGroup.permissionGroup.id)); + long countOfUserManagementRolesWithMigrationFlag028Set = + mongoTemplate.count(queryUserManagementRolesWithWithMigrationFlag028Set, PermissionGroup.class); + int attempt = 0; + + while (countOfUserManagementRolesWithMigrationFlag028Set > 0 && attempt < migrationRetries) { + List userManagementRolesWithMigrationFlag028Set = + mongoTemplate.find(queryUserManagementRolesWithWithMigrationFlag028Set, PermissionGroup.class); + userManagementRolesWithMigrationFlag028Set.parallelStream().forEach(userManagementRole -> { + if (userManagementRoleIdToUserIdMap.containsKey(userManagementRole.getId()) + && StringUtils.isNotEmpty(userManagementRoleIdToUserIdMap.get(userManagementRole.getId()))) { + Criteria criteriaUserManagementRoleById = Criteria.where( + fieldName(QPermissionGroup.permissionGroup.id)) + .is(userManagementRole.getId()); + Query queryUserManagementRoleById = new Query(criteriaUserManagementRoleById); + Update updateDefaultDomainIdOfUserManagementRole = new Update(); + + updateDefaultDomainIdOfUserManagementRole.set( + fieldName(QPermissionGroup.permissionGroup.defaultDomainId), + userManagementRoleIdToUserIdMap.get(userManagementRole.getId())); + + updateDefaultDomainIdOfUserManagementRole.set( + fieldName(QPermissionGroup.permissionGroup.defaultDomainType), User.class.getSimpleName()); + + updateDefaultDomainIdOfUserManagementRole.unset( + Migration028TagUserManagementRolesWithoutDefaultDomainTypeAndId + .MIGRATION_FLAG_028_TAG_USER_MANAGEMENT_ROLE_WITHOUT_DEFAULT_DOMAIN_TYPE_AND_ID); + mongoTemplate.updateFirst( + queryUserManagementRoleById, + updateDefaultDomainIdOfUserManagementRole, + PermissionGroup.class); + } + }); + countOfUserManagementRolesWithMigrationFlag028Set = + mongoTemplate.count(queryUserManagementRolesWithWithMigrationFlag028Set, PermissionGroup.class); + attempt += 1; + } + + if (countOfUserManagementRolesWithMigrationFlag028Set != 0) { + String reasonForFailure = "All user management roles were not tagged."; + throw new AppsmithException(AppsmithError.MIGRATION_FAILED, migrationId, reasonForFailure, migrationNote); + } + } +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration030TagUsersWithNoUserManagementRoles.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration030TagUsersWithNoUserManagementRoles.java new file mode 100644 index 0000000000..11555cc0bd --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration030TagUsersWithNoUserManagementRoles.java @@ -0,0 +1,70 @@ +package com.appsmith.server.migrations.db.ce; + +import com.appsmith.external.models.QBaseDomain; +import com.appsmith.server.domains.PermissionGroup; +import com.appsmith.server.domains.User; +import io.mongock.api.annotations.ChangeUnit; +import io.mongock.api.annotations.Execution; +import io.mongock.api.annotations.RollbackExecution; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.data.mongodb.core.MongoTemplate; +import org.springframework.data.mongodb.core.query.Criteria; +import org.springframework.data.mongodb.core.query.Query; +import org.springframework.data.mongodb.core.query.Update; + +import java.util.List; + +import static com.appsmith.server.repositories.ce.BaseAppsmithRepositoryCEImpl.fieldName; + +@Slf4j +@RequiredArgsConstructor +@ChangeUnit(order = "030", id = "tag-users-with-no-user-management-roles") +public class Migration030TagUsersWithNoUserManagementRoles { + + private final MongoTemplate mongoTemplate; + + public static final String MIGRATION_FLAG_030_TAG_USER_WITHOUT_USER_MANAGEMENT_ROLE = + "tagUserWithoutUserManagementRole"; + + @RollbackExecution + public void rollbackExecution() {} + + @Execution + public void tagUsersWithNoUserManagementRoles() { + Criteria criteriaDefaultDomainTypeExists = + Criteria.where("defaultDomainType").exists(true); + Criteria criteriaDefaultDomainTypeIsUser = + Criteria.where("defaultDomainType").is(User.class.getSimpleName()); + + Criteria criteriaUserManagementRoles = + new Criteria().andOperator(criteriaDefaultDomainTypeExists, criteriaDefaultDomainTypeIsUser); + Query queryUserManagementRoles = new Query(criteriaUserManagementRoles); + queryUserManagementRoles.fields().include("defaultDomainId"); + + List userManagementRoles = mongoTemplate.find(queryUserManagementRoles, PermissionGroup.class); + + List userIdsWithUserManagementRoles = userManagementRoles.stream() + .map(PermissionGroup::getDefaultDomainId) + .toList(); + + Criteria criteriaUsersWithNoUserManagementRoles = + Criteria.where(fieldName(QBaseDomain.baseDomain.id)).nin(userIdsWithUserManagementRoles); + Criteria criteriaUsersPoliciesExists = + Criteria.where(fieldName(QBaseDomain.baseDomain.policies)).exists(true); + Criteria criteriaUsersPoliciesNotEmpty = + Criteria.where(fieldName(QBaseDomain.baseDomain.policies)).not().size(0); + Criteria criteriaUsersWithNoUserManagementRolesAndUserPoliciesExists = new Criteria() + .andOperator( + criteriaUsersWithNoUserManagementRoles, + criteriaUsersPoliciesExists, + criteriaUsersPoliciesNotEmpty); + Query queryUsersWithNoUserManagementRoles = + new Query(criteriaUsersWithNoUserManagementRolesAndUserPoliciesExists); + + Update updateSetMigrationFlag = new Update(); + updateSetMigrationFlag.set(MIGRATION_FLAG_030_TAG_USER_WITHOUT_USER_MANAGEMENT_ROLE, Boolean.TRUE); + + mongoTemplate.updateMulti(queryUsersWithNoUserManagementRoles, updateSetMigrationFlag, User.class); + } +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration031CreateUserManagementRolesForUsersTaggedIn030.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration031CreateUserManagementRolesForUsersTaggedIn030.java new file mode 100644 index 0000000000..b450af9daf --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration031CreateUserManagementRolesForUsersTaggedIn030.java @@ -0,0 +1,110 @@ +package com.appsmith.server.migrations.db.ce; + +import com.appsmith.external.models.Policy; +import com.appsmith.external.models.QBaseDomain; +import com.appsmith.server.constants.FieldName; +import com.appsmith.server.domains.PermissionGroup; +import com.appsmith.server.domains.QUser; +import com.appsmith.server.domains.User; +import com.appsmith.server.dtos.Permission; +import com.appsmith.server.exceptions.AppsmithError; +import com.appsmith.server.exceptions.AppsmithException; +import com.appsmith.server.migrations.utils.CompatibilityUtils; +import com.appsmith.server.solutions.PolicySolution; +import io.mongock.api.annotations.ChangeUnit; +import io.mongock.api.annotations.Execution; +import io.mongock.api.annotations.RollbackExecution; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.data.mongodb.core.MongoTemplate; +import org.springframework.data.mongodb.core.query.Criteria; +import org.springframework.data.mongodb.core.query.Query; +import org.springframework.data.mongodb.core.query.Update; + +import java.util.Map; +import java.util.Set; + +import static com.appsmith.server.acl.AclPermission.MANAGE_USERS; +import static com.appsmith.server.repositories.ce.BaseAppsmithRepositoryCEImpl.fieldName; +import static java.lang.Boolean.TRUE; + +@Slf4j +@RequiredArgsConstructor +@ChangeUnit(order = "031", id = "create-user-management-roles-for-users-tagged-in-migration-030") +public class Migration031CreateUserManagementRolesForUsersTaggedIn030 { + private final MongoTemplate mongoTemplate; + private final PolicySolution policySolution; + + private static final int migrationRetries = 5; + private static final String migrationNote = "This will not have any adverse effects on the Data. Restarting the " + + "server will begin the migration from where it left."; + private static final String migrationId = "create-user-management-roles-for-users-tagged-in-migration-030"; + + @RollbackExecution + public void rollbackExecution() {} + + @Execution + public void createUserManagementRolesForUsersTaggedInMigration030() { + Criteria criteriaUsersTaggedInMigration030 = Criteria.where( + Migration030TagUsersWithNoUserManagementRoles + .MIGRATION_FLAG_030_TAG_USER_WITHOUT_USER_MANAGEMENT_ROLE) + .is(TRUE); + + Query queryUsersTaggedInMigration030 = new Query(criteriaUsersTaggedInMigration030); + queryUsersTaggedInMigration030.fields().include(fieldName(QUser.user.id)); + queryUsersTaggedInMigration030.fields().include(fieldName(QUser.user.policies)); + queryUsersTaggedInMigration030.fields().include(fieldName(QUser.user.email)); + + Query optimisedQueryUsersTaggedInMigration030 = CompatibilityUtils.optimizeQueryForNoCursorTimeout( + mongoTemplate, queryUsersTaggedInMigration030, User.class); + + int attempt = 0; + long countUsersTaggedInMigration030 = mongoTemplate.count(optimisedQueryUsersTaggedInMigration030, User.class); + + while (attempt < migrationRetries && countUsersTaggedInMigration030 > 0) { + mongoTemplate.stream(optimisedQueryUsersTaggedInMigration030, User.class) + .forEach(user -> { + User userWithUpdatedPolicies = createUserManagementRoleAndGetUserWithUpdatedPolicies(user); + Update updateMigrationFlagAndPoliciesForUser = new Update(); + updateMigrationFlagAndPoliciesForUser.unset( + Migration030TagUsersWithNoUserManagementRoles + .MIGRATION_FLAG_030_TAG_USER_WITHOUT_USER_MANAGEMENT_ROLE); + updateMigrationFlagAndPoliciesForUser.set( + fieldName(QUser.user.policies), userWithUpdatedPolicies.getPolicies()); + Criteria criteriaUserId = Criteria.where(fieldName(QBaseDomain.baseDomain.id)) + .is(user.getId()); + Query queryUserId = new Query(criteriaUserId); + mongoTemplate.updateFirst(queryUserId, updateMigrationFlagAndPoliciesForUser, User.class); + }); + attempt += 1; + countUsersTaggedInMigration030 = mongoTemplate.count(optimisedQueryUsersTaggedInMigration030, User.class); + } + + if (countUsersTaggedInMigration030 > 0) { + String reasonForFailure = "All user management roles were not created."; + throw new AppsmithException(AppsmithError.MIGRATION_FAILED, migrationId, reasonForFailure, migrationNote); + } + } + + private User createUserManagementRoleAndGetUserWithUpdatedPolicies(User user) { + // Create user management permission group + PermissionGroup userManagementRole = new PermissionGroup(); + userManagementRole.setName(user.getUsername() + FieldName.SUFFIX_USER_MANAGEMENT_ROLE); + // Add CRUD permissions for user to the group + userManagementRole.setPermissions(Set.of(new Permission(user.getId(), MANAGE_USERS))); + userManagementRole.setDefaultDomainId(user.getId()); + userManagementRole.setDefaultDomainType(User.class.getSimpleName()); + + // Assign the permission group to the user + userManagementRole.setAssignedToUserIds(Set.of(user.getId())); + + PermissionGroup savedUserManagementRole = mongoTemplate.save(userManagementRole); + + Map crudUserPolicies = + policySolution.generatePolicyFromPermissionGroupForObject(savedUserManagementRole, user.getId()); + + User updatedWithPolicies = policySolution.addPoliciesToExistingObject(crudUserPolicies, user); + + return updatedWithPolicies; + } +} From ab3ebc94f53ef1b824f690b7d009f66cb01ff731 Mon Sep 17 00:00:00 2001 From: Aishwarya-U-R <91450662+Aishwarya-U-R@users.noreply.github.com> Date: Fri, 13 Oct 2023 20:35:05 +0530 Subject: [PATCH 28/32] test: Cypress | Skip MsSQL_Basic_Spec.ts spec in CI (#28074) - This PR skips MsSQL_Basic_Spe.ts from CI runs since its runing out of memory in multiple CI runs - Until this is added to TED or we find other ways - will be run locally during regression - Script fix (non-breaking change which fixes an issue) - [X] Added `Test Plan Approved` label after changes were reviewed (cherry picked from commit 2a302e234dfecb5751b3217878f6caf0f7a7c47a) --- app/client/cypress_ci_custom.config.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/client/cypress_ci_custom.config.ts b/app/client/cypress_ci_custom.config.ts index 025ee81137..9ffd3a80b2 100644 --- a/app/client/cypress_ci_custom.config.ts +++ b/app/client/cypress_ci_custom.config.ts @@ -58,6 +58,8 @@ export default defineConfig({ "cypress/e2e/EE/Enterprise/MultipleEnv/ME_airtable_spec.ts", "cypress/e2e/Regression/ServerSide/Datasources/ElasticSearch_Basic_Spec.ts", "cypress/e2e/Regression/ServerSide/Datasources/Oracle_Spec.ts", + "cypress/e2e/Regression/ClientSide/Widgets/Others/MapWidget_Spec.ts", + "cypress/e2e/Sanity/Datasources/MsSQL_Basic_Spec.ts", ], }, }); From 18553c6a8d6cb3b1cfa2443422ae23f97d825edc Mon Sep 17 00:00:00 2001 From: Ankita Kinger Date: Mon, 16 Oct 2023 23:13:38 +0530 Subject: [PATCH 29/32] fix: Refactoring the code for label wrapper on admin settings (#28123) ## Description Refactoring the code for label wrapper on admin settings #### PR fixes following issue(s) Fixes [#28099](https://github.com/appsmithorg/appsmith/issues/28099) #### Type of change - Bug fix (non-breaking change which fixes an issue) ## Testing #### How Has This Been Tested? - [x] Manual - [ ] JUnit - [ ] Jest - [ ] Cypress ## Checklist: #### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --- .../pages/AdminSettings/FormGroup/Button.tsx | 50 ++++++++--------- .../AdminSettings/FormGroup/Checkbox.tsx | 55 +++++++++---------- .../pages/AdminSettings/FormGroup/Common.tsx | 14 ++++- .../pages/AdminSettings/FormGroup/Radio.tsx | 26 +++++---- .../AdminSettings/FormGroup/TextInput.tsx | 27 ++------- 5 files changed, 81 insertions(+), 91 deletions(-) diff --git a/app/client/src/pages/AdminSettings/FormGroup/Button.tsx b/app/client/src/pages/AdminSettings/FormGroup/Button.tsx index e5c3dfbcd6..c1bef4609f 100644 --- a/app/client/src/pages/AdminSettings/FormGroup/Button.tsx +++ b/app/client/src/pages/AdminSettings/FormGroup/Button.tsx @@ -1,14 +1,21 @@ import { SETTINGS_FORM_NAME } from "@appsmith/constants/forms"; import React from "react"; -import { Button, Text } from "design-system"; +import { Button } from "design-system"; import { useDispatch, useSelector } from "react-redux"; import { getFormValues } from "redux-form"; import styled from "styled-components"; -import type { SettingComponentProps } from "./Common"; +import { FormGroup, type SettingComponentProps } from "./Common"; const ButtonWrapper = styled.div` width: 357px; - margin-bottom: 8px; + + .styled-label { + padding: 0 0 0.5rem; + } + + .admin-settings-form-group-label { + font-weight: var(--ads-v2-h5-font-weight); + } `; export const StyledButton = styled(Button)` @@ -22,28 +29,21 @@ export default function ButtonComponent({ setting }: SettingComponentProps) { const settings = useSelector(formValuesSelector); return ( - - {setting.label} - - { - if (setting.action) { - setting.action(dispatch, settings); - } - }} - size="md" - > - {setting.text} - + + { + if (setting.action) { + setting.action(dispatch, settings); + } + }} + size="md" + > + {setting.text} + + ); } diff --git a/app/client/src/pages/AdminSettings/FormGroup/Checkbox.tsx b/app/client/src/pages/AdminSettings/FormGroup/Checkbox.tsx index 60c38c818b..5d9ad0ba7a 100644 --- a/app/client/src/pages/AdminSettings/FormGroup/Checkbox.tsx +++ b/app/client/src/pages/AdminSettings/FormGroup/Checkbox.tsx @@ -2,13 +2,12 @@ import React, { memo } from "react"; import type { WrappedFieldInputProps, WrappedFieldMetaProps } from "redux-form"; import { Field, getFormValues } from "redux-form"; import styled from "styled-components"; -import type { SettingComponentProps } from "./Common"; +import { FormGroup, type SettingComponentProps } from "./Common"; import type { FormTextFieldProps } from "components/utils/ReduxFormTextField"; -import { Checkbox, Text } from "design-system"; +import { Checkbox } from "design-system"; import { useSelector } from "react-redux"; import { SETTINGS_FORM_NAME } from "@appsmith/constants/forms"; import { isTenantConfig } from "@appsmith/utils/adminSettingsHelpers"; -import BusinessTag from "components/BusinessTag"; const CheckboxWrapper = styled.div` display: grid; @@ -65,7 +64,13 @@ function FieldCheckboxWithCheckboxText(props: CheckboxProps) { } const StyledFieldCheckboxGroup = styled.div` - margin-bottom: 8px; + .styled-label { + padding: 0 0 0.5rem; + } + + .admin-settings-form-group-label { + font-weight: var(--ads-v2-h5-font-weight); + } `; const formValuesSelector = getFormValues(SETTINGS_FORM_NAME); @@ -75,32 +80,22 @@ export function CheckboxComponent({ setting }: SettingComponentProps) { return ( -
- - {setting.label} - - {setting.isFeatureEnabled === false && } -
- + + + ); } diff --git a/app/client/src/pages/AdminSettings/FormGroup/Common.tsx b/app/client/src/pages/AdminSettings/FormGroup/Common.tsx index 0198b5bc6b..254c3e03de 100644 --- a/app/client/src/pages/AdminSettings/FormGroup/Common.tsx +++ b/app/client/src/pages/AdminSettings/FormGroup/Common.tsx @@ -3,6 +3,8 @@ import React from "react"; import styled from "styled-components"; import { Icon, Tooltip, Text } from "design-system"; import type { Setting } from "@appsmith/pages/AdminSettings/config/types"; +import EnterpriseTag from "components/EnterpriseTag"; +import BusinessTag from "components/BusinessTag"; interface FieldHelperProps { setting: Setting; @@ -28,6 +30,8 @@ export const StyledFormGroup = styled.div` export const StyledLabel = styled.div` margin-bottom: 4px; + display: flex; + align-items: center; `; export const StyledSubtext = styled(Text)` @@ -48,7 +52,7 @@ export function FormGroup({ children, className, setting }: FieldHelperProps) { className={`${className}`} data-testid="admin-settings-form-group" > - + {setting.label && ( )} +
+ {setting.isFeatureEnabled === false && + (setting.isEnterprise === true ? ( + + ) : ( + + ))} +
{children} {setting.subText && ( diff --git a/app/client/src/pages/AdminSettings/FormGroup/Radio.tsx b/app/client/src/pages/AdminSettings/FormGroup/Radio.tsx index 198d8e66d2..81e3624bbe 100644 --- a/app/client/src/pages/AdminSettings/FormGroup/Radio.tsx +++ b/app/client/src/pages/AdminSettings/FormGroup/Radio.tsx @@ -2,7 +2,7 @@ import React, { useEffect, useState } from "react"; import type { ReactElement } from "react"; import { FieldError } from "design-system-old"; import { Popover2 } from "@blueprintjs/popover2"; -import type { SettingComponentProps } from "./Common"; +import { FormGroup, type SettingComponentProps } from "./Common"; import type { WrappedFieldInputProps, WrappedFieldMetaProps } from "redux-form"; import { Field } from "redux-form"; import styled, { createGlobalStyle } from "styled-components"; @@ -87,6 +87,16 @@ const PopoverStyles = createGlobalStyle` } `; +const StyledFormGroup = styled(FormGroup)` + .styled-label { + padding: 0 0 0.5rem; + } + + .admin-settings-form-group-label { + font-weight: var(--ads-v2-h5-font-weight); + } +`; + type RadioGroupProps = SettingComponentProps; function RadioFieldWrapper( @@ -201,20 +211,12 @@ export default function RadioField({ setting }: RadioGroupProps) { const controlTypeProps = setting.controlTypeProps as RadioOptionProps; return ( -
- - {setting.label} - -
+ ); } diff --git a/app/client/src/pages/AdminSettings/FormGroup/TextInput.tsx b/app/client/src/pages/AdminSettings/FormGroup/TextInput.tsx index ecbc2b57e7..2662119630 100644 --- a/app/client/src/pages/AdminSettings/FormGroup/TextInput.tsx +++ b/app/client/src/pages/AdminSettings/FormGroup/TextInput.tsx @@ -1,42 +1,23 @@ import FormTextField from "components/utils/ReduxFormTextField"; import { createMessage } from "@appsmith/constants/messages"; import React from "react"; -import type { SettingComponentProps } from "./Common"; -import BusinessTag from "components/BusinessTag"; -import EnterpriseTag from "components/EnterpriseTag"; -import styled from "styled-components"; - -const LabelWrapper = styled.div` - display: flex; - gap: 4px; - align-items: center; -`; +import { FormGroup, type SettingComponentProps } from "./Common"; export default function TextInput({ setting }: SettingComponentProps) { - const inputLabel = setting.label ? ( - -
{setting.label}
- {setting.isFeatureEnabled === false && - (setting.isEnterprise === true ? : )} - - ) : ( - "" - ); return ( -
setting.placeholder || "")} type={setting.controlSubType} /> -
+ ); } From 0c9510704cb91eb100b295f1e10db6d3b588f206 Mon Sep 17 00:00:00 2001 From: Manish Kumar <107841575+sondermanish@users.noreply.github.com> Date: Mon, 16 Oct 2023 10:11:12 +0530 Subject: [PATCH 30/32] chore: added ce support for KB changes (#28068) ## Description > TL;DR: CE support for PR: https://github.com/appsmithorg/appsmith-ee/pull/2645/ #### PR fixes following issue(s) Fixes https://github.com/appsmithorg/appsmith-ee/issues/2644 #### Type of change - Chore (housekeeping or task changes that don't impact user perception) #### How Has This Been Tested? - [ ] Manual ## Checklist: #### Dev activity - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --- .../server/constants/SerialiseApplicationObjective.java | 3 ++- .../export/internal/ImportExportApplicationServiceCEImpl.java | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/SerialiseApplicationObjective.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/SerialiseApplicationObjective.java index 1889f901ff..d546144bf5 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/SerialiseApplicationObjective.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/SerialiseApplicationObjective.java @@ -3,5 +3,6 @@ package com.appsmith.server.constants; public enum SerialiseApplicationObjective { // For which purpose we are serialising the application from DB VERSION_CONTROL, - SHARE + SHARE, + KNOWLEDGE_BASE_GENERATION } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/export/internal/ImportExportApplicationServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/export/internal/ImportExportApplicationServiceCEImpl.java index b7546cb9c5..4bf860f25f 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/export/internal/ImportExportApplicationServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/export/internal/ImportExportApplicationServiceCEImpl.java @@ -173,7 +173,9 @@ public class ImportExportApplicationServiceCEImpl implements ImportExportApplica return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.APPLICATION_ID)); } - boolean isGitSync = SerialiseApplicationObjective.VERSION_CONTROL.equals(serialiseFor); + boolean isGitSync = SerialiseApplicationObjective.VERSION_CONTROL.equals(serialiseFor) + || SerialiseApplicationObjective.KNOWLEDGE_BASE_GENERATION.equals(serialiseFor); + exportingMetaDTO.setIsGitSync(isGitSync); exportingMetaDTO.setExportWithConfiguration(false); From bd6f403420c828ff36669a6887944b98bd3b8486 Mon Sep 17 00:00:00 2001 From: Shrikant Sharat Kandula Date: Wed, 18 Oct 2023 07:58:56 +0530 Subject: [PATCH 31/32] fix: Fail startup when supervisor creds are missing --- deploy/docker/fs/opt/appsmith/entrypoint.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/deploy/docker/fs/opt/appsmith/entrypoint.sh b/deploy/docker/fs/opt/appsmith/entrypoint.sh index 7eee9ea9dd..0a2957fcf7 100644 --- a/deploy/docker/fs/opt/appsmith/entrypoint.sh +++ b/deploy/docker/fs/opt/appsmith/entrypoint.sh @@ -143,6 +143,12 @@ unset_unused_variables() { unset APPSMITH_RECAPTCHA_SECRET_KEY unset APPSMITH_RECAPTCHA_ENABLED fi + + export APPSMITH_SUPERVISOR_USER="${APPSMITH_SUPERVISOR_USER:-appsmith}" + if [[ -z "${APPSMITH_SUPERVISOR_PASSWORD-}" ]]; then + APPSMITH_SUPERVISOR_PASSWORD="$(tr -dc A-Za-z0-9 Date: Wed, 18 Oct 2023 12:52:40 +0530 Subject: [PATCH 32/32] fix: removed datasourceContextCaching for RestAPI (#28160) ## Description > Rest Api used cached datasourceContext which was providing older tokens. Now when we look for datasourceContext we provide a new context. #### PR fixes following issue(s) Fixes #27699 #### Type of change - Bug fix (non-breaking change which fixes an issue) #### How Has This Been Tested? - [ ] Manual ## Checklist: #### Dev activity - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --- .../external/constants/PluginConstants.java | 2 + .../ce/DatasourceContextServiceCEImpl.java | 70 +++++++----- .../DatasourceContextServiceTest.java | 102 +++++++++++++++--- 3 files changed, 137 insertions(+), 37 deletions(-) diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/PluginConstants.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/PluginConstants.java index 3d12fee22c..883835c55a 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/PluginConstants.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/PluginConstants.java @@ -10,6 +10,8 @@ public interface PluginConstants { String DYNAMO_PLUGIN = "dynamo-plugin"; String AMAZON_S3_PLUGIN = "amazons3-plugin"; String GOOGLE_SHEETS_PLUGIN = "google-sheets-plugin"; + String REST_API_PLUGIN = "restapi-plugin"; + String GRAPH_QL_PLUGIN = "graphql-plugin"; } public static final String DEFAULT_REST_DATASOURCE = "DEFAULT_REST_DATASOURCE"; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/DatasourceContextServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/DatasourceContextServiceCEImpl.java index 5594ad7cb6..1b24142b42 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/DatasourceContextServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/DatasourceContextServiceCEImpl.java @@ -1,5 +1,6 @@ package com.appsmith.server.services.ce; +import com.appsmith.external.constants.PluginConstants; import com.appsmith.external.dtos.ExecutePluginDTO; import com.appsmith.external.dtos.RemoteDatasourceDTO; import com.appsmith.external.exceptions.pluginExceptions.StaleConnectionException; @@ -70,6 +71,7 @@ public class DatasourceContextServiceCEImpl implements DatasourceContextServiceC * value instead of creating a new connection for each subscription of the source publisher. * * @param datasourceStorage - datasource storage for which a new datasource context / connection needs to be created + * @param plugin * @param pluginExecutor - plugin executor associated with the datasource's plugin * @param monitor - unique monitor object per datasource id. Lock is acquired on this monitor object. * @param datasourceContextIdentifier - key for the datasourceContextMaps. @@ -78,6 +80,7 @@ public class DatasourceContextServiceCEImpl implements DatasourceContextServiceC */ public Mono> getCachedDatasourceContextMono( DatasourceStorage datasourceStorage, + Plugin plugin, PluginExecutor pluginExecutor, Object monitor, DatasourceContextIdentifier datasourceContextIdentifier) { @@ -114,7 +117,7 @@ public class DatasourceContextServiceCEImpl implements DatasourceContextServiceC /* Create a fresh datasource context */ DatasourceContext datasourceContext = new DatasourceContext<>(); - if (datasourceContextIdentifier.isKeyValid()) { + if (datasourceContextIdentifier.isKeyValid() && shouldCacheContextForThisPlugin(plugin)) { /* For this datasource, either the context doesn't exist, or the context is stale. Replace (or add) with the new connection in the context map. */ datasourceContextMap.put(datasourceContextIdentifier, datasourceContext); @@ -138,13 +141,25 @@ public class DatasourceContextServiceCEImpl implements DatasourceContextServiceC datasourceContext) .cache(); /* Cache the value so that further evaluations don't result in new connections */ - if (datasourceContextIdentifier.isKeyValid()) { + if (datasourceContextIdentifier.isKeyValid() && shouldCacheContextForThisPlugin(plugin)) { datasourceContextMonoMap.put(datasourceContextIdentifier, datasourceContextMonoCache); } return datasourceContextMonoCache; } } + /** + * determines whether we should cache context for given plugin + * it gives false if plugin is rest-api or graph-ql + * @param plugin + * @return + */ + public boolean shouldCacheContextForThisPlugin(Plugin plugin) { + // !(a || b) => (!a) & (!b) + return !PluginConstants.PackageName.REST_API_PLUGIN.equals(plugin.getPackageName()) + && !PluginConstants.PackageName.GRAPH_QL_PLUGIN.equals(plugin.getPackageName()); + } + public Mono updateDatasourceAndSetAuthentication(Object connection, DatasourceStorage datasourceStorage) { Mono datasourceStorageMono = Mono.just(datasourceStorage); if (connection instanceof UpdatableConnection updatableConnection) { @@ -162,33 +177,38 @@ public class DatasourceContextServiceCEImpl implements DatasourceContextServiceC protected Mono> createNewDatasourceContext( DatasourceStorage datasourceStorage, DatasourceContextIdentifier datasourceContextIdentifier) { log.debug("Datasource context doesn't exist. Creating connection."); - Mono pluginMono = pluginService.findById(datasourceStorage.getPluginId()); + Mono pluginMono = + pluginService.findById(datasourceStorage.getPluginId()).cache(); - return pluginExecutorHelper.getPluginExecutor(pluginMono).flatMap(pluginExecutor -> { + return pluginMono + .zipWith(pluginExecutorHelper.getPluginExecutor(pluginMono)) + .flatMap(tuple2 -> { + Plugin plugin = tuple2.getT1(); + PluginExecutor pluginExecutor = tuple2.getT2(); - /** - * Keep one monitor object against each datasource id. The synchronized method - * `getCachedDatasourceContextMono` would then acquire lock on the monitor object which is unique - * for each datasourceId hence ensuring that if competing threads want to create datasource context - * on different datasource id then they are not blocked on each other and can run concurrently. - * Only threads that want to create a new datasource context on the same datasource id would be - * synchronized. - */ - Object monitor = new Object(); - if (datasourceContextIdentifier.isKeyValid()) { - if (datasourceContextSynchronizationMonitorMap.get(datasourceContextIdentifier) == null) { - synchronized (this) { - datasourceContextSynchronizationMonitorMap.computeIfAbsent( - datasourceContextIdentifier, k -> new Object()); + /** + * Keep one monitor object against each datasource id. The synchronized method + * `getCachedDatasourceContextMono` would then acquire lock on the monitor object which is unique + * for each datasourceId hence ensuring that if competing threads want to create datasource context + * on different datasource id then they are not blocked on each other and can run concurrently. + * Only threads that want to create a new datasource context on the same datasource id would be + * synchronized. + */ + Object monitor = new Object(); + if (datasourceContextIdentifier.isKeyValid()) { + if (datasourceContextSynchronizationMonitorMap.get(datasourceContextIdentifier) == null) { + synchronized (this) { + datasourceContextSynchronizationMonitorMap.computeIfAbsent( + datasourceContextIdentifier, k -> new Object()); + } + } + + monitor = datasourceContextSynchronizationMonitorMap.get(datasourceContextIdentifier); } - } - monitor = datasourceContextSynchronizationMonitorMap.get(datasourceContextIdentifier); - } - - return getCachedDatasourceContextMono( - datasourceStorage, pluginExecutor, monitor, datasourceContextIdentifier); - }); + return getCachedDatasourceContextMono( + datasourceStorage, plugin, pluginExecutor, monitor, datasourceContextIdentifier); + }); } public boolean getIsStale( diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/DatasourceContextServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/DatasourceContextServiceTest.java index e5e93b5e64..717a795037 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/DatasourceContextServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/DatasourceContextServiceTest.java @@ -1,9 +1,14 @@ package com.appsmith.server.services; +import com.appsmith.external.constants.PluginConstants; +import com.appsmith.external.helpers.restApiUtils.connections.APIConnection; +import com.appsmith.external.helpers.restApiUtils.connections.APIConnectionFactory; +import com.appsmith.external.helpers.restApiUtils.connections.BearerTokenAuthentication; import com.appsmith.external.helpers.restApiUtils.connections.OAuth2ClientCredentials; import com.appsmith.external.models.ApiKeyAuth; import com.appsmith.external.models.AuthenticationDTO; import com.appsmith.external.models.BasicAuth; +import com.appsmith.external.models.BearerTokenAuth; import com.appsmith.external.models.DBAuth; import com.appsmith.external.models.Datasource; import com.appsmith.external.models.DatasourceConfiguration; @@ -146,6 +151,7 @@ public class DatasourceContextServiceTest { @WithUserDetails(value = "api_user") public void testDatasourceCache_afterDatasourceDeleted_doesNotReturnOldConnection() { // Never require the datasource connection to be stale + Plugin emptyPlugin = new Plugin(); doReturn(false).doReturn(false).when(datasourceContextService).getIsStale(any(), any()); MockPluginExecutor mockPluginExecutor = new MockPluginExecutor(); @@ -169,7 +175,7 @@ public class DatasourceContextServiceTest { Object monitor = new Object(); // Create one instance of datasource connection Mono> dsContextMono1 = datasourceContextService.getCachedDatasourceContextMono( - datasourceStorage, spyMockPluginExecutor, monitor, datasourceContextIdentifier); + datasourceStorage, emptyPlugin, spyMockPluginExecutor, monitor, datasourceContextIdentifier); Datasource datasource = new Datasource(); datasource.setId("id1"); @@ -197,7 +203,7 @@ public class DatasourceContextServiceTest { Mono> dsContextMono2 = datasourceService .archiveById("id1") .flatMap(deleted -> datasourceContextService.getCachedDatasourceContextMono( - datasourceStorage, spyMockPluginExecutor, monitor, datasourceContextIdentifier)); + datasourceStorage, emptyPlugin, spyMockPluginExecutor, monitor, datasourceContextIdentifier)); StepVerifier.create(dsContextMono1) .assertNext(dsContext1 -> { @@ -349,7 +355,7 @@ public class DatasourceContextServiceTest { @WithUserDetails(value = "api_user") public void testCachedDatasourceCreate() { doReturn(false).doReturn(false).when(datasourceContextService).getIsStale(any(), any()); - + Plugin emptyPlugin = new Plugin(); MockPluginExecutor mockPluginExecutor = new MockPluginExecutor(); MockPluginExecutor spyMockPluginExecutor = spy(mockPluginExecutor); /* Return two different connection objects if `datasourceCreate` method is called twice */ @@ -369,11 +375,11 @@ public class DatasourceContextServiceTest { Object monitor = new Object(); DatasourceContext dsContext1 = (DatasourceContext) datasourceContextService .getCachedDatasourceContextMono( - datasourceStorage, spyMockPluginExecutor, monitor, datasourceContextIdentifier) + datasourceStorage, emptyPlugin, spyMockPluginExecutor, monitor, datasourceContextIdentifier) .block(); DatasourceContext dsContext2 = (DatasourceContext) datasourceContextService .getCachedDatasourceContextMono( - datasourceStorage, spyMockPluginExecutor, monitor, datasourceContextIdentifier) + datasourceStorage, emptyPlugin, spyMockPluginExecutor, monitor, datasourceContextIdentifier) .block(); /* They can only be equal if the `datasourceCreate` method was called only once */ @@ -388,6 +394,7 @@ public class DatasourceContextServiceTest { @Test @WithUserDetails(value = "api_user") public void testDatasourceCreate_withUpdatableConnection_recreatesConnectionAlways() { + Plugin emptyPlugin = new Plugin(); MockPluginExecutor mockPluginExecutor = new MockPluginExecutor(); Mockito.when(pluginExecutorHelper.getPluginExecutor(Mockito.any())).thenReturn(Mono.just(mockPluginExecutor)); @@ -452,7 +459,11 @@ public class DatasourceContextServiceTest { Object monitor = new Object(); final DatasourceContext dsc1 = (DatasourceContext) datasourceContextService .getCachedDatasourceContextMono( - createdDatasourceStorage, spyMockPluginExecutor, monitor, datasourceContextIdentifier) + createdDatasourceStorage, + emptyPlugin, + spyMockPluginExecutor, + monitor, + datasourceContextIdentifier) .block(); assertNotNull(dsc1); assertTrue(dsc1.getConnection() instanceof UpdatableConnection); @@ -461,7 +472,11 @@ public class DatasourceContextServiceTest { final DatasourceContext dsc2 = (DatasourceContext) datasourceContextService .getCachedDatasourceContextMono( - createdDatasourceStorage, spyMockPluginExecutor, monitor, datasourceContextIdentifier) + createdDatasourceStorage, + emptyPlugin, + spyMockPluginExecutor, + monitor, + datasourceContextIdentifier) .block(); assertNotNull(dsc2); assertTrue(dsc2.getConnection() instanceof UpdatableConnection); @@ -479,6 +494,7 @@ public class DatasourceContextServiceTest { public void testDatasourceContextIsInvalid_whenCachedDatasourceContextMono_isInErrorState() { doReturn(false).when(datasourceContextService).getIsStale(any(), any()); + Plugin emptyPlugin = new Plugin(); MockPluginExecutor mockPluginExecutor = new MockPluginExecutor(); MockPluginExecutor spyMockPluginExecutor = spy(mockPluginExecutor); @@ -499,7 +515,7 @@ public class DatasourceContextServiceTest { Mono> failedDatasourceContextMono = datasourceContextService.getCachedDatasourceContextMono( - datasourceStorage, spyMockPluginExecutor, monitor, datasourceContextIdentifier); + datasourceStorage, emptyPlugin, spyMockPluginExecutor, monitor, datasourceContextIdentifier); StepVerifier.create(failedDatasourceContextMono) .expectError(RuntimeException.class) @@ -512,14 +528,14 @@ public class DatasourceContextServiceTest { /** * This test verifies that if a cached datasource context Mono goes to an error state, then that Mono is invalidated * and a new datasource context mono is created on calling - * {@link com.appsmith.server.services.ce.DatasourceContextServiceCEImpl#getCachedDatasourceContextMono(DatasourceStorage, PluginExecutor, Object, DatasourceContextIdentifier)} + * {@link com.appsmith.server.services.ce.DatasourceContextServiceCEImpl#getCachedDatasourceContextMono(DatasourceStorage, Plugin, PluginExecutor, Object, DatasourceContextIdentifier)} * and not fetched from the cache. */ @Test @WithUserDetails(value = "api_user") public void testNewDatasourceContextCreate_whenCachedDatasourceContextMono_isInErrorState() { doReturn(false).doReturn(false).when(datasourceContextService).getIsStale(any(), any()); - + Plugin emptyPlugin = new Plugin(); MockPluginExecutor mockPluginExecutor = new MockPluginExecutor(); MockPluginExecutor spyMockPluginExecutor = spy(mockPluginExecutor); @@ -541,13 +557,13 @@ public class DatasourceContextServiceTest { Mono> failedDatasourceContextMono = datasourceContextService.getCachedDatasourceContextMono( - datasourceStorage, spyMockPluginExecutor, monitor, datasourceContextIdentifier); + datasourceStorage, emptyPlugin, spyMockPluginExecutor, monitor, datasourceContextIdentifier); StepVerifier.create(failedDatasourceContextMono) .expectError(RuntimeException.class) .verify(); Mono> validDatasourceContextMono = datasourceContextService.getCachedDatasourceContextMono( - datasourceStorage, spyMockPluginExecutor, monitor, datasourceContextIdentifier); + datasourceStorage, emptyPlugin, spyMockPluginExecutor, monitor, datasourceContextIdentifier); StepVerifier.create(validDatasourceContextMono) .assertNext(validDatasourceContext -> @@ -645,4 +661,66 @@ public class DatasourceContextServiceTest { }) .verifyComplete(); } + + @Test + @WithUserDetails(value = "api_user") + public void verifyDatasourceContextHasRightCredentialsAfterVariableReplacement() { + String datasourceId = "datasourceId"; + String environmentId = "environmentId"; + + Plugin restApiPlugin = pluginService + .findByPackageName(PluginConstants.PackageName.REST_API_PLUGIN) + .block(); + String primaryBearerToken = "bearerToken1"; + BearerTokenAuth authenticationDTO = new BearerTokenAuth(); + authenticationDTO.setBearerToken(primaryBearerToken); + DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setAuthentication(authenticationDTO); + + DatasourceStorage datasourceStorage = new DatasourceStorage(); + datasourceStorage.setDatasourceId(datasourceId); + datasourceStorage.setEnvironmentId(environmentId); + datasourceStorage.setDatasourceConfiguration(datasourceConfiguration); + datasourceStorage.setPluginId(restApiPlugin.getId()); + datasourceStorage.setPluginName(restApiPlugin.getPluginName()); + + MockPluginExecutor mockPluginExecutor = new MockPluginExecutor(); + MockPluginExecutor spyMockPluginExecutor = spy(mockPluginExecutor); + + Mockito.when(pluginExecutorHelper.getPluginExecutor(Mockito.any())) + .thenReturn(Mono.just(spyMockPluginExecutor)); + + doReturn(APIConnectionFactory.createConnection(datasourceStorage.getDatasourceConfiguration())) + .when(spyMockPluginExecutor) + .datasourceCreate(any()); + + Mono> datasourceContextMono = + datasourceContextService.getDatasourceContext(datasourceStorage, restApiPlugin); + StepVerifier.create(datasourceContextMono) + .assertNext(datasourceContext -> { + assertThat(datasourceContext.getConnection()).isInstanceOf(APIConnection.class); + assertThat(((BearerTokenAuthentication) datasourceContext.getConnection()).getBearerToken()) + .isEqualTo(primaryBearerToken); + }) + .verifyComplete(); + + String updatedBearerToken = "bearerToken2"; + BearerTokenAuth bearerTokenAuth = new BearerTokenAuth(); + bearerTokenAuth.setBearerToken(updatedBearerToken); + datasourceStorage.getDatasourceConfiguration().setAuthentication(bearerTokenAuth); + + doReturn(APIConnectionFactory.createConnection(datasourceStorage.getDatasourceConfiguration())) + .when(spyMockPluginExecutor) + .datasourceCreate(any()); + + Mono> datasourceContextMono2 = + datasourceContextService.getDatasourceContext(datasourceStorage, restApiPlugin); + StepVerifier.create(datasourceContextMono) + .assertNext(datasourceContext -> { + assertThat(datasourceContext.getConnection()).isInstanceOf(APIConnection.class); + assertThat(((BearerTokenAuthentication) datasourceContext.getConnection()).getBearerToken()) + .isEqualTo(updatedBearerToken); + }) + .verifyComplete(); + } }