feat: backend changes for updating run behaviour to automatic #40671
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
|
|
||
| import com.appsmith.external.dtos.DslExecutableDTO; | ||
| import com.appsmith.external.dtos.LayoutExecutableUpdateDTO; | ||
| import com.appsmith.external.enums.FeatureFlagEnum; | ||
| import com.appsmith.external.helpers.MustacheHelper; | ||
| import com.appsmith.external.models.ActionDTO; | ||
| import com.appsmith.external.models.CreatorContextType; | ||
|
|
@@ -15,9 +16,11 @@ | |
| import com.appsmith.server.domains.NewPage; | ||
| import com.appsmith.server.exceptions.AppsmithError; | ||
| import com.appsmith.server.exceptions.AppsmithException; | ||
| import com.appsmith.server.helpers.CollectionUtils; | ||
| import com.appsmith.server.helpers.ObservationHelperImpl; | ||
| import com.appsmith.server.onload.executables.ExecutableOnLoadService; | ||
| import com.appsmith.server.services.AstService; | ||
| import com.appsmith.server.services.FeatureFlagService; | ||
| import com.fasterxml.jackson.core.type.TypeReference; | ||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||
| import io.micrometer.observation.ObservationRegistry; | ||
|
|
@@ -90,6 +93,7 @@ public class OnLoadExecutablesUtilCEImpl implements OnLoadExecutablesUtilCE { | |
| private final Set<String> APPSMITH_GLOBAL_VARIABLES = Set.of(); | ||
| private final ObservationRegistry observationRegistry; | ||
| private final ObservationHelperImpl observationHelper; | ||
| private final FeatureFlagService featureFlagService; | ||
|
|
||
| /** | ||
| * This function computes the sequenced on page load executables. | ||
|
|
@@ -320,103 +324,117 @@ public Mono<Boolean> updateExecutablesRunBehaviour( | |
|
|
||
| return existingOnLoadExecutablesMono | ||
| .zipWith(creatorContextExecutablesFlux.collectList()) | ||
| .flatMap(tuple -> featureFlagService | ||
| .check(FeatureFlagEnum.release_reactive_actions_enabled) | ||
| .flatMap(isReactiveActionsEnabled -> { | ||
| List<Executable> existingOnLoadExecutables = tuple.getT1(); | ||
| List<Executable> creatorContextExecutables = tuple.getT2(); | ||
|
|
||
| // 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); | ||
| } | ||
|
|
||
| // 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); | ||
| } | ||
|
|
||
| // Extract names of existing page load actions and new page load actions for quick lookup. | ||
| Set<String> existingOnLoadExecutableNames = existingOnLoadExecutables.stream() | ||
| .map(Executable::getUserExecutableName) | ||
| .collect(Collectors.toSet()); | ||
|
|
||
| Set<String> newOnLoadExecutableNames = onLoadExecutables.stream() | ||
| .map(Executable::getUserExecutableName) | ||
| .collect(Collectors.toSet()); | ||
|
|
||
| // Calculate the actions which would need to be updated from execute on load TRUE to FALSE. | ||
| Set<String> turnedOffExecutableNames = new HashSet<>(); | ||
| turnedOffExecutableNames.addAll(existingOnLoadExecutableNames); | ||
| turnedOffExecutableNames.removeAll(newOnLoadExecutableNames); | ||
|
|
||
| // Calculate the actions which would need to be updated from execute on load FALSE to TRUE | ||
| Set<String> turnedOnExecutableNames = new HashSet<>(); | ||
| turnedOnExecutableNames.addAll(newOnLoadExecutableNames); | ||
| turnedOnExecutableNames.removeAll(existingOnLoadExecutableNames); | ||
|
|
||
| for (Executable executable : creatorContextExecutables) { | ||
|
|
||
| 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 no longer an onload executable, turn the execute on load to | ||
| // false | ||
| if (turnedOffExecutableNames.contains(executableName)) { | ||
| executable.setRunBehaviour(RunBehaviourEnum.MANUAL); | ||
| 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) { | ||
|
Collaborator
There was a problem hiding this comment. Do you plan to include the auto-commit change in this PR?
Contributor
Author
There was a problem hiding this comment. We won't need auto commit changes for this right? We are not introducing new prop now, we are introducing new state for that new prop we added in milestone 1
Collaborator
There was a problem hiding this comment. There is a case which can cause diff on top of the clean state of the app. Let’s assume the app is connected to Git and an action is set to run on page load (
Contributor
Author
There was a problem hiding this comment. That's a very valid concern and thanks for raising this!! @subrata71 @infinitetrooper @pedro-santos-rodrigues Can you please check this edge case and let me know what should happen in this case? I have triggered EE DP here in case you need to test this scenario
Contributor
There was a problem hiding this comment. As far as I understood the scenario you're discussing, it seems that this won't be an issue since a change from Am I missing anything here, or with that point it gets clear the expected behavior?
Collaborator
There was a problem hiding this comment. @pedro-santos-rodrigues Could you please review the scenario I shared? Do you agree that, in this specific case, it would lead to uncommitted changes? If so, do you think this is something we should be concerned about? |
||
| executable.setRunBehaviour(RunBehaviourEnum.AUTOMATIC); | ||
sneha122 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } 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 (!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)); | ||
| })); | ||
| } | ||
|
|
||
| @Override | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.