From dae62816541a900555df58438e3cbc30e5b8585d Mon Sep 17 00:00:00 2001 From: sneha122 Date: Thu, 22 May 2025 15:49:40 +0530 Subject: [PATCH] feat: updated toast messages for on page load actions (#40726) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description This PR: - updates the toast message that we see on add/remove of query binding with widget after the introduction of automatic run behaviour. We have updated the toast messages as per https://miro.com/app/board/uXjVIQ1GcX4=/?moveToWidget=3458764621410290209&cot=14 - Also resolves #40720 **Steps To Reproduce** 1. Create Query1 2. Create Table1 3. Edit Query1 body to be -> SELECT * FROM public."users" LIMIT {{Query2.data.length}}; 4. Attach this query1 to table1 -> We should a toast message saying Query1 will execute automatically on page load Now add Query2 and see the toast message -> It says "Query1, Query2 will execute automatically on page load instead it should just be Query2 in the message Ref: https://github.com/appsmithorg/appsmith/issues/40656#issuecomment-2896912279 Fixes #40720 #40471 _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.Table" ### :mag: Cypress test results > [!TIP] > ๐ŸŸข ๐ŸŸข ๐ŸŸข All cypress tests have passed! ๐ŸŽ‰ ๐ŸŽ‰ ๐ŸŽ‰ > Workflow run: > Commit: 141e2754cf202ee54d7373a0ef3ed0ee1217c876 > Cypress dashboard. > Tags: `@tag.Table` > Spec: >
Thu, 22 May 2025 08:31:09 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No ## Summary by CodeRabbit - **Refactor** - Improved how on-load executables are managed based on a feature flag, dynamically adjusting their run behavior and user messages. - **Tests** - Added new tests to verify correct messaging and behavior updates for executables under different feature flag conditions. Co-authored-by: โ€œsneha122โ€ <โ€œsneha@appsmith.comโ€> --- .../internal/OnLoadExecutablesUtilCEImpl.java | 241 ++++++++++-------- .../OnLoadExecutablesUtilCEImplTest.java | 240 +++++++++++++++++ 2 files changed, 375 insertions(+), 106 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java index 3d8143e2be..d8ba6331e8 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java @@ -312,129 +312,158 @@ public class OnLoadExecutablesUtilCEImpl implements OnLoadExecutablesUtilCE { Flux creatorContextExecutablesFlux = this.getAllExecutablesByCreatorIdFlux(creatorId, creatorType).cache(); - // Before we update the actions, fetch all the actions which are currently set to execute on load. - Mono> existingOnLoadExecutablesMono = creatorContextExecutablesFlux - .flatMap(executable -> { - if (RunBehaviourEnum.ON_PAGE_LOAD.equals(executable.getRunBehaviour())) { - return Mono.just(executable); - } - return Mono.empty(); - }) - .collectList(); + // Check the feature flag first to determine which RunBehaviourEnum to use for comparison + return featureFlagService + .check(FeatureFlagEnum.release_reactive_actions_enabled) + .flatMap(isReactiveActionsEnabled -> { + RunBehaviourEnum runBehaviourToCheck = + isReactiveActionsEnabled ? RunBehaviourEnum.AUTOMATIC : RunBehaviourEnum.ON_PAGE_LOAD; - return existingOnLoadExecutablesMono - .zipWith(creatorContextExecutablesFlux.collectList()) - .flatMap(tuple -> featureFlagService - .check(FeatureFlagEnum.release_reactive_actions_enabled) - .flatMap(isReactiveActionsEnabled -> { - List existingOnLoadExecutables = tuple.getT1(); - List creatorContextExecutables = tuple.getT2(); + // Before we update the actions, fetch all the actions which are currently set to execute on load. + Mono> existingOnLoadExecutablesMono = creatorContextExecutablesFlux + .flatMap(executable -> { + if (runBehaviourToCheck.equals(executable.getRunBehaviour())) { + return Mono.just(executable); + } + return Mono.empty(); + }) + .collectList(); - // There are no actions in this page. No need to proceed further since no actions would get - // updated - if (CollectionUtils.isNullOrEmpty(creatorContextExecutables)) { - return Mono.just(FALSE); - } + return existingOnLoadExecutablesMono + .zipWith(creatorContextExecutablesFlux.collectList()) + .flatMap(tuple -> { + List existingOnLoadExecutables = tuple.getT1(); + List creatorContextExecutables = tuple.getT2(); - // No actions require an update if no actions have been found as page load actions as well - // as - // existing on load page actions are empty - if (CollectionUtils.isNullOrEmpty(existingOnLoadExecutables) - && (onLoadExecutables == null - || CollectionUtils.isNullOrEmpty(onLoadExecutables))) { - return Mono.just(FALSE); - } + // There are no actions in this page. No need to proceed further since no actions would + // get + // updated + if (CollectionUtils.isNullOrEmpty(creatorContextExecutables)) { + return Mono.just(FALSE); + } - // Extract names of existing page load actions and new page load actions for quick lookup. - Set existingOnLoadExecutableNames = existingOnLoadExecutables.stream() - .map(Executable::getUserExecutableName) - .collect(Collectors.toSet()); + // No actions require an update if no actions have been found as page load actions as + // well + // as + // existing on load page actions are empty + if (CollectionUtils.isNullOrEmpty(existingOnLoadExecutables) + && (onLoadExecutables == null + || CollectionUtils.isNullOrEmpty(onLoadExecutables))) { + return Mono.just(FALSE); + } - Set newOnLoadExecutableNames = onLoadExecutables.stream() - .map(Executable::getUserExecutableName) - .collect(Collectors.toSet()); + // Extract names of existing page load actions and new page load actions for quick + // lookup. + Set existingOnLoadExecutableNames = existingOnLoadExecutables.stream() + .map(Executable::getUserExecutableName) + .collect(Collectors.toSet()); - // Calculate the actions which would need to be updated from execute on load TRUE to FALSE. - Set turnedOffExecutableNames = new HashSet<>(); - turnedOffExecutableNames.addAll(existingOnLoadExecutableNames); - turnedOffExecutableNames.removeAll(newOnLoadExecutableNames); + Set newOnLoadExecutableNames = onLoadExecutables.stream() + .map(Executable::getUserExecutableName) + .collect(Collectors.toSet()); - // Calculate the actions which would need to be updated from execute on load FALSE to TRUE - Set turnedOnExecutableNames = new HashSet<>(); - turnedOnExecutableNames.addAll(newOnLoadExecutableNames); - turnedOnExecutableNames.removeAll(existingOnLoadExecutableNames); + // Calculate the actions which would need to be updated from execute on load TRUE to + // FALSE. + Set turnedOffExecutableNames = new HashSet<>(); + turnedOffExecutableNames.addAll(existingOnLoadExecutableNames); + turnedOffExecutableNames.removeAll(newOnLoadExecutableNames); - for (Executable executable : creatorContextExecutables) { + // Calculate the actions which would need to be updated from execute on load FALSE to + // TRUE + Set turnedOnExecutableNames = new HashSet<>(); + turnedOnExecutableNames.addAll(newOnLoadExecutableNames); + turnedOnExecutableNames.removeAll(existingOnLoadExecutableNames); - String executableName = executable.getUserExecutableName(); - // If a user has ever set execute on load, this field can not be changed automatically. - // It has - // to be explicitly changed by the user again. Add the executable to update only if this - // condition is false. - if (FALSE.equals(executable.getUserSetOnLoad())) { + for (Executable executable : creatorContextExecutables) { - // If this executable is no longer an onload executable, turn the execute on load to - // false - if (turnedOffExecutableNames.contains(executableName)) { - executable.setRunBehaviour(RunBehaviourEnum.MANUAL); - toUpdateExecutables.add(executable); - } + String executableName = executable.getUserExecutableName(); + // If a user has ever set execute on load, this field can not be changed + // automatically. + // It has + // to be explicitly changed by the user again. Add the executable to update only if + // this + // condition is false. + if (FALSE.equals(executable.getUserSetOnLoad())) { - // If this executable is newly found to be on load, turn execute on load to true - if (turnedOnExecutableNames.contains(executableName)) { - // Use the prefetched feature flag value - if (isReactiveActionsEnabled) { - executable.setRunBehaviour(RunBehaviourEnum.AUTOMATIC); - } else { - executable.setRunBehaviour(RunBehaviourEnum.ON_PAGE_LOAD); + // If this executable is no longer an onload executable, turn the execute on + // load to + // false + if (turnedOffExecutableNames.contains(executableName)) { + executable.setRunBehaviour(RunBehaviourEnum.MANUAL); + toUpdateExecutables.add(executable); } - toUpdateExecutables.add(executable); + + // If this executable is newly found to be on load, turn execute on load to true + if (turnedOnExecutableNames.contains(executableName)) { + // Use the prefetched feature flag value + if (isReactiveActionsEnabled) { + executable.setRunBehaviour(RunBehaviourEnum.AUTOMATIC); + } else { + executable.setRunBehaviour(RunBehaviourEnum.ON_PAGE_LOAD); + } + toUpdateExecutables.add(executable); + } + + } else { + // Remove the executable name from either of the lists (if present) because this + // executable + // should not be updated + turnedOnExecutableNames.remove(executableName); + turnedOffExecutableNames.remove(executableName); + } + } + + // Add newly turned on page actions to report back to the caller + executableUpdatesRef.addAll(addExecutableUpdatesForExecutableNames( + creatorContextExecutables, turnedOnExecutableNames)); + + // Add newly turned off page actions to report back to the caller + executableUpdatesRef.addAll(addExecutableUpdatesForExecutableNames( + creatorContextExecutables, turnedOffExecutableNames)); + + for (Executable executable : creatorContextExecutables) { + String executableName = executable.getUserExecutableName(); + if (Boolean.FALSE.equals(executable.isOnLoadMessageAllowed())) { + turnedOffExecutableNames.remove(executableName); + turnedOnExecutableNames.remove(executableName); + } + } + + // Now add messagesRef that would eventually be displayed to the developer user + // informing + // them + // about the action setting change. + if (isReactiveActionsEnabled) { + if (!turnedOffExecutableNames.isEmpty()) { + messagesRef.add( + turnedOffExecutableNames.toString() + + " will no longer run automatically. You can run it manually when needed."); } + if (!turnedOnExecutableNames.isEmpty()) { + messagesRef.add( + turnedOnExecutableNames.toString() + + " will run automatically on page load or when a variable it depends on changes"); + } } else { - // Remove the executable name from either of the lists (if present) because this - // executable - // should not be updated - turnedOnExecutableNames.remove(executableName); - turnedOffExecutableNames.remove(executableName); + if (!turnedOffExecutableNames.isEmpty()) { + messagesRef.add(turnedOffExecutableNames.toString() + + " will no longer be executed on page load"); + } + + if (!turnedOnExecutableNames.isEmpty()) { + messagesRef.add(turnedOnExecutableNames.toString() + + " will be executed automatically on page load"); + } } - } - // Add newly turned on page actions to report back to the caller - executableUpdatesRef.addAll(addExecutableUpdatesForExecutableNames( - creatorContextExecutables, turnedOnExecutableNames)); - - // Add newly turned off page actions to report back to the caller - executableUpdatesRef.addAll(addExecutableUpdatesForExecutableNames( - creatorContextExecutables, turnedOffExecutableNames)); - - for (Executable executable : creatorContextExecutables) { - String executableName = executable.getUserExecutableName(); - if (Boolean.FALSE.equals(executable.isOnLoadMessageAllowed())) { - turnedOffExecutableNames.remove(executableName); - turnedOnExecutableNames.remove(executableName); - } - } - - // Now add messagesRef that would eventually be displayed to the developer user informing - // them - // about the action setting change. - if (!turnedOffExecutableNames.isEmpty()) { - messagesRef.add(turnedOffExecutableNames.toString() - + " will no longer be executed on page load"); - } - - if (!turnedOnExecutableNames.isEmpty()) { - messagesRef.add(turnedOnExecutableNames.toString() - + " will be executed automatically on page load"); - } - - // Finally update the actions which require an update - return Flux.fromIterable(toUpdateExecutables) - .flatMap(executable -> this.updateUnpublishedExecutable( - executable.getId(), executable, creatorType)) - .then(Mono.just(TRUE)); - })); + // Finally update the actions which require an update + return Flux.fromIterable(toUpdateExecutables) + .flatMap(executable -> this.updateUnpublishedExecutable( + executable.getId(), executable, creatorType)) + .then(Mono.just(TRUE)); + }); + }); } @Override diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImplTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImplTest.java index 8ab3205d3b..dae9acada6 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImplTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImplTest.java @@ -236,6 +236,246 @@ public class OnLoadExecutablesUtilCEImplTest { verify(executableOnLoadService, times(0)).updateUnpublishedExecutable(anyString(), any(ActionDTO.class)); } + @Test + public void whenFeatureFlagOn_andExecutableTurnedOn_shouldShowReactiveMessage() { + // Setup + ActionDTO existingAction = new ActionDTO(); + existingAction.setName("TestApi"); + existingAction.setUserSetOnLoad(false); + existingAction.setRunBehaviour(RunBehaviourEnum.MANUAL); + existingAction.setId("1"); + + ActionDTO updatedAction = new ActionDTO(); + updatedAction.setName("TestApi"); + updatedAction.setUserSetOnLoad(false); + updatedAction.setRunBehaviour(RunBehaviourEnum.AUTOMATIC); + updatedAction.setId("1"); + + List onLoadExecutables = List.of(updatedAction); + List executableUpdates = new ArrayList<>(); + List messages = new ArrayList<>(); + + when(executableOnLoadService.getAllExecutablesByCreatorIdFlux(any())).thenReturn(Flux.just(existingAction)); + when(featureFlagService.check(any())).thenReturn(Mono.just(true)); + when(executableOnLoadService.updateUnpublishedExecutable(eq("1"), any())) + .thenReturn(Mono.just(updatedAction)); + + // Execute and verify + StepVerifier.create(onLoadExecutablesUtilCE.updateExecutablesRunBehaviour( + onLoadExecutables, "creatorId", executableUpdates, messages, CreatorContextType.PAGE)) + .expectNext(true) + .verifyComplete(); + + // Assert + assert messages.size() == 1; + assert messages.get(0).contains("TestApi"); + assert messages.get(0).contains("will run automatically on page load or when a variable it depends on changes"); + } + + @Test + public void whenFeatureFlagOff_andExecutableTurnedOn_shouldShowPageLoadMessage() { + // Setup + ActionDTO existingAction = new ActionDTO(); + existingAction.setName("TestApi"); + existingAction.setUserSetOnLoad(false); + existingAction.setRunBehaviour(RunBehaviourEnum.MANUAL); + existingAction.setId("1"); + + ActionDTO updatedAction = new ActionDTO(); + updatedAction.setName("TestApi"); + updatedAction.setUserSetOnLoad(false); + updatedAction.setRunBehaviour(RunBehaviourEnum.ON_PAGE_LOAD); + updatedAction.setId("1"); + + List onLoadExecutables = List.of(updatedAction); + List executableUpdates = new ArrayList<>(); + List messages = new ArrayList<>(); + + when(executableOnLoadService.getAllExecutablesByCreatorIdFlux(any())).thenReturn(Flux.just(existingAction)); + when(featureFlagService.check(any())).thenReturn(Mono.just(false)); + when(executableOnLoadService.updateUnpublishedExecutable(eq("1"), any())) + .thenReturn(Mono.just(updatedAction)); + + // Execute and verify + StepVerifier.create(onLoadExecutablesUtilCE.updateExecutablesRunBehaviour( + onLoadExecutables, "creatorId", executableUpdates, messages, CreatorContextType.PAGE)) + .expectNext(true) + .verifyComplete(); + + // Assert + assert messages.size() == 1; + assert messages.get(0).contains("TestApi"); + assert messages.get(0).contains("will be executed automatically on page load"); + } + + @Test + public void whenFeatureFlagOn_andExecutableTurnedOff_shouldShowManualMessage() { + // Setup + ActionDTO existingAction = new ActionDTO(); + existingAction.setName("TestApi"); + existingAction.setUserSetOnLoad(false); + existingAction.setRunBehaviour(RunBehaviourEnum.AUTOMATIC); + existingAction.setId("1"); + + ActionDTO updatedAction = new ActionDTO(); + updatedAction.setName("TestApi"); + updatedAction.setUserSetOnLoad(false); + updatedAction.setRunBehaviour(RunBehaviourEnum.MANUAL); + updatedAction.setId("1"); + + List onLoadExecutables = new ArrayList<>(); // Empty list means turning off + List executableUpdates = new ArrayList<>(); + List messages = new ArrayList<>(); + + when(executableOnLoadService.getAllExecutablesByCreatorIdFlux(any())).thenReturn(Flux.just(existingAction)); + when(featureFlagService.check(any())).thenReturn(Mono.just(true)); + when(executableOnLoadService.updateUnpublishedExecutable(eq("1"), any())) + .thenReturn(Mono.just(updatedAction)); + + // Execute and verify + StepVerifier.create(onLoadExecutablesUtilCE.updateExecutablesRunBehaviour( + onLoadExecutables, "creatorId", executableUpdates, messages, CreatorContextType.PAGE)) + .expectNext(true) + .verifyComplete(); + + // Assert + assert messages.size() == 1; + assert messages.get(0).contains("TestApi"); + assert messages.get(0).contains("will no longer run automatically"); + } + + @Test + public void whenOnlyOneActionChange_shouldShowOnlyThatEntityInToastMessage() { + // Setup + ActionDTO existingAction1 = new ActionDTO(); + existingAction1.setName("Api1"); + existingAction1.setUserSetOnLoad(false); + existingAction1.setRunBehaviour(RunBehaviourEnum.AUTOMATIC); + existingAction1.setId("1"); + + ActionDTO existingAction2 = new ActionDTO(); + existingAction2.setName("Api2"); + existingAction2.setUserSetOnLoad(false); + existingAction2.setRunBehaviour(RunBehaviourEnum.MANUAL); + existingAction2.setId("2"); + + ActionDTO unchangedAction1 = new ActionDTO(); + unchangedAction1.setName("Api1"); + unchangedAction1.setUserSetOnLoad(false); + unchangedAction1.setRunBehaviour(RunBehaviourEnum.AUTOMATIC); + unchangedAction1.setId("1"); + + ActionDTO updatedAction2 = new ActionDTO(); + updatedAction2.setName("Api2"); + updatedAction2.setUserSetOnLoad(false); + updatedAction2.setRunBehaviour(RunBehaviourEnum.AUTOMATIC); + updatedAction2.setId("2"); + + List onLoadExecutables = + List.of(updatedAction2, unchangedAction1); // Only Api2 is in the onLoad list + List executableUpdates = new ArrayList<>(); + List messages = new ArrayList<>(); + + when(executableOnLoadService.getAllExecutablesByCreatorIdFlux(any())) + .thenReturn(Flux.just(existingAction1, existingAction2)); + when(featureFlagService.check(any())).thenReturn(Mono.just(true)); + + // Mock different behaviors for different executable IDs + when(executableOnLoadService.updateUnpublishedExecutable(eq("2"), any())) + .thenReturn(Mono.just(updatedAction2)); + + // Execute and verify + StepVerifier.create(onLoadExecutablesUtilCE.updateExecutablesRunBehaviour( + onLoadExecutables, "creatorId", executableUpdates, messages, CreatorContextType.PAGE)) + .expectNext(true) + .verifyComplete(); + + // Assert + assert messages.size() == 1; + assert messages.stream() + .anyMatch(msg -> msg.contains("Api2") + && msg.contains("will run automatically on page load or when a variable it depends on changes") + && !msg.contains("Api1")); + } + + @Test + public void whenMultipleExecutablesChange_shouldShowAllMessages() { + // Setup + ActionDTO existingAction1 = new ActionDTO(); + existingAction1.setName("Api1"); + existingAction1.setUserSetOnLoad(false); + existingAction1.setRunBehaviour(RunBehaviourEnum.AUTOMATIC); + existingAction1.setId("1"); + + ActionDTO existingAction2 = new ActionDTO(); + existingAction2.setName("Api2"); + existingAction2.setUserSetOnLoad(false); + existingAction2.setRunBehaviour(RunBehaviourEnum.MANUAL); + existingAction2.setId("2"); + + ActionDTO updatedAction1 = new ActionDTO(); + updatedAction1.setName("Api1"); + updatedAction1.setUserSetOnLoad(false); + updatedAction1.setRunBehaviour(RunBehaviourEnum.MANUAL); + updatedAction1.setId("1"); + + ActionDTO updatedAction2 = new ActionDTO(); + updatedAction2.setName("Api2"); + updatedAction2.setUserSetOnLoad(false); + updatedAction2.setRunBehaviour(RunBehaviourEnum.AUTOMATIC); + updatedAction2.setId("2"); + + List onLoadExecutables = List.of(updatedAction2); // Only Api2 is in the onLoad list + List executableUpdates = new ArrayList<>(); + List messages = new ArrayList<>(); + + when(executableOnLoadService.getAllExecutablesByCreatorIdFlux(any())) + .thenReturn(Flux.just(existingAction1, existingAction2)); + when(featureFlagService.check(any())).thenReturn(Mono.just(true)); + + // Mock different behaviors for different executable IDs + when(executableOnLoadService.updateUnpublishedExecutable(eq("1"), any())) + .thenReturn(Mono.just(updatedAction1)); + when(executableOnLoadService.updateUnpublishedExecutable(eq("2"), any())) + .thenReturn(Mono.just(updatedAction2)); + + // Execute and verify + StepVerifier.create(onLoadExecutablesUtilCE.updateExecutablesRunBehaviour( + onLoadExecutables, "creatorId", executableUpdates, messages, CreatorContextType.PAGE)) + .expectNext(true) + .verifyComplete(); + + // Assert + assert messages.size() == 2; + assert messages.stream() + .anyMatch(msg -> msg.contains("Api1") && msg.contains("will no longer run automatically")); + assert messages.stream() + .anyMatch(msg -> msg.contains("Api2") + && msg.contains( + "will run automatically on page load or when a variable it depends on changes")); + } + + @Test + public void whenNoExecutables_shouldReturnFalse() { + // Setup + List onLoadExecutables = new ArrayList<>(); + List executableUpdates = new ArrayList<>(); + List messages = new ArrayList<>(); + + when(executableOnLoadService.getAllExecutablesByCreatorIdFlux(any())).thenReturn(Flux.empty()); + when(featureFlagService.check(any())).thenReturn(Mono.just(true)); + + // Execute and verify + StepVerifier.create(onLoadExecutablesUtilCE.updateExecutablesRunBehaviour( + onLoadExecutables, "creatorId", executableUpdates, messages, CreatorContextType.PAGE)) + .expectNext(false) + .verifyComplete(); + + // Assert + assert messages.isEmpty(); + assert executableUpdates.isEmpty(); + } + // Helper methods to create test executables private List createTestExecutables(String... names) { List executables = new ArrayList<>();