fix: Delete / Archive / Block conversations and users#75
fix: Delete / Archive / Block conversations and users#75thierryskoda wants to merge 1 commit intomainfrom
Conversation
WalkthroughThis update introduces new consent and conversation management features, including query and mutation utilities for consent states, improved conversation deletion logic, and enhanced debug menu options. Several utility functions and hooks are added or refactored for type safety and explicitness. Some group-related utilities and tests are removed, and navigation and screen naming are updated for clarity. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DebugMenu
participant XMTPService
participant Alert
User->>DebugMenu: Select "Get last 10 XMTP messages"
DebugMenu->>XMTPService: Fetch conversation ID, sync, get last 10 messages
XMTPService-->>DebugMenu: Return messages or error
DebugMenu->>Alert: Show messages or error
User->>DebugMenu: Select "Get disappearing message settings"
DebugMenu->>XMTPService: Fetch conversation ID, sync, get settings
XMTPService-->>DebugMenu: Return settings or error
DebugMenu->>Alert: Show settings or error
sequenceDiagram
participant User
participant ConsentMutation
participant ReactQueryCache
User->>ConsentMutation: Update consent for inbox ID
ConsentMutation->>ReactQueryCache: Optimistically update consent state
ConsentMutation->>ConsentMutation: Perform async update
alt Success
ConsentMutation-->>ReactQueryCache: Confirm cache update
else Error
ConsentMutation-->>ReactQueryCache: Rollback to previous consent state
end
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
| consent: IConsentState | ||
| }, | ||
| ) { | ||
| return reactQueryClient.setQueryData(getConsentForInboxIdQueryOptions(args).queryKey, args) |
There was a problem hiding this comment.
The call to reactQueryClient.setQueryData in setConsentForInboxIdQueryData is caching the entire args object rather than just the IConsentState that your queryFn returns. This leads to inconsistent data shapes in the cache for the key generated by getConsentForInboxIdQueryOptions. Consider passing only args.consent to setQueryData so that the cached data matches the shape returned by the query function.
- return reactQueryClient.setQueryData(getConsentForInboxIdQueryOptions(args).queryKey, args)
+ return reactQueryClient.setQueryData(getConsentForInboxIdQueryOptions(args).queryKey, args.consent)🚀 Reply to ask Macroscope to explain or update this suggestion.
👍 Helpful? React to give us feedback.
There was a problem hiding this comment.
Actionable comments posted: 16
🔭 Outside diff range comments (1)
features/consent/use-allow-group.mutation.ts (1)
75-82:⚠️ Potential issueCritical: Incorrect peer inbox ID in consent update.
The
peerInboxIdparameter is incorrectly set toclientInboxIdon line 78, which means the user is updating consent for themselves rather than for the intended peer inbox IDs.Apply this fix to use the correct peer inbox IDs:
...(inboxIdsToAllow.length > 0 ? [ - executeUpdateConsentForInboxIdMutation({ - peerInboxId: clientInboxId, - consent: "allowed", - clientInboxId, - }), + ...inboxIdsToAllow.map((peerInboxId) => + executeUpdateConsentForInboxIdMutation({ + peerInboxId, + consent: "allowed", + clientInboxId, + }), + ), ] : []),
🧹 Nitpick comments (5)
utils/time.utils.ts (1)
49-81: Good implementation with some considerations for time approximations.The function correctly implements hierarchical time conversion with proper pluralization. However, consider these points:
- Approximate calculations: Using 30 days/month and 365 days/year may be misleading for precise durations
- Missing edge case: No handling for negative values (could cause unexpected behavior)
For UI display purposes, these approximations are likely acceptable, but consider adding a comment documenting this behavior.
Consider adding input validation:
export function getHumanReadableTimeFromMs(ms: number) { + if (ms < 0) { + return "invalid duration" + } if (ms < 1000) { return "less than a second" }Or add a JSDoc comment documenting the approximations:
+/** + * Converts milliseconds to human-readable time string. + * Note: Uses approximate values (30 days/month, 365 days/year) for display purposes. + */ export function getHumanReadableTimeFromMs(ms: number) {components/prompt-async.ts (1)
29-29: Consider simplifying the return type.The return type
Promise<{ value: string | undefined }>wraps the result in an object. Consider returningPromise<string | undefined>directly for simpler usage, unless the object wrapper serves a specific purpose.- return new Promise<{ value: string | undefined }>((resolve) => { + return new Promise<string | undefined>((resolve) => { Alert.prompt( title, message, [ { text: cancelText, style: "cancel", - onPress: () => resolve({ value: undefined }), + onPress: () => resolve(undefined), }, { text: submitText, style: "default", - onPress: (value?: string) => resolve({ value }), + onPress: (value?: string) => resolve(value), }, ],features/consent/use-deny-group.mutation.ts (1)
16-28: Consider renaming the function to reflect conversation-based logic.The function name
useDenyGroupMutationsuggests group-based operations, but the implementation now uses conversation-based consent updates withupdateXmtpConsentForConversationForInbox. This naming inconsistency could confuse developers about the function's actual behavior.Consider renaming to
useDenyConversationMutationor adding a comment explaining why the group naming is retained:-export const useDenyGroupMutation = (args: { +export const useDenyConversationMutation = (args: { clientInboxId: IXmtpInboxId xmtpConversationId: IXmtpConversationId }) => {features/consent/consent.utils.ts (1)
18-30: Question the necessity of this conversion function.While the implementation is correct, since
IXmtpConsentStateis defined asConsentState(from the relevant code snippets), this function is essentially performing an identity mapping. The conversion appears redundant unless there are plans to diverge these types in the future.Consider whether this bidirectional conversion is necessary or if the types could be used interchangeably:
-export function convertXmtpConsentStateToConsentState( - consentState: IXmtpConsentState, -): IConsentState { - if (consentState === "allowed") { - return "allowed" - } - - if (consentState === "unknown") { - return "unknown" - } - - return "denied" -} +// Consider if this function is needed given that IXmtpConsentState === ConsentStateutils/react-query/react-query.utils.ts (1)
10-15: Consider removing redundant runtime filtering.The runtime filtering of the "caller" key seems redundant given the compile-time type constraint. The type assertion on line 10 removes the compile-time safety you've added. Consider maintaining type safety throughout:
- const { baseStr, ...rest } = args as { baseStr: string; [key: string]: unknown } - - // Make sure caller isn't in the rest of the keys. - const filteredEntries = Object.entries(rest).filter(([key]) => key !== "caller") - - return [baseStr, ...filteredEntries.map(([key, value]) => `${key}: ${String(value)}`)] + const { baseStr, ...rest } = args + + return [baseStr, ...Object.entries(rest).map(([key, value]) => `${key}: ${String(value)}`)]If you need to support legacy code that might pass "caller", consider creating a separate migration function instead of mixing compile-time and runtime checks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
components/debug-menu.tsx(2 hunks)components/prompt-async.ts(1 hunks)features/archived-conversations/archived-conversations.screen.tsx(1 hunks)features/archived-conversations/denied-conversations.query.ts(1 hunks)features/archived-conversations/use-archived-conversations-for-current-sender.ts(3 hunks)features/consent/consent-for-inbox-id.query.ts(1 hunks)features/consent/consent.utils.ts(1 hunks)features/consent/update-consent-for-inbox-id.mutation.ts(1 hunks)features/consent/use-allow-dm.mutation.ts(3 hunks)features/consent/use-allow-group.mutation.ts(4 hunks)features/consent/use-deny-dm.mutation.ts(3 hunks)features/consent/use-deny-group.mutation.ts(2 hunks)features/consent/use-group-consent-for-current-sender.ts(0 hunks)features/conversation/conversation-chat/conversation-consent-popup/conversation-consent-popup-dm.tsx(5 hunks)features/conversation/conversation-chat/conversation-consent-popup/conversation-consent-popup-group.tsx(2 hunks)features/conversation/conversation-create/mutations/create-conversation-and-send-first-message.mutation.ts(2 hunks)features/conversation/conversation-list/hooks/use-delete-dm.ts(5 hunks)features/conversation/conversation-list/hooks/use-delete-group.ts(2 hunks)features/conversation/conversation-requests-list/delete-conversations.mutation.ts(2 hunks)features/conversation/queries/conversation.query.ts(1 hunks)features/current-user/current-user.query.ts(2 hunks)features/groups/utils/getGroupMemberActions.test.ts(0 hunks)features/groups/utils/getGroupMemberActions.ts(0 hunks)features/groups/utils/groupActionHandlers.ts(0 hunks)features/preferred-display-info/use-preferred-display-info.ts(2 hunks)features/profiles/profile-me.tsx(1 hunks)features/xmtp/xmtp-consent/xmtp-consent.ts(3 hunks)features/xmtp/xmtp-conversations/xmtp-conversations-sync.ts(1 hunks)features/xmtp/xmtp-disappearing-messages/xmtp-disappearing-messages.ts(1 hunks)navigation/app-navigator.tsx(2 hunks)navigation/navigation.utils.ts(1 hunks)stores/app-state-store/app-state-store.service.ts(2 hunks)utils/react-query/react-query.helpers.ts(2 hunks)utils/react-query/react-query.utils.ts(1 hunks)utils/time.utils.ts(1 hunks)
💤 Files with no reviewable changes (4)
- features/groups/utils/getGroupMemberActions.test.ts
- features/consent/use-group-consent-for-current-sender.ts
- features/groups/utils/getGroupMemberActions.ts
- features/groups/utils/groupActionHandlers.ts
🧰 Additional context used
🧬 Code Graph Analysis (16)
navigation/app-navigator.tsx (1)
features/archived-conversations/archived-conversations.screen.tsx (1)
ArchivedConversationsScreen(11-34)
features/consent/use-deny-group.mutation.ts (1)
features/xmtp/xmtp-consent/xmtp-consent.ts (1)
updateXmtpConsentForConversationForInbox(76-102)
features/consent/use-deny-dm.mutation.ts (2)
features/xmtp/xmtp-consent/xmtp-consent.ts (1)
updateXmtpConsentForConversationForInbox(76-102)features/consent/update-consent-for-inbox-id.mutation.ts (1)
executeUpdateConsentForInboxIdMutation(63-69)
features/xmtp/xmtp-disappearing-messages/xmtp-disappearing-messages.ts (2)
features/xmtp/xmtp.helpers.ts (1)
wrapXmtpCallWithDuration(23-106)utils/date.ts (1)
getTodayNs(95-97)
features/archived-conversations/use-archived-conversations-for-current-sender.ts (1)
features/conversation/conversation-list/conversations-allowed-consent.query.ts (1)
useAllowedConsentConversationsQuery(27-29)
features/profiles/profile-me.tsx (1)
design-system/list-item.tsx (1)
ListItem(35-68)
features/consent/consent.utils.ts (1)
features/xmtp/xmtp.types.ts (1)
IXmtpConsentState(56-56)
navigation/navigation.utils.ts (1)
navigation/navigation.types.tsx (1)
NavigationParamList(3-56)
utils/react-query/react-query.helpers.ts (2)
utils/logger/logger.ts (1)
queryLogger(121-121)utils/react-query/react-query.client.ts (1)
reactQueryClient(10-143)
features/conversation/conversation-list/hooks/use-delete-group.ts (1)
features/xmtp/xmtp-consent/xmtp-consent.ts (1)
updateXmtpConsentForConversationForInbox(76-102)
features/consent/use-allow-group.mutation.ts (2)
features/xmtp/xmtp-consent/xmtp-consent.ts (1)
updateXmtpConsentForConversationForInbox(76-102)features/consent/update-consent-for-inbox-id.mutation.ts (1)
executeUpdateConsentForInboxIdMutation(63-69)
features/conversation/conversation-requests-list/delete-conversations.mutation.ts (4)
features/xmtp/xmtp.types.ts (1)
IXmtpConversationId(51-51)features/authentication/multi-inbox.store.ts (1)
getSafeCurrentSender(290-296)features/convos-identities/convos-identities.service.ts (1)
ensureDeviceIdentityForInboxId(13-29)features/conversation/conversation-metadata/conversation-metadata.api.ts (1)
deleteConversationMetadata(116-127)
features/current-user/current-user.query.ts (1)
utils/react-query/react-query.helpers.ts (1)
ensureQueryDataBetter(117-125)
features/conversation/conversation-chat/conversation-consent-popup/conversation-consent-popup-group.tsx (11)
features/conversation/conversation-chat/conversation.store-context.tsx (1)
useCurrentXmtpConversationIdSafe(108-110)features/authentication/multi-inbox.store.ts (1)
useSafeCurrentSender(298-304)navigation/use-navigation.tsx (1)
useRouter(34-108)features/consent/use-allow-group.mutation.ts (1)
useAllowGroupMutation(170-172)features/conversation/conversation-requests-list/delete-conversations.mutation.ts (1)
useDeleteConversationsMutation(17-107)features/groups/queries/group.query.ts (1)
ensureGroupQueryData(143-145)features/consent/consent-for-inbox-id.query.ts (1)
ensureConsentForInboxIdQueryData(55-57)features/preferred-display-info/use-preferred-display-info.ts (1)
ensurePreferredDisplayInfo(224-288)components/action-sheet.tsx (1)
showActionSheet(68-73)utils/capture-error.ts (1)
captureErrorWithToast(53-67)utils/error.ts (1)
GenericError(83-87)
features/preferred-display-info/use-preferred-display-info.ts (5)
features/xmtp/xmtp-inbox-id/xmtp-inbox-id-from-eth-address.query.ts (1)
getXmtpInboxIdFromEthAddressQueryOptions(18-35)features/profiles/profiles.query.ts (1)
getProfileQueryConfig(20-47)utils/react-query/react-query.constants.ts (1)
reactQueryFreshDataQueryOptions(20-23)features/social-profiles/social-profiles-for-inbox-id.query.ts (1)
getSocialProfilesForInboxIdQueryOptions(15-52)features/social-profiles/social-profiles.query.ts (1)
getSocialProfilesForEthAddressQueryOptions(24-49)
features/conversation/conversation-list/hooks/use-delete-dm.ts (1)
features/conversation/conversation-requests-list/delete-conversations.mutation.ts (1)
useDeleteConversationsMutation(17-107)
🪛 Biome (1.9.4)
utils/react-query/react-query.utils.ts
[error] 8-8: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
🔇 Additional comments (28)
navigation/navigation.utils.ts (1)
170-176: Excellent type-safe navigation utility implementation!The function provides clean, type-safe access to route parameters with proper generic constraints and null handling. The implementation correctly leverages TypeScript's type system to ensure compile-time safety.
utils/react-query/react-query.helpers.ts (1)
99-99: Excellent improvement to query enablement logic!The change from
!args.enabledto"enabled" in args && !args.enabledis a significant improvement that ensures queries are only skipped whenenabledis explicitly set tofalse, not when it'sundefined. This aligns better with React Query's expected behavior and prevents unintended query skipping.This change enhances type safety and makes the intent more explicit.
Also applies to: 110-110, 118-118
features/current-user/current-user.query.ts (1)
48-48: LGTM! Improved error handling with helper function.The change to use
ensureQueryDataBetteradds valuable validation by checking if the query is disabled before attempting to ensure data. This prevents potential runtime issues and provides better error messages when trying to ensure data for disabled queries.features/xmtp/xmtp-disappearing-messages/xmtp-disappearing-messages.ts (1)
80-90: LGTM! Enhanced debugging information.The dynamic descriptive label including
conversationIdandretentionDurationInNsimproves debugging and monitoring capabilities. This will make it easier to trace specific disappearing message operations in logs.features/conversation/conversation-list/hooks/use-delete-group.ts (2)
12-12: Approve the import update for conversation-based consent management.The import change aligns with the broader refactoring from group-based to conversation-based consent management mentioned in the PR objectives.
87-91: Approve the function call update.The function call and parameter name changes are consistent with the new conversation-based consent API. The logic remains correct, updating consent for the conversation when deleting and blocking.
features/archived-conversations/archived-conversations.screen.tsx (1)
5-5: Update import path aligns with refactoring.The import change from blocked-conversations to archived-conversations feature directory is consistent with the component's new purpose.
stores/app-state-store/app-state-store.service.ts (1)
108-114: LGTM! Good security enhancement.The authentication gating logic properly prevents app state side effects from executing for unauthenticated users. This is a security improvement that ensures streaming, notifications, and conversation query invalidations only occur when appropriate.
features/archived-conversations/use-archived-conversations-for-current-sender.ts (1)
11-11: LGTM! Variable renaming improves clarity.The renaming from
conversationIdstoallowedConversationIdsbetter reflects the semantic meaning and improves code readability throughout the hook.Also applies to: 18-18, 28-28, 38-38
features/consent/use-deny-dm.mutation.ts (1)
33-37: LGTM! Consent state mutation update is correct.The replacement of
setXmtpConsentStateForInboxIdwithexecuteUpdateConsentForInboxIdMutationmaintains the same parameters while using the updated mutation pattern consistent with the new consent management system.features/conversation/queries/conversation.query.ts (1)
151-159: LGTM! Good optimization to check conversation existence early.The refactor to retrieve and validate conversation data before fetching messages is a smart optimization that prevents unnecessary work when the conversation doesn't exist in the query cache.
features/consent/use-allow-dm.mutation.ts (1)
3-3:Details
✅ Verification successful
Verify the refactoring from group-based to conversation-based consent is consistent.
The refactoring looks correct, changing from group-based consent (
updateXmtpConsentForGroupsForInbox,groupIds) to conversation-based consent (updateXmtpConsentForConversationForInbox,conversationIds). This aligns with the broader changes mentioned in the AI summary.Also applies to: 16-16, 26-36
🏁 Script executed:
#!/bin/bash # Verify that all usages of the old group-based consent functions have been updated echo "Searching for any remaining usage of old group-based consent functions..." # Check for old function usage rg -A 3 "updateXmtpConsentForGroupsForInbox|setXmtpConsentStateForInboxId" --type ts echo "Searching for new conversation-based consent function usage..." # Check new function usage rg -A 3 "updateXmtpConsentForConversationForInbox|executeUpdateConsentForInboxIdMutation" --type tsLength of output: 12799
All group-based consent functions have been replaced.
The search confirms that there are no remaining calls to the old
updateXmtpConsentForGroupsForInboxAPI. The only usage ofsetXmtpConsentStateForInboxIdis inside the newexecuteUpdateConsentForInboxIdMutationinfeatures/consent/update-consent-for-inbox-id.mutation.ts, which is the expected low-level implementation for inbox-level consent updates. All other mutations now consistently use the conversation-based APIs (updateXmtpConsentForConversationForInboxandexecuteUpdateConsentForInboxIdMutation).No further changes required.
features/conversation/conversation-chat/conversation-consent-popup/conversation-consent-popup-dm.tsx (2)
44-44: Consider improving error message consistency.The error messages could be more specific and consistent:
- Line 44: The dialog text is clear and user-friendly
- Line 58-59: "Error deleting conversation" is good
- Line 78: "Error joining conversation" is appropriate
The error handling looks good overall.
Also applies to: 58-59, 78-78
6-6:Details
✅ Verification successful
LGTM! The refactoring from blocking to deleting conversations is well-implemented.
The changes correctly update the component to use the new deletion mutation with improved user messaging. The
alsoDenyInviterConsent: trueparameter appropriately ensures that deleting also prevents future messages.Also applies to: 31-31, 34-65
🏁 Script executed:
#!/bin/bash # Verify the new delete conversations mutation interface echo "Checking the deleteConversationsMutation interface..." # Find the mutation definition ast-grep --pattern 'export function useDeleteConversationsMutation() { $$$ }' # Also check the mutation function signature rg -A 10 "useDeleteConversationsMutation|deleteConversationsAsync" --type tsLength of output: 28989
Approved: deleteConversations mutation interface matches component usage
The refactoring to use
useDeleteConversationsMutationwith{ conversationIds, alsoDenyInviterConsent }aligns perfectly with the mutation signature indelete-conversations.mutation.ts. No further changes needed.features/preferred-display-info/use-preferred-display-info.ts (1)
55-64: Excellent refactoring to extract query options and improve enabled logic.The refactoring improves code organization by extracting query option objects and correctly combines the hook's
enabledparameter with each query's ownenabledproperty. This pattern (enabled && queryOptions.enabled !== false) ensures proper query control flow.Also applies to: 79-85, 87-98, 102-111
components/debug-menu.tsx (1)
469-571: Well-implemented debug menu additions!The new XMTP debug options are properly implemented with:
- Appropriate null checks for conversation context
- Proper loading state management
- Comprehensive error handling
- User-friendly formatting of disappearing message settings
features/conversation/conversation-list/hooks/use-delete-dm.ts (1)
50-77: Clean refactoring to use centralized deletion mutation!The migration to
useDeleteConversationsMutationis well-executed:
- Proper use of the
alsoDenyInviterConsentflag- Efficient concurrent execution with
Promise.allfor delete and block- Maintained error handling
features/xmtp/xmtp-consent/xmtp-consent.ts (3)
12-28: Good error handling improvement!The addition of try-catch error handling with contextual error messages improves debugging and error tracking.
30-47: Well-structured new function for consent state retrieval.The new
getXmtpConsentStateForInboxIdfunction follows the established error handling pattern and provides clear functionality for retrieving consent states.
76-102: Consistent refactoring from groups to conversations.The function renaming and parameter updates align well with the PR's objective of transitioning from group-based to conversation-based consent management.
features/consent/update-consent-for-inbox-id.mutation.ts (1)
1-73: Well-implemented mutation with proper optimistic updates!The mutation follows React Query best practices with optimistic updates in
onMutateand proper rollback inonError. The imperative execution function and React hook provide flexible usage options.features/conversation/conversation-chat/conversation-consent-popup/conversation-consent-popup-group.tsx (2)
3-33: Good setup with proper mutation configuration!The new imports are well-organized and the mutations are correctly initialized with the required parameters.
103-141: Clean implementation with proper error handling!The
onAcceptcallback is correctly updated to use the new mutation with explicit parameters, and the error handling with toast notifications provides good user feedback.features/consent/consent-for-inbox-id.query.ts (1)
1-53: Well-structured query implementation with proper type safety!The query options function correctly handles the enabled state with
skipToken, and the type definitions with bothIArgsandIStrictArgsprovide good type safety for different use cases.features/archived-conversations/denied-conversations.query.ts (4)
1-8: Clean imports and good organization.The imports are well-organized and appropriately scoped for the functionality provided.
9-11: Good type safety practice.Using
Awaited<ReturnType<...>>ensures type consistency between the query function and its exported type.
99-120: Well-structured query options with proper React Query patterns.The query options function follows React Query best practices with proper enablement logic and metadata inclusion.
122-146: Standard cache management utilities implemented correctly.Both invalidation and refetch utilities properly use the query key from options and follow React Query patterns.
| export function promptAsync(args: { | ||
| title: string | ||
| message?: string | ||
| type?: "plain-text" | "secure-text" | ||
| defaultValue?: string | ||
| submitText?: string | ||
| cancelText?: string | ||
| keyboardType?: string | ||
| options?: AlertOptions | ||
| }) { | ||
| const { | ||
| title, | ||
| message, | ||
| type = "plain-text", | ||
| defaultValue = "", | ||
| submitText = "OK", | ||
| cancelText = "Cancel", | ||
| keyboardType, | ||
| options, | ||
| } = args | ||
|
|
||
| return new Promise<{ value: string | undefined }>((resolve) => { | ||
| Alert.prompt( | ||
| title, | ||
| message, | ||
| [ | ||
| { | ||
| text: cancelText, | ||
| style: "cancel", | ||
| onPress: () => resolve({ value: undefined }), | ||
| }, | ||
| { | ||
| text: submitText, | ||
| style: "default", | ||
| onPress: (value?: string) => resolve({ value }), | ||
| }, | ||
| ], | ||
| type, | ||
| defaultValue, | ||
| keyboardType, | ||
| options, | ||
| ) | ||
| }) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add platform compatibility considerations.
Alert.prompt is iOS-only in React Native. Consider adding platform checks or documenting this limitation to prevent runtime errors on Android.
Add platform check or documentation:
+import { Platform } from "react-native"
+
/**
* Creates an Alert with a text input that returns a Promise which resolves with
* the input value when submitted, or undefined if canceled.
+ * Note: iOS only - Alert.prompt is not available on Android
*/
export function promptAsync(args: {
// ... existing args
}) {
+ if (Platform.OS !== 'ios') {
+ return Promise.resolve({ value: undefined })
+ }
+
// ... rest of implementation
}🏁 Script executed:
#!/bin/bash
# Check if there are any cross-platform prompt solutions already in use
rg -A 5 -B 5 "Alert\.prompt|react-native-prompt" --type ts --type tsxLength of output: 99
#!/bin/bash
# Search for usage of Alert.prompt in TS and TSX files
rg -A 5 -B 5 "Alert\.prompt" --glob "*.ts" --glob "*.tsx"
# Search for any import or reference to react-native-prompt
rg -A 5 -B 5 "react-native-prompt" --glob "*.ts" --glob "*.tsx"#!/bin/bash
# Search for any usage of Alert.prompt across the repository
rg -A5 -B5 "Alert\.prompt" .
# Search for any references to react-native-prompt (cross-platform lib)
rg -A5 -B5 "react-native-prompt" .
# Check if Platform is already imported in prompt-async file
rg "import.*Platform" -g "*prompt-async.ts"#!/bin/bash
# Search for any usage of Alert.prompt across the repository
rg -A5 -B5 "Alert\.prompt" .
# Search for any references to a cross-platform prompt library
rg -A5 -B5 "react-native-prompt" .
# Check if Platform is already imported in the prompt-async implementation
rg "import.*Platform" -g "components/prompt-async.ts"🏁 Script executed:
#!/bin/bash
# Search for Alert.prompt usage in TypeScript files
rg -n "Alert\\.prompt" --glob "*.ts" --glob "*.tsx" .
# Check if Platform is imported in prompt-async.ts
rg -n "import .*Platform" components/prompt-async.ts
# Look for any cross-platform prompt libraries
rg -n "react-native-prompt" --glob "*.ts" --glob "*.tsx" .Length of output: 218
Add iOS‐only guard around Alert.prompt
Alert.prompt is only supported on iOS. Without a Platform check, calling this on Android will crash at runtime.
Please update components/prompt-async.ts to:
• Import Platform from react-native.
• Short‐circuit and resolve immediately on non‐iOS platforms.
• Document the iOS‐only limitation in the JSDoc.
For example:
+ import { Platform } from "react-native"
/**
* Creates an Alert with a text input and returns a Promise.
+ * Note: Alert.prompt is iOS-only; on Android this returns { value: undefined } immediately.
*/
export function promptAsync(args: { /* … */ }) {
+ if (Platform.OS !== "ios") {
+ return Promise.resolve({ value: undefined })
+ }
return new Promise<{ value: string | undefined }>((resolve) => {
Alert.prompt(
/* … */
)
})
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function promptAsync(args: { | |
| title: string | |
| message?: string | |
| type?: "plain-text" | "secure-text" | |
| defaultValue?: string | |
| submitText?: string | |
| cancelText?: string | |
| keyboardType?: string | |
| options?: AlertOptions | |
| }) { | |
| const { | |
| title, | |
| message, | |
| type = "plain-text", | |
| defaultValue = "", | |
| submitText = "OK", | |
| cancelText = "Cancel", | |
| keyboardType, | |
| options, | |
| } = args | |
| return new Promise<{ value: string | undefined }>((resolve) => { | |
| Alert.prompt( | |
| title, | |
| message, | |
| [ | |
| { | |
| text: cancelText, | |
| style: "cancel", | |
| onPress: () => resolve({ value: undefined }), | |
| }, | |
| { | |
| text: submitText, | |
| style: "default", | |
| onPress: (value?: string) => resolve({ value }), | |
| }, | |
| ], | |
| type, | |
| defaultValue, | |
| keyboardType, | |
| options, | |
| ) | |
| }) | |
| } | |
| import { Alert, Platform } from "react-native" | |
| /** | |
| * Creates an Alert with a text input and returns a Promise which resolves with | |
| * the input value when submitted, or undefined if canceled. | |
| * Note: Alert.prompt is iOS-only; on Android this returns { value: undefined } immediately. | |
| */ | |
| export function promptAsync(args: { | |
| title: string | |
| message?: string | |
| type?: "plain-text" | "secure-text" | |
| defaultValue?: string | |
| submitText?: string | |
| cancelText?: string | |
| keyboardType?: string | |
| options?: AlertOptions | |
| }) { | |
| // Short-circuit on non-iOS platforms since Alert.prompt is unavailable there | |
| if (Platform.OS !== "ios") { | |
| return Promise.resolve({ value: undefined }) | |
| } | |
| const { | |
| title, | |
| message, | |
| type = "plain-text", | |
| defaultValue = "", | |
| submitText = "OK", | |
| cancelText = "Cancel", | |
| keyboardType, | |
| options, | |
| } = args | |
| return new Promise<{ value: string | undefined }>((resolve) => { | |
| Alert.prompt( | |
| title, | |
| message, | |
| [ | |
| { | |
| text: cancelText, | |
| style: "cancel", | |
| onPress: () => resolve({ value: undefined }), | |
| }, | |
| { | |
| text: submitText, | |
| style: "default", | |
| onPress: (value?: string) => resolve({ value }), | |
| }, | |
| ], | |
| type, | |
| defaultValue, | |
| keyboardType, | |
| options, | |
| ) | |
| }) | |
| } |
🤖 Prompt for AI Agents
In components/prompt-async.ts lines 8 to 51, Alert.prompt is used without
checking platform compatibility, which causes runtime crashes on Android since
Alert.prompt is iOS-only. Fix this by importing Platform from react-native,
adding a check to short-circuit and resolve immediately with undefined on
non-iOS platforms, and update the JSDoc to document that this function is
iOS-only.
| options={{ animation: "fade" }} | ||
| /> | ||
| <AppNativeStack.Screen name="Blocked" component={BlockedConversationsScreen} /> | ||
| <AppNativeStack.Screen name="Blocked" component={ArchivedConversationsScreen} /> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider updating the screen name to match the component.
The screen is named "Blocked" but uses the ArchivedConversationsScreen component. This mismatch between the screen name and component could cause confusion for developers navigating to this screen programmatically.
Consider updating the screen name to align with the component purpose:
-<AppNativeStack.Screen name="Blocked" component={ArchivedConversationsScreen} />
+<AppNativeStack.Screen name="ArchivedConversations" component={ArchivedConversationsScreen} />Note: This change would require updating all navigation calls that reference the "Blocked" screen name throughout the codebase.
🏁 Script executed:
#!/bin/bash
# Find all navigation calls to the "Blocked" screen
rg -A 3 -B 3 '"Blocked"' --type tsx --type tsLength of output: 75
I couldn’t search .tsx types directly. Let’s retry without type filters:
🏁 Script executed:
#!/bin/bash
# Search all .ts and .tsx files for any references to "Blocked"
rg '"Blocked"' -A3 -B3 --glob '*.ts' --glob '*.tsx'Length of output: 1073
Rename “Blocked” screen to “ArchivedConversations”
To keep the screen name in sync with the ArchivedConversationsScreen component, update both the navigator entry and all calls to that route:
• navigation/app-navigator.tsx
- <AppNativeStack.Screen name="Blocked" component={ArchivedConversationsScreen} />
+ <AppNativeStack.Screen name="ArchivedConversations" component={ArchivedConversationsScreen} />• features/profiles/profile-me.tsx
- router.navigate("Blocked")
+ router.navigate("ArchivedConversations")Be sure to search for any other occurrences of "Blocked" in navigation calls and update them accordingly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <AppNativeStack.Screen name="Blocked" component={ArchivedConversationsScreen} /> | |
| // navigation/app-navigator.tsx | |
| <AppNativeStack.Screen name="ArchivedConversations" component={ArchivedConversationsScreen} /> | |
| // features/profiles/profile-me.tsx | |
| router.navigate("ArchivedConversations") |
🤖 Prompt for AI Agents
In navigation/app-navigator.tsx at line 210, rename the screen from "Blocked" to
"ArchivedConversations" to match the component ArchivedConversationsScreen.
Then, update all navigation calls referencing "Blocked" throughout the codebase,
including features/profiles/profile-me.tsx and any other files where "Blocked"
is used as a route name, to use "ArchivedConversations" instead. This ensures
consistency between screen names and their components.
| export function ArchivedConversationsScreen() { | ||
| const { data: blockedConversationsIds = [] } = useBlockedConversationsForCurrentAccount() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Address naming inconsistency between component and internal variables.
The component is renamed to ArchivedConversationsScreen but the internal hook is still called useBlockedConversationsForCurrentAccount and the variable is blockedConversationsIds. This creates confusion about whether the component handles archived or blocked conversations.
Consider updating the variable names to match the component's new purpose:
-export function ArchivedConversationsScreen() {
- const { data: blockedConversationsIds = [] } = useBlockedConversationsForCurrentAccount()
+export function ArchivedConversationsScreen() {
+ const { data: archivedConversationsIds = [] } = useBlockedConversationsForCurrentAccount()
// Update usage below
- {blockedConversationsIds.length > 0 ? (
- <ConversationList conversationsIds={blockedConversationsIds} />
+ {archivedConversationsIds.length > 0 ? (
+ <ConversationList conversationsIds={archivedConversationsIds} />Also consider renaming the hook itself to reflect its new purpose if it indeed handles archived conversations.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function ArchivedConversationsScreen() { | |
| const { data: blockedConversationsIds = [] } = useBlockedConversationsForCurrentAccount() | |
| export function ArchivedConversationsScreen() { | |
| const { data: archivedConversationsIds = [] } = useBlockedConversationsForCurrentAccount() | |
| // Update usage below | |
| {archivedConversationsIds.length > 0 ? ( | |
| <ConversationList conversationsIds={archivedConversationsIds} /> | |
| ) : ( | |
| <EmptyState message="No archived conversations" /> | |
| )} | |
| } |
🤖 Prompt for AI Agents
In features/archived-conversations/archived-conversations.screen.tsx around
lines 11 to 12, the variable and hook names refer to blocked conversations while
the component is named ArchivedConversationsScreen, causing confusion. Rename
the variable from blockedConversationsIds to archivedConversationsIds and update
the hook name from useBlockedConversationsForCurrentAccount to
useArchivedConversationsForCurrentAccount (or a similar name reflecting archived
conversations). Ensure all references to these names in the file are updated
accordingly to maintain consistency.
| consentStates: ConsentState[] | ||
| caller: string | ||
| }) { | ||
| const { clientInboxId, consentStates = ["allowed", "unknown", "denied"], caller } = args | ||
| const { clientInboxId, consentStates, caller } = args |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify that all callers provide the now-required consentStates parameter.
Making consentStates a required parameter is a breaking change. All existing callers that relied on the default value ["allowed", "unknown", "denied"] must now explicitly provide this parameter.
Please verify that all callers of syncAllXmtpConversations have been updated to provide the consentStates parameter:
If any callers are missing the parameter, they will need to be updated to prevent runtime errors.
🏁 Script executed:
#!/bin/bash
# Description: Find all calls to syncAllXmtpConversations and verify they provide consentStates
# Expected: All calls should have consentStates parameter explicitly provided
# Search for function calls
rg -A 10 "syncAllXmtpConversations\(" --type=ts --type=tsxLength of output: 88
🏁 Script executed:
#!/bin/bash
# Search for all invocations of syncAllXmtpConversations in .ts and .tsx files
rg -C2 "syncAllXmtpConversations\(" -g "*.ts" -g "*.tsx"Length of output: 2782
Fix Required: Add missing consentStates to all syncAllXmtpConversations calls
The syncAllXmtpConversations function signature no longer provides a default for consentStates, so every invocation must now include this array. Update each call to pass the appropriate consentStates (e.g. the previous default ["allowed","unknown","denied"] or your project’s ConsentState enum values).
Callsites needing update:
- features/archived-conversations/denied-conversations.query.ts
await syncAllXmtpConversations({ clientInboxId: inboxId, caller: "getDeniedConversationsQueryFn",
- consentStates: ["allowed", "unknown", "denied"],
})
- features/conversation/conversation-requests-list/conversations-unknown-consent.query.ts
```ts
await syncAllXmtpConversations({
clientInboxId: inboxId,
caller: "getUnknownConversationsQueryFn",
+ consentStates: ["allowed", "unknown", "denied"],
})
- features/conversation/conversation-list/conversations-allowed-consent.query.ts
await syncAllXmtpConversations({ clientInboxId, caller: "getAllowedConsentConversationsQueryFn",
- consentStates: ["allowed", "unknown", "denied"],
})
- components/debug-menu.tsx (three invocations)
```tsx
await syncAllXmtpConversations({
clientInboxId,
caller: "debugMenuListAllowed",
+ consentStates: ["allowed", "unknown", "denied"],
})
// Repeat for debugMenuListDenied and debugMenuListUnknown
Please apply these changes to prevent runtime errors.
🤖 Prompt for AI Agents
In features/xmtp/xmtp-conversations/xmtp-conversations-sync.ts around lines 103
to 106, the syncAllXmtpConversations function now requires the consentStates
parameter with no default value. You need to update all calls to
syncAllXmtpConversations across the codebase to explicitly pass the
consentStates array, typically ["allowed", "unknown", "denied"], to avoid
runtime errors. Locate each invocation and add the consentStates property with
the appropriate array to the argument object.
| <ListItem | ||
| chevron | ||
| title={translate("Archive")} | ||
| title="Security line" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use translated string for consistency.
The title "Security line" appears to be hardcoded while other titles use the translate() function. For consistency and internationalization support, this should also be translated.
- title="Security line"
+ title={translate("Security line")}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| title="Security line" | |
| title={translate("Security line")} |
🤖 Prompt for AI Agents
In features/profiles/profile-me.tsx at line 113, the title "Security line" is
hardcoded while other titles use the translate() function. Replace the hardcoded
string with a call to translate() passing the appropriate key for "Security
line" to ensure consistency and support for internationalization.
| type IDeleteConversationsMutationArgs = { | ||
| conversationIds: IXmtpConversationId[] | ||
| alsoDenyInviterConsent: boolean | ||
| } | ||
|
|
||
| export const useDeleteConversationsMutation = () => { | ||
| return useMutation({ | ||
| mutationFn: async (conversationIds: IXmtpConversationId[]) => { | ||
| mutationFn: async (args: IDeleteConversationsMutationArgs) => { | ||
| const { conversationIds, alsoDenyInviterConsent } = args | ||
| const currentSender = getSafeCurrentSender() |
There was a problem hiding this comment.
Unused parameter alsoDenyInviterConsent due to commented code.
The alsoDenyInviterConsent parameter is accepted but not used because the consent denial logic is commented out. This creates a confusing API where the parameter has no effect.
Either:
- Remove the parameter if the consent denial feature is not needed:
-type IDeleteConversationsMutationArgs = {
- conversationIds: IXmtpConversationId[]
- alsoDenyInviterConsent: boolean
-}
+type IDeleteConversationsMutationArgs = {
+ conversationIds: IXmtpConversationId[]
+}- Or uncomment and properly implement the consent denial logic if it's still needed.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
features/conversation/conversation-requests-list/delete-conversations.mutation.ts
around lines 12 to 21, the parameter alsoDenyInviterConsent is accepted but
unused due to commented-out consent denial logic. To fix this, either remove the
alsoDenyInviterConsent parameter from the IDeleteConversationsMutationArgs type
and related code if the feature is not needed, or uncomment and properly
implement the consent denial logic to make use of this parameter as intended.
| const handleDeleteGroup = useCallback(async () => { | ||
| try { | ||
| const group = await ensureGroupQueryData({ | ||
| clientInboxId: currentSender.inboxId, | ||
| xmtpConversationId, | ||
| caller: "ConversationConsentPopupGroup", | ||
| }) | ||
|
|
||
| const { groupName } = useGroupName({ xmtpConversationId }) | ||
| if (!group) { | ||
| throw new Error("Group not found while deleting group in ConversationConsentPopupGroup") | ||
| } | ||
|
|
||
| const handleDeclineGroup = useCallback(async () => { | ||
| groupRemoveRestoreHandler( | ||
| "unknown", // To display "Remove & Block inviter" | ||
| colorScheme, | ||
| groupName, | ||
| allowGroup, | ||
| denyGroup, | ||
| )((success: boolean) => { | ||
| if (success) { | ||
| navigation.pop() | ||
| const hasAllowedConsentForAddedBy = await ensureConsentForInboxIdQueryData({ | ||
| clientInboxId: currentSender.inboxId, | ||
| inboxIdToCheck: group.addedByInboxId, | ||
| caller: "ConversationConsentPopupGroup", | ||
| }) | ||
|
|
||
| let options = [translate("Delete"), translate("Cancel")] | ||
| let destructiveButtonIndex: number | number[] = 0 | ||
| let cancelButtonIndex = 1 | ||
|
|
||
| if (hasAllowedConsentForAddedBy === "allowed") { | ||
| const { displayName: addedByDisplayName } = await ensurePreferredDisplayInfo({ | ||
| inboxId: group.addedByInboxId, | ||
| caller: "ConversationConsentPopupGroup", | ||
| }) | ||
| options = [ | ||
| translate("Delete"), | ||
| `Delete and Block ${shortDisplayName(addedByDisplayName)} (invited you)`, | ||
| translate("Cancel"), | ||
| ] | ||
| destructiveButtonIndex = [0, 1] | ||
| cancelButtonIndex = 2 | ||
| } | ||
| // If not successful, do nothing (user canceled) | ||
| }) | ||
| }, [groupName, colorScheme, allowGroup, denyGroup, navigation]) | ||
|
|
||
| showActionSheet({ | ||
| options: { | ||
| options, | ||
| cancelButtonIndex, | ||
| destructiveButtonIndex, | ||
| title: `If you delete this conversation, you won't be able to see any messages from them anymore.`, | ||
| }, | ||
| callback: async (selectedIndex?: number) => { | ||
| if (selectedIndex === 0) { | ||
| await deleteConversationsAsync({ | ||
| conversationIds: [xmtpConversationId], | ||
| alsoDenyInviterConsent: false, | ||
| }) | ||
| router.goBack() | ||
| } else if (selectedIndex === 1 && hasAllowedConsentForAddedBy === "allowed") { | ||
| await deleteConversationsAsync({ | ||
| conversationIds: [xmtpConversationId], | ||
| alsoDenyInviterConsent: true, | ||
| }) | ||
| router.goBack() | ||
| } | ||
| }, | ||
| }) | ||
| } catch (error) { | ||
| captureErrorWithToast( | ||
| new GenericError({ error, additionalMessage: `Failed to delete group` }), | ||
| { | ||
| message: "Failed to delete group", | ||
| }, | ||
| ) | ||
| } | ||
| }, [deleteConversationsAsync, currentSender.inboxId, xmtpConversationId, router]) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use translation for action sheet title and consider refactoring for complexity.
The implementation is correct, but there are two improvements to consider:
- Essential: The action sheet title should use the
translatefunction for internationalization. - Optional: This function is quite complex and could benefit from being broken down into smaller helper functions.
Apply this diff to fix the internationalization issue:
- title: `If you delete this conversation, you won't be able to see any messages from them anymore.`,
+ title: translate("If you delete this conversation, you won't be able to see any messages from them anymore."),For the optional refactoring, consider extracting the action sheet logic into a separate function like showDeleteGroupActionSheet.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleDeleteGroup = useCallback(async () => { | |
| try { | |
| const group = await ensureGroupQueryData({ | |
| clientInboxId: currentSender.inboxId, | |
| xmtpConversationId, | |
| caller: "ConversationConsentPopupGroup", | |
| }) | |
| const { groupName } = useGroupName({ xmtpConversationId }) | |
| if (!group) { | |
| throw new Error("Group not found while deleting group in ConversationConsentPopupGroup") | |
| } | |
| const handleDeclineGroup = useCallback(async () => { | |
| groupRemoveRestoreHandler( | |
| "unknown", // To display "Remove & Block inviter" | |
| colorScheme, | |
| groupName, | |
| allowGroup, | |
| denyGroup, | |
| )((success: boolean) => { | |
| if (success) { | |
| navigation.pop() | |
| const hasAllowedConsentForAddedBy = await ensureConsentForInboxIdQueryData({ | |
| clientInboxId: currentSender.inboxId, | |
| inboxIdToCheck: group.addedByInboxId, | |
| caller: "ConversationConsentPopupGroup", | |
| }) | |
| let options = [translate("Delete"), translate("Cancel")] | |
| let destructiveButtonIndex: number | number[] = 0 | |
| let cancelButtonIndex = 1 | |
| if (hasAllowedConsentForAddedBy === "allowed") { | |
| const { displayName: addedByDisplayName } = await ensurePreferredDisplayInfo({ | |
| inboxId: group.addedByInboxId, | |
| caller: "ConversationConsentPopupGroup", | |
| }) | |
| options = [ | |
| translate("Delete"), | |
| `Delete and Block ${shortDisplayName(addedByDisplayName)} (invited you)`, | |
| translate("Cancel"), | |
| ] | |
| destructiveButtonIndex = [0, 1] | |
| cancelButtonIndex = 2 | |
| } | |
| // If not successful, do nothing (user canceled) | |
| }) | |
| }, [groupName, colorScheme, allowGroup, denyGroup, navigation]) | |
| showActionSheet({ | |
| options: { | |
| options, | |
| cancelButtonIndex, | |
| destructiveButtonIndex, | |
| title: `If you delete this conversation, you won't be able to see any messages from them anymore.`, | |
| }, | |
| callback: async (selectedIndex?: number) => { | |
| if (selectedIndex === 0) { | |
| await deleteConversationsAsync({ | |
| conversationIds: [xmtpConversationId], | |
| alsoDenyInviterConsent: false, | |
| }) | |
| router.goBack() | |
| } else if (selectedIndex === 1 && hasAllowedConsentForAddedBy === "allowed") { | |
| await deleteConversationsAsync({ | |
| conversationIds: [xmtpConversationId], | |
| alsoDenyInviterConsent: true, | |
| }) | |
| router.goBack() | |
| } | |
| }, | |
| }) | |
| } catch (error) { | |
| captureErrorWithToast( | |
| new GenericError({ error, additionalMessage: `Failed to delete group` }), | |
| { | |
| message: "Failed to delete group", | |
| }, | |
| ) | |
| } | |
| }, [deleteConversationsAsync, currentSender.inboxId, xmtpConversationId, router]) | |
| showActionSheet({ | |
| options: { | |
| options, | |
| cancelButtonIndex, | |
| destructiveButtonIndex, | |
| title: translate( | |
| "If you delete this conversation, you won't be able to see any messages from them anymore." | |
| ), | |
| }, | |
| callback: async (selectedIndex?: number) => { | |
| if (selectedIndex === 0) { | |
| await deleteConversationsAsync({ | |
| conversationIds: [xmtpConversationId], | |
| alsoDenyInviterConsent: false, | |
| }) | |
| router.goBack() | |
| } else if (selectedIndex === 1 && hasAllowedConsentForAddedBy === "allowed") { | |
| await deleteConversationsAsync({ | |
| conversationIds: [xmtpConversationId], | |
| alsoDenyInviterConsent: true, | |
| }) | |
| router.goBack() | |
| } | |
| }, | |
| }) |
🤖 Prompt for AI Agents
In
features/conversation/conversation-chat/conversation-consent-popup/conversation-consent-popup-group.tsx
around lines 34 to 101, the action sheet title is currently a hardcoded string
and should be wrapped with the translate function for internationalization
support. Replace the title string with translate("If you delete this
conversation, you won't be able to see any messages from them anymore.").
Additionally, consider refactoring the handleDeleteGroup function by extracting
the action sheet display logic into a separate helper function, such as
showDeleteGroupActionSheet, to reduce complexity and improve readability.
| export function setConsentForInboxIdQueryData( | ||
| args: IStrictArgs & { | ||
| consent: IConsentState | ||
| }, | ||
| ) { | ||
| return reactQueryClient.setQueryData(getConsentForInboxIdQueryOptions(args).queryKey, args) | ||
| } |
There was a problem hiding this comment.
Fix incorrect setQueryData implementation.
The function is setting the entire args object as the query data, but it should only set the consent state value.
Apply this diff to fix the issue:
- return reactQueryClient.setQueryData(getConsentForInboxIdQueryOptions(args).queryKey, args)
+ return reactQueryClient.setQueryData(getConsentForInboxIdQueryOptions(args).queryKey, args.consent)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function setConsentForInboxIdQueryData( | |
| args: IStrictArgs & { | |
| consent: IConsentState | |
| }, | |
| ) { | |
| return reactQueryClient.setQueryData(getConsentForInboxIdQueryOptions(args).queryKey, args) | |
| } | |
| export function setConsentForInboxIdQueryData( | |
| args: IStrictArgs & { | |
| consent: IConsentState | |
| }, | |
| ) { | |
| return reactQueryClient.setQueryData( | |
| getConsentForInboxIdQueryOptions(args).queryKey, | |
| args.consent | |
| ) | |
| } |
🤖 Prompt for AI Agents
In features/consent/consent-for-inbox-id.query.ts around lines 67 to 73, the
setConsentForInboxIdQueryData function incorrectly sets the entire args object
as the query data. To fix this, update the function to set only the consent
state value from args.consent as the query data by passing args.consent to
reactQueryClient.setQueryData instead of args.
| async function getDeniedConversationsQueryFn(args: { inboxId: IXmtpInboxId }) { | ||
| const { inboxId } = args | ||
|
|
||
| if (!inboxId) { | ||
| throw new Error("InboxId is required") | ||
| } | ||
|
|
||
| await syncAllXmtpConversations({ | ||
| clientInboxId: inboxId, | ||
| caller: "getDeniedConversationsQueryFn", | ||
| consentStates: ["denied"], | ||
| }) | ||
|
|
||
| const deniedConsentXmtpConversations = await getXmtpConversations({ | ||
| clientInboxId: inboxId, | ||
| consentStates: ["denied"], | ||
| caller: "getDeniedConversationsQueryFn", | ||
| }) | ||
|
|
||
| const convosConversations = await Promise.all( | ||
| deniedConsentXmtpConversations.map(convertXmtpConversationToConvosConversation), | ||
| ) | ||
|
|
||
| for (const conversation of convosConversations) { | ||
| setConversationQueryData({ | ||
| clientInboxId: inboxId, | ||
| xmtpConversationId: conversation.xmtpId, | ||
| conversation, | ||
| }) | ||
| } | ||
|
|
||
| return convosConversations.map((c) => c.xmtpId) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and consider performance optimization.
The core query function has good structure but needs improvements:
- Missing error handling: The async operations could fail and should be wrapped in try-catch blocks
- Performance consideration: Converting all conversations in parallel is good, but consider pagination for large datasets
async function getDeniedConversationsQueryFn(args: { inboxId: IXmtpInboxId }) {
const { inboxId } = args
if (!inboxId) {
throw new Error("InboxId is required")
}
+ try {
await syncAllXmtpConversations({
clientInboxId: inboxId,
caller: "getDeniedConversationsQueryFn",
consentStates: ["denied"],
})
const deniedConsentXmtpConversations = await getXmtpConversations({
clientInboxId: inboxId,
consentStates: ["denied"],
caller: "getDeniedConversationsQueryFn",
})
const convosConversations = await Promise.all(
deniedConsentXmtpConversations.map(convertXmtpConversationToConvosConversation),
)
for (const conversation of convosConversations) {
setConversationQueryData({
clientInboxId: inboxId,
xmtpConversationId: conversation.xmtpId,
conversation,
})
}
return convosConversations.map((c) => c.xmtpId)
+ } catch (error) {
+ console.error('Failed to fetch denied conversations:', error)
+ throw new Error(`Failed to fetch denied conversations: ${error instanceof Error ? error.message : 'Unknown error'}`)
+ }
}🤖 Prompt for AI Agents
In features/archived-conversations/denied-conversations.query.ts around lines 13
to 45, add try-catch blocks around the async operations to handle potential
errors gracefully. Wrap the entire function body in a try block and catch any
errors to throw or handle them appropriately. Additionally, implement pagination
when fetching and converting conversations to avoid performance issues with
large datasets, such as limiting the number of conversations processed at once
and supporting fetching subsequent pages.
| export const getDeniedConsentConversationsQueryData = (args: { inboxId: IXmtpInboxId }) => { | ||
| return reactQueryClient.getQueryData(getDeniedConsentConversationsQueryOptions(args).queryKey) | ||
| } | ||
|
|
||
| export function addConversationToDeniedConsentConversationsQuery(args: { | ||
| clientInboxId: IXmtpInboxId | ||
| conversationId: IXmtpConversationId | ||
| }) { | ||
| const { clientInboxId, conversationId } = args | ||
|
|
||
| return reactQueryClient.setQueryData( | ||
| getDeniedConsentConversationsQueryOptions({ | ||
| inboxId: clientInboxId, | ||
| caller: "addConversationToDeniedConsentConversationsQuery", | ||
| }).queryKey, | ||
| (previousConversationIds) => { | ||
| if (!previousConversationIds) { | ||
| return [conversationId] | ||
| } | ||
|
|
||
| const conversationExists = previousConversationIds.includes(conversationId) | ||
|
|
||
| if (conversationExists) { | ||
| return previousConversationIds | ||
| } | ||
|
|
||
| return [conversationId, ...previousConversationIds] | ||
| }, | ||
| ) | ||
| } | ||
|
|
||
| export function removeConversationFromDeniedConsentConversationsQuery(args: { | ||
| clientInboxId: IXmtpInboxId | ||
| conversationId: IXmtpConversationId | ||
| }) { | ||
| const { clientInboxId, conversationId } = args | ||
|
|
||
| return reactQueryClient.setQueryData( | ||
| getDeniedConsentConversationsQueryOptions({ | ||
| inboxId: clientInboxId, | ||
| caller: "removeConversationFromDeniedConsentConversationsQuery", | ||
| }).queryKey, | ||
| (previousConversationIds) => { | ||
| if (!previousConversationIds) { | ||
| return [] | ||
| } | ||
|
|
||
| return previousConversationIds.filter((id) => id !== conversationId) | ||
| }, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation and fix naming inconsistency.
The cache utility functions need improvements:
- Missing validation: Functions don't validate if
inboxIdis provided - Naming inconsistency: The
callerparameter in line 60 is not used in the query key
export function addConversationToDeniedConsentConversationsQuery(args: {
clientInboxId: IXmtpInboxId
conversationId: IXmtpConversationId
}) {
const { clientInboxId, conversationId } = args
+ if (!clientInboxId || !conversationId) {
+ throw new Error("Both clientInboxId and conversationId are required")
+ }
return reactQueryClient.setQueryData(
getDeniedConsentConversationsQueryOptions({
inboxId: clientInboxId,
- caller: "addConversationToDeniedConsentConversationsQuery",
}).queryKey,
(previousConversationIds) => {
if (!previousConversationIds) {
return [conversationId]
}
const conversationExists = previousConversationIds.includes(conversationId)
if (conversationExists) {
return previousConversationIds
}
return [conversationId, ...previousConversationIds]
},
)
}
export function removeConversationFromDeniedConsentConversationsQuery(args: {
clientInboxId: IXmtpInboxId
conversationId: IXmtpConversationId
}) {
const { clientInboxId, conversationId } = args
+ if (!clientInboxId || !conversationId) {
+ throw new Error("Both clientInboxId and conversationId are required")
+ }
return reactQueryClient.setQueryData(
getDeniedConsentConversationsQueryOptions({
inboxId: clientInboxId,
- caller: "removeConversationFromDeniedConsentConversationsQuery",
}).queryKey,
(previousConversationIds) => {
if (!previousConversationIds) {
return []
}
return previousConversationIds.filter((id) => id !== conversationId)
},
)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const getDeniedConsentConversationsQueryData = (args: { inboxId: IXmtpInboxId }) => { | |
| return reactQueryClient.getQueryData(getDeniedConsentConversationsQueryOptions(args).queryKey) | |
| } | |
| export function addConversationToDeniedConsentConversationsQuery(args: { | |
| clientInboxId: IXmtpInboxId | |
| conversationId: IXmtpConversationId | |
| }) { | |
| const { clientInboxId, conversationId } = args | |
| return reactQueryClient.setQueryData( | |
| getDeniedConsentConversationsQueryOptions({ | |
| inboxId: clientInboxId, | |
| caller: "addConversationToDeniedConsentConversationsQuery", | |
| }).queryKey, | |
| (previousConversationIds) => { | |
| if (!previousConversationIds) { | |
| return [conversationId] | |
| } | |
| const conversationExists = previousConversationIds.includes(conversationId) | |
| if (conversationExists) { | |
| return previousConversationIds | |
| } | |
| return [conversationId, ...previousConversationIds] | |
| }, | |
| ) | |
| } | |
| export function removeConversationFromDeniedConsentConversationsQuery(args: { | |
| clientInboxId: IXmtpInboxId | |
| conversationId: IXmtpConversationId | |
| }) { | |
| const { clientInboxId, conversationId } = args | |
| return reactQueryClient.setQueryData( | |
| getDeniedConsentConversationsQueryOptions({ | |
| inboxId: clientInboxId, | |
| caller: "removeConversationFromDeniedConsentConversationsQuery", | |
| }).queryKey, | |
| (previousConversationIds) => { | |
| if (!previousConversationIds) { | |
| return [] | |
| } | |
| return previousConversationIds.filter((id) => id !== conversationId) | |
| }, | |
| ) | |
| } | |
| export const getDeniedConsentConversationsQueryData = (args: { inboxId: IXmtpInboxId }) => { | |
| return reactQueryClient.getQueryData( | |
| getDeniedConsentConversationsQueryOptions(args).queryKey, | |
| ) | |
| } | |
| export function addConversationToDeniedConsentConversationsQuery(args: { | |
| clientInboxId: IXmtpInboxId | |
| conversationId: IXmtpConversationId | |
| }) { | |
| const { clientInboxId, conversationId } = args | |
| if (!clientInboxId || !conversationId) { | |
| throw new Error("Both clientInboxId and conversationId are required") | |
| } | |
| return reactQueryClient.setQueryData( | |
| getDeniedConsentConversationsQueryOptions({ | |
| inboxId: clientInboxId, | |
| }).queryKey, | |
| (previousConversationIds) => { | |
| if (!previousConversationIds) { | |
| return [conversationId] | |
| } | |
| const conversationExists = previousConversationIds.includes(conversationId) | |
| if (conversationExists) { | |
| return previousConversationIds | |
| } | |
| return [conversationId, ...previousConversationIds] | |
| }, | |
| ) | |
| } | |
| export function removeConversationFromDeniedConsentConversationsQuery(args: { | |
| clientInboxId: IXmtpInboxId | |
| conversationId: IXmtpConversationId | |
| }) { | |
| const { clientInboxId, conversationId } = args | |
| if (!clientInboxId || !conversationId) { | |
| throw new Error("Both clientInboxId and conversationId are required") | |
| } | |
| return reactQueryClient.setQueryData( | |
| getDeniedConsentConversationsQueryOptions({ | |
| inboxId: clientInboxId, | |
| }).queryKey, | |
| (previousConversationIds) => { | |
| if (!previousConversationIds) { | |
| return [] | |
| } | |
| return previousConversationIds.filter((id) => id !== conversationId) | |
| }, | |
| ) | |
| } |
🤖 Prompt for AI Agents
In features/archived-conversations/denied-conversations.query.ts between lines
47 and 97, add validation to ensure the inboxId argument is provided before
proceeding in the cache utility functions. Also, fix the naming inconsistency by
ensuring the caller parameter used in setQueryData calls matches the expected
query key structure, so it is properly included and consistent across the
functions.
Replace conversation blocking with deletion functionality and implement new consent management system for XMTP messaging
📍Where to Start
Start with the consent management implementation in consent-for-inbox-id.query.ts as it forms the foundation for the new messaging permissions system.
Macroscope summarized ee484a7.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Refactor
Chores
Tests