From 0094aab735dd40c1cbcecf0374792d81a2187f73 Mon Sep 17 00:00:00 2001 From: Hetu Nandu Date: Tue, 4 Apr 2023 10:29:00 +0530 Subject: [PATCH] fix: [Git] Avoid 404 when checking out a branch (#21894) ## Description After checkout, we will now check if the resource the user was accessing is available in the incoming branch. Instead of calling the apis to check this, we will listen to the success action and then handle check if the current resource is still available in the branch. If not, we will navigate the user to the home page of the app so that they do not see a 404 error > Don't show a 404 error when a resource is not available in the checked out branch, instead take them to the home page of the app Fixes #17234 Fixes #20883 Media ## Type of change - Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? - Manual - Have a git connected app - Create a new branch - Create a new API/Query/Page on the new branch - Switch back to the original branch - Test: The app should not show 404 error but be navigated to the home page of the app - Cypress Updated the existing cypress tests that avoided the error to make sure they test the fix instead ### Test Plan > Add Testsmith test cases links that relate to this PR ### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) ## Checklist: ### Dev activity - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag ### QA activity: - [ ] Test plan has been approved by relevant developers - [ ] Test plan has been peer reviewed by QA - [ ] Cypress test cases have been added and approved by either SDET or manual QA - [ ] Organized project review call with relevant stakeholders after Round 1/2 of QA - [ ] Added Test Plan Approved label after reveiwing all Cypress test --- .../Git/GitSync/SwitchBranches_spec.js | 22 ++++-- .../src/entities/Engine/AppEditorEngine.ts | 2 +- .../src/entities/URLRedirect/URLAssembly.ts | 9 ++- .../src/pages/Editor/APIEditor/index.tsx | 2 +- .../src/pages/Editor/EntityNotFoundPane.tsx | 4 - app/client/src/sagas/GitSyncSagas.ts | 73 +++++++++++++++++-- 6 files changed, 93 insertions(+), 19 deletions(-) diff --git a/app/client/cypress/integration/Regression_TestSuite/ClientSideTests/Git/GitSync/SwitchBranches_spec.js b/app/client/cypress/integration/Regression_TestSuite/ClientSideTests/Git/GitSync/SwitchBranches_spec.js index e9290ad502..cff95a6447 100644 --- a/app/client/cypress/integration/Regression_TestSuite/ClientSideTests/Git/GitSync/SwitchBranches_spec.js +++ b/app/client/cypress/integration/Regression_TestSuite/ClientSideTests/Git/GitSync/SwitchBranches_spec.js @@ -109,11 +109,19 @@ describe("Git sync:", function () { // reflect in api sidebar after the call passes. // eslint-disable-next-line cypress/no-unnecessary-waiting cy.wait(2000); + + // A switch here should not show a 404 page + cy.switchGitBranch(parentBranchKey); + // When entity not found, takes them to the home page + cy.get(`.t--entity.page`) + .contains("Page1") + .closest(".t--entity") + .should("be.visible") + .should("have.class", "activePage"); + cy.GlobalSearchEntity("ParentPage1"); cy.contains("ParentPage1").click(); - cy.switchGitBranch(parentBranchKey); - cy.get(`.t--entity-name:contains("ChildPage1")`).should("not.exist"); cy.CheckAndUnfoldEntityItem("Queries/JS"); cy.get(`.t--entity-name:contains("ChildApi1")`).should("not.exist"); @@ -235,7 +243,7 @@ describe("Git sync:", function () { }); // Validate the error faced when user switches between the branches - it("6. error faced when user switches branch with new page", function () { + it("6. no error faced when user switches branch with new page", function () { cy.goToEditFromPublish(); //Adding since skipping 6th case cy.generateUUID().then((uuid) => { _.gitSync.CreateGitBranch(childBranchKey, true); @@ -247,9 +255,13 @@ describe("Git sync:", function () { cy.wait(400); cy.get(gitSyncLocators.branchListItem).contains("master").click(); cy.wait(4000); - cy.contains("Page not found"); + cy.get(`.t--entity.page`) + .contains("Page1") + .closest(".t--entity") + .should("be.visible") + .should("have.class", "activePage"); + cy.get(".t--canvas-artboard").should("be.visible"); }); - cy.go("back"); cy.reload(); }); diff --git a/app/client/src/entities/Engine/AppEditorEngine.ts b/app/client/src/entities/Engine/AppEditorEngine.ts index 368c5870bd..1664338e7c 100644 --- a/app/client/src/entities/Engine/AppEditorEngine.ts +++ b/app/client/src/entities/Engine/AppEditorEngine.ts @@ -211,7 +211,7 @@ export default class AppEditorEngine extends AppEngine { branch: branchInStore, }), ); - // init of temporay remote url from old application + // init of temporary remote url from old application yield put(remoteUrlInputValue({ tempRemoteUrl: "" })); // add branch query to path and fetch status if (branchInStore) { diff --git a/app/client/src/entities/URLRedirect/URLAssembly.ts b/app/client/src/entities/URLRedirect/URLAssembly.ts index eced9bdd72..45bbb79a98 100644 --- a/app/client/src/entities/URLRedirect/URLAssembly.ts +++ b/app/client/src/entities/URLRedirect/URLAssembly.ts @@ -201,6 +201,7 @@ export class URLBuilder { persistExistingParams = false, suffix, pageId, + branch, } = builderParams; if (!pageId) { @@ -215,7 +216,13 @@ export class URLBuilder { persistExistingParams, ); - const modifiedQueryParams = { ...queryParamsToPersist, ...params }; + const branchParams = branch ? { branch: encodeURIComponent(branch) } : {}; + + const modifiedQueryParams = { + ...queryParamsToPersist, + ...params, + ...branchParams, + }; const queryString = getQueryStringfromObject(modifiedQueryParams); diff --git a/app/client/src/pages/Editor/APIEditor/index.tsx b/app/client/src/pages/Editor/APIEditor/index.tsx index 65abac570a..b73062d6a2 100644 --- a/app/client/src/pages/Editor/APIEditor/index.tsx +++ b/app/client/src/pages/Editor/APIEditor/index.tsx @@ -174,7 +174,7 @@ class ApiEditor extends React.Component { pluginId, plugins, } = this.props; - if (!this.props.pluginId && this.props.match.params.apiId) { + if (!pluginId && apiId) { return ; } if (isCreating || !isEditorInitialized) { diff --git a/app/client/src/pages/Editor/EntityNotFoundPane.tsx b/app/client/src/pages/Editor/EntityNotFoundPane.tsx index 232df18ac1..be4add0e28 100644 --- a/app/client/src/pages/Editor/EntityNotFoundPane.tsx +++ b/app/client/src/pages/Editor/EntityNotFoundPane.tsx @@ -67,10 +67,6 @@ function EntityNotFoundPane(props: Props) {

{createMessage(INVALID_URL_ERROR)}

{createMessage(PAGE_NOT_FOUND_ERROR)}

-

Pathname: {history.location.pathname}

-

Search: {history.location.search}

-

Hash: {history.location.hash}

-

Action: {history.action}