chore: git mod - fixes toast issues#39140
Conversation
WalkthroughThis pull request updates the discard functionality by changing its signature across components, hooks, sagas, and actions to require a success message string parameter. The changes modify how the discard operation is invoked and how success feedback is communicated through toast notifications. Additionally, the pull and protected branch update sagas now include toast displays that conditionally use the provided success message. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Component
participant H as useDiscard
participant S as DiscardSaga
participant T as Toast
C->>H: call discard(successMessage)
H->>S: dispatch discardInit(successMessage)
S->>T: if successMessage provided, toast.show(successMessage)
sequenceDiagram
participant PC as Pull Component
participant PS as PullSaga
participant T as Toast
PC->>PS: initiate pull request
PS->>T: toast.show(createMessage(DISCARD_AND_PULL_SUCCESS))
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
Bug
github-actions
bot
added
Bug
![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
🧹 Nitpick comments (1)
app/client/src/git/sagas/pullSaga.ts (1)
60-74: Consider adding error toast notifications for consistency.While success cases show toast notifications, error cases only log to console and Sentry. Consider adding error toast notifications for better user feedback.
} catch (e) { if (response && response.responseMeta.error) { const { error } = response.responseMeta; yield put(gitArtifactActions.pullError({ artifactDef, error })); + toast.show(error.message || "Pull failed", { + kind: "error", + }); } else { log.error(e); captureException(e); + toast.show("Failed to pull changes", { + kind: "error", + }); } }🧰 Tools
🪛 Biome (1.9.4)
[error] 61-61: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/client/src/git/components/OpsModal/TabDeploy/TabDeployView.tsx(2 hunks)app/client/src/git/components/QuickActions/QuickActionsView.tsx(3 hunks)app/client/src/git/hooks/useDiscard.ts(1 hunks)app/client/src/git/sagas/discardSaga.ts(2 hunks)app/client/src/git/sagas/pullSaga.ts(2 hunks)app/client/src/git/sagas/updateProtectedBranchesSaga.ts(2 hunks)app/client/src/git/store/actions/discardActions.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (11)
app/client/src/git/sagas/pullSaga.ts (2)
17-18: LGTM! Import statements are well-organized.The new imports for toast notifications are correctly placed and follow project conventions.
56-58: Verify the message constant usage.The saga is named
pullSagabut usesDISCARD_AND_PULL_SUCCESSmessage constant. Please verify if this is the intended message for a regular pull operation or if a different message constant should be used.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for other usages of DISCARD_AND_PULL_SUCCESS to understand the context rg "DISCARD_AND_PULL_SUCCESS" -A 5Length of output: 3523
Message constant usage verified
The constant DISCARD_AND_PULL_SUCCESS is defined as returning "Pulled from remote successfully" and is used consistently across both pullSaga and other components like QuickActionsView. Despite its name suggesting a "discard and pull" action, its message accurately reflects a successful pull operation. No changes are needed here.
app/client/src/git/store/actions/discardActions.ts (1)
7-10: LGTM! Clean interface extension.The optional successMessage property is well-integrated into the existing interface.
app/client/src/git/hooks/useDiscard.ts (1)
15-28: LGTM! Clean hook implementation.The useCallback implementation correctly handles the new successMessage parameter and includes all necessary dependencies.
app/client/src/git/sagas/discardSaga.ts (1)
17-40: LGTM! Well-structured toast implementation.The conditional toast display is properly placed before the reload delay, ensuring user visibility.
app/client/src/git/sagas/updateProtectedBranchesSaga.ts (1)
1-3: LGTM! Consistent toast implementation.The success notification follows the established pattern and is properly placed after the success actions.
Also applies to: 47-49
app/client/src/git/components/QuickActions/QuickActionsView.tsx (3)
8-8: LGTM!The import is correctly added and used in the component.
109-109: LGTM!The success message is correctly passed to the discard function.
31-31: Breaking change: Verify parent components.The
discardprop type now requires a success message parameter.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Find all components that pass the discard prop to QuickActionsView ast-grep --pattern 'discard={$_}'Length of output: 35
I'll run a new shell script to search for instances where QuickActionsView is used with the discard prop. This should help verify whether the parent components provide the required parameter.
🏁 Script executed:
#!/bin/bash echo "Searching for QuickActionsView usage with discard prop..." rg --heading --line-number "<QuickActionsView[^>]*discard=" || echo "No matches found."Length of output: 231
Breaking Change Verification:
The new discard prop now requires a success message parameter. Our search for QuickActionsView usages with a discard prop did not reveal any instances, which suggests that no parent components are currently passing this prop incorrectly. Please ensure that any indirect invocations or dynamic prop assignments are manually verified to cover all cases.app/client/src/git/components/OpsModal/TabDeploy/TabDeployView.tsx (2)
242-242: LGTM!The success message is correctly passed to the discard function.
78-78: 🏁 Script executed:#!/bin/bash # Find all components that pass the discard prop to TabDeployView ast-grep --pattern 'discard={$_}'Length of output: 35
github-actions
bot
added
Bug
## Description Discard toast issue, introduced in #39140 ## Automation /ok-to-test tags="@tag.Git" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!CAUTION] > 🔴 🔴 🔴 Some tests have failed. > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/13240898404> > Commit: f8e760a > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=13240898404&attempt=4&selectiontype=test&testsstatus=failed&specsstatus=fail" target="_blank">Cypress dashboard</a>. > Tags: @tag.Git > Spec: > The following are new failures, please fix them before merging the PR: <ol> > <li>cypress/e2e/Regression/ServerSide/QueryPane/Mongo1_spec.ts</ol> > <a href="https://internal.appsmith.com/app/cypress-dashboard/identified-flaky-tests-65890b3c81d7400d08fa9ee3?branch=master" target="_blank">List of identified flaky tests</a>. > <hr>Mon, 10 Feb 2025 22:21:53 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 - **Bug Fixes** - Updated the discard confirmation so users now receive a clear success notification, ensuring more accurate feedback when changes are discarded. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Description Fixes issue with missing success toast related to git Fixes appsmithorg#38872 Fixes appsmithorg#38862 ## 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/13216997415> > Commit: 95ccaed > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=13216997415&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Git` > Spec: > <hr>Sat, 08 Feb 2025 17:43:52 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 - Discard operations now display clear, contextual success messages to enhance user feedback. - Toast notifications have been added for pull actions and protected branch updates, providing real-time confirmation of successful operations. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Description Discard toast issue, introduced in appsmithorg#39140 ## Automation /ok-to-test tags="@tag.Git" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!CAUTION] > 🔴 🔴 🔴 Some tests have failed. > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/13240898404> > Commit: f8e760a > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=13240898404&attempt=4&selectiontype=test&testsstatus=failed&specsstatus=fail" target="_blank">Cypress dashboard</a>. > Tags: @tag.Git > Spec: > The following are new failures, please fix them before merging the PR: <ol> > <li>cypress/e2e/Regression/ServerSide/QueryPane/Mongo1_spec.ts</ol> > <a href="https://internal.appsmith.com/app/cypress-dashboard/identified-flaky-tests-65890b3c81d7400d08fa9ee3?branch=master" target="_blank">List of identified flaky tests</a>. > <hr>Mon, 10 Feb 2025 22:21:53 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 - **Bug Fixes** - Updated the discard confirmation so users now receive a clear success notification, ensuring more accurate feedback when changes are discarded. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 participants
Description
Fixes issue with missing success toast related to git
Fixes #38872
Fixes #38862
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/13216997415
Commit: 95ccaed
Cypress dashboard.
Tags:
@tag.GitSpec:
Sat, 08 Feb 2025 17:43:52 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit