From 785dfdd55ac11a94455f92180ed146e34a8376c5 Mon Sep 17 00:00:00 2001 From: sneha122 Date: Fri, 30 May 2025 16:12:46 +0530 Subject: [PATCH] fix: generate crud run behaviour updates (#40792) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description This PR ensures reactivity aspects are enabled for generate page from data flow as well. With these changes whenever we generate a page from data, we need to check the reactivity feature flag: if it's enabled, generated actions which are bound to widgets become automatic. If it's disabled, generated actions which are bound to widgets become page load. **Steps to test** 1. On the DP, sign up with any @integration-appsmith.com email domain and generate page from data, check the select query for its run behaviour -> It should be automatic 2. On the DP, sign up with any other email and generate page from data, check the select query for its run behaviour -> It should be page load Fixes #40789 _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.Sanity" ### :mag: Cypress test results > [!TIP] > ๐ŸŸข ๐ŸŸข ๐ŸŸข All cypress tests have passed! ๐ŸŽ‰ ๐ŸŽ‰ ๐ŸŽ‰ > Workflow run: > Commit: e07e20226a64406a281eb1ad6cf1277bd46186ae > Cypress dashboard. > Tags: `@tag.Sanity` > Spec: >
Fri, 30 May 2025 08:37:41 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - The run behavior of certain actions (SelectQuery, FindQuery, ListFiles) when creating CRUD pages from database tables now adapts based on a feature flag. If enabled, actions run automatically; if not, they run on page load as before. - **Tests** - Added new tests to verify correct action run behavior for both enabled and disabled feature flag scenarios. --------- Co-authored-by: โ€œsneha122โ€ <โ€œsneha@appsmith.comโ€> --- .../CreateDBTablePageSolutionImpl.java | 7 +- .../ce/CreateDBTablePageSolutionCEImpl.java | 64 +++++---- .../CreateDBTablePageSolutionTests.java | 133 ++++++++++++++++++ 3 files changed, 178 insertions(+), 26 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/CreateDBTablePageSolutionImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/CreateDBTablePageSolutionImpl.java index de21fb2774..71eeb0381c 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/CreateDBTablePageSolutionImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/CreateDBTablePageSolutionImpl.java @@ -10,6 +10,7 @@ import com.appsmith.server.newpages.base.NewPageService; import com.appsmith.server.plugins.base.PluginService; import com.appsmith.server.services.AnalyticsService; import com.appsmith.server.services.ApplicationPageService; +import com.appsmith.server.services.FeatureFlagService; import com.appsmith.server.services.LayoutActionService; import com.appsmith.server.services.SessionUserService; import com.appsmith.server.solutions.ce.CreateDBTablePageSolutionCEImpl; @@ -37,7 +38,8 @@ public class CreateDBTablePageSolutionImpl extends CreateDBTablePageSolutionCEIm PagePermission pagePermission, DatasourceStructureSolution datasourceStructureSolution, EnvironmentPermission environmentPermission, - JsonSchemaMigration jsonSchemaMigration) { + JsonSchemaMigration jsonSchemaMigration, + FeatureFlagService featureFlagService) { super( datasourceService, datasourceStorageService, @@ -55,6 +57,7 @@ public class CreateDBTablePageSolutionImpl extends CreateDBTablePageSolutionCEIm pagePermission, datasourceStructureSolution, environmentPermission, - jsonSchemaMigration); + jsonSchemaMigration, + featureFlagService); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/CreateDBTablePageSolutionCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/CreateDBTablePageSolutionCEImpl.java index 1b131a27a3..732ae192b4 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/CreateDBTablePageSolutionCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/CreateDBTablePageSolutionCEImpl.java @@ -4,6 +4,7 @@ import com.appsmith.external.constants.ActionCreationSourceTypeEnum; import com.appsmith.external.constants.AnalyticsEvents; import com.appsmith.external.converters.HttpMethodConverter; import com.appsmith.external.converters.ISOStringToInstantConverter; +import com.appsmith.external.enums.FeatureFlagEnum; import com.appsmith.external.git.constants.ce.RefType; import com.appsmith.external.helpers.AppsmithBeanUtils; import com.appsmith.external.models.ActionConfiguration; @@ -39,6 +40,7 @@ import com.appsmith.server.newpages.base.NewPageService; import com.appsmith.server.plugins.base.PluginService; import com.appsmith.server.services.AnalyticsService; import com.appsmith.server.services.ApplicationPageService; +import com.appsmith.server.services.FeatureFlagService; import com.appsmith.server.services.LayoutActionService; import com.appsmith.server.services.SessionUserService; import com.appsmith.server.solutions.ApplicationPermission; @@ -96,6 +98,7 @@ public class CreateDBTablePageSolutionCEImpl implements CreateDBTablePageSolutio private final DatasourceStructureSolution datasourceStructureSolution; private final EnvironmentPermission environmentPermission; private final JsonSchemaMigration jsonSchemaMigration; + private final FeatureFlagService featureFlagService; private static final String FILE_PATH = "CRUD-DB-Table-Template-Application.json"; @@ -441,30 +444,43 @@ public class CreateDBTablePageSolutionCEImpl implements CreateDBTablePageSolutio String tableNameInAction = tuple.getT5(); String savedPageId = tuple.getT6(); log.debug("Going to clone actions from template application for page {}", savedPageId); - return cloneActionsFromTemplateApplication( - datasourceStorage, - tableNameInAction, - savedPageId, - templateActionList, - mappedColumnsAndTableName, - deletedWidgets, - pluginSpecificParams, - templateAutogeneratedKey.toString()) - .flatMap(actionDTO -> StringUtils.equals(actionDTO.getName(), SELECT_QUERY) - || StringUtils.equals(actionDTO.getName(), FIND_QUERY) - || StringUtils.equals(actionDTO.getName(), LIST_QUERY) - ? layoutActionService.setRunBehaviour( - actionDTO.getId(), RunBehaviourEnum.ON_PAGE_LOAD) - : Mono.just(actionDTO)) - .then(applicationPageService - .getPage(savedPageId, false) - .flatMap(pageDTO -> { - CRUDPageResponseDTO crudPage = new CRUDPageResponseDTO(); - crudPage.setPage(pageDTO); - createSuccessMessageAndSetAsset(plugin, crudPage); - return sendGenerateCRUDPageAnalyticsEvent( - crudPage, datasourceStorage, plugin.getName()); - })); + return featureFlagService + .check(FeatureFlagEnum.release_reactive_actions_enabled) + .flatMap(isEnabled -> { + RunBehaviourEnum runBehaviour = (isEnabled != null && isEnabled) + ? RunBehaviourEnum.AUTOMATIC + : RunBehaviourEnum.ON_PAGE_LOAD; + return cloneActionsFromTemplateApplication( + datasourceStorage, + tableNameInAction, + savedPageId, + templateActionList, + mappedColumnsAndTableName, + deletedWidgets, + pluginSpecificParams, + templateAutogeneratedKey.toString()) + .flatMap(actionDTO -> { + if (StringUtils.equals(actionDTO.getName(), SELECT_QUERY) + || StringUtils.equals(actionDTO.getName(), FIND_QUERY) + || StringUtils.equals(actionDTO.getName(), LIST_QUERY)) { + // Use the previously fetched flag value + // This preserves the contract and documents the intention + return layoutActionService.setRunBehaviour( + actionDTO.getId(), runBehaviour); + } else { + return Mono.just(actionDTO); + } + }) + .then(applicationPageService + .getPage(savedPageId, false) + .flatMap(pageDTO -> { + CRUDPageResponseDTO crudPage = new CRUDPageResponseDTO(); + crudPage.setPage(pageDTO); + createSuccessMessageAndSetAsset(plugin, crudPage); + return sendGenerateCRUDPageAnalyticsEvent( + crudPage, datasourceStorage, plugin.getName()); + })); + }); }); } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/CreateDBTablePageSolutionTests.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/CreateDBTablePageSolutionTests.java index 01e128f933..b86f0064d5 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/CreateDBTablePageSolutionTests.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/CreateDBTablePageSolutionTests.java @@ -1,5 +1,6 @@ package com.appsmith.server.solutions; +import com.appsmith.external.enums.FeatureFlagEnum; import com.appsmith.external.git.constants.ce.RefType; import com.appsmith.external.models.ActionConfiguration; import com.appsmith.external.models.ActionDTO; @@ -40,6 +41,7 @@ import com.appsmith.server.newpages.base.NewPageService; import com.appsmith.server.repositories.PluginRepository; import com.appsmith.server.services.ApplicationPageService; import com.appsmith.server.services.DatasourceStructureService; +import com.appsmith.server.services.FeatureFlagService; import com.appsmith.server.services.WorkspaceService; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang.StringUtils; @@ -50,6 +52,7 @@ import org.mockito.Mockito; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.boot.test.mock.mockito.SpyBean; import org.springframework.security.test.context.support.WithUserDetails; import org.springframework.test.annotation.DirtiesContext; import reactor.core.publisher.Mono; @@ -67,6 +70,7 @@ import static com.appsmith.server.constants.ArtifactType.APPLICATION; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.spy; @Slf4j @@ -176,6 +180,9 @@ public class CreateDBTablePageSolutionTests { private final CRUDPageResourceDTO resource = new CRUDPageResourceDTO(); + @SpyBean + private FeatureFlagService featureFlagService; + @BeforeEach public void setup() { @@ -694,6 +701,132 @@ public class CreateDBTablePageSolutionTests { .verifyComplete(); } + @Test + @WithUserDetails(value = "api_user") + public void createPageWithValidPageIdForMySqlDS_FeatureFlagEnabled() { + doReturn(Mono.just(true)).when(featureFlagService).check(FeatureFlagEnum.release_reactive_actions_enabled); + + resource.setApplicationId(testApp.getId()); + PageDTO newPage = new PageDTO(); + newPage.setApplicationId(testApp.getId()); + newPage.setName("crud-admin-page-mysql-ff-enabled"); + StringBuilder pluginName = new StringBuilder(); + + Mono datasourceMono = pluginRepository.findByName("MySQL").flatMap(plugin -> { + pluginName.append(plugin.getName()); + Datasource datasource = new Datasource(); + datasource.setPluginId(plugin.getId()); + datasource.setWorkspaceId(testWorkspace.getId()); + datasource.setName("MySql-CRUD-Page-Table-DS-FF-Enabled"); + + HashMap storages = new HashMap<>(); + storages.put( + testDefaultEnvironmentId, + new DatasourceStorageDTO(null, testDefaultEnvironmentId, datasourceConfiguration)); + datasource.setDatasourceStorages(storages); + + return datasourceService.create(datasource).flatMap(datasource1 -> { + DatasourceStorageStructure datasourceStorageStructure = new DatasourceStorageStructure(); + datasourceStorageStructure.setDatasourceId(datasource1.getId()); + datasourceStorageStructure.setEnvironmentId(testDefaultEnvironmentId); + datasourceStorageStructure.setStructure(structure); + + return datasourceStructureService + .save(datasourceStorageStructure) + .thenReturn(datasource1); + }); + }); + + Mono resultMono = datasourceMono + .flatMap(datasource1 -> { + resource.setDatasourceId(datasource1.getId()); + return applicationPageService.createPage(newPage); + }) + .flatMap(savedPage -> + solution.createPageFromDBTable(savedPage.getId(), resource, testDefaultEnvironmentId)); + + StepVerifier.create(resultMono.zipWhen(crudPageResponseDTO -> + getActions(crudPageResponseDTO.getPage().getId()))) + .assertNext(tuple -> { + CRUDPageResponseDTO crudPageResponseDTO = tuple.getT1(); + List actions = tuple.getT2(); + for (NewAction action : actions) { + String name = action.getUnpublishedAction().getName(); + if (SELECT_QUERY.equals(name) || FIND_QUERY.equals(name) || LIST_QUERY.equals(name)) { + assertThat(action.getUnpublishedAction().getRunBehaviour()) + .isEqualTo(RunBehaviourEnum.AUTOMATIC); + } else { + assertThat(action.getUnpublishedAction().getRunBehaviour()) + .isEqualTo(RunBehaviourEnum.MANUAL); + } + } + }) + .verifyComplete(); + } + + @Test + @WithUserDetails(value = "api_user") + public void createPageWithValidPageIdForMySqlDS_FeatureFlagDisabled() { + doReturn(Mono.just(false)).when(featureFlagService).check(FeatureFlagEnum.release_reactive_actions_enabled); + + resource.setApplicationId(testApp.getId()); + PageDTO newPage = new PageDTO(); + newPage.setApplicationId(testApp.getId()); + newPage.setName("crud-admin-page-mysql-ff-disabled"); + StringBuilder pluginName = new StringBuilder(); + + Mono datasourceMono = pluginRepository.findByName("MySQL").flatMap(plugin -> { + pluginName.append(plugin.getName()); + Datasource datasource = new Datasource(); + datasource.setPluginId(plugin.getId()); + datasource.setWorkspaceId(testWorkspace.getId()); + datasource.setName("MySql-CRUD-Page-Table-DS-FF-Disabled"); + + HashMap storages = new HashMap<>(); + storages.put( + testDefaultEnvironmentId, + new DatasourceStorageDTO(null, testDefaultEnvironmentId, datasourceConfiguration)); + datasource.setDatasourceStorages(storages); + + return datasourceService.create(datasource).flatMap(datasource1 -> { + DatasourceStorageStructure datasourceStorageStructure = new DatasourceStorageStructure(); + datasourceStorageStructure.setDatasourceId(datasource1.getId()); + datasourceStorageStructure.setEnvironmentId(testDefaultEnvironmentId); + datasourceStorageStructure.setStructure(structure); + + return datasourceStructureService + .save(datasourceStorageStructure) + .thenReturn(datasource1); + }); + }); + + Mono resultMono = datasourceMono + .flatMap(datasource1 -> { + resource.setDatasourceId(datasource1.getId()); + return applicationPageService.createPage(newPage); + }) + .flatMap(savedPage -> + solution.createPageFromDBTable(savedPage.getId(), resource, testDefaultEnvironmentId)); + + StepVerifier.create(resultMono.zipWhen(crudPageResponseDTO -> + getActions(crudPageResponseDTO.getPage().getId()))) + .assertNext(tuple -> { + CRUDPageResponseDTO crudPageResponseDTO = tuple.getT1(); + List actions = tuple.getT2(); + for (NewAction action : actions) { + String name = action.getUnpublishedAction().getName(); + if (SELECT_QUERY.equals(name) || FIND_QUERY.equals(name) || LIST_QUERY.equals(name)) { + assertThat(action.getUnpublishedAction().getRunBehaviour()) + .isEqualTo(RunBehaviourEnum.ON_PAGE_LOAD); + } else { + assertThat(action.getUnpublishedAction().getRunBehaviour()) + .isEqualTo(RunBehaviourEnum.MANUAL); + } + } + }) + .verifyComplete(); + } + @Test @WithUserDetails(value = "api_user") public void createPageWithValidPageIdForRedshiftDS() {