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
This commit is contained in:
Hetu Nandu 2023-04-04 10:29:00 +05:30 committed by GitHub
parent 0b9a9a2da1
commit 0094aab735
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 93 additions and 19 deletions

View File

@ -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();
});

View File

@ -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) {

View File

@ -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);

View File

@ -174,7 +174,7 @@ class ApiEditor extends React.Component<Props> {
pluginId,
plugins,
} = this.props;
if (!this.props.pluginId && this.props.match.params.apiId) {
if (!pluginId && apiId) {
return <EntityNotFoundPane />;
}
if (isCreating || !isEditorInitialized) {

View File

@ -67,10 +67,6 @@ function EntityNotFoundPane(props: Props) {
<div className="page-details">
<p className="bold-text">{createMessage(INVALID_URL_ERROR)}</p>
<p className="page-message">{createMessage(PAGE_NOT_FOUND_ERROR)}</p>
<p>Pathname: {history.location.pathname}</p>
<p>Search: {history.location.search}</p>
<p>Hash: {history.location.hash}</p>
<p>Action: {history.action}</p>
<Button
category={Category.secondary}
className="button-position"

View File

@ -97,8 +97,12 @@ import type { Workspace } from "@appsmith/constants/workspaceConstants";
import { log } from "loglevel";
import GIT_ERROR_CODES from "constants/GitErrorCodes";
import { builderURL } from "RouteBuilder";
import { APP_MODE } from "../entities/App";
import type { GitDiscardResponse } from "../reducers/uiReducers/gitSyncReducer";
import { APP_MODE } from "entities/App";
import type { GitDiscardResponse } from "reducers/uiReducers/gitSyncReducer";
import { FocusEntity, identifyEntityFromPath } from "navigation/FocusEntity";
import { getActions, getJSCollections } from "selectors/entitiesSelector";
import type { Action } from "entities/Action";
import type { JSCollectionDataState } from "reducers/entityReducers/jsActionsReducer";
export function* handleRepoLimitReachedError(response?: ApiResponse) {
const { responseMeta } = response || {};
@ -316,7 +320,7 @@ function* updateGlobalGitConfig(action: ReduxAction<GitConfig>) {
const trimRemotePrefix = (branch: string) => branch.replace(/^origin\//, "");
function* switchBranch(action: ReduxAction<string>) {
let response: ApiResponse | undefined;
let response: ApiResponse<ApplicationPayload> | undefined;
try {
const branch = action.payload;
const applicationId: string = yield select(getCurrentApplicationId);
@ -327,10 +331,65 @@ function* switchBranch(action: ReduxAction<string>) {
getLogToSentryFromResponse(response),
);
if (isValidResponse) {
const trimmedBranch = trimRemotePrefix(branch);
const updatedPath = addBranchParam(trimmedBranch);
history.push(updatedPath);
if (!response || !isValidResponse) {
return;
}
const trimmedBranch = trimRemotePrefix(branch);
const destinationHref = addBranchParam(trimmedBranch);
const entityInfo = identifyEntityFromPath(
destinationHref.slice(0, destinationHref.indexOf("?")),
);
// Check if page exists in the branch. If not, instead of 404, take them to
// the app home page
const page = response.data.pages.find(
(page) => page.id === entityInfo.pageId,
);
const homePage = response.data.pages.find((page) => page.isDefault);
if (!page) {
if (homePage) {
history.push(
builderURL({ pageId: homePage.id, branch: trimmedBranch }),
);
return;
}
}
// Page exists, so we will try to go to the destination
history.push(destinationHref);
let shouldGoToHomePage = false;
// It is possible that the action does not exist in the incoming branch
// so here instead of showing the 404 page, we will navigate them to the
// home page
if ([FocusEntity.API, FocusEntity.QUERY].includes(entityInfo.entity)) {
// Wait for fetch actions success, check if action id in actions state
// or else navigate to home
yield take(ReduxActionTypes.FETCH_ACTIONS_SUCCESS);
const actions: Action[] = yield select(getActions);
if (!actions.find((action) => action.id === entityInfo.id)) {
shouldGoToHomePage = true;
}
}
// Same for JS Objects
if (entityInfo.entity === FocusEntity.JS_OBJECT) {
yield take(ReduxActionTypes.FETCH_JS_ACTIONS_SUCCESS);
const jsActions: JSCollectionDataState = yield select(getJSCollections);
if (!jsActions.find((action) => action.config.id === entityInfo.id)) {
shouldGoToHomePage = true;
}
}
if (shouldGoToHomePage) {
if (homePage) {
// We will replace so that the user does not go back to the 404 url
history.replace(
builderURL({ pageId: homePage.id, persistExistingParams: true }),
);
}
}
} catch (e) {
// non api error