chore: revert git rebase with discard (#24479)

Due to one of the corrupted app state in release, we are going to revert
this and retest it and release later.

https://theappsmith.slack.com/archives/CTHN8GX5Y/p1686637493386649

app/client/cypress/e2e/Regression/ClientSide/Git/GitDiscardChange/DiscardChanges_spec.js

@@ -226,7 +226,7 @@ describe("Git discard changes:", function () {
     _.agHelper
       .GetElement(gitSyncLocators.discardChanges)
       .children()
-      .should("have.text", "Discard & pull");
+      .should("have.text", "Discard changes");
     _.agHelper.GetNClick(gitSyncLocators.discardChanges);
     _.agHelper.AssertContains(
       Cypress.env("MESSAGES").DISCARD_CHANGES_WARNING(),

app/client/cypress/support/gitSync.js

@@ -412,7 +412,7 @@ Cypress.Commands.add("gitDiscardChanges", () => {
   //cy.wait(6000);
   cy.get(gitSyncLocators.discardChanges)
     .children()
-    .should("have.text", "Discard & pull");
+    .should("have.text", "Discard changes");
 
   cy.get(gitSyncLocators.discardChanges).click();
   cy.contains(Cypress.env("MESSAGES").DISCARD_CHANGES_WARNING());

app/client/src/api/GitSyncAPI.tsx

@@ -172,8 +172,10 @@ class GitSyncAPI extends Api {
     });
   }
 
-  static discardChanges(applicationId: string) {
+  static discardChanges(applicationId: string, doPull: boolean) {
-    return Api.put(`${GitSyncAPI.baseURL}/discard/app/${applicationId}`);
+    return Api.put(
+      `${GitSyncAPI.baseURL}/discard/app/${applicationId}?doPull=${doPull}`,
+    );
   }
 }
 

app/client/src/ce/constants/messages.test.ts

@@ -294,8 +294,7 @@ describe("messages without input", () => {
     { key: "MERGED_SUCCESSFULLY", value: "Merged successfully" },
     {
       key: "DISCARD_CHANGES_WARNING",
-      value:
+      value: "Discarding these changes will pull previous changes from Git.",
-        "This action will replace your local changes with the latest remote version.",
     },
     {
       key: "DISCARD_SUCCESS",
@@ -311,7 +310,7 @@ describe("messages without input", () => {
     },
     {
       key: "DISCARD_CHANGES",
-      value: "Discard & pull",
+      value: "Discard changes",
     },
     {
       key: "IMPORTING_APP_FROM_GIT",

app/client/src/ce/constants/messages.ts

@@ -874,8 +874,8 @@ export const CONNECTING_TO_REPO_DISABLED = () =>
 export const DURING_ONBOARDING_TOUR = () => "during the onboarding tour";
 export const MERGED_SUCCESSFULLY = () => "Merged successfully";
 export const DISCARD_CHANGES_WARNING = () =>
-  "This action will replace your local changes with the latest remote version.";
+  "Discarding these changes will pull previous changes from Git.";
-export const DISCARD_CHANGES = () => "Discard & pull";
+export const DISCARD_CHANGES = () => "Discard changes";
 
 // GIT DEPLOY begin
 export const DEPLOY = () => "Deploy";
@@ -891,8 +891,6 @@ export const CHANGES_USER_AND_MIGRATION = () =>
   "Appsmith update and user changes since last commit";
 export const CURRENT_PAGE_DISCARD_WARNING = (page: string) =>
   `Current page (${page}) is in the discard list.`;
-export const DISCARD_MESSAGE = () =>
-  `Some changes may reappear after discarding them, these changes support new features in Appsmith. You can safely commit them to your repository.`;
 // GIT DEPLOY end
 
 // GIT CHANGE LIST begin

app/client/src/pages/Editor/gitSync/Tabs/Deploy.tsx

@@ -25,6 +25,7 @@ import {
 } from "design-system";
 import {
   getConflictFoundDocUrlDeploy,
+  getDiscardDocUrl,
   getGitCommitAndPushError,
   getGitDiscardError,
   getGitStatus,
@@ -112,6 +113,7 @@ function Deploy() {
   const pullFailed = useSelector(getPullFailed);
   const commitInputRef = useRef<HTMLInputElement>(null);
   const upstreamErrorDocumentUrl = useSelector(getUpstreamErrorDocUrl);
+  const discardDocUrl = useSelector(getDiscardDocUrl);
   const [commitMessage, setCommitMessage] = useState(
     gitMetaData?.remoteUrl && lastDeployedAt ? "" : FIRST_COMMIT,
   );
@@ -408,6 +410,7 @@ function Deploy() {
 
           {showDiscardWarning && (
             <DiscardChangesWarning
+              discardDocUrl={discardDocUrl}
               onCloseDiscardChangesWarning={onCloseDiscardWarning}
             />
           )}

app/client/src/pages/Editor/gitSync/components/DiscardChangesWarning.tsx

@@ -1,8 +1,8 @@
 import React from "react";
 import {
   createMessage,
+  CURRENT_PAGE_DISCARD_WARNING,
   DISCARD_CHANGES_WARNING,
-  DISCARD_MESSAGE,
 } from "@appsmith/constants/messages";
 import { useSelector } from "react-redux";
 import { getCurrentPageName } from "selectors/editorSelectors";
@@ -15,10 +15,9 @@ const Container = styled.div`
 `;
 
 export default function DiscardChangesWarning({
+  discardDocUrl,
   onCloseDiscardChangesWarning,
 }: any) {
-  const discardDocUrl =
-    "https://docs.appsmith.com/advanced-concepts/version-control-with-git/commit-and-push";
   const currentPageName = useSelector(getCurrentPageName) || "";
   const modifiedPageList = useSelector(getGitStatus)?.modified.map(
     (page: string) => page.toLocaleLowerCase(),
@@ -44,7 +43,9 @@ export default function DiscardChangesWarning({
       >
         <Text kind="heading-xs">{createMessage(DISCARD_CHANGES_WARNING)}</Text>
         <br />
-        {isCurrentPageDiscardable ? createMessage(DISCARD_MESSAGE) : null}
+        {isCurrentPageDiscardable
+          ? createMessage(CURRENT_PAGE_DISCARD_WARNING, currentPageName)
+          : null}
       </Callout>
     </Container>
   );

app/client/src/sagas/GitSyncSagas.ts

@@ -930,7 +930,8 @@ function* discardChanges() {
   let response: ApiResponse<GitDiscardResponse>;
   try {
     const appId: string = yield select(getCurrentApplicationId);
-    response = yield GitSyncAPI.discardChanges(appId);
+    const doPull = true;
+    response = yield GitSyncAPI.discardChanges(appId, doPull);
     const isValidResponse: boolean = yield validateResponse(
       response,
       false,

app/server/appsmith-git/src/main/java/com/appsmith/git/service/GitExecutorImpl.java

@@ -23,8 +23,6 @@ import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.ListBranchCommand;
 import org.eclipse.jgit.api.MergeCommand;
 import org.eclipse.jgit.api.MergeResult;
-import org.eclipse.jgit.api.RebaseCommand;
-import org.eclipse.jgit.api.RebaseResult;
 import org.eclipse.jgit.api.ResetCommand;
 import org.eclipse.jgit.api.Status;
 import org.eclipse.jgit.api.TransportConfigCallback;
@@ -558,6 +556,16 @@ public class GitExecutorImpl implements GitExecutor {
         .subscribeOn(scheduler);
     }
 
+    private int getModifiedQueryCount(Set<String> jsObjectsModified, int modifiedCount, String filePath) {
+        String queryName = filePath.substring(filePath.lastIndexOf("/") + 1);
+        String pageName = filePath.split("/")[1];
+        if (!jsObjectsModified.contains(pageName + queryName)) {
+            jsObjectsModified.add(pageName + queryName);
+            modifiedCount++;
+        }
+        return modifiedCount;
+    }
+
     @Override
     public Mono<String> mergeBranch(Path repoSuffix, String sourceBranch, String destinationBranch) {
         return Mono.fromCallable(() -> {
@@ -717,7 +725,7 @@ public class GitExecutorImpl implements GitExecutor {
                 config.save();
                 return git.getRepository().getBranch();
             }
-        }).timeout(Duration.ofMillis(Constraint.TIMEOUT_MILLIS))
+        })
         .subscribeOn(scheduler);
     }
 
@@ -779,29 +787,6 @@ public class GitExecutorImpl implements GitExecutor {
                         log.error("Error while resetting the commit, {}", e.getMessage());
                     }
                     return Mono.just(false);
-                })
+                });
-                .timeout(Duration.ofMillis(Constraint.TIMEOUT_MILLIS))
-                .subscribeOn(scheduler);
-    }
-
-    public Mono<Boolean> rebaseBranch(Path repoSuffix, String branchName) {
-        return this.checkoutToBranch(repoSuffix, branchName)
-                .flatMap(isCheckedOut -> {
-                    try (Git git = Git.open(createRepoPath(repoSuffix).toFile())) {
-                        RebaseResult result = git.rebase().setUpstream("origin/" + branchName).call();
-                        if (result.getStatus().isSuccessful()) {
-                            return Mono.just(true);
-                        } else {
-                            log.error("Error while rebasing the branch, {}", result.getStatus().name());
-                            git.rebase().setUpstream("origin/master").setOperation(RebaseCommand.Operation.ABORT).call();
-                            return Mono.just(false);
-                        }
-                    } catch (GitAPIException | IOException e) {
-                        log.error("Error while rebasing the branch, {}", e.getMessage());
-                        return Mono.just(false);
-                    }
-                })
-                .timeout(Duration.ofMillis(Constraint.TIMEOUT_MILLIS))
-                .subscribeOn(scheduler);
     }
 }

app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/GitExecutor.java

@@ -175,6 +175,4 @@ public interface GitExecutor {
     Mono<Boolean> testConnection(String publicKey, String privateKey, String remoteUrl);
 
     Mono<Boolean> resetHard(Path repoSuffix, String branchName);
-
-    Mono<Boolean> rebaseBranch(Path repoSuffix, String branchName);
 }

app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/GitControllerCE.java

@@ -253,9 +253,10 @@ public class GitControllerCE {
     @JsonView(Views.Public.class)
     @PutMapping("/discard/app/{defaultApplicationId}")
     public Mono<ResponseDTO<Application>> discardChanges(@PathVariable String defaultApplicationId,
+                                                         @RequestParam(required = false, defaultValue = "true") Boolean doPull,
                                                          @RequestHeader(name = FieldName.BRANCH_NAME) String branchName) {
         log.debug("Going to discard changes for branch {} with defaultApplicationId {}", branchName, defaultApplicationId);
-        return service.discardChanges(defaultApplicationId, branchName)
+        return service.discardChanges(defaultApplicationId, branchName, doPull)
                 .map(result -> new ResponseDTO<>((HttpStatus.OK.value()), result, null));
     }
 

app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/GitServiceCE.java

@@ -70,7 +70,7 @@ public interface GitServiceCE {
 
     Mono<Application> deleteBranch(String defaultApplicationId, String branchName);
 
-    Mono<Application> discardChanges(String defaultApplicationId, String branchName);
+    Mono<Application> discardChanges(String defaultApplicationId, String branchName, Boolean doPull);
 
     Mono<List<GitDocsDTO>> getGitDocUrls();
 

app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/GitServiceCEImpl.java

@@ -83,6 +83,7 @@ import java.io.IOException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.time.Duration;
+import java.util.ArrayList;
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -2189,7 +2190,7 @@ public class GitServiceCEImpl implements GitServiceCE {
     }
 
     @Override
-    public Mono<Application> discardChanges(String defaultApplicationId, String branchName) {
+    public Mono<Application> discardChanges(String defaultApplicationId, String branchName, Boolean doPull) {
 
         if (StringUtils.isEmptyOrNull(defaultApplicationId)) {
             return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.APPLICATION_ID));
@@ -2200,7 +2201,16 @@ public class GitServiceCEImpl implements GitServiceCE {
         Mono<Application> defaultApplicationMono = this.getApplicationById(defaultApplicationId);
 
         Mono<Application> discardChangeMono;
-
+        if (Boolean.TRUE.equals(doPull)) {
+            discardChangeMono = defaultApplicationMono
+                    // Add file lock before proceeding with the git operation
+                    .flatMap(application -> addFileLock(defaultApplicationId).thenReturn(application))
+                    .flatMap(defaultApplication -> this.pullAndRehydrateApplication(defaultApplication, branchName))
+                    .map(GitPullDTO::getApplication)
+                    .flatMap(application -> releaseFileLock(defaultApplicationId)
+                                .then(this.addAnalyticsForGitOperation(AnalyticsEvents.GIT_DISCARD_CHANGES.getEventName(), application, null)))
+                    .map(responseUtils::updateApplicationWithDefaultResources);
+        } else {
             // Rehydrate the application from local file system
             discardChangeMono = branchedApplicationMono
                     // Add file lock before proceeding with the git operation
@@ -2211,17 +2221,13 @@ public class GitServiceCEImpl implements GitServiceCE {
                             return Mono.error(new AppsmithException(AppsmithError.INVALID_GIT_CONFIGURATION, GIT_CONFIG_ERROR));
                         }
                         Path repoSuffix = Paths.get(branchedApplication.getWorkspaceId(), gitData.getDefaultApplicationId(), gitData.getRepoName());
-                    return gitExecutor.rebaseBranch(repoSuffix, branchName)
+                        return gitExecutor.checkoutToBranch(repoSuffix, branchName)
-                            .flatMap(rebaseStatus -> {
+                                .then(fileUtils.reconstructApplicationJsonFromGitRepo(
-                                if (rebaseStatus.equals(Boolean.FALSE)) {
-                                    return Mono.error(new AppsmithException(AppsmithError.GIT_ACTION_FAILED, "rebase branch", "Unable to rebase branch - " + branchName));
-                                }
-                                return fileUtils.reconstructApplicationJsonFromGitRepo(
                                         branchedApplication.getWorkspaceId(),
                                         branchedApplication.getGitApplicationMetadata().getDefaultApplicationId(),
                                         branchedApplication.getGitApplicationMetadata().getRepoName(),
-                                        branchName);
+                                        branchName)
-                            })
+                                )
                                 .flatMap(applicationJson ->
                                         importExportApplicationService
                                             .importApplicationInWorkspaceFromGit(branchedApplication.getWorkspaceId(), applicationJson, branchedApplication.getId(), branchName)
@@ -2230,7 +2236,7 @@ public class GitServiceCEImpl implements GitServiceCE {
                     .flatMap(application -> releaseFileLock(defaultApplicationId)
                             .then(this.addAnalyticsForGitOperation(AnalyticsEvents.GIT_DISCARD_CHANGES.getEventName(), application, null)))
                     .map(responseUtils::updateApplicationWithDefaultResources);
-
+        }
 
         return Mono.create(sink -> discardChangeMono
                 .subscribe(sink::success, sink::error, null, sink.currentContext())

app/server/appsmith-server/src/test/java/com/appsmith/server/services/GitServiceTest.java

@@ -3155,10 +3155,17 @@ public class GitServiceTest {
                 .thenReturn(Mono.just(Paths.get("path")));
         Mockito.when(gitFileUtils.reconstructApplicationJsonFromGitRepo(Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyString()))
                 .thenReturn(Mono.just(applicationJson));
-        Mockito.when(gitExecutor.rebaseBranch(Mockito.any(Path.class), Mockito.anyString()))
+        Mockito.when(gitExecutor.pullApplication(
+                        Mockito.any(Path.class), Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyString()))
+                .thenReturn(Mono.just(mergeStatusDTO));
+        Mockito.when(gitExecutor.getStatus(Mockito.any(Path.class), Mockito.anyString()))
+                .thenReturn(Mono.just(gitStatusDTO));
+        Mockito.when(gitExecutor.fetchRemote(Mockito.any(Path.class), Mockito.anyString(), Mockito.anyString(), eq(true), Mockito.anyString(), Mockito.anyBoolean()))
+                .thenReturn(Mono.just("fetched"));
+        Mockito.when(gitExecutor.resetToLastCommit(Mockito.any(Path.class), Mockito.anyString()))
                 .thenReturn(Mono.just(true));
 
-        Mono<Application> applicationMono = gitService.discardChanges(application.getId(), application.getGitApplicationMetadata().getBranchName());
+        Mono<Application> applicationMono = gitService.discardChanges(application.getId(), application.getGitApplicationMetadata().getBranchName(), true);
 
         StepVerifier
                 .create(applicationMono)
@@ -3198,7 +3205,7 @@ public class GitServiceTest {
                 .thenReturn(Mono.just("fetched"));
 
         gitService
-                .discardChanges(application.getId(), application.getGitApplicationMetadata().getBranchName())
+                .discardChanges(application.getId(), application.getGitApplicationMetadata().getBranchName(), true)
                 .timeout(Duration.ofNanos(100))
                 .subscribe();