diff --git a/app/client/cypress/e2e/Regression/ClientSide/Git/GitSync/GitBranchProtect_spec.ts b/app/client/cypress/e2e/Regression/ClientSide/Git/GitSync/GitBranchProtect_spec.ts index 91f342963c..5f53ebffac 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/Git/GitSync/GitBranchProtect_spec.ts +++ b/app/client/cypress/e2e/Regression/ClientSide/Git/GitSync/GitBranchProtect_spec.ts @@ -2,37 +2,9 @@ import { featureFlagIntercept } from "../../../../../support/Objects/FeatureFlag import * as _ from "../../../../../support/Objects/ObjectsCore"; let guid: any; -let repoName1: any; -let repoName2: any; +let repoName: any; describe("Git Branch Protection", function () { - it("Issue 28056 - 1 : Check if protection is not enabled when feature flag is disabled", function () { - _.agHelper.GenerateUUID(); - cy.get("@guid").then((uid) => { - guid = uid; - const wsName = "GitBranchProtect-1" + uid; - const appName = "GitBranchProtect-1" + uid; - _.homePage.CreateNewWorkspace(wsName, true); - _.homePage.CreateAppInWorkspace(wsName, appName); - featureFlagIntercept({ - release_git_connect_v2_enabled: true, - release_git_branch_protection_enabled: false, - }); - cy.wait(1000); - _.gitSync.CreateNConnectToGitV2(); - cy.get("@gitRepoName").then((repName) => { - repoName1 = repName; - _.agHelper.AssertElementExist(_.entityExplorer._entityExplorerWrapper); - _.agHelper.AssertElementExist(_.propPane._propertyPaneSidebar); - _.agHelper.AssertElementEnabledDisabled( - _.gitSync._bottomBarCommit, - 0, - false, - ); - }); - }); - }); - it("Issue 28056 - 2 : Check if protection is enabled when feature flag is enabled", function () { _.agHelper.GenerateUUID(); cy.get("@guid").then((uid) => { @@ -43,7 +15,6 @@ describe("Git Branch Protection", function () { _.homePage.CreateAppInWorkspace(wsName, appName); featureFlagIntercept({ release_git_connect_v2_enabled: true, - release_git_branch_protection_enabled: true, }); cy.wait(1000); @@ -54,7 +25,7 @@ describe("Git Branch Protection", function () { _.gitSync.CreateNConnectToGitV2(); cy.get("@gitRepoName").then((repName) => { - repoName2 = repName; + repoName = repName; cy.wait("@gitProtectApi").then((res1) => { expect(res1.response).to.have.property("statusCode", 200); _.agHelper.AssertElementVisibility( @@ -76,7 +47,6 @@ describe("Git Branch Protection", function () { }); after(() => { - _.gitSync.DeleteTestGithubRepo(repoName1); - _.gitSync.DeleteTestGithubRepo(repoName2); + _.gitSync.DeleteTestGithubRepo(repoName); }); }); diff --git a/app/client/cypress/e2e/Regression/ClientSide/Git/GitSync/GitConnectV2_spec.ts b/app/client/cypress/e2e/Regression/ClientSide/Git/GitSync/GitConnectV2_spec.ts index 79950ddb47..cfcc878870 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/Git/GitSync/GitConnectV2_spec.ts +++ b/app/client/cypress/e2e/Regression/ClientSide/Git/GitSync/GitConnectV2_spec.ts @@ -5,6 +5,7 @@ let ws1Name: string; let ws2Name: string; let app1Name: string; let repoName: any; +let branchName: any; describe("Git Connect V2", function () { before(() => { @@ -36,17 +37,21 @@ describe("Git Connect V2", function () { release_git_connect_v2_enabled: true, }); - _.entityExplorer.DragDropWidgetNVerify(_.draggableWidgets.TEXT, 300, 300); - _.propPane.RenameWidget("Text1", "MyText"); - _.propPane.UpdatePropertyFieldValue("Text", "Hello World"); - _.gitSync.CommitAndPush(); + _.gitSync.CreateGitBranch("test", true); + cy.get("@gitbranchName").then((bName) => { + branchName = bName; + _.entityExplorer.DragDropWidgetNVerify(_.draggableWidgets.TEXT, 300, 300); + _.propPane.RenameWidget("Text1", "MyText"); + _.propPane.UpdatePropertyFieldValue("Text", "Hello World"); + _.gitSync.CommitAndPush(); - _.gitSync.ImportAppFromGitV2(ws2Name, repoName); - - _.entityExplorer.ExpandCollapseEntity("Widgets"); - _.entityExplorer.AssertEntityPresenceInExplorer("MyText"); - _.entityExplorer.SelectEntityByName("MyText"); - _.propPane.ValidatePropertyFieldValue("Text", "Hello World"); + _.gitSync.ImportAppFromGitV2(ws2Name, repoName); + _.gitSync.SwitchGitBranch(branchName); + _.entityExplorer.ExpandCollapseEntity("Widgets"); + _.entityExplorer.AssertEntityPresenceInExplorer("MyText"); + _.entityExplorer.SelectEntityByName("MyText"); + _.propPane.ValidatePropertyFieldValue("Text", "Hello World"); + }); }); after(() => { diff --git a/app/client/cypress/e2e/Regression/ClientSide/Git/GitWithJSLibrary/GitwithCustomJSLibrary_spec.js b/app/client/cypress/e2e/Regression/ClientSide/Git/GitWithJSLibrary/GitwithCustomJSLibrary_spec.js index 4a82a357c0..b78c0ec540 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/Git/GitWithJSLibrary/GitwithCustomJSLibrary_spec.js +++ b/app/client/cypress/e2e/Regression/ClientSide/Git/GitWithJSLibrary/GitwithCustomJSLibrary_spec.js @@ -44,7 +44,7 @@ describe("excludeForAirgap", "Tests JS Library with Git", () => { installer.uninstallLibrary("uuidjs"); installer.assertUnInstall("uuidjs"); // discard js library uninstallation - cy.gitDiscardChanges(); + gitSync.DiscardChanges(); // verify js library is present entityExplorer.ExpandCollapseEntity("Libraries"); installer.AssertLibraryinExplorer("uuidjs"); diff --git a/app/client/cypress/support/Pages/GitSync.ts b/app/client/cypress/support/Pages/GitSync.ts index 40dd9f1a76..99139d7211 100644 --- a/app/client/cypress/support/Pages/GitSync.ts +++ b/app/client/cypress/support/Pages/GitSync.ts @@ -474,9 +474,9 @@ export class GitSync { this.agHelper.AssertContains( Cypress.env("MESSAGES").DISCARDING_AND_PULLING_CHANGES(), ); + this.agHelper.AssertContains("Discarded changes successfully"); this.assertHelper.AssertNetworkStatus("@discardChanges"); this.assertHelper.AssertNetworkStatus("@gitStatus"); - this.agHelper.AssertContains("Discarded changes successfully"); this.agHelper.AssertElementExist(this._bottomBarCommit, 0, 30000); } diff --git a/app/client/src/actions/gitSyncActions.ts b/app/client/src/actions/gitSyncActions.ts index c08068a911..1427ac984b 100644 --- a/app/client/src/actions/gitSyncActions.ts +++ b/app/client/src/actions/gitSyncActions.ts @@ -196,8 +196,11 @@ export const fetchGitRemoteStatusSuccess = (payload: GitRemoteStatusData) => ({ payload, }); -export const discardChanges = () => ({ +export const discardChanges = ( + payload: { successToastMessage?: string } | undefined | null = {}, +) => ({ type: ReduxActionTypes.GIT_DISCARD_CHANGES, + payload, }); export const discardChangesSuccess = (payload: any) => ({ diff --git a/app/client/src/ce/constants/messages.ts b/app/client/src/ce/constants/messages.ts index 958fa34d65..865e2c916f 100644 --- a/app/client/src/ce/constants/messages.ts +++ b/app/client/src/ce/constants/messages.ts @@ -508,7 +508,6 @@ export const PAGE_SERVER_UNAVAILABLE_ERROR_MESSAGES = ( export const POST = () => "Post"; export const CANCEL = () => "Cancel"; export const REMOVE = () => "Remove"; -export const CREATE = () => "Create"; // Showcase Carousel export const NEXT = () => "NEXT"; @@ -843,6 +842,7 @@ export const COMMITTING_AND_PUSHING_CHANGES = () => export const DISCARDING_AND_PULLING_CHANGES = () => "Discarding and pulling changes..."; export const DISCARD_SUCCESS = () => "Discarded changes successfully."; +export const DISCARD_AND_PULL_SUCCESS = () => "Pulled from remote successfully"; export const IS_MERGING = () => "Merging changes..."; @@ -1033,7 +1033,7 @@ export const ADD_DEPLOY_KEY_STEP_TITLE = () => export const HOW_TO_ADD_DEPLOY_KEY = () => "How to paste SSH Key in repo and give write access?"; export const CONSENT_ADDED_DEPLOY_KEY = () => - "I've added deploy key and gave it write access"; + "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"; @@ -1082,13 +1082,16 @@ export const BRANCH_PROTECTION_CHANGE_RULE = () => "You can remove protection on your default branch in Git settings."; export const BRANCH_TOOLTIP_TITLE = () => "🚫 This is a protected branch"; export const BRANCH_TOOLTIP_MESSAGE = () => - "You can remove protection on your default branch in Git settings."; + "Please create a new branch or checkout an existing one to edit the app."; export const GO_TO_SETTINGS = () => "Go to settings"; export const NOW_PROTECT_BRANCH = () => "You can now protect your default branch."; export const APPSMITH_ENTERPRISE = () => "Appsmith Enterprise"; export const PROTECT_BRANCH_SUCCESS = () => "Changed protected branches"; -export const UPDATE_DEFAULT_BRANCH_SUCCESS = () => "Updated default branch"; +export const UPDATE_DEFAULT_BRANCH_SUCCESS = (branchName: string) => + `Updated default branch ${!!branchName ? `to ${branchName}` : ""}`; +export const CONTACT_ADMIN_FOR_GIT = () => + "Please contact your workspace admin to connect your app to a git repo"; // Git Branch Protection end export const NAV_DESCRIPTION = () => diff --git a/app/client/src/ce/entities/FeatureFlag.ts b/app/client/src/ce/entities/FeatureFlag.ts index 06cbf116fc..4660a6118a 100644 --- a/app/client/src/ce/entities/FeatureFlag.ts +++ b/app/client/src/ce/entities/FeatureFlag.ts @@ -28,8 +28,6 @@ export const FEATURE_FLAG = { ab_show_templates_instead_of_blank_canvas_enabled: "ab_show_templates_instead_of_blank_canvas_enabled", release_app_sidebar_enabled: "release_app_sidebar_enabled", - release_git_branch_protection_enabled: - "release_git_branch_protection_enabled", license_git_branch_protection_enabled: "license_git_branch_protection_enabled", license_widget_rtl_support_enabled: "license_widget_rtl_support_enabled", @@ -61,7 +59,6 @@ export const DEFAULT_FEATURE_FLAG_VALUE: FeatureFlags = { release_anvil_enabled: false, ab_show_templates_instead_of_blank_canvas_enabled: false, release_app_sidebar_enabled: false, - release_git_branch_protection_enabled: false, license_git_branch_protection_enabled: false, license_widget_rtl_support_enabled: false, }; diff --git a/app/client/src/ce/sagas/ApplicationSagas.tsx b/app/client/src/ce/sagas/ApplicationSagas.tsx index f2a50f6ccb..23a7ce406c 100644 --- a/app/client/src/ce/sagas/ApplicationSagas.tsx +++ b/app/client/src/ce/sagas/ApplicationSagas.tsx @@ -65,7 +65,6 @@ import { createMessage, DELETING_APPLICATION, DELETING_MULTIPLE_APPLICATION, - DISCARD_SUCCESS, ERROR_IMPORTING_APPLICATION_TO_WORKSPACE, } from "@appsmith/constants/messages"; import { APP_MODE } from "entities/App"; @@ -300,12 +299,6 @@ export function* fetchAppAndPagesSaga( }, }); - if (localStorage.getItem("GIT_DISCARD_CHANGES") === "success") { - toast.show(createMessage(DISCARD_SUCCESS), { - kind: "success", - }); - localStorage.setItem("GIT_DISCARD_CHANGES", ""); - } yield put({ type: ReduxActionTypes.SET_APP_VERSION_ON_WORKER, payload: response.data.application?.evaluationVersion, diff --git a/app/client/src/ce/utils/analyticsUtilTypes.ts b/app/client/src/ce/utils/analyticsUtilTypes.ts index d78da69057..cafbf8ad91 100644 --- a/app/client/src/ce/utils/analyticsUtilTypes.ts +++ b/app/client/src/ce/utils/analyticsUtilTypes.ts @@ -185,6 +185,9 @@ export type EventName = | "GS_GENERATE_KEY_BUTTON_CLICK" | "GS_CONNECT_BUTTON_ON_GIT_SYNC_MODAL_CLICK" | "GS_START_USING_GIT" + | "GS_DEFAULT_BRANCH_UPDATE" + | "GS_PROTECTED_BRANCHES_UPDATE" + | "GS_OPEN_GIT_SETTINGS" | "GIT_DISCARD_WARNING" | "GIT_DISCARD_CANCEL" | "GIT_DISCARD" diff --git a/app/client/src/components/designSystems/appsmith/header/DeployLinkButton.tsx b/app/client/src/components/designSystems/appsmith/header/DeployLinkButton.tsx index 8853985a15..93ca6edc78 100644 --- a/app/client/src/components/designSystems/appsmith/header/DeployLinkButton.tsx +++ b/app/client/src/components/designSystems/appsmith/header/DeployLinkButton.tsx @@ -12,6 +12,7 @@ import { } from "@appsmith/constants/messages"; import { Button } from "design-system"; import { KBEditorMenuItem } from "@appsmith/pages/Editor/KnowledgeBase/KBEditorMenuItem"; +import { useIsGitAdmin } from "pages/Editor/gitSync/hooks/useIsGitAdmin"; interface Props { trigger: ReactNode; @@ -21,6 +22,7 @@ interface Props { export const DeployLinkButton = (props: Props) => { const dispatch = useDispatch(); const isGitConnected = useSelector(getIsGitConnected); + const isGitAdmin = useIsGitAdmin(); const goToGitConnectionPopup = () => { AnalyticsUtil.logEvent("GS_CONNECT_GIT_CLICK", { @@ -46,7 +48,7 @@ export const DeployLinkButton = (props: Props) => { /> - {!isGitConnected && ( + {!isGitConnected && isGitAdmin && ( { +const getPullBtnStatus = (gitStatus: any, pullFailed: boolean) => { const { behindCount, isClean } = gitStatus || {}; let message = createMessage(NO_COMMITS_TO_PULL); - let disabled = isProtected ? false : behindCount === 0; + let disabled = behindCount === 0; if (!isClean) { disabled = true; message = createMessage(CANNOT_PULL_WITH_LOCAL_UNCOMMITTED_CHANGES); @@ -187,7 +186,7 @@ const getQuickActionButtons = ({ }, { className: "t--bottom-bar-pull", - count: isProtectedMode ? undefined : gitStatus?.behindCount, + count: gitStatus?.behindCount, icon: "down-arrow-2", onClick: () => !pullDisabled && pull(), tooltipText: pullTooltipMessage, @@ -236,21 +235,35 @@ const OuterContainer = styled.div` height: 100%; `; +const CenterDiv = styled.div` + text-align: center; +`; + function ConnectGitPlaceholder() { const dispatch = useDispatch(); const isInGuidedTour = useSelector(inGuidedTour); - const isTooltipEnabled = isInGuidedTour; - const tooltipContent = !isInGuidedTour ? ( - <> -
{createMessage(NOT_LIVE_FOR_YOU_YET)}
-
{createMessage(COMING_SOON)}
- - ) : ( - <> -
{createMessage(CONNECTING_TO_REPO_DISABLED)}
-
{createMessage(DURING_ONBOARDING_TOUR)}
- - ); + const isGitAdmin = useIsGitAdmin(); + const isTooltipEnabled = isInGuidedTour || !isGitAdmin; + const tooltipContent = useMemo(() => { + if (!isGitAdmin) { + return {createMessage(CONTACT_ADMIN_FOR_GIT)}; + } + if (isInGuidedTour) { + return ( + <> +
{createMessage(CONNECTING_TO_REPO_DISABLED)}
+
{createMessage(DURING_ONBOARDING_TOUR)}
+ + ); + } + return ( + <> +
{createMessage(NOT_LIVE_FOR_YOU_YET)}
+
{createMessage(COMING_SOON)}
+ + ); + }, [isInGuidedTour, isGitAdmin]); + const isGitConnectionEnabled = !isInGuidedTour; return ( @@ -265,6 +278,7 @@ function ConnectGitPlaceholder() { {isGitConnectionEnabled ? ( - ); - }; - - const postBranchProtectionActions = () => { + const branchProtectionActions = () => { return ( <> diff --git a/app/client/src/pages/Editor/gitSync/Tabs/GitSettings/GitProtectedBranches.tsx b/app/client/src/pages/Editor/gitSync/Tabs/GitSettings/GitProtectedBranches.tsx index 123a247191..870b9fb670 100644 --- a/app/client/src/pages/Editor/gitSync/Tabs/GitSettings/GitProtectedBranches.tsx +++ b/app/client/src/pages/Editor/gitSync/Tabs/GitSettings/GitProtectedBranches.tsx @@ -2,6 +2,7 @@ import { APPSMITH_ENTERPRISE, BRANCH_PROTECTION, BRANCH_PROTECTION_DESC, + LEARN_MORE, UPDATE, createMessage, } from "@appsmith/constants/messages"; @@ -20,6 +21,9 @@ import styled from "styled-components"; import { useFeatureFlag } from "utils/hooks/useFeatureFlag"; import { FEATURE_FLAG } from "@appsmith/entities/FeatureFlag"; import { useAppsmithEnterpriseLink } from "./hooks"; +import { REMOTE_BRANCH_PREFIX } from "../../constants"; +import AnalyticsUtil from "utils/AnalyticsUtil"; +import { DOCS_BRANCH_PROTECTION_URL } from "constants/ThirdPartyConstants"; const Container = styled.div` padding-top: 16px; @@ -56,16 +60,36 @@ function GitProtectedBranches() { const dispatch = useDispatch(); const unfilteredBranches = useSelector(getGitBranches); - const branches = unfilteredBranches.filter( - (b) => !b.branchName.includes("origin/"), - ); + const defaultBranch = useSelector(getDefaultGitBranchName); + + const branchNames = useMemo(() => { + const returnVal: string[] = []; + for (const unfilteredBranch of unfilteredBranches) { + if (unfilteredBranch.branchName === defaultBranch) { + returnVal.unshift(unfilteredBranch.branchName); + } else if (unfilteredBranch.branchName.includes(REMOTE_BRANCH_PREFIX)) { + const localBranchName = unfilteredBranch.branchName.replace( + REMOTE_BRANCH_PREFIX, + "", + ); + if (!returnVal.includes(localBranchName)) { + returnVal.push( + unfilteredBranch.branchName.replace(REMOTE_BRANCH_PREFIX, ""), + ); + } + } else { + returnVal.push(unfilteredBranch.branchName); + } + } + return returnVal; + }, [unfilteredBranches, defaultBranch]); + const isGitProtectedFeatureLicensed = useFeatureFlag( FEATURE_FLAG.license_git_branch_protection_enabled, ); - const defaultBranch = useSelector(getDefaultGitBranchName); const protectedBranches = useSelector(getProtectedBranchesSelector); const isUpdateLoading = useSelector(getIsUpdateProtectedBranchesLoading); - const [selectedValues, setSelectedValues] = useState(); + const [selectedValues, setSelectedValues] = useState([]); const enterprisePricingLink = useAppsmithEnterpriseLink( "git_branch_protection", @@ -82,6 +106,7 @@ function GitProtectedBranches() { const updateIsDisabled = !areProtectedBranchesDifferent; const handleUpdate = () => { + sendAnalyticsEvent(); dispatch( updateGitProtectedBranchesInit({ protectedBranches: selectedValues ?? [], @@ -89,6 +114,25 @@ function GitProtectedBranches() { ); }; + const sendAnalyticsEvent = () => { + const eventData = { + branches_added: [] as string[], + branches_removed: [] as string[], + protected_branches: selectedValues, + }; + for (const val of selectedValues) { + if (!protectedBranches.includes(val)) { + eventData.branches_added.push(val); + } + } + for (const val of protectedBranches) { + if (!selectedValues.includes(val)) { + eventData.branches_removed.push(val); + } + } + AnalyticsUtil.logEvent("GS_PROTECTED_BRANCHES_UPDATE", eventData); + }; + return ( @@ -96,7 +140,10 @@ function GitProtectedBranches() { {createMessage(BRANCH_PROTECTION)} - {createMessage(BRANCH_PROTECTION_DESC)} + {createMessage(BRANCH_PROTECTION_DESC)}{" "} + + {createMessage(LEARN_MORE)} + {!isGitProtectedFeatureLicensed && ( @@ -114,20 +161,22 @@ function GitProtectedBranches() { triggerNode.parentNode} isMultiSelect maxTagTextLength={8} onChange={(v) => setSelectedValues(v)} value={selectedValues} > - {branches.map((b) => ( + {branchNames.map((branchName) => ( ))} diff --git a/app/client/src/pages/Editor/gitSync/Tabs/GitSettings/index.test.tsx b/app/client/src/pages/Editor/gitSync/Tabs/GitSettings/index.test.tsx new file mode 100644 index 0000000000..97cfecd713 --- /dev/null +++ b/app/client/src/pages/Editor/gitSync/Tabs/GitSettings/index.test.tsx @@ -0,0 +1,19 @@ +/* eslint-disable jest/no-focused-tests */ +import React from "react"; +import { render, screen } from "test/testUtils"; +import GitSettings from "."; +import type { AppState } from "@appsmith/reducers"; + +jest.mock("../../hooks/useIsGitAdmin", () => ({ + useIsGitAdmin: () => false, +})); + +describe("GitSettings test for git admin disabled", () => { + it("Branch protection, default branch and disconnect disabled when not ", () => { + const initialState: Partial = {}; + render(, { initialState }); + expect(screen.queryByTestId("t--git-protected-branches-select")).toBe(null); + expect(screen.queryByTestId("t--git-default-branch-select")).toBe(null); + expect(screen.queryByTestId("t--git-disconnect-btn")).toBe(null); + }); +}); diff --git a/app/client/src/pages/Editor/gitSync/Tabs/GitSettings/index.tsx b/app/client/src/pages/Editor/gitSync/Tabs/GitSettings/index.tsx index ce72c2db9f..40ebbfbbb2 100644 --- a/app/client/src/pages/Editor/gitSync/Tabs/GitSettings/index.tsx +++ b/app/client/src/pages/Editor/gitSync/Tabs/GitSettings/index.tsx @@ -5,8 +5,7 @@ import styled from "styled-components"; import { Divider, ModalBody } from "design-system"; import GitDefaultBranch from "./GitDefaultBranch"; import GitProtectedBranches from "./GitProtectedBranches"; -import { useSelector } from "react-redux"; -import { getIsGitProtectedFeatureEnabled } from "selectors/gitSyncSelectors"; +import { useIsGitAdmin } from "../../hooks/useIsGitAdmin"; const Container = styled.div` overflow: auto; @@ -20,21 +19,20 @@ const StyledDivider = styled(Divider)` `; function GitSettings() { - const isGitProtectedFeatureEnabled = useSelector( - getIsGitProtectedFeatureEnabled, - ); + const isGitAdmin = useIsGitAdmin(); + return ( - {isGitProtectedFeatureEnabled ? ( + {isGitAdmin ? ( <> ) : null} - + {isGitAdmin && } ); diff --git a/app/client/src/pages/Editor/gitSync/components/BranchList.tsx b/app/client/src/pages/Editor/gitSync/components/BranchList.tsx index cafdd034e3..516e3d6931 100644 --- a/app/client/src/pages/Editor/gitSync/components/BranchList.tsx +++ b/app/client/src/pages/Editor/gitSync/components/BranchList.tsx @@ -6,7 +6,7 @@ import { useDispatch, useSelector } from "react-redux"; import { createNewBranchInit, fetchBranchesInit, - // setIsGitSyncModalOpen, + fetchGitProtectedBranchesInit, switchGitBranchInit, } from "actions/gitSyncActions"; import { @@ -15,6 +15,7 @@ import { getFetchingBranches, getGitBranches, getGitBranchNames, + getIsGetProtectedBranchesLoading, getProtectedBranchesSelector, } from "selectors/gitSyncSelectors"; @@ -54,7 +55,6 @@ import { RemoteBranchList } from "./RemoteBranchList"; import { LocalBranchList } from "./LocalBranchList"; import type { Theme } from "constants/DefaultTheme"; import { Space } from "./StyledComponents"; -// import { GitSyncModalTab } from "entities/GitSync"; const ListContainer = styled.div` flex: 1; @@ -69,11 +69,6 @@ const BranchDropdownContainer = styled.div` display: flex; flex-direction: column; - // & .title { - // ${getTypographyByKey("h3")}; - // color: var(--ads-v2-color-fg-emphasis-plus); - // } - padding: ${(props) => props.theme.spaces[5]}px; min-height: 0; `; @@ -246,6 +241,7 @@ export default function BranchList(props: { source: "BRANCH_LIST_POPUP_FROM_BOTTOM_BAR", }); dispatch(fetchBranchesInit({ pruneBranches: true })); + dispatch(fetchGitProtectedBranchesInit()); }; const branches = useSelector(getGitBranches); @@ -254,6 +250,9 @@ export default function BranchList(props: { const fetchingBranches = useSelector(getFetchingBranches); const defaultBranch = useSelector(getDefaultGitBranchName); const protectedBranches = useSelector(getProtectedBranchesSelector); + const isGetProtectedBranchesLoading = useSelector( + getIsGetProtectedBranchesLoading, + ); const [searchText, changeSearchTextInState] = useState(""); const changeSearchText = (text: string) => { changeSearchTextInState(removeSpecialChars(text)); @@ -340,6 +339,9 @@ export default function BranchList(props: { switchBranch, protectedBranches, ); + + const loading = fetchingBranches || isGetProtectedBranchesLoading; + return (
- {fetchingBranches && ( + {loading && (
)} - {!fetchingBranches && ( + {!loading && ( - {fetchingBranches && } - {!fetchingBranches && ( + {loading && } + {!loading && ( {/* keeping it commented for future use */} {/* { + const workspace = useSelector(getCurrentAppWorkspace); + return hasCreateNewAppPermission(workspace.userPermissions); +}; diff --git a/app/client/src/sagas/GitSyncSagas.ts b/app/client/src/sagas/GitSyncSagas.ts index ee685cdf16..df2e59308d 100644 --- a/app/client/src/sagas/GitSyncSagas.ts +++ b/app/client/src/sagas/GitSyncSagas.ts @@ -10,6 +10,7 @@ import { import { actionChannel, call, + delay, fork, put, select, @@ -84,6 +85,7 @@ import { import { createMessage, DELETE_BRANCH_SUCCESS, + DISCARD_SUCCESS, ERROR_GIT_AUTH_FAIL, ERROR_GIT_INVALID_REMOTE, GIT_USER_UPDATED_SUCCESSFULLY, @@ -96,7 +98,7 @@ import { addBranchParam, GIT_BRANCH_QUERY_KEY } from "constants/routes"; import { getCurrentGitBranch, getDisconnectingGitApplication, - getIsGitProtectedFeatureEnabled, + getIsGitConnectV2Enabled, getIsGitStatusLiteEnabled, } from "selectors/gitSyncSelectors"; import { initEditor } from "actions/initActions"; @@ -219,6 +221,9 @@ function* connectToGitSaga(action: ConnectToGitReduxAction) { const applicationId: string = yield select(getCurrentApplicationId); const currentPageId: string = yield select(getCurrentPageId); response = yield GitSyncAPI.connect(action.payload, applicationId); + const isGitConnectV2Enabled: boolean = yield select( + getIsGitConnectV2Enabled, + ); const isValidResponse: boolean = yield validateResponse( response, @@ -231,10 +236,7 @@ function* connectToGitSaga(action: ConnectToGitReduxAction) { yield put(connectToGitSuccess(response?.data)); const defaultBranch = response?.data?.gitApplicationMetadata?.branchName; - const isGitProtectedFeatureEnabled: boolean = yield select( - getIsGitProtectedFeatureEnabled, - ); - if (isGitProtectedFeatureEnabled) { + if (isGitConnectV2Enabled) { yield put( updateGitProtectedBranchesInit({ protectedBranches: defaultBranch ? [defaultBranch] : [], @@ -1024,11 +1026,13 @@ export function* deleteBranch({ payload }: ReduxAction) { yield put(fetchBranchesInit({ pruneBranches: true })); } } catch (error) { - yield put(deleteBranchError(error)); + yield put(deleteBranchError({ error, show: true })); } } -function* discardChanges() { +function* discardChanges({ + payload, +}: ReduxAction<{ successToastMessage: string } | null | undefined>) { let response: ApiResponse; try { const appId: string = yield select(getCurrentApplicationId); @@ -1040,10 +1044,15 @@ function* discardChanges() { ); if (isValidResponse) { yield put(discardChangesSuccess(response.data)); - // const applicationId: string = response.data.id; + const successToastMessage = + payload?.successToastMessage ?? createMessage(DISCARD_SUCCESS); + toast.show(successToastMessage, { + kind: "success", + }); + // adding delay to show toast animation before reloading + yield delay(500); const pageId: string = response.data?.pages?.find((page: any) => page.isDefault)?.id || ""; - localStorage.setItem("GIT_DISCARD_CHANGES", "success"); const branch = response.data.gitApplicationMetadata.branchName; window.open(builderURL({ pageId, branch }), "_self"); } else { @@ -1053,21 +1062,13 @@ function* discardChanges() { show: true, }), ); - localStorage.setItem("GIT_DISCARD_CHANGES", "failure"); } } catch (error) { yield put(discardChangesFailure({ error, show: true })); - localStorage.setItem("GIT_DISCARD_CHANGES", "failure"); } } function* fetchGitProtectedBranchesSaga() { - const isGitProtectedFeatureEnabled: boolean = yield select( - getIsGitProtectedFeatureEnabled, - ); - if (!isGitProtectedFeatureEnabled) { - return; - } let response: ApiResponse; try { const appId: string = yield select(getCurrentApplicationId); @@ -1104,12 +1105,6 @@ function* fetchGitProtectedBranchesSaga() { function* updateGitProtectedBranchesSaga({ payload, }: ReduxAction<{ protectedBranches: string[] }>) { - const isGitProtectedFeatureEnabled: boolean = yield select( - getIsGitProtectedFeatureEnabled, - ); - if (!isGitProtectedFeatureEnabled) { - return; - } const { protectedBranches } = payload; const applicationId: string = yield select(getCurrentApplicationId); let response: ApiResponse; diff --git a/app/client/src/selectors/gitSyncSelectors.tsx b/app/client/src/selectors/gitSyncSelectors.tsx index e96dc1558c..12d82d113e 100644 --- a/app/client/src/selectors/gitSyncSelectors.tsx +++ b/app/client/src/selectors/gitSyncSelectors.tsx @@ -240,7 +240,6 @@ export const getIsUpdateProtectedBranchesLoading = (state: AppState) => { ); }; -export const getIsGitProtectedFeatureEnabled = createSelector( - selectFeatureFlags, - (flags) => !!flags?.release_git_branch_protection_enabled, -); +export const getIsGetProtectedBranchesLoading = (state: AppState) => { + return state.ui.gitSync.protectedBranchesLoading; +}; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/GitPrivateRepoHelperCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/GitPrivateRepoHelperCE.java index 7e0865e4c9..f47a921d1d 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/GitPrivateRepoHelperCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/GitPrivateRepoHelperCE.java @@ -1,8 +1,11 @@ package com.appsmith.server.helpers.ce; +import com.appsmith.server.domains.GitApplicationMetadata; import reactor.core.publisher.Mono; public interface GitPrivateRepoHelperCE { Mono isRepoLimitReached(String workspaceId, Boolean isClearCache); + + Mono isBranchProtected(GitApplicationMetadata metaData, String branchName); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/GitPrivateRepoHelperCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/GitPrivateRepoHelperCEImpl.java index 51ecb5e063..bedf96342f 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/GitPrivateRepoHelperCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/GitPrivateRepoHelperCEImpl.java @@ -1,11 +1,14 @@ package com.appsmith.server.helpers.ce; +import com.appsmith.server.domains.GitApplicationMetadata; import com.appsmith.server.helpers.GitCloudServicesUtils; import com.appsmith.server.services.ApplicationService; import lombok.RequiredArgsConstructor; import org.springframework.stereotype.Component; import reactor.core.publisher.Mono; +import java.util.List; + @Component @RequiredArgsConstructor public class GitPrivateRepoHelperCEImpl implements GitPrivateRepoHelperCE { @@ -40,4 +43,18 @@ public class GitPrivateRepoHelperCEImpl implements GitPrivateRepoHelperCE { }); }); } + + @Override + public Mono isBranchProtected(GitApplicationMetadata metaData, String branchName) { + boolean result = false; + if (metaData != null) { + String defaultBranch = metaData.getDefaultBranchName(); + List branchProtectionRules = metaData.getBranchProtectionRules(); + + result = branchProtectionRules != null + && branchName.equals(defaultBranch) + && branchProtectionRules.contains(branchName); + } + return Mono.just(result); + } } 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 476d169885..261d9e74bc 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 @@ -359,12 +359,6 @@ public class GitServiceCEImpl implements GitServiceCE { return this.commitApplication(commitDTO, defaultApplicationId, branchName, doAmend, true); } - private boolean isBranchProtected(GitApplicationMetadata metaData, String branchName) { - return metaData != null - && metaData.getBranchProtectionRules() != null - && metaData.getBranchProtectionRules().contains(branchName); - } - /** * This method will make a commit to local repo and is used internally in flows like create, merge branch * Since the lock is already acquired by the other flows, we do not need to acquire file lock again @@ -439,14 +433,16 @@ public class GitServiceCEImpl implements GitServiceCE { boolean isSystemGenerated = isSystemGeneratedTemp; Mono commitMono = this.getApplicationById(defaultApplicationId) - .map(application -> { - if (isBranchProtected(application.getGitApplicationMetadata(), branchName)) { + .zipWhen(application -> + gitPrivateRepoHelper.isBranchProtected(application.getGitApplicationMetadata(), branchName)) + .map(objects -> { + if (objects.getT2()) { throw new AppsmithException( AppsmithError.GIT_ACTION_FAILED, "commit", "Cannot commit to protected branch " + branchName); } - return application; + return objects.getT1(); }) .flatMap(application -> { GitApplicationMetadata gitData = application.getGitApplicationMetadata(); @@ -2727,14 +2723,16 @@ public class GitServiceCEImpl implements GitServiceCE { @Override public Mono deleteBranch(String defaultApplicationId, String branchName) { Mono deleteBranchMono = getApplicationById(defaultApplicationId) - .map(application -> { - if (isBranchProtected(application.getGitApplicationMetadata(), branchName)) { + .zipWhen(application -> + gitPrivateRepoHelper.isBranchProtected(application.getGitApplicationMetadata(), branchName)) + .map(objects -> { + if (objects.getT2()) { throw new AppsmithException( AppsmithError.GIT_ACTION_FAILED, "delete", "Cannot delete protected branch " + branchName); } - return application; + return objects.getT1(); }) .flatMap(application -> addFileLock(defaultApplicationId).map(status -> application)) .flatMap(application -> { @@ -3336,11 +3334,20 @@ public class GitServiceCEImpl implements GitServiceCE { @Override public Mono> getProtectedBranches(String defaultApplicationId) { - return getApplicationById(defaultApplicationId) - .flatMap(application -> { - GitApplicationMetadata gitApplicationMetadata = application.getGitApplicationMetadata(); - return Mono.justOrEmpty(gitApplicationMetadata.getBranchProtectionRules()); - }) - .defaultIfEmpty(List.of()); + return getApplicationById(defaultApplicationId).map(application -> { + GitApplicationMetadata gitApplicationMetadata = application.getGitApplicationMetadata(); + /* + user may have multiple branches as protected, but we only return the default branch + as protected branch if it's present in the list of protected branches + */ + List protectedBranches = gitApplicationMetadata.getBranchProtectionRules(); + String defaultBranchName = gitApplicationMetadata.getDefaultBranchName(); + + if (!CollectionUtils.isNullOrEmpty(protectedBranches) && protectedBranches.contains(defaultBranchName)) { + return List.of(defaultBranchName); + } else { + return List.of(); + } + }); } } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ce/GitPrivateRepoCEImplTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ce/GitPrivateRepoCEImplTest.java index 94ae79b1b9..9499ed84a3 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ce/GitPrivateRepoCEImplTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ce/GitPrivateRepoCEImplTest.java @@ -1,5 +1,6 @@ package com.appsmith.server.helpers.ce; +import com.appsmith.server.domains.GitApplicationMetadata; import com.appsmith.server.helpers.GitCloudServicesUtils; import com.appsmith.server.helpers.GitPrivateRepoHelper; import com.appsmith.server.services.ApplicationService; @@ -12,12 +13,18 @@ import org.mockito.Mockito; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.boot.test.mock.mockito.SpyBean; import org.springframework.security.test.context.support.WithUserDetails; +import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.junit.jupiter.SpringExtension; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; +import java.util.List; + import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyString; @@ -25,15 +32,16 @@ import static org.mockito.ArgumentMatchers.anyString; @ExtendWith(SpringExtension.class) @SpringBootTest @Slf4j +@DirtiesContext public class GitPrivateRepoCEImplTest { @Autowired GitPrivateRepoHelper gitPrivateRepoHelper; - @MockBean + @SpyBean GitCloudServicesUtils gitCloudServicesUtils; - @MockBean + @SpyBean ApplicationService applicationService; /** @@ -88,4 +96,34 @@ public class GitPrivateRepoCEImplTest { .assertNext(aBoolean -> assertEquals(true, aBoolean)) .verifyComplete(); } + + boolean isBranchProtected(GitApplicationMetadata metaData, String branchName) { + return Boolean.TRUE.equals( + gitPrivateRepoHelper.isBranchProtected(metaData, branchName).block()); + } + + @Test + @WithUserDetails(value = "api_user") + public void isBranchProtected() { + GitApplicationMetadata metaData = new GitApplicationMetadata(); + + assertFalse(isBranchProtected(null, "master")); + assertFalse(isBranchProtected(metaData, "master")); + + metaData.setDefaultBranchName("master2"); + assertFalse(isBranchProtected(metaData, "master")); + + metaData.setDefaultBranchName("master"); + assertFalse(isBranchProtected(metaData, "master")); + + metaData.setBranchProtectionRules(List.of("dev")); + assertFalse(isBranchProtected(metaData, "master")); + + metaData.setBranchProtectionRules(List.of("dev")); + assertFalse(isBranchProtected(metaData, "dev")); + + metaData.setBranchProtectionRules(List.of("dev", "master")); + assertFalse(isBranchProtected(metaData, "dev")); + assertTrue(isBranchProtected(metaData, "master")); + } } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/GitServiceCETest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/GitServiceCETest.java index 17946d4363..68ed489ba8 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/GitServiceCETest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/GitServiceCETest.java @@ -4227,7 +4227,7 @@ public class GitServiceCETest { StepVerifier.create(branchListMono) .assertNext(branches -> { - assertThat(branches).containsExactlyInAnyOrder("main", "develop"); + assertThat(branches).isNullOrEmpty(); }) .verifyComplete(); } @@ -4401,4 +4401,49 @@ public class GitServiceCETest { }) .verifyComplete(); } + + @Test + @WithUserDetails("api_user") + public void getProtectedBranches_WhenUserHasMultipleBranchProtected_ReturnsEmptyOrDefaultBranchOnly() { + Application testApplication = new Application(); + testApplication.setName("App" + UUID.randomUUID()); + testApplication.setWorkspaceId(workspaceId); + + Mono applicationMono = applicationPageService + .createApplication(testApplication) + .flatMap(application -> { + GitApplicationMetadata gitApplicationMetadata = new GitApplicationMetadata(); + gitApplicationMetadata.setDefaultApplicationId(application.getId()); + gitApplicationMetadata.setDefaultBranchName("master"); + // include default branch in the list of the protected branches + gitApplicationMetadata.setBranchProtectionRules(List.of("master", "develop")); + application.setGitApplicationMetadata(gitApplicationMetadata); + return applicationRepository.save(application); + }) + .cache(); + + Mono> branchListMonoWithDefaultBranch = + applicationMono.flatMap(application -> gitService.getProtectedBranches(application.getId())); + + StepVerifier.create(branchListMonoWithDefaultBranch) + .assertNext(branchList -> { + assertThat(branchList).containsExactly("master"); + }) + .verifyComplete(); + + Mono> branchListMonoWithoutDefaultBranch = applicationMono + .flatMap(application -> { + GitApplicationMetadata gitApplicationMetadata = application.getGitApplicationMetadata(); + // remove the default branch from the protected branches + gitApplicationMetadata.setBranchProtectionRules(List.of("develop", "feature")); + return applicationRepository.save(application); + }) + .flatMap(application -> gitService.getProtectedBranches(application.getId())); + + StepVerifier.create(branchListMonoWithoutDefaultBranch) + .assertNext(branchList -> { + assertThat(branchList).isNullOrEmpty(); + }) + .verifyComplete(); + } }