Skip to content

chore: removing branch protection trigger from git connect flow #34118

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
brayn003 merged 4 commits into from
Jun 17, 2024
Merged

chore: removing branch protection trigger from git connect flow #34118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
727c94a
fix: removing branch protection when git repo connected
Jun 9, 2024
caa6ff1
fix: updating test cases
Jun 9, 2024
ba90874
fix: design changes
Jun 10, 2024
6ef7f8b
fix: review fixes
Jun 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions app/client/cypress/e2e/Regression/ClientSide/Git/GitSync/GitBranchProtect_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { featureFlagIntercept } from "../../../../../support/Objects/FeatureFlags";
import * as _ from "../../../../../support/Objects/ObjectsCore";
import {
AppSidebar,
Expand All @@ -17,17 +16,25 @@ describe("Git Branch Protection", { tags: ["@tag.Git"] }, function () {
const appName = "GitBranchProtect-2" + uid;
_.homePage.CreateNewWorkspace(wsName, true);
_.homePage.CreateAppInWorkspace(wsName, appName);
_.gitSync.CreateNConnectToGit("repoprotect", true, true);
cy.wait(1000);

cy.intercept({
method: "POST",
url: /\/api\/v1\/git\/branch\/app\/.*\/protected/,
}).as("gitProtectApi");

_.gitSync.CreateNConnectToGit("repoprotect", true, true, false);
cy.get("@gitRepoName").then((repName) => {
repoName = repName;
_.gitSync.OpenGitSettingsModal();
_.agHelper.GetNClick(_.gitSync._settingsTabBranch);
_.agHelper.GetNClick(_.gitSync._protectedBranchesSelect);
_.agHelper.GetNClick(
`${_.gitSync._protectedBranchesSelect} .rc-select-item`,
0,
);
_.agHelper.GetNClick(_.gitSync._branchProtectionUpdateBtn);
cy.wait("@gitProtectApi").then((res1) => {
_.agHelper.GetNClick(_.gitSync._closeGitSettingsModal);
expect(res1.response).to.have.property("statusCode", 200);
_.agHelper.AssertElementVisibility(
AppSidebar.locators.sidebar,
Expand Down
8 changes: 4 additions & 4 deletions app/client/cypress/e2e/Regression/ClientSide/Git/GitSync/RepoLimitExceededErrorModal_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,26 +32,26 @@ describe(
homePage.NavigateToHome();
homePage.CreateNewApplication();
onboarding.closeIntroModal();
gitSync.CreateNConnectToGit(repoName1, true, true, false);
gitSync.CreateNConnectToGit(repoName1, true, true);
cy.get("@gitRepoName").then((repName) => {
repoName1 = repName;
});
homePage.NavigateToHome();
homePage.CreateNewApplication();
onboarding.closeIntroModal();
gitSync.CreateNConnectToGit(repoName2, true, true, false);
gitSync.CreateNConnectToGit(repoName2, true, true);
cy.get("@gitRepoName").then((repName) => {
repoName2 = repName;
});
homePage.NavigateToHome();
homePage.CreateNewApplication();
gitSync.CreateNConnectToGit(repoName3, true, true, false);
gitSync.CreateNConnectToGit(repoName3, true, true);
cy.get("@gitRepoName").then((repName) => {
repoName3 = repName;
});
homePage.NavigateToHome();
homePage.CreateNewApplication();
gitSync.CreateNConnectToGit(repoName4, false, true, false);
gitSync.CreateNConnectToGit(repoName4, false, true);
cy.get("@gitRepoName").then((repName) => {
repoName4 = repName;
});
Expand Down
16 changes: 1 addition & 15 deletions app/client/cypress/support/Pages/GitSync.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export class GitSync {
public _gitSyncModal = "[data-testid=t--git-sync-modal]";
private _closeGitSyncModal =
"//div[@data-testid='t--git-sync-modal']//button[@aria-label='Close']";
private _closeGitSettingsModal =
public _closeGitSettingsModal =
"//div[@data-testid='t--git-settings-modal']//button[@aria-label='Close']";
//private _closeGitSyncModal = ".ads-v2-modal__content-header-close-button";
private _gitRepoInput =
Expand Down Expand Up @@ -118,7 +118,6 @@ export class GitSync {
repoName = "Repo",
Copy link
Contributor

@ayushpahwa ayushpahwa Jun 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be better to assign all items to public/private explicitly for clarity. In most cases, not all of these variables are to be public.

Copy link
Contributor Author

@brayn003 brayn003 Jun 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the items are already explicitly marked as private/public.
Also, we are planning to pick up refactoring tests in future sprints. It is not in the scope for this PR

assertConnect = true,
privateFlag = false,
removeDefaultBranchProtection = true,
) {
this.agHelper.GenerateUUID();
cy.get("@guid").then((uid) => {
Expand Down Expand Up @@ -181,19 +180,6 @@ export class GitSync {
this.agHelper.AssertElementExist(this._bottomBarCommit, 0, 30000);
}

if (removeDefaultBranchProtection) {
cy.wait([`@protected-${repoName}`, `@branches-${repoName}`]).then(
(interceptions) => {
if (
interceptions[0]?.response?.statusCode === 200 &&
interceptions[1]?.response?.statusCode === 200
) {
this.clearBranchProtection();
}
},
);
}

cy.wrap(repoName).as("gitRepoName");
});
}
Expand Down
2 changes: 1 addition & 1 deletion app/client/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@
"d3-geo": "^3.1.0",
"dayjs": "^1.10.6",
"deep-diff": "^1.0.2",
"design-system": "npm:@appsmithorg/design-system@2.1.41",
"design-system": "npm:@appsmithorg/design-system@2.1.42",
"design-system-old": "npm:@appsmithorg/design-system-old@1.1.17",
"downloadjs": "^1.4.7",
"echarts": "^5.4.2",
Expand Down
34 changes: 17 additions & 17 deletions app/client/src/ce/constants/messages.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1066,12 +1066,6 @@ export const HOW_TO_ADD_DEPLOY_KEY = () =>
export const CONSENT_ADDED_DEPLOY_KEY = () =>
"I've added the deploy key and gave it write access";
export const PREVIOUS_STEP = () => "Previous step";
export const GIT_CONNECT_SUCCESS_TITLE = () =>
"Successfully connected to your Git remote repository";
export const GIT_CONNECT_SUCCESS_MESSAGE = () =>
"Now you can start collaborating with your team members by committing, merging and deploying your app";
export const START_USING_GIT = () => "Start using Git";
export const OPEN_GIT_SETTINGS = () => "Open Git settings";
export const GIT_AUTHOR = () => "Git author";
export const DISCONNECT_GIT = () => "Disconnect Git";
export const DISCONNECT_GIT_MESSAGE = () =>
Expand Down Expand Up @@ -1114,16 +1108,6 @@ export const DEFAULT_BRANCH_DESC = () =>
export const BRANCH_PROTECTION = () => "Branch protection";
export const BRANCH_PROTECTION_DESC = () =>
"Protected branches enable you to enforce Git workflows. Changes to the app are not allowed in the protected branches.";
export const BRANCH_PROTECTION_RULES_AS_FOLLOWS = () =>
"Branch protection rules follow as,";
export const BRANCH_PROTECTION_RULE_1 = () =>
"Commit and merge are not allowed.";
export const BRANCH_PROTECTION_RULE_2 = () =>
"Users can’t create or edit queries, widgets, and JS Objects.";
export const BRANCH_PROTECTION_RULE_3 = () =>
"You can still pull the latest changes and create new branches to edit the app.";
export const BRANCH_PROTECTION_CHANGE_RULE = () =>
"You can remove protection on your default branch in Git settings.";
export const GO_TO_SETTINGS = () => "Go to settings";
export const NOW_PROTECT_BRANCH = () =>
"You can now protect your default branch.";
Expand All @@ -1141,9 +1125,25 @@ export const BRANCH_PROTECTION_CALLOUT_UNPROTECT = () => "Unprotect branch";
export const BRANCH_PROTECTION_CALLOUT_UNPROTECT_LOADING = () =>
"Unprotecting branch ...";
export const BRANCH_PROTECTION_PROTECTED = () => "Protected";

// Git Branch Protection end

// Git Connection Success
export const GIT_CONNECT_SUCCESS_TITLE = () => "Successfully connected to Git";
export const GIT_CONNECT_SUCCESS_MESSAGE = () =>
"Now you can start collaborating with your team members by committing, merging and deploying your app";
export const GIT_CONNECT_SUCCESS_ACTION_CONTINUE = () =>
"Continue to edit application";
export const GIT_CONNECT_SUCCESS_ACTION_SETTINGS = () => "Protect your branch";
export const GIT_CONNECT_SUCCESS_PROTECTION_MSG = () =>
"We recommend protecting your default branch to have a seamless collaboration.";
export const GIT_CONNECT_SUCCESS_REPO_NAME = () => "Repository name";
export const GIT_CONNECT_SUCCESS_DEFAULT_BRANCH = () => "Default branch";
export const GIT_CONNECT_SUCCESS_DEFAULT_BRANCH_TOOLTIP = () =>
"This is the base branch of the app. Users launching the app from the dashboard will see the deployed version from this branch.";
export const GIT_CONNECT_SUCCESS_PROTECTION_DOC_CTA = () =>
"Learn more about branch protection";
// Git Connection Success end

export const GENERAL = () => "General";
export const BRANCH = () => "Branch";

Expand Down
2 changes: 1 addition & 1 deletion app/client/src/pages/Editor/IDE/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import MainPane from "./MainPane";
import RightPane from "./RightPane";
import classNames from "classnames";
import { tailwindLayers } from "constants/Layers";
import ProtectedCallout from "./ProtectedCallout";
import { protectedModeSelector } from "selectors/gitSyncSelectors";
import ProtectedCallout from "./ProtectedCallout";

/**
* OldName: MainContainer
Expand Down
Loading