Skip to content

fix: Delete / Archive / Block conversations and users#75

Open
thierryskoda wants to merge 1 commit intomainfrom
ts/fix-delete-archive-block
Open

fix: Delete / Archive / Block conversations and users#75
thierryskoda wants to merge 1 commit intomainfrom
ts/fix-delete-archive-block

Conversation

@thierryskoda
Copy link
Copy Markdown
Contributor

@thierryskoda thierryskoda commented May 23, 2025

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

    • Added debug menu options to fetch the last 10 messages and view disappearing message settings for a conversation.
    • Introduced a utility for prompting user input with a text dialog.
    • Added support for managing and querying denied consent conversations and consent states for inbox IDs.
    • Added a function to convert durations to human-readable time strings.
  • Improvements

    • Enhanced conversation and consent management, including batch consent updates on conversation creation and improved consent mutation flows.
    • Updated deletion flows for conversations and groups, offering clearer user options and improved error handling.
    • Refined navigation and settings: "Blocked" screen now displays archived conversations, and settings menu items were reordered.
    • Improved query logic, cache handling, and type safety across various features.
  • Bug Fixes

    • Fixed query execution to only skip when explicitly disabled, preventing unintended skips.
  • Refactor

    • Renamed components and variables for clarity (e.g., BlockedConversationsScreen to ArchivedConversationsScreen).
    • Replaced group-based consent logic with conversation-based logic throughout relevant features.
  • Chores

    • Gated app state operations to only run when the user is signed in.
  • Tests

    • Removed obsolete test files and unused utility functions related to group member actions.

@thierryskoda thierryskoda requested a review from a team as a code owner May 23, 2025 14:08
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented May 23, 2025

Walkthrough

This 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

File(s) / Path(s) Change Summary
components/debug-menu.tsx Added two debug menu options: fetch last 10 XMTP messages and get disappearing message settings, with loading and error handling.
components/prompt-async.ts Added promptAsync function for promise-based text input dialogs.
features/archived-conversations/archived-conversations.screen.tsx
navigation/app-navigator.tsx
Renamed and switched screen/component from Blocked to Archived conversations; updated imports and usage.
features/archived-conversations/denied-conversations.query.ts New module for querying, caching, and manipulating denied consent conversations.
features/archived-conversations/use-archived-conversations-for-current-sender.ts Renamed variable to allowedConversationIds and added debug logging.
features/consent/consent-for-inbox-id.query.ts New utilities and hooks for querying and managing consent state for inbox IDs.
features/consent/consent.utils.ts Added function to convert XMTP consent state to local consent state.
features/consent/update-consent-for-inbox-id.mutation.ts New mutation utilities for updating consent state, with optimistic updates and rollback.
features/consent/use-allow-dm.mutation.ts
features/consent/use-deny-dm.mutation.ts
features/consent/use-deny-group.mutation.ts
Updated to use conversation-based consent mutation utilities and new mutation function.
features/consent/use-allow-group.mutation.ts Switched to conversation-based consent updates; made some arguments required.
features/consent/use-group-consent-for-current-sender.ts
features/groups/utils/getGroupMemberActions.ts
features/groups/utils/getGroupMemberActions.test.ts
features/groups/utils/groupActionHandlers.ts
Deleted group consent and group member action utilities and related tests.
features/conversation/conversation-chat/conversation-consent-popup/conversation-consent-popup-dm.tsx Switched from blocking to deleting conversations with optional inviter consent denial.
features/conversation/conversation-chat/conversation-consent-popup/conversation-consent-popup-group.tsx Refactored group consent popup to provide explicit deletion and block options with confirmation dialogs.
features/conversation/conversation-create/mutations/create-conversation-and-send-first-message.mutation.ts On success, now updates consent state for all inbox IDs involved in the new conversation.
features/conversation/conversation-list/hooks/use-delete-dm.ts
features/conversation/conversation-list/hooks/use-delete-group.ts
Refactored to use centralized conversation deletion mutation and updated consent logic.
features/conversation/conversation-requests-list/delete-conversations.mutation.ts Mutation now accepts an object with conversation IDs and a flag; consent update logic commented out.
features/conversation/queries/conversation.query.ts Early return if conversation data not found before fetching messages in update logic.
features/current-user/current-user.query.ts Uses new helper for ensuring query data.
features/preferred-display-info/use-preferred-display-info.ts Refactored to extract and reuse query option objects and improved enabled flag logic.
features/profiles/profile-me.tsx Swapped titles and navigation targets for Archive and Security line list items.
features/xmtp/xmtp-consent/xmtp-consent.ts Added consent state getter; refactored group consent update to conversation consent update; improved error handling.
features/xmtp/xmtp-conversations/xmtp-conversations-sync.ts Made consentStates parameter required in conversation sync function.
features/xmtp/xmtp-disappearing-messages/xmtp-disappearing-messages.ts Improved logging for disappearing message settings update.
navigation/navigation.utils.ts Added utility to retrieve current route parameters.
stores/app-state-store/app-state-store.service.ts App state listeners now gated on user being signed in.
utils/react-query/react-query.helpers.ts Improved enabled flag checks for query helpers.
utils/react-query/react-query.utils.ts Refactored query key generator for type safety and to forbid "caller" key.
utils/time.utils.ts Added function to get human-readable time from milliseconds.

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
Loading
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
Loading

Poem

🐇✨
New consents and queries, a warren of care,
Debugging with messages floating in air.
Deletions are smarter, the routes are more neat,
Time is now readable, updates complete!
Old group code is buried, new flows in the sun—
This hop through the codebase was quite a fun run!
✨🐇

Note

⚡️ AI Code Reviews for VS Code, Cursor, Windsurf

CodeRabbit 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.
Learn more here.

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

consent: IConsentState
},
) {
return reactQueryClient.setQueryData(getConsentForInboxIdQueryOptions(args).queryKey, args)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 16

🔭 Outside diff range comments (1)
features/consent/use-allow-group.mutation.ts (1)

75-82: ⚠️ Potential issue

Critical: Incorrect peer inbox ID in consent update.

The peerInboxId parameter is incorrectly set to clientInboxId on 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:

  1. Approximate calculations: Using 30 days/month and 365 days/year may be misleading for precise durations
  2. 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 returning Promise<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 useDenyGroupMutation suggests group-based operations, but the implementation now uses conversation-based consent updates with updateXmtpConsentForConversationForInbox. This naming inconsistency could confuse developers about the function's actual behavior.

Consider renaming to useDenyConversationMutation or 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 IXmtpConsentState is defined as ConsentState (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 === ConsentState
utils/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

📥 Commits

Reviewing files that changed from the base of the PR and between 0742ffe and ee484a7.

📒 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.enabled to "enabled" in args && !args.enabled is a significant improvement that ensures queries are only skipped when enabled is explicitly set to false, not when it's undefined. 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 ensureQueryDataBetter adds 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 conversationId and retentionDurationInNs improves 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 conversationIds to allowedConversationIds better 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 setXmtpConsentStateForInboxId with executeUpdateConsentForInboxIdMutation maintains 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 ts

Length of output: 12799


All group-based consent functions have been replaced.

The search confirms that there are no remaining calls to the old updateXmtpConsentForGroupsForInbox API. The only usage of setXmtpConsentStateForInboxId is inside the new executeUpdateConsentForInboxIdMutation in features/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 (updateXmtpConsentForConversationForInbox and executeUpdateConsentForInboxIdMutation).

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: true parameter 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 ts

Length of output: 28989


Approved: deleteConversations mutation interface matches component usage

The refactoring to use useDeleteConversationsMutation with { conversationIds, alsoDenyInviterConsent } aligns perfectly with the mutation signature in delete-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 enabled parameter with each query's own enabled property. 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 useDeleteConversationsMutation is well-executed:

  • Proper use of the alsoDenyInviterConsent flag
  • Efficient concurrent execution with Promise.all for 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 getXmtpConsentStateForInboxId function 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 onMutate and proper rollback in onError. 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 onAccept callback 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 both IArgs and IStrictArgs provide 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.

Comment on lines +8 to +51
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,
)
})
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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 tsx

Length 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.

Suggested change
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} />
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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 ts

Length 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.

Suggested change
<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.

Comment on lines +11 to 12
export function ArchivedConversationsScreen() {
const { data: blockedConversationsIds = [] } = useBlockedConversationsForCurrentAccount()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Suggested change
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.

Comment on lines +103 to +106
consentStates: ConsentState[]
caller: string
}) {
const { clientInboxId, consentStates = ["allowed", "unknown", "denied"], caller } = args
const { clientInboxId, consentStates, caller } = args
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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=tsx

Length 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"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Suggested change
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.

Comment on lines +12 to 21
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()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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:

  1. Remove the parameter if the consent denial feature is not needed:
-type IDeleteConversationsMutationArgs = {
-  conversationIds: IXmtpConversationId[]
-  alsoDenyInviterConsent: boolean
-}
+type IDeleteConversationsMutationArgs = {
+  conversationIds: IXmtpConversationId[]
+}
  1. 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.

Comment on lines +34 to +101
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])
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use translation for action sheet title and consider refactoring for complexity.

The implementation is correct, but there are two improvements to consider:

  1. Essential: The action sheet title should use the translate function for internationalization.
  2. 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.

Suggested change
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.

Comment on lines +67 to +73
export function setConsentForInboxIdQueryData(
args: IStrictArgs & {
consent: IConsentState
},
) {
return reactQueryClient.setQueryData(getConsentForInboxIdQueryOptions(args).queryKey, args)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
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.

Comment on lines +13 to +45
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)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling and consider performance optimization.

The core query function has good structure but needs improvements:

  1. Missing error handling: The async operations could fail and should be wrapped in try-catch blocks
  2. 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.

Comment on lines +47 to +97
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)
},
)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add input validation and fix naming inconsistency.

The cache utility functions need improvements:

  1. Missing validation: Functions don't validate if inboxId is provided
  2. Naming inconsistency: The caller parameter 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.

Suggested change
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant