Skip to content

Commit

Permalink
fix: git merge button visibility on branch change after a successful …
Browse files Browse the repository at this point in the history
…merge (#38847)

## Description
The `mergeSuccess` status in the reducer was not getting reset on branch
change.
Introduced a new action to reset it on change

Fixes #38844

## Automation

/ok-to-test tags="@tag.Git"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/12983463132>
> Commit: f012fbb
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=12983463132&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Git`
> Spec:
> <hr>Mon, 27 Jan 2025 07:39:33 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [ ] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
- Enhanced merge state management with a new `resetMergeSuccessState`
function
	- Improved ability to reset merge success state during branch selection

- **Bug Fixes**
	- Resolved potential state persistence issues in merge operations

- **Refactor**
- Updated hooks and components to support more flexible merge state
handling

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
ashit-rath authored Jan 27, 2025
1 parent e89788d commit 56018b0
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ interface TabMergeViewProps {
mergeStatus: FetchMergeStatusResponseData | null;
protectedBranches: FetchProtectedBranchesResponseData | null;
resetMergeState: () => void;
resetMergeSuccessState: () => void;
}

export default function TabMergeView({
Expand All @@ -99,6 +100,7 @@ export default function TabMergeView({
mergeStatus = null,
protectedBranches = null,
resetMergeState = noop,
resetMergeSuccessState = noop,
}: TabMergeViewProps) {
const [selectedBranchOption, setSelectedBranchOption] =
useState<BranchOption>();
Expand Down Expand Up @@ -208,9 +210,15 @@ export default function TabMergeView({
// when user selects a branch to merge
if (currentBranch && selectedBranchOption?.value) {
fetchMergeStatus(currentBranch, selectedBranchOption?.value);
resetMergeSuccessState();
}
},
[currentBranch, selectedBranchOption?.value, fetchMergeStatus],
[
currentBranch,
selectedBranchOption?.value,
fetchMergeStatus,
resetMergeSuccessState,
],
);

useEffect(
Expand Down
2 changes: 2 additions & 0 deletions app/client/src/git/components/OpsModal/TabMerge/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export default function TabMerge() {
mergeError,
mergeStatus,
resetMergeState,
resetMergeSuccessState,
} = useMerge();
const { isFetchStatusLoading, status } = useStatus();
const { branches, currentBranch, fetchBranches, isFetchBranchesLoading } =
Expand All @@ -42,6 +43,7 @@ export default function TabMerge() {
mergeStatus={mergeStatus}
protectedBranches={protectedBranches}
resetMergeState={resetMergeState}
resetMergeSuccessState={resetMergeSuccessState}
/>
);
}
7 changes: 7 additions & 0 deletions app/client/src/git/hooks/useMerge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ export default function useMerge() {
}
}, [artifactDef, dispatch]);

const resetMergeSuccessState = useCallback(() => {
if (artifactDef) {
dispatch(gitArtifactActions.resetMergeSuccessState({ artifactDef }));
}
}, [artifactDef, dispatch]);

return {
isMergeLoading: mergeState?.loading ?? false,
mergeError: mergeState?.error ?? null,
Expand All @@ -77,5 +83,6 @@ export default function useMerge() {
clearMergeStatus,
isMergeSuccess: isMergeSuccess ?? false,
resetMergeState,
resetMergeSuccessState,
};
}
6 changes: 6 additions & 0 deletions app/client/src/git/store/actions/mergeActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,9 @@ export const resetMergeStateAction = createArtifactAction((state) => {

return state;
});

export const resetMergeSuccessAction = createArtifactAction((state) => {
state.ui.mergeSuccess = false;

return state;
});
2 changes: 2 additions & 0 deletions app/client/src/git/store/gitArtifactSlice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ import {
mergeInitAction,
mergeSuccessAction,
resetMergeStateAction,
resetMergeSuccessAction,
} from "./actions/mergeActions";
import {
pollAutocommitProgressStopAction,
Expand Down Expand Up @@ -192,6 +193,7 @@ export const gitArtifactSlice = createSlice({
mergeSuccess: mergeSuccessAction,
mergeError: mergeErrorAction,
resetMergeState: resetMergeStateAction,
resetMergeSuccessState: resetMergeSuccessAction,
pullInit: pullInitAction,
pullSuccess: pullSuccessAction,
pullError: pullErrorAction,
Expand Down

0 comments on commit 56018b0

Please sign in to comment.