fix: fixing unprotect logic in callout #34244
Conversation
WalkthroughThe changes ensure that unprotecting a branch in the IDE's protected branches management only unprotects the selected branch, avoiding issues where all branches get unprotected simultaneously. Key updates include improved hook usage for state management and the addition of relevant test cases to confirm the functionality. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant UI as User Interface
participant Store as Redux Store
participant Backend as Backend Service
User->>UI: Clicks "Unprotect" on a specific branch
UI->>Store: Dispatches action to unprotect specific branch
Store->>Backend: API request to unprotect the branch
Backend->>Store: Confirms branch unprotected
Store->>UI: Updates state without the unprotected branch
UI->>User: Displays updated branches list
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
github-actions
bot
added
Branch protection
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/client/src/pages/Editor/IDE/ProtectedCallout.test.tsx (3 hunks)
- app/client/src/pages/Editor/IDE/ProtectedCallout.tsx (3 hunks)
Additional comments not posted (6)
app/client/src/pages/Editor/IDE/ProtectedCallout.tsx (4)
30-31: The usage ofuseSelectorto fetchcurrentBranchandprotectedBrancheslooks correct and is consistent with Redux best practices.
38-43: Good implementation of filtering logic to handle branch unprotection correctly. This should resolve the issue where all branches were being unprotected.
56-62: The conditional rendering based onisLoadingis a good practice to enhance user experience during asynchronous operations.
9-13: Ensure that the newly added selectors are properly tested.Verification successful
The
getProtectedBranchesSelectoris implemented and used in multiple places within the codebase, including test-related files. This confirms that the new selectors are properly integrated into the codebase.
app/client/src/selectors/gitSyncSelectors.tsx: Implementation and export.app/client/src/pages/Editor/gitSync/Tabs/Merge.tsx: Usage inuseSelector.app/client/src/pages/Editor/gitSync/components/BranchList.tsx: Usage inuseSelector.app/client/src/pages/Editor/gitSync/GitSettingsModal/TabBranch/GitProtectedBranches.tsx: Usage inuseSelector.app/client/src/pages/Editor/IDE/ProtectedCallout.tsx: Usage inuseSelector.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for test cases involving the new selectors. # Test: Search for the new selectors in test files. ast-grep --lang typescript --pattern $'import { getCurrentGitBranch, getProtectedBranchesSelector } from "selectors/gitSyncSelectors"; $$$'Length of output: 662
Script:
#!/bin/bash # Description: Search for test cases involving the new selectors. # Test: Search for `getCurrentGitBranch` selector in test files. ast-grep --lang typescript --pattern 'import { getCurrentGitBranch } from "selectors/gitSyncSelectors"; $$$' # Test: Search for `getProtectedBranchesSelector` selector in test files. ast-grep --lang typescript --pattern 'import { getProtectedBranchesSelector } from "selectors/gitSyncSelectors"; $$$'Length of output: 1223
Script:
#!/bin/bash # Description: Search for test cases involving the new selectors. # Test: Search for `getCurrentGitBranch` selector in test files. ast-grep --lang typescript --pattern 'import { getCurrentGitBranch } from "selectors/gitSyncSelectors";' # Test: Search for `getProtectedBranchesSelector` selector in test files. ast-grep --lang typescript --pattern 'import { getProtectedBranchesSelector } from "selectors/gitSyncSelectors";'Length of output: 1273
Script:
#!/bin/bash # Description: Verify the implementation and usage of `getProtectedBranchesSelector`. # Test: Search for the implementation of `getProtectedBranchesSelector`. ast-grep --lang typescript --pattern 'function getProtectedBranchesSelector($_) { $$$ }' # Test: Search for any usage of `getProtectedBranchesSelector` in the codebase. rg 'getProtectedBranchesSelector'Length of output: 1233
app/client/src/pages/Editor/IDE/ProtectedCallout.test.tsx (2)
43-50: Good use of mocking foruseDispatchto isolate the component behavior in tests.
91-118: The new test case effectively checks that only the current branch is unprotected, which aligns with the PR's objectives. However, ensure that the test covers all edge cases, such as when the current branch is not in the list of protected branches.
Description
Clicking "Unprotect branch" in the branch protection callout was making all the branches as unprotected. This PR fixes that and only allows the current branch to be unprotected
Fixes #33310 #34005
Automation
/ok-to-test tags="@tag.Git"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9511853863
Commit: 1ccab8a
Cypress dashboard url: Click here!
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes