diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java index 45fe4a882efc..c3468404b723 100755 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java @@ -5,6 +5,7 @@ import com.appsmith.external.models.ActionDTO; import com.appsmith.external.models.CreatorContextType; import com.appsmith.external.models.Datasource; +import com.appsmith.external.models.PluginType; import com.appsmith.server.acl.AclPermission; import com.appsmith.server.constants.FieldName; import com.appsmith.server.datasources.base.DatasourceService; @@ -246,11 +247,20 @@ protected Mono updateActionBasedOnContextType(NewAction newAction, Ac return updateSingleAction(newAction.getId(), action) .name(UPDATE_SINGLE_ACTION) .tap(Micrometer.observation(observationRegistry)) - .flatMap(updatedAction -> updateLayoutService - .updatePageLayoutsByPageId(pageId) - .name(UPDATE_PAGE_LAYOUT_BY_PAGE_ID) - .tap(Micrometer.observation(observationRegistry)) - .thenReturn(updatedAction)) + .flatMap(updatedAction -> { + // Update page layout is skipped for JS actions here because when JSobject is updated, we first + // update all actions, action + // collection and then we update the page layout, hence updating page layout with each action update + // is not required here + if (action.getPluginType() != PluginType.JS) { + return updateLayoutService + .updatePageLayoutsByPageId(pageId) + .name(UPDATE_PAGE_LAYOUT_BY_PAGE_ID) + .tap(Micrometer.observation(observationRegistry)) + .thenReturn(updatedAction); + } + return Mono.just(updatedAction); + }) .zipWhen(actionDTO -> newPageService.findPageById(pageId, pagePermission.getEditPermission(), false)) .map(tuple2 -> { ActionDTO actionDTO = tuple2.getT1(); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceTest.java index b13dd94a39c1..f201b0a3bff4 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceTest.java @@ -28,6 +28,7 @@ import com.appsmith.server.dtos.WorkspacePluginStatus; import com.appsmith.server.helpers.MockPluginExecutor; import com.appsmith.server.helpers.PluginExecutorHelper; +import com.appsmith.server.layouts.UpdateLayoutService; import com.appsmith.server.newactions.base.NewActionService; import com.appsmith.server.newpages.base.NewPageService; import com.appsmith.server.plugins.base.PluginService; @@ -47,6 +48,7 @@ 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; @@ -134,6 +136,9 @@ public class ActionCollectionServiceTest { @MockBean PluginExecutor pluginExecutor; + @SpyBean + UpdateLayoutService updateLayoutService; + Application testApp = null; PageDTO testPage = null; @@ -700,4 +705,68 @@ public void testDeleteActionCollection_afterApplicationPublish_clearsActionColle }) .verifyComplete(); } + + @Test + @WithUserDetails(value = "api_user") + public void testUpdateUnpublishedActionCollection_withValidCollection_callsPageLayoutOnlyOnce() { + Mockito.when(pluginExecutorHelper.getPluginExecutor(Mockito.any())).thenReturn(Mono.just(pluginExecutor)); + Mockito.when(pluginExecutor.getHintMessages(Mockito.any(), Mockito.any())) + .thenReturn(Mono.zip(Mono.just(new HashSet<>()), Mono.just(new HashSet<>()))); + + ActionCollectionDTO actionCollectionDTO = new ActionCollectionDTO(); + actionCollectionDTO.setName("testCollection1"); + actionCollectionDTO.setPageId(testPage.getId()); + actionCollectionDTO.setApplicationId(testApp.getId()); + actionCollectionDTO.setWorkspaceId(workspaceId); + actionCollectionDTO.setPluginId(datasource.getPluginId()); + actionCollectionDTO.setVariables(List.of(new JSValue("test", "String", "test", true))); + actionCollectionDTO.setBody("collectionBody"); + actionCollectionDTO.setPluginType(PluginType.JS); + + // Create actions + ActionDTO action1 = new ActionDTO(); + action1.setName("testAction1"); + action1.setActionConfiguration(new ActionConfiguration()); + action1.getActionConfiguration().setBody("mockBody"); + action1.getActionConfiguration().setIsValid(false); + + ActionDTO action2 = new ActionDTO(); + action2.setName("testAction2"); + action2.setActionConfiguration(new ActionConfiguration()); + action2.getActionConfiguration().setBody("mockBody"); + action2.getActionConfiguration().setIsValid(false); + + ActionDTO action3 = new ActionDTO(); + action3.setName("testAction3"); + action3.setActionConfiguration(new ActionConfiguration()); + action3.getActionConfiguration().setBody("mockBody"); + action3.getActionConfiguration().setIsValid(false); + + actionCollectionDTO.setActions(List.of(action1, action2, action3)); + + ActionCollectionDTO createdActionCollectionDTO = + layoutCollectionService.createCollection(actionCollectionDTO).block(); + assert createdActionCollectionDTO != null; + assert createdActionCollectionDTO.getId() != null; + String createdActionCollectionId = createdActionCollectionDTO.getId(); + + applicationPageService.publish(testApp.getId(), true).block(); + + actionCollectionDTO.getActions().get(0).getActionConfiguration().setBody("updatedBody"); + + final Mono updatedActionCollectionDTO = + layoutCollectionService.updateUnpublishedActionCollection( + createdActionCollectionId, actionCollectionDTO); + + StepVerifier.create(updatedActionCollectionDTO) + .assertNext(actionCollectionDTO1 -> { + assertEquals(createdActionCollectionId, actionCollectionDTO1.getId()); + + // This invocation will happen here twice, once during create collection and once during update + // collection as expected + Mockito.verify(updateLayoutService, Mockito.times(2)) + .updatePageLayoutsByPageId(Mockito.anyString()); + }) + .verifyComplete(); + } }