-
Notifications
You must be signed in to change notification settings - Fork 111
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.
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
feat: add wallet and account to appZi feedback for debugging #5472
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
WalkthroughThis change updates the feedback and survey functionalities by extending the data structures and function signatures. The Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant AB as AppziButton
participant O as openFeedbackAppzi
participant US as updateAppziSettings
U->>AB: Click feedback button
AB->>AB: Retrieve wallet details (account, walletName, chainId)
AB->>O: Invoke showFeedbackModal with wallet info
O->>US: Call updateAppziSettings with account, walletName, chainId
sequenceDiagram
participant UO as _updateOrders
participant TN as _triggerNps
participant TS as triggerAppziSurvey
UO->>TN: Call _triggerNps(pending, chainId, account)
TN->>TS: Invoke triggerAppziSurvey with account & settings
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
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/cowswap-frontend/src/appzi.ts (1)
132-132
: Add trailing comma for consistent parameter styling.This is a minor stylistic change that enhances readability and helps with future parameter additions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/cowswap-frontend/src/appzi.ts
(3 hunks)apps/cowswap-frontend/src/common/updaters/orders/PendingOrdersUpdater.ts
(3 hunks)apps/cowswap-frontend/src/legacy/components/AppziButton/index.tsx
(4 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
apps/cowswap-frontend/src/legacy/components/AppziButton/index.tsx
[error] 6-6: There should be at least one empty line between import groups (import/order)
[error] 7-7: There should be at least one empty line between import groups (import/order)
[error] 7-7: react
import should occur before import of @cowprotocol/assets/cow-swap/feedback.svg
(import/order)
[error] 68-68: React Hook "useWalletInfo" is called conditionally. React Hooks must be called in the exact same order in every component render (react-hooks/rules-of-hooks)
[error] 69-69: React Hook "useWalletDetails" is called conditionally. React Hooks must be called in the exact same order in every component render (react-hooks/rules-of-hooks)
[error] 71-71: React Hook "useCallback" is called conditionally. React Hooks must be called in the exact same order in every component render (react-hooks/rules-of-hooks)
🔇 Additional comments (10)
apps/cowswap-frontend/src/legacy/components/AppziButton/index.tsx (4)
3-3
: Add wallet-related imports to enhance user feedback context.The addition of wallet-related hooks provides valuable context for user feedback, making support interactions more effective.
24-26
: Improve transition property formatting for better readability.The transition properties have been properly formatted with line breaks for better readability.
35-37
: Improve transition property formatting for better readability.The transition properties have been properly formatted with line breaks for better readability.
76-76
: Improved onClick handler readability with callback abstraction.The onClick handler now uses a callback function, which improves readability and maintainability.
apps/cowswap-frontend/src/appzi.ts (3)
65-65
: Add wallet name to enhance feedback context.Adding
walletName
to the settings allows support teams to better understand the user's environment, enhancing troubleshooting capabilities.
93-93
: Update call to updateAppziSettings with wallet information.The function now passes account, chainId, and walletName to provide richer context for user feedback.
89-91
:✅ Verification successful
Function signature updated to include wallet information.
The function now properly accepts wallet-related information, ensuring that feedback contains important context about the user's session.
🏁 Script executed:
#!/bin/bash # Description: Verify that the openFeedbackAppzi function is called with the correct parameters # throughout the codebase # Search for all calls to openFeedbackAppzi to ensure they pass the required parameters rg -A 3 "openFeedbackAppzi" --type ts --type tsxLength of output: 76
🏁 Script executed:
#!/bin/bash # Re-run verification: Search for calls to openFeedbackAppzi in TS and TSX files using explicit globs rg -A 3 "openFeedbackAppzi" -g "*.ts" -g "*.tsx"Length of output: 1190
Final verification: Function signature and usage are correct.
- The
openFeedbackAppzi
function now accepts wallet-related information as required.- Verification confirms that calls (e.g., in
apps/cowswap-frontend/src/legacy/components/AppziButton/index.tsx
) pass theaccount
,chainId
, andwalletName
parameters.Approved as is.
apps/cowswap-frontend/src/common/updaters/orders/PendingOrdersUpdater.ts (3)
203-203
: Update _triggerNps call to include account information.The function call now correctly passes the account parameter to _triggerNps, ensuring wallet information is included in user feedback.
343-343
: Function signature updated to include account parameter.The _triggerNps function now accepts an account parameter, allowing it to pass this information to the Appzi feedback system.
357-357
: Include account information in NPS survey data.Adding the account parameter to triggerAppziSurvey ensures that support teams can correctly identify which wallet is experiencing issues with long-pending orders.
apps/cowswap-frontend/src/legacy/components/AppziButton/index.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @anxolin , great job!
There are some issues (nipicks) like:
-
wallet shows 'injected' when the app is opened inside a walelt's browser (like imToken, MM). It is a known issue related to this task Detect injected wallet name #792
-
inside the safe, it does not capture the image (however, it is on Prod also)
-
it does not show a connection type (e.g. connected to Safe using WC), just shows a wallet name. Not sure whether we should fix this
Tanks @elena-zh ! I've added this into our side quests. I agree with the suggestions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/cowswap-frontend/src/legacy/components/AppziButton/index.tsx (2)
25-27
: Consider consolidating transition properties for brevity.
The multi-line transitions are fully valid; however, you may consider consolidating properties into single lines to improve readability. This is purely a style preference and doesn’t affect functionality.Also applies to: 36-38
78-78
: Confirm user consent regarding PII.
This button action transmits the wallet address and name. Verify that users are aware of this data sharing for support purposes, and that it aligns with your privacy policies.Need help creating a modal or tooltip explaining the implications of sharing wallet details?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/cowswap-frontend/src/legacy/components/AppziButton/index.tsx
(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Cypress
- GitHub Check: Setup
🔇 Additional comments (1)
apps/cowswap-frontend/src/legacy/components/AppziButton/index.tsx (1)
66-71
: Ensure handling of undefined or empty wallet details.
Ifaccount
,chainId
, orwalletName
might be undefined when the user isn’t connected, verify thatopenFeedbackAppzi
gracefully handles these values without throwing errors or storing unwanted null data.
Summary
This PR adds the wallet address and the wallet name to appZi button.
Users ask for help on various issues, this additional context gives support important information to help the user.
To Test
Additional tests
Summary by CodeRabbit