diff --git a/app/client/cypress/e2e/Regression/ClientSide/Git/GitDiscardChange/DiscardChanges_spec.js b/app/client/cypress/e2e/Regression/ClientSide/Git/GitDiscardChange/DiscardChanges_spec.js index ba5fb10ba5..5fa26283df 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/Git/GitDiscardChange/DiscardChanges_spec.js +++ b/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(), diff --git a/app/client/cypress/support/gitSync.js b/app/client/cypress/support/gitSync.js index e19dbd1494..5844db1eea 100644 --- a/app/client/cypress/support/gitSync.js +++ b/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()); diff --git a/app/client/src/api/GitSyncAPI.tsx b/app/client/src/api/GitSyncAPI.tsx index 6488bbd0f6..ee90237784 100644 --- a/app/client/src/api/GitSyncAPI.tsx +++ b/app/client/src/api/GitSyncAPI.tsx @@ -172,8 +172,10 @@ class GitSyncAPI extends Api { }); } - static discardChanges(applicationId: string) { - return Api.put(`${GitSyncAPI.baseURL}/discard/app/${applicationId}`); + static discardChanges(applicationId: string, doPull: boolean) { + return Api.put( + `${GitSyncAPI.baseURL}/discard/app/${applicationId}?doPull=${doPull}`, + ); } } diff --git a/app/client/src/ce/constants/messages.test.ts b/app/client/src/ce/constants/messages.test.ts index 65252f9471..0350e7fe0e 100644 --- a/app/client/src/ce/constants/messages.test.ts +++ b/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: - "This action will replace your local changes with the latest remote version.", + value: "Discarding these changes will pull previous changes from Git.", }, { 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", diff --git a/app/client/src/ce/constants/messages.ts b/app/client/src/ce/constants/messages.ts index 81afc37bf0..e7d5564c6a 100644 --- a/app/client/src/ce/constants/messages.ts +++ b/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."; -export const DISCARD_CHANGES = () => "Discard & pull"; + "Discarding these changes will pull previous changes from Git."; +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 diff --git a/app/client/src/pages/Editor/gitSync/Tabs/Deploy.tsx b/app/client/src/pages/Editor/gitSync/Tabs/Deploy.tsx index 41a9c3463c..cd2880f722 100644 --- a/app/client/src/pages/Editor/gitSync/Tabs/Deploy.tsx +++ b/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(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 && ( )} diff --git a/app/client/src/pages/Editor/gitSync/components/DiscardChangesWarning.tsx b/app/client/src/pages/Editor/gitSync/components/DiscardChangesWarning.tsx index fd140645af..62a3fbe3fe 100644 --- a/app/client/src/pages/Editor/gitSync/components/DiscardChangesWarning.tsx +++ b/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({ > {createMessage(DISCARD_CHANGES_WARNING)}
- {isCurrentPageDiscardable ? createMessage(DISCARD_MESSAGE) : null} + {isCurrentPageDiscardable + ? createMessage(CURRENT_PAGE_DISCARD_WARNING, currentPageName) + : null} ); diff --git a/app/client/src/sagas/GitSyncSagas.ts b/app/client/src/sagas/GitSyncSagas.ts index 2bc7d7a820..efdda1ed83 100644 --- a/app/client/src/sagas/GitSyncSagas.ts +++ b/app/client/src/sagas/GitSyncSagas.ts @@ -930,7 +930,8 @@ function* discardChanges() { let response: ApiResponse; 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, diff --git a/app/server/appsmith-git/src/main/java/com/appsmith/git/service/GitExecutorImpl.java b/app/server/appsmith-git/src/main/java/com/appsmith/git/service/GitExecutorImpl.java index b770de6910..36f1bcbc21 100644 --- a/app/server/appsmith-git/src/main/java/com/appsmith/git/service/GitExecutorImpl.java +++ b/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 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 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 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); + }); } } diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/GitExecutor.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/GitExecutor.java index 27d3f83e02..aeab3167d5 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/GitExecutor.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/GitExecutor.java @@ -175,6 +175,4 @@ public interface GitExecutor { Mono testConnection(String publicKey, String privateKey, String remoteUrl); Mono resetHard(Path repoSuffix, String branchName); - - Mono rebaseBranch(Path repoSuffix, String branchName); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/GitControllerCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/GitControllerCE.java index 06ef5d3def..a04c67dbc0 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/GitControllerCE.java +++ b/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> 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)); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/GitServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/GitServiceCE.java index 51659ed8f2..01868109d7 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/GitServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/GitServiceCE.java @@ -70,7 +70,7 @@ public interface GitServiceCE { Mono deleteBranch(String defaultApplicationId, String branchName); - Mono discardChanges(String defaultApplicationId, String branchName); + Mono discardChanges(String defaultApplicationId, String branchName, Boolean doPull); Mono> getGitDocUrls(); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/GitServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/GitServiceCEImpl.java index 0829c3f108..c207c8e0fd 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/GitServiceCEImpl.java +++ b/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 discardChanges(String defaultApplicationId, String branchName) { + public Mono discardChanges(String defaultApplicationId, String branchName, Boolean doPull) { if (StringUtils.isEmptyOrNull(defaultApplicationId)) { return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.APPLICATION_ID)); @@ -2200,37 +2201,42 @@ public class GitServiceCEImpl implements GitServiceCE { Mono defaultApplicationMono = this.getApplicationById(defaultApplicationId); Mono discardChangeMono; - - // Rehydrate the application from local file system - discardChangeMono = branchedApplicationMono - // Add file lock before proceeding with the git operation - .flatMap(application -> addFileLock(defaultApplicationId).thenReturn(application)) - .flatMap(branchedApplication -> { - GitApplicationMetadata gitData = branchedApplication.getGitApplicationMetadata(); - if (gitData == null || StringUtils.isEmptyOrNull(gitData.getDefaultApplicationId())) { - 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) - .flatMap(rebaseStatus -> { - if (rebaseStatus.equals(Boolean.FALSE)) { - return Mono.error(new AppsmithException(AppsmithError.GIT_ACTION_FAILED, "rebase branch", "Unable to rebase branch - " + branchName)); - } - return fileUtils.reconstructApplicationJsonFromGitRepo( + 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 + .flatMap(application -> addFileLock(defaultApplicationId).thenReturn(application)) + .flatMap(branchedApplication -> { + GitApplicationMetadata gitData = branchedApplication.getGitApplicationMetadata(); + if (gitData == null || StringUtils.isEmptyOrNull(gitData.getDefaultApplicationId())) { + return Mono.error(new AppsmithException(AppsmithError.INVALID_GIT_CONFIGURATION, GIT_CONFIG_ERROR)); + } + Path repoSuffix = Paths.get(branchedApplication.getWorkspaceId(), gitData.getDefaultApplicationId(), gitData.getRepoName()); + return gitExecutor.checkoutToBranch(repoSuffix, branchName) + .then(fileUtils.reconstructApplicationJsonFromGitRepo( branchedApplication.getWorkspaceId(), branchedApplication.getGitApplicationMetadata().getDefaultApplicationId(), branchedApplication.getGitApplicationMetadata().getRepoName(), - branchName); - }) - .flatMap(applicationJson -> - importExportApplicationService + branchName) + ) + .flatMap(applicationJson -> + importExportApplicationService .importApplicationInWorkspaceFromGit(branchedApplication.getWorkspaceId(), applicationJson, branchedApplication.getId(), branchName) - ); - }) - .flatMap(application -> releaseFileLock(defaultApplicationId) - .then(this.addAnalyticsForGitOperation(AnalyticsEvents.GIT_DISCARD_CHANGES.getEventName(), application, null))) - .map(responseUtils::updateApplicationWithDefaultResources); - + ); + }) + .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()) diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/GitServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/GitServiceTest.java index c04f8b6459..e587097d52 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/GitServiceTest.java +++ b/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 applicationMono = gitService.discardChanges(application.getId(), application.getGitApplicationMetadata().getBranchName()); + Mono 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();