Feature: V3 split towards a V3 Only World #1189
Conversation
Performance Comparison ReportSignificant Changes To DurationThere are no entries Meaningless Changes To DurationShow entries
Show details
Render Count ChangesThere are no entries Render IssuesThere are no entries Added ScenariosThere are no entries Removed ScenariosThere are no entries |
alexrisch
force-pushed
the
feature/v3-split
branch
from
fcfdee4 to
cffc384
Compare
alexrisch
force-pushed
the
release/3.0.0
branch
2 times, most recently
from
07b66ae to
514e4b8
Compare
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (6)
⛔ Files ignored due to path filters (1)
You can disable this status message by setting the WalkthroughThe changes in this pull request involve significant refactoring across multiple components and files within the application. Key modifications include the removal of unused imports and functionality, simplification of state management, and the introduction of new components and hooks. Notable updates include the elimination of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant XmtpClient
participant NotificationService
participant Preferences
User->>App: Open App
App->>XmtpClient: Initialize Client
XmtpClient->>Preferences: Load Preferences
Preferences-->>XmtpClient: Return Preferences
XmtpClient-->>App: Client Initialized
App->>NotificationService: Check Notifications
NotificationService-->>App: Return Notifications
App->>User: Display Notifications
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Started some work on V3 Message Component Cleaned up several Components Cleaned up list merging Better query handling for messages
Message Status Chat message work
Message Status Chat message work
* feat: Update to use inbox id endpoint Added new query to use inbox endpoint Updated usage for group avatar/members Updated socials type to match what is being returned from the server * feat: Remove Inbox Id store Remove inbox id store * Updates Added tests Split util files Added helper util functions Added helper hooks * Updated to map and filter
Added helpers Updated naming to match v3 rather than just groups Updated Pinned Conversations and Conversation List
Fix for missing function
Moved util functions to separate files Removed helpers/index files Prepared media attachments for split from V2 handling :
Added V3 Message and V3Message to Display Added conversation context to help reduce overall changes
Added blocked view support Cleaned up tsc Added helper query config Added API setting pinned conversations
* feat: Remove V2 * feat: Remove V2 Removes almost all V2 Handling Removes App Database Cleans up queries Bumps to V3 SDK Updated native iOS notifications to work with V3 only Removes some frames functionality Leaves the app utterly broken
Fix Android Build Update Conversation List to have hooks
* wip * wip * wip * wip * wip * wip * wip * remove unused rn patches * wip * wip * fix: New DM Work Updated handling when sending new Conversations --------- Co-authored-by: Thierry <skodathierry@gmail.com>
alexrisch
force-pushed
the
feature/v3-split
branch
from
0183b42 to
b4ed98d
Compare
Updated Tests to work on V3 Only
* fix tsc * remove unused test
Added new query Updated use of permissions Added new translations Updated hooks Added new utils
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
🛑 Comments failed to post (65)
components/PinnedConversations/PinnedConversations.tsx (1)
11-20:
⚠️ Potential issueHandle potential null currentAccount case
The non-null assertion (
currentAccount!) could lead to runtime errors if the account is not available.Consider adding a null check:
- const { isLoading } = useV3ConversationListQuery( - currentAccount!, + if (!currentAccount) return null; + const { isLoading } = useV3ConversationListQuery( + currentAccount, { refetchOnWindowFocus: false, refetchOnMount: false, }, "PinnedConversations" );📝 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 PinnedConversations = ({ topics }: Props) => { const currentAccount = useCurrentAccount(); if (!currentAccount) return null; const { isLoading } = useV3ConversationListQuery( currentAccount, { refetchOnWindowFocus: false, refetchOnMount: false, }, "PinnedConversations" );components/PinnedConversations/PinnedV3Conversation.tsx (2)
9-11: 🛠️ Refactor suggestion
Strengthen type safety in props definition.
The
topicprop is defined asstringbut later cast toConversationTopic. Consider usingConversationTopicdirectly in the props type to catch type mismatches at compile time.type PinnedV3ConversationProps = { - topic: string; + topic: ConversationTopic; };Committable suggestion skipped: line range outside the PR's diff.
18-25: 🛠️ Refactor suggestion
Improve type safety and type discrimination.
The component uses type casting which could be unsafe. Consider using type guards or discriminated unions for safer type narrowing.
+type ConversationType = GroupWithCodecsType | DmWithCodecsType; +function isGroupConversation( + conversation: ConversationType +): conversation is GroupWithCodecsType { + return conversationIsGroup(conversation); +} export const PinnedV3Conversation = ({ topic }: PinnedV3ConversationProps) => { const conversation = useConversationListGroupItem(topic as ConversationTopic); if (!conversation) { return null; } - if (conversationIsGroup(conversation)) { + if (isGroupConversation(conversation)) { return ( - <PinnedV3GroupConversation group={conversation as GroupWithCodecsType} /> + <PinnedV3GroupConversation group={conversation} /> ); } return ( - <PinnedV3DMConversation conversation={conversation as DmWithCodecsType} /> + <PinnedV3DMConversation conversation={conversation} /> ); };Committable suggestion skipped: line range outside the PR's diff.
components/StateHandlers/HydrationStateHandler.tsx (2)
23-25: 🛠️ Refactor suggestion
Remove commented code block
Remove the commented code block as it's no longer needed. If this is part of the V3 migration, document the removal reason in the commit message or add a TODO comment explaining the transition plan.
- // accounts.map((address) => { - // refreshProfileForAddress(address, address); - // });📝 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.
3-3: 🛠️ Refactor suggestion
Remove commented import statement
Instead of commenting out the import, it should be removed entirely if it's no longer needed. If this is part of a gradual migration to V3, consider adding a TODO comment explaining the transition plan.
-// import { refreshProfileForAddress } from "../../data/helpers/profiles/profilesUpdate";📝 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.components/Chat/Message/MessageContext.tsx (2)
46-48: 🛠️ Refactor suggestion
Add context existence check in the custom hook
The hook should verify that it's being used within a provider context.
export function useMessageContext(): IMessageContextType { - return useContext(MessageContext); + const context = useContext(MessageContext); + if (!context) { + throw new Error('useMessageContext must be used within a MessageContextProvider'); + } + return context; }📝 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 useMessageContext(): IMessageContextType { const context = useContext(MessageContext); if (!context) { throw new Error('useMessageContext must be used within a MessageContextProvider'); } return context; }
15-17: 🛠️ Refactor suggestion
Improve context initialization safety
The current initialization with an empty object cast to
IMessageContextTypecould lead to runtime errors if the context is accidentally used outside of the provider. Consider providing meaningful defaults or throwing an error.-const MessageContext = createContext<IMessageContextType>( - {} as IMessageContextType -); +const MessageContext = createContext<IMessageContextType>({ + showTime: () => { + throw new Error('MessageContext not initialized'); + }, + hideTime: () => { + throw new Error('MessageContext not initialized'); + }, + toggleTime: () => { + throw new Error('MessageContext not initialized'); + }, + showTimeAV: { value: false } as SharedValue<boolean>, +});Committable suggestion skipped: line range outside the PR's diff.
components/Chat/Message/MessageSender.tsx (1)
26-36: 🛠️ Refactor suggestion
Add error handling for missing profile data
Consider handling cases where profile data might be undefined to prevent potential runtime errors.
export const MessageSender = ({ senderAddress }: MessageSenderProps) => { const senderSocials = useProfilesStore( (s) => getProfile(senderAddress, s.profiles)?.socials ); const name = getPreferredName(senderSocials, senderAddress); - return <MessageSenderDumb name={name} />; + if (!name) { + return null; // or a fallback UI + } + return <MessageSenderDumb name={name} />; };📝 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.type MessageSenderProps = { senderAddress: string; }; export const MessageSender = ({ senderAddress }: MessageSenderProps) => { const senderSocials = useProfilesStore( (s) => getProfile(senderAddress, s.profiles)?.socials ); const name = getPreferredName(senderSocials, senderAddress); if (!name) { return null; // or a fallback UI } return <MessageSenderDumb name={name} />; };components/Conversation/ConversationTitleDumb.tsx (1)
13-18: 🛠️ Refactor suggestion
Consider making title prop required and adding prop validation
As a presentational component, rendering without a title could lead to undesirable UI states. Consider:
- Making the
titleprop required- Adding validation for empty strings
type ConversationTitleDumbProps = { - title?: string; + title: string; avatarComponent?: React.ReactNode; onLongPress?: () => void; onPress?: () => void; };📝 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.type ConversationTitleDumbProps = { title: string; avatarComponent?: React.ReactNode; onLongPress?: () => void; onPress?: () => void; };components/PinnedConversations/PinnedV3DMConversation.tsx (2)
17-27:
⚠️ Potential issueConsider safer handling of currentAccount
The non-null assertion (
!) oncurrentAccountcould lead to runtime errors if the account is not available. Consider adding a guard clause or error boundary.export const PinnedV3DMConversation = ({ conversation, }: PinnedV3DMConversationProps) => { const currentAccount = useCurrentAccount(); + if (!currentAccount) { + return null; // or an error state component + } const { data: peerInboxId } = useDmPeerInboxOnConversationList( - currentAccount!, + currentAccount, conversation );📝 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 PinnedV3DMConversation = ({ conversation, }: PinnedV3DMConversationProps) => { const currentAccount = useCurrentAccount(); if (!currentAccount) { return null; // or an error state component } const { data: peerInboxId } = useDmPeerInboxOnConversationList( currentAccount, conversation ); const preferredName = usePreferredInboxName(peerInboxId); const preferredAvatar = usePreferredInboxAvatar(peerInboxId);
34-35: 💡 Codebase verification
The hardcoded showUnread flag should use the showUnreadOnConversation utility
The
showUnreadflag is incorrectly hardcoded tofalsewhen it should be using the availableshowUnreadOnConversationutility. The component has access to theconversationprop and can access the current user's address, which are required parameters for the unread status calculation.
- In
PinnedV3DMConversation.tsx, replace the hardcodedshowUnread = falsewith theshowUnreadOnConversationutility- Required data for the utility:
conversation(already available as prop)currentAccount(already available fromuseCurrentAccount)- Need to add access to
topicsData,initialLoadDoneOnce, andlastMessagePreview🔗 Analysis chain
Verify the hardcoded showUnread flag
The
showUnreadflag is hardcoded tofalse. Is this intentional or should it be derived from the conversation state?🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for unread status implementations in other conversation components rg -A 5 "showUnread" --type typescriptLength of output: 73
Script:
#!/bin/bash # Search for unread status implementations in other conversation components rg -A 5 "showUnread"Length of output: 3784
Script:
#!/bin/bash # Search for unread-related implementations in the codebase rg -A 5 "unread" --type ts --type tsxLength of output: 65
Script:
#!/bin/bash # Find conversation-related components fd -e tsx -e ts "Conversation"Length of output: 2176
Script:
#!/bin/bash # Check the implementation of showUnreadOnConversation utility cat utils/conversation/showUnreadOnConversation.tsLength of output: 950
Script:
#!/bin/bash # Check if there are any props or state related to unread status in parent components rg -A 5 "PinnedV3DMConversation"Length of output: 1837
Script:
#!/bin/bash # Check for any usage of showUnreadOnConversation utility in the component or its parent rg -l "showUnreadOnConversation" components/PinnedConversations/ # Check the full implementation of PinnedV3DMConversation to see available props and data cat components/PinnedConversations/PinnedV3DMConversation.tsxLength of output: 2035
components/PinnedConversations/PinnedV3GroupConversation.tsx (2)
19-23:
⚠️ Potential issueAdd null check for currentAccount
The non-null assertion (
!) oncurrentAccountcould lead to runtime errors if the account is not available.const currentAccount = useCurrentAccount(); + if (!currentAccount) { + return null; // or handle the no-account case appropriately + } const { memberData } = useGroupConversationListAvatarInfo( - currentAccount!, + currentAccount, group );📝 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 currentAccount = useCurrentAccount(); if (!currentAccount) { return null; // or handle the no-account case appropriately } const { memberData } = useGroupConversationListAvatarInfo( currentAccount, group );
33-36:
⚠️ Potential issueRemove empty useEffect
The
useEffecthook contains only early returns without any side effects. This appears to be unused code.- useEffect(() => { - if (!group) return; - if (group.imageUrlSquare) return; - }, [group]);📝 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.components/Chat/Message/messageContextStore.tsx (1)
29-32: 🛠️ Refactor suggestion
Implement prop comparison for store updates
The current implementation updates the store on every prop change without checking if the values actually changed. This could lead to unnecessary re-renders.
Consider implementing a comparison:
useEffect(() => { - // TODO: Check if the props have made something change in the store? - storeRef.current?.setState(props); + const currentState = storeRef.current?.getState(); + const hasChanges = Object.entries(props).some( + ([key, value]) => currentState[key as keyof IMessageContextStoreProps] !== value + ); + if (hasChanges) { + storeRef.current?.setState(props); + } }, [props]);Committable suggestion skipped: line range outside the PR's diff.
components/ParsedText/ParsedText.utils.ts (2)
29-34:
⚠️ Potential issueAddress potential security and safety concerns
Two issues need attention:
- User-provided regex patterns could lead to ReDoS attacks
- Non-null assertion on
match.indexshould be handled more safelyConsider adding:
- Timeout protection for regex execution
- Safe fallback for index
- const regex = new RegExp( - pattern.pattern.source, - pattern.pattern.flags.includes("g") - ? pattern.pattern.flags - : pattern.pattern.flags + "g" - ); + const regex = createSafeRegex(pattern.pattern); + - const index = match.index!; + const index = match.index ?? lastIndex; // Add this helper function + function createSafeRegex(pattern: RegExp): RegExp { + const flags = pattern.flags.includes("g") ? pattern.flags : pattern.flags + "g"; + // Consider adding pattern validation or using a regex safety library + return new RegExp(pattern.source, flags); + }Also applies to: 42-42
86-89: 🛠️ Refactor suggestion
Remove @ts-ignore by improving type definitions
The ts-ignore comment suggests a typing issue with the value function call.
Consider improving the type definitions:
+ type MatchFunction = (text: string, index: number) => void; + type PatternValue = string | boolean | MatchFunction; if (typeof value === "function") { - // @ts-ignore - acc[key] = () => value(text, index); + acc[key] = () => (value as MatchFunction)(text, index); }📝 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.type MatchFunction = (text: string, index: number) => void; type PatternValue = string | boolean | MatchFunction; acc[key] = () => (value as MatchFunction)(text, index); } else {components/Chat/Message/MessageStatusDumb.tsx (1)
53-84:
⚠️ Potential issueReview animation timing and dependencies
There are several concerns with the current implementation:
- The setTimeout delay of 100ms lacks explanation
- The nested requestAnimationFrame might be unnecessary
- The effect is missing dependencies (status, opacity, height, scale, timingConfig)
Consider these improvements:
useEffect( () => { const prevStatus = prevStatusRef.current; prevStatusRef.current = status; + // Allow time for layout calculations before starting animations setTimeout(() => { - requestAnimationFrame(() => { if ( isSentOrDelivered && (prevStatus === "sending" || prevStatus === "prepared") ) { opacity.value = withTiming(1, timingConfig); height.value = withTiming(22, timingConfig); scale.value = withTiming(1, timingConfig); setRenderText(true); } else if (isSentOrDelivered && !isLatestSettledFromMe) { opacity.value = withTiming(0, timingConfig); height.value = withTiming(0, timingConfig); scale.value = withTiming(0, timingConfig); setTimeout(() => setRenderText(false), timingConfig.duration); } else if (isLatestSettledFromMe) { opacity.value = 1; height.value = 22; scale.value = 1; setRenderText(true); } - }); }, 100); }, - // eslint-disable-next-line react-hooks/exhaustive-deps - [isLatestSettledFromMe, isSentOrDelivered] + [ + isLatestSettledFromMe, + isSentOrDelivered, + status, + opacity, + height, + scale, + timingConfig + ] );📝 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.useEffect( () => { const prevStatus = prevStatusRef.current; prevStatusRef.current = status; // Allow time for layout calculations before starting animations setTimeout(() => { if ( isSentOrDelivered && (prevStatus === "sending" || prevStatus === "prepared") ) { opacity.value = withTiming(1, timingConfig); height.value = withTiming(22, timingConfig); scale.value = withTiming(1, timingConfig); setRenderText(true); } else if (isSentOrDelivered && !isLatestSettledFromMe) { opacity.value = withTiming(0, timingConfig); height.value = withTiming(0, timingConfig); scale.value = withTiming(0, timingConfig); setTimeout(() => setRenderText(false), timingConfig.duration); } else if (isLatestSettledFromMe) { opacity.value = 1; height.value = 22; scale.value = 1; setRenderText(true); } }, 100); }, [ isLatestSettledFromMe, isSentOrDelivered, status, opacity, height, scale, timingConfig ] );components/Chat/Message/MessageSenderAvatar.tsx (1)
82-85: 🛠️ Refactor suggestion
Add runtime check for currentAccount
The non-null assertion on
currentAccount!could lead to runtime errors. Consider adding a proper runtime check.- const { data: senderSocials } = useInboxProfileSocialsQuery( - currentAccount!, - inboxId - ); + if (!currentAccount) { + return null; + } + const { data: senderSocials } = useInboxProfileSocialsQuery( + currentAccount, + inboxId + );📝 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.if (!currentAccount) { return null; } const { data: senderSocials } = useInboxProfileSocialsQuery( currentAccount, inboxId );components/Chat/Message/Message.utils.tsx (2)
111-118: 🛠️ Refactor suggestion
Handle potential null case explicitly instead of using non-null assertion.
The
getCurrentAccountInboxIdQueryOptionsfunction uses a non-null assertion (!) which could lead to runtime errors ifcurrentAccountis null.Consider this safer approach:
function getCurrentAccountInboxIdQueryOptions() { const currentAccount = getCurrentAccount(); return { queryKey: ["inboxId", currentAccount], - queryFn: () => getInboxId(currentAccount!), + queryFn: () => { + if (!currentAccount) { + throw new Error("No current account found"); + } + return getInboxId(currentAccount); + }, enabled: !!currentAccount, }; }📝 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.function getCurrentAccountInboxIdQueryOptions() { const currentAccount = getCurrentAccount(); return { queryKey: ["inboxId", currentAccount], queryFn: () => { if (!currentAccount) { throw new Error("No current account found"); } return getInboxId(currentAccount); }, enabled: !!currentAccount, }; }
39-93: 🛠️ Refactor suggestion
Improve type safety by replacing
anywith proper types.The type guard functions currently use
anytype for the message parameter, while commented code suggestsDecodedMessageWithCodecsTypewas intended. This reduces type safety.Apply this pattern to all type guard functions:
-export function isTextMessage( - message: any -): message is DecodedMessage<[TextCodec]> { +export function isTextMessage( + message: DecodedMessageWithCodecsType +): message is DecodedMessage<[TextCodec]> {Committable suggestion skipped: line range outside the PR's diff.
components/ConversationFlashList.tsx (1)
62-85: 🛠️ Refactor suggestion
Improve renderItem type safety and maintainability.
The current implementation could benefit from:
- Type guards for better type safety
- Extraction of rendering logic into separate components
- Error boundary protection
+type V3Conversation = GroupWithCodecsType | DmWithCodecsType; + +const isV3Conversation = (item: any): item is V3Conversation => { + return "lastMessage" in item && "version" in unwrapConversationContainer(item); +}; + +const isHiddenRequestsButton = ( + item: any +): item is ConversationFlatListHiddenRequestItem => { + return "topic" in item && item.topic === "hiddenRequestsButton"; +}; + const renderItem = useCallback(({ item }: { item: FlatListItemType }) => { - if ("lastMessage" in item) { + try { + if (isV3Conversation(item)) { const conversation = unwrapConversationContainer(item); if (conversation.version === ConversationVersion.GROUP) { return ( <V3GroupConversationListItem group={conversation as GroupWithCodecsType} /> ); } else { return <V3DMListItem conversation={conversation as DmWithCodecsType} />; } } - if (item.topic === "hiddenRequestsButton") { - const hiddenRequestItem = item as ConversationFlatListHiddenRequestItem; + if (isHiddenRequestsButton(item)) { return ( <HiddenRequestsButton - spamCount={hiddenRequestItem.spamCount} - toggleActivated={hiddenRequestItem.toggleActivated} + spamCount={item.spamCount} + toggleActivated={item.toggleActivated} /> ); } return null; + } catch (error) { + console.error('Error rendering item:', error); + return null; + } }, []);📝 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.type V3Conversation = GroupWithCodecsType | DmWithCodecsType; const isV3Conversation = (item: any): item is V3Conversation => { return "lastMessage" in item && "version" in unwrapConversationContainer(item); }; const isHiddenRequestsButton = ( item: any ): item is ConversationFlatListHiddenRequestItem => { return "topic" in item && item.topic === "hiddenRequestsButton"; }; const renderItem = useCallback(({ item }: { item: FlatListItemType }) => { try { if (isV3Conversation(item)) { const conversation = unwrapConversationContainer(item); if (conversation.version === ConversationVersion.GROUP) { return ( <V3GroupConversationListItem group={conversation as GroupWithCodecsType} /> ); } else { return <V3DMListItem conversation={conversation as DmWithCodecsType} />; } } if (isHiddenRequestsButton(item)) { return ( <HiddenRequestsButton spamCount={item.spamCount} toggleActivated={item.toggleActivated} /> ); } return null; } catch (error) { console.error('Error rendering item:', error); return null; } }, []);components/Onboarding/init-xmtp-client.ts (2)
33-39:
⚠️ Potential issueEnsure error in async callback is properly propagated
Throwing an error inside the async callback passed to
createXmtpClientFromSignermay not propagate to the outertry...catchblock as expected. Ensure that thecreateXmtpClientFromSignerfunction is designed to handle errors thrown in the callback and that these errors will correctly trigger thecatchblock ininitXmtpClient.
87-87:
⚠️ Potential issueImplement missing logic in
performLogoutAndSaveKeyfunctionThe
performLogoutAndSaveKeyfunction currently contains only logging statements without actual logic to perform logout or save the XMTP key. Ensure that the necessary functionality is implemented to handle logout operations and key management.async function performLogoutAndSaveKey(address: string) { logger.debug("Waiting for logout tasks"); await waitForLogoutTasksDone(500); logger.debug("Logout tasks done, saving xmtp key"); + // TODO: Implement logic to save XMTP key and perform logout tasks + // Example: + // await saveXmtpKey(address); + // Additional logout operations as needed logger.debug("XMTP Key saved"); }Also applies to: 99-104
components/Chat/Message/RepliableMessageWrapper.tsx (2)
76-78:
⚠️ Potential issueAvoid accessing internal properties of Swipeable component
Accessing internal properties like
swipeableRef.current?.state.rowTranslationand(translation as any)._valueis not recommended because these are not part of the public API and may change in future versions, leading to potential breakage.Consider using publicly exposed APIs or alternative approaches to achieve the desired functionality.
76-77:
⚠️ Potential issuePotential null reference when accessing
_valueof undefinedIf
swipeableRef.currentortranslationisundefined, accessing(translation as any)._valuewill result in a runtime error.Consider adding a null check before accessing
_value.Apply this diff to ensure safety:
const translation = swipeableRef.current?.state.rowTranslation; -const translationValue = (translation as any)._value; +const translationValue = translation ? (translation as any)._value : 0;📝 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 translation = swipeableRef.current?.state.rowTranslation; const translationValue = translation ? (translation as any)._value : 0;components/ParsedText/ParsedText.tsx (2)
11-11:
⚠️ Potential issueEnsure consistency of the
parseprop's optionalityThe
parseprop is currently marked as required inParsedTextProps(line 11), but you check for its existence ingetParsedText(lines 46-48). This inconsistency might lead to errors or confusion. Consider makingparseoptional in the type definition or ensure it's always provided.Also applies to: 46-48
17-17: 🛠️ Refactor suggestion
Provide a default value for the
parsepropTo prevent
parsefrom beingundefined, you can assign a default empty array when destructuring:- const { parse, childrenProps = {}, ...rest } = props; + const { parse = [], childrenProps = {}, ...rest } = props;This ensures
parseis always an array, simplifying checks in your functions.📝 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 { parse = [], childrenProps = {}, ...rest } = props;components/Chat/ConsentPopup/ConsentPopup.tsx (2)
83-89: 🛠️ Refactor suggestion
⚠️ Potential issueInconsistent navigation and missing
awaitin consent acceptanceIn the consent acceptance handler,
consentToInboxIdsOnProtocolByAccountis called withoutawait, which may cause the consent action to not complete before other operations occur. Additionally, unlike the block action, there's no navigation after accepting consent, which could lead to an inconsistent user experience.Apply this diff to await the function and ensure consistent navigation:
onPress={async () => { - const inboxId = await conversation.peerInboxId(); - consentToInboxIdsOnProtocolByAccount({ + try { + const inboxId = await conversation.peerInboxId(); + await consentToInboxIdsOnProtocolByAccount({ account: currentAccount(), inboxIds: [inboxId], consent: "allow", }); + navigation.pop(); + } catch (error) { + // Handle error appropriately, e.g., show an alert + console.error(error); + } }}Committable suggestion skipped: line range outside the PR's diff.
64-71: 🛠️ Refactor suggestion
⚠️ Potential issueMissing
awaitand error handling for asynchronous consent actionThe
consentToInboxIdsOnProtocolByAccountfunction appears to be asynchronous but is not awaited. This could lead to unexpected behavior if the consent action hasn't completed beforenavigation.pop()is called. Additionally, there's no error handling for potential exceptions from the asynchronous calls.Apply this diff to await the function and add error handling:
async (selectedIndex?: number) => { if (selectedIndex === 0) { - const inboxId = await conversation.peerInboxId(); - consentToInboxIdsOnProtocolByAccount({ + try { + const inboxId = await conversation.peerInboxId(); + await consentToInboxIdsOnProtocolByAccount({ account: currentAccount(), inboxIds: [inboxId], consent: "deny", }); navigation.pop(); + } catch (error) { + // Handle error appropriately, e.g., show an alert + console.error(error); + } } }📝 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.async (selectedIndex?: number) => { if (selectedIndex === 0) { try { const inboxId = await conversation.peerInboxId(); await consentToInboxIdsOnProtocolByAccount({ account: currentAccount(), inboxIds: [inboxId], consent: "deny", }); navigation.pop(); } catch (error) { // Handle error appropriately, e.g., show an alert console.error(error); } }android/app/src/main/java/com/converse/xmtp/Conversations.kt (1)
68-68: 🛠️ Refactor suggestion
Update error messages to reflect 'conversation' instead of 'group'
In the
sentryTrackErrorcalls withingetNewConversationandgetConversation, the error message says"Could not sync new group":
- In
getNewConversation(line 68)- In
getConversation(line 79)Since these functions handle conversations, please update the error messages to accurately reflect the context and avoid confusion.
Apply this diff to update the error messages:
- sentryTrackError(error, mapOf("message" to "Could not sync new group")) + sentryTrackError(error, mapOf("message" to "Could not sync new conversation"))Also applies to: 79-79
components/Chat/Attachment/AddAttachmentButton.tsx (1)
174-179:
⚠️ Potential issueHandle upload errors by updating status and notifying the user
When an error occurs during the attachment upload process, the
composerMediaPreviewStatusremains as"uploading", and the user isn't informed of the failure. Consider updating the status to reflect the error and providing user feedback.Apply this diff to update the status and notify the user:
} catch (error) { sentryTrackMessage("ATTACHMENT_UPLOAD_ERROR", { error }); + setComposerMediaPreviewStatus("error"); + showToast("Attachment upload failed. Please try again."); } } catch (error) { sentryTrackError(error); + setComposerMediaPreviewStatus("error"); + showToast("An error occurred while processing the attachment."); }Note: Ensure that a user notification method like
showToastis available and appropriate for your application's UI to inform users about the error.Committable suggestion skipped: line range outside the PR's diff.
components/Chat/ChatGroupUpdatedMessage.tsx (2)
63-103: 🛠️ Refactor suggestion
Refactor duplicated logic in
ChatGroupMemberLeftandChatGroupMemberJoinedcomponentsThe
ChatGroupMemberLeftandChatGroupMemberJoinedcomponents share a significant amount of duplicated code. Consider refactoring to extract the shared logic into a single reusable component to improve maintainability and reduce code duplication.For example, you could create a generic
ChatGroupMemberUpdatecomponent that accepts props for the action type (e.g., "joined" or "left") and the corresponding translation key. This would reduce redundancy and make future updates easier.Also applies to: 109-149
99-100:
⚠️ Potential issueIncorrect translation key in
ChatGroupMemberLeftcomponentIn the
ChatGroupMemberLeftcomponent, the translation key used is"group_member_joined", but it should be"group_member_left"to accurately reflect that a member has left the group.Apply this diff to fix the translation key:
- {translate("group_member_joined")} + {translate("group_member_left")}Committable suggestion skipped: line range outside the PR's diff.
components/Chat/ChatDumb.tsx (2)
161-167:
⚠️ Potential issueHandle cases where
messageIdis not found initemsIn the
scrollToMessagefunction, if themessageIdis not found,findIndexreturns-1. Attempting to scroll to index-1can cause errors. The current conditionif (index !== undefined)does not prevent this.Update the condition to check if
indexis greater than or equal to0:- if (index !== undefined) { + if (index !== undefined && index >= 0) { messageListRef.current?.scrollToIndex({ index, viewPosition: 0.5, animated: !!data.animated, }); }📝 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.if (index !== undefined && index >= 0) { messageListRef.current?.scrollToIndex({ index, viewPosition: 0.5, animated: !!data.animated, }); }
189-214:
⚠️ Potential issueAssign the
messageListRefto theReanimatedFlashListcomponentThe
messageListRefis defined but not attached to theReanimatedFlashList. Without assigning the ref, thescrollToMessagefunction will not work as intended because it won't have access to the list methods.Apply this diff to fix the issue:
<ReanimatedFlashList contentContainerStyle={styles.chat} data={items} refreshing={refreshing} extraData={extraData} renderItem={renderItem} onLayout={handleOnLayout} keyboardDismissMode="interactive" automaticallyAdjustContentInsets={false} contentInsetAdjustmentBehavior="never" inverted keyExtractor={keyExtractor} getItemType={getItemType} keyboardShouldPersistTaps="handled" estimatedItemSize={34} showsVerticalScrollIndicator={Platform.OS === "ios"} pointerEvents="auto" ListFooterComponent={ListFooterComponent} + ref={messageListRef} } />📝 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.<ReanimatedFlashList contentContainerStyle={styles.chat} data={items} refreshing={refreshing} extraData={extraData} renderItem={renderItem} onLayout={handleOnLayout} keyboardDismissMode="interactive" automaticallyAdjustContentInsets={false} contentInsetAdjustmentBehavior="never" // Causes a glitch on Android, no sure we need it for now // maintainVisibleContentPosition={{ // minIndexForVisible: 0, // autoscrollToTopThreshold: 100, // }} // estimatedListSize={Dimensions.get("screen")} inverted keyExtractor={keyExtractor} getItemType={getItemType} keyboardShouldPersistTaps="handled" estimatedItemSize={34} // Size glitch on Android showsVerticalScrollIndicator={Platform.OS === "ios"} pointerEvents="auto" ListFooterComponent={ListFooterComponent} ref={messageListRef} />android/app/src/main/java/com/converse/Spam.kt (2)
57-59: 🛠️ Refactor suggestion
Avoid blocking the thread with
runBlockingincomputeDmSpamScoreUsing
runBlockingcan block the current thread, potentially causing performance issues, especially if this function is called on the main thread. Consider convertingcomputeDmSpamScoreinto a suspending function to leverage coroutines properly and prevent blocking.Apply this diff to refactor the function:
-fun computeDmSpamScore(address: String, message: String?, contentType: String, apiURI: String?, appContext: Context): Double { - var senderScore = runBlocking { - getSenderSpamScore(appContext, address, apiURI); - } +suspend fun computeDmSpamScore(address: String, message: String?, contentType: String, apiURI: String?, appContext: Context): Double { + val senderScore = getSenderSpamScore(appContext, address, apiURI) return senderScore + getMessageSpamScore(message, contentType) }📝 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.suspend fun computeDmSpamScore(address: String, message: String?, contentType: String, apiURI: String?, appContext: Context): Double { val senderScore = getSenderSpamScore(appContext, address, apiURI) return senderScore + getMessageSpamScore(message, contentType) }
158-160:
⚠️ Potential issueAvoid swallowing exceptions without handling or logging
Swallowing exceptions without proper handling or logging can make debugging difficult and obscure the root cause of issues. Consider logging the exception or providing more specific error handling to improve maintainability.
Apply this diff to log the exception:
} catch (e: Exception) { - return 0.0 + Log.e("SpamKt", "Error in computeSpamScoreDmWelcome", e) + return 0.0 }📝 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.} catch (e: Exception) { Log.e("SpamKt", "Error in computeSpamScoreDmWelcome", e) return 0.0 }🧰 Tools
🪛 detekt (1.23.7)
[warning] 158-158: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
components/V3DMListItem.tsx (2)
280-280:
⚠️ Potential issuePass the correct
isUnreadprop toConversationListItemDumbThe
isUnreadprop is currently hardcoded tofalse. To properly reflect the unread status of the conversation, you should pass the computedisUnreadvalue.Apply this diff to fix the issue:
- isUnread={false} + isUnread={isUnread}Committable suggestion skipped: line range outside the PR's diff.
48-48:
⚠️ Potential issueAvoid using non-null assertion
!without proper null checksUsing the non-null assertion operator
!onuseCurrentAccount()can lead to runtime errors if it returnsnullorundefined. Consider adding a null check or handling the case whencurrentAccountis not available.Apply this diff to safely handle the potential null value:
- const currentAccount = useCurrentAccount()!; + const currentAccount = useCurrentAccount(); + if (!currentAccount) { + // Handle the null case, e.g., navigate to login or return null + return null; + }Committable suggestion skipped: line range outside the PR's diff.
components/Conversation/V3Conversation.tsx (3)
220-223:
⚠️ Potential issuePrevent index out-of-bounds errors in
MessagecomponentAccessing
messages.ids[index + 1]andmessages.ids[index - 1]without bounds checking may cause runtime errors whenindexis at the start or end of the array.Apply this diff to prevent out-of-bounds access:
220 <V3Message 221 messageId={messageId} 222- previousMessageId={messages.ids[index + 1]} 223- nextMessageId={messages.ids[index - 1]} + previousMessageId={index + 1 < messages.ids.length ? messages.ids[index + 1] : null} + nextMessageId={index > 0 ? messages.ids[index - 1] : null} 224 />📝 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.messageId={messageId} previousMessageId={index + 1 < messages.ids.length ? messages.ids[index + 1] : null} nextMessageId={index > 0 ? messages.ids[index - 1] : null} />
54-60:
⚠️ Potential issueHandle undefined
topicto prevent potential crashesThe
topicmay be undefined, as indicated by the TODO comment. CallinginitializeCurrentConversationwith an undefinedtopiccould lead to unexpected behavior or runtime errors.Apply this diff to handle the case when
topicis undefined:54 // TODO: Handle when topic is not defined 55 const messageToPrefill = textPrefill ?? getDraftMessage(topic) ?? ""; 56+ if (topic) { 57 initializeCurrentConversation({ 58 topic, 59 peerAddress, 60 inputValue: messageToPrefill, 61 }); 62+ } else { 63+ // Handle the undefined topic case appropriately 64+ }Committable suggestion skipped: line range outside the PR's diff.
177-179: 🛠️ Refactor suggestion
Avoid using
@ts-ignoreby properly typingAnimated.FlatListUsing
@ts-ignoresuppresses TypeScript errors and can hide legitimate issues. It's better to correctly typeAnimated.FlatListto resolve the TypeScript error without ignoring it.Consider updating the import to include the correct type definitions:
16 import Animated, { 17 AnimatedProps, 18 useAnimatedKeyboard, 19 useAnimatedStyle, 20 } from "react-native-reanimated"; + import { FlatList } from "react-native"; + const AnimatedFlatList = Animated.createAnimatedComponent(FlatList) as typeof FlatList;Then, update the usage:
175- <Animated.FlatList + <AnimatedFlatListCommittable suggestion skipped: line range outside the PR's diff.
components/Chat/Attachment/AttachmentMessagePreview.tsx (6)
65-68:
⚠️ Potential issueAdd validation to prevent division by zero when calculating aspect ratio
When computing the
aspectRatio, there's a potential for a division by zero ifattachment.imageSize.heightis zero. This would lead to a runtime error. Consider adding a check to ensureattachment.imageSize.heightis not zero before performing the division.Apply this diff to add the necessary validation:
const aspectRatio = fitAspectRatio && attachment.imageSize + && attachment.imageSize.height !== 0 ? attachment.imageSize.width / attachment.imageSize.height : undefined;📝 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 aspectRatio = fitAspectRatio && attachment.imageSize && attachment.imageSize.height !== 0 ? attachment.imageSize.width / attachment.imageSize.height : undefined;
27-30:
⚠️ Potential issueInclude
remoteMessageContentinqueryKeyto prevent caching issuesIn the
useRemoteAttachmenthook, thequeryKeycurrently includes onlymessageId. SinceremoteMessageContentis a dependency forfetchAttachment, it should be included in thequeryKeyto ensure proper caching and data consistency. This prevents stale or incorrect data whenremoteMessageContentchanges butmessageIdremains the same.Apply this diff to include
remoteMessageContentin thequeryKey:return useQuery({ - queryKey: ["attachment", messageId], + queryKey: ["attachment", messageId, remoteMessageContent], queryFn: () => fetchAttachment(messageId, remoteMessageContent), });📝 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.return useQuery({ queryKey: ["attachment", messageId, remoteMessageContent], queryFn: () => fetchAttachment(messageId, remoteMessageContent), });
306-307:
⚠️ Potential issueValidate
content.contentLengthto handle invalid valuesParsing
content.contentLengthusingparseFloatwithout validation could result inNaN, leading to unexpected behavior in the comparison. Ensure thatcontent.contentLengthis a valid number before performing the comparison toMAX_AUTOMATIC_DOWNLOAD_ATTACHMENT_SIZE.Apply this diff to add validation:
if ( content.contentLength && + !isNaN(parseFloat(content.contentLength)) && parseFloat(content.contentLength) <= MAX_AUTOMATIC_DOWNLOAD_ATTACHMENT_SIZE ) {Alternatively, ensure that
content.contentLengthis always provided as a number, eliminating the need for parsing.📝 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.content.contentLength && !isNaN(parseFloat(content.contentLength)) && parseFloat(content.contentLength) <= MAX_AUTOMATIC_DOWNLOAD_ATTACHMENT_SIZE
172-174:
⚠️ Potential issueImplement the
openInWebviewfunctionality or remove the unusedonPresshandlerThe
onPresshandler for thePressableScalecomponent is currently empty and includes a commented-out reference toopenInWebview. This may cause confusion and indicates incomplete functionality. If the intent is to allow users to view unsupported attachments in a web browser, implement theopenInWebviewfunction. Otherwise, remove theonPresshandler to clean up the code.Apply this diff if you intend to implement
openInWebview:- onPress={() => { - // openInWebview - }} + onPress={openInWebview}Ensure that
openInWebviewis defined and handles the navigation appropriately.Committable suggestion skipped: line range outside the PR's diff.
98-101:
⚠️ Potential issueInclude
remoteMessageContentinqueryKeyto ensure proper data fetchingIn the
RemoteAttachmentPreviewcomponent, theuseQueryhook'squeryKeydoes not includeremoteMessageContent, which is a dependency forfetchAttachment. Omitting it can lead to inappropriate caching behavior whenremoteMessageContentchanges.Apply this diff to update the
queryKey:useQuery({ - queryKey: ["attachment", messageId], + queryKey: ["attachment", messageId, remoteMessageContent], queryFn: () => fetchAttachment(messageId, remoteMessageContent), });📝 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.} = useQuery({ queryKey: ["attachment", messageId, remoteMessageContent], queryFn: () => fetchAttachment(messageId, remoteMessageContent), });
293-325:
⚠️ Potential issueAdd error handling to
fetchAttachmentto gracefully handle failuresThe
fetchAttachmentfunction performs several asynchronous operations that may fail, such as fetching the local attachment or fetching and decoding the remote attachment. Currently, there's no try-catch block to handle exceptions, which could lead to unhandled promise rejections.Apply this diff to wrap the asynchronous operations in a try-catch block:
async function fetchAttachment( messageId: string, content: RemoteAttachmentContent ) { + try { const localAttachment = await getLocalAttachmentForMessageId(messageId); if (localAttachment) { return localAttachment; } if ( content.contentLength && !isNaN(parseFloat(content.contentLength)) && parseFloat(content.contentLength) <= MAX_AUTOMATIC_DOWNLOAD_ATTACHMENT_SIZE ) { const decryptedLocalAttachment = await fetchAndDecodeRemoteAttachment({ account: getCurrentAccount()!, messageId: messageId, remoteAttachmentContent: content, }); const result = await handleDecryptedLocalAttachment({ messageId: messageId, decryptedLocalAttachment: decryptedLocalAttachment, }); return result; } + throw new Error("Attachment exceeds maximum automatic download size."); + } catch (error) { + console.error("Error fetching attachment:", error); + throw error; + } }This ensures that any errors are caught and can be handled appropriately by the calling code.
📝 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.async function fetchAttachment( messageId: string, content: RemoteAttachmentContent ) { try { const localAttachment = await getLocalAttachmentForMessageId(messageId); console.log("localAttachment:", localAttachment); if (localAttachment) { return localAttachment; } if ( content.contentLength && parseFloat(content.contentLength) <= MAX_AUTOMATIC_DOWNLOAD_ATTACHMENT_SIZE ) { const decryptedLocalAttachment = await fetchAndDecodeRemoteAttachment({ account: getCurrentAccount()!, messageId: messageId, remoteAttachmentContent: content, }); console.log("decryptedLocalAttachment", decryptedLocalAttachment); const result = await handleDecryptedLocalAttachment({ messageId: messageId, decryptedLocalAttachment: decryptedLocalAttachment, }); console.log("result:", result); return result; } throw new Error("Attachment exceeds maximum automatic download size."); } catch (error) { console.error("Error fetching attachment:", error); throw error; } }components/ConversationListItem/ConversationListItemDumb.tsx (1)
303-331:
⚠️ Potential issuePotential logical error in
rightIsDestructiveusage inrenderRightActionsThe
renderRightActionsfunction seems to have the logic inverted for therightIsDestructiveprop. WhenrightIsDestructiveistrue, it currently renders a non-destructive style with a checkmark icon. Conversely, whenrightIsDestructiveisfalse, it renders a destructive style with a trash icon. This may lead to unintended behavior where destructive actions are not properly indicated.Apply this diff to correct the logic:
- if (rightIsDestructive) { - return ( - <RectButton style={styles.rightAction} onPress={onRightActionPress}> - <Icon - icon="checkmark" - color={themedInversePrimaryColor} - size={PictoSizes.swipableItem} - /> - </RectButton> - ); - } else { - return ( - <RectButton - style={styles.rightActionRed} - onPress={onRightActionPress} - > - <Icon icon="trash" color="white" size={PictoSizes.swipableItem} /> - </RectButton> - ); - } + if (rightIsDestructive) { + return ( + <RectButton + style={styles.rightActionRed} + onPress={onRightActionPress} + > + <Icon icon="trash" color="white" size={PictoSizes.swipableItem} /> + </RectButton> + ); + } else { + return ( + <RectButton style={styles.rightAction} onPress={onRightActionPress}> + <Icon + icon="checkmark" + color={themedInversePrimaryColor} + size={PictoSizes.swipableItem} + /> + </RectButton> + ); + }📝 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 renderRightActions = useCallback(() => { if (rightIsDestructive) { return ( <RectButton style={styles.rightActionRed} onPress={onRightActionPress} > <Icon icon="trash" color="white" size={PictoSizes.swipableItem} /> </RectButton> ); } else { return ( <RectButton style={styles.rightAction} onPress={onRightActionPress}> <Icon icon="checkmark" color={themedInversePrimaryColor} size={PictoSizes.swipableItem} /> </RectButton> ); } }, [ rightIsDestructive, styles.rightAction, styles.rightActionRed, onRightActionPress, themedInversePrimaryColor, ]);components/V3GroupConversationListItem.tsx (5)
47-47:
⚠️ Potential issueEnsure
currentAccountis not null before usageThe usage of
useCurrentAccount()assumescurrentAccountis always available by using the non-null assertion operator!. IfcurrentAccountcan be null or undefined, this may lead to runtime errors.Apply this diff to add a null check:
-const currentAccount = useCurrentAccount()!; +const currentAccount = useCurrentAccount(); +if (!currentAccount) { + // Handle the null case appropriately, e.g., show an error or redirect + return null; +}Ensure this check is applied at both occurrences in the code.
Also applies to: 274-274
283-285:
⚠️ Potential issueHandle asynchronous operations properly with
awaitThe functions
Haptics.mediumImpactAsync()andHaptics.successNotificationAsync()are asynchronous but are not being awaited. Not awaiting these promises may lead to unhandled promise rejections or unexpected behavior.Apply this diff to properly await the asynchronous calls:
const triggerHapticFeedback = useCallback(() => { - return Haptics.mediumImpactAsync(); + Haptics.mediumImpactAsync(); }, []); const onWillLeftSwipe = useCallback(() => { - Haptics.successNotificationAsync(); + Haptics.successNotificationAsync(); }, []);Alternatively, if you want to handle any potential errors:
const triggerHapticFeedback = useCallback(async () => { + try { await Haptics.mediumImpactAsync(); + } catch (error) { + console.error('Haptic feedback failed', error); + } }, []); const onWillLeftSwipe = useCallback(async () => { + try { await Haptics.successNotificationAsync(); + } catch (error) { + console.error('Haptic feedback failed', error); + } }, []);Also applies to: 308-309
143-146:
⚠️ Potential issueAwait asynchronous action functions in callbacks
In the
showActionSheetWithOptionscallbacks withinhandleDeleteandhandleRestore, the selected actions may be asynchronous, but they are not being awaited. This could result in unhandled promise rejections or actions not completing as expected.Modify the callbacks to await the actions:
showActionSheetWithOptions( // ...existing code... - async (selectedIndex?: number) => { + (selectedIndex?: number) => { if (selectedIndex !== undefined && selectedIndex < actions.length) { - actions[selectedIndex](); + const actionResult = actions[selectedIndex](); + if (actionResult instanceof Promise) { + actionResult.catch(error => console.error('Action failed', error)); + } } } );Or make the callback function
asyncand useawait:showActionSheetWithOptions( // ...existing code... async (selectedIndex?: number) => { if (selectedIndex !== undefined && selectedIndex < actions.length) { - actions[selectedIndex](); + await actions[selectedIndex](); } } );Ensure this change is applied to both
handleDeleteandhandleRestore.Also applies to: 190-193
202-204:
⚠️ Potential issueCorrect the pinning functionality to avoid overwriting existing pins
The
setPinnedConversations([topic])action overwrites the existing list of pinned conversations with a new array containing only the current topic. This could unintentionally remove previously pinned conversations.Apply this diff to append the current topic to the existing list:
-action: () => { - setPinnedConversations([topic]); - closeContextMenu(); +action: () => { + setPinnedConversations((prevPinned) => [...prevPinned, topic]); + closeContextMenu();📝 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.setPinnedConversations((prevPinned) => [...prevPinned, topic]); closeContextMenu(); },
288-289: 🛠️ Refactor suggestion
Review the usage of
runOnJSfor calling JavaScript functionsThe
runOnJSfunction is intended to be used within Reanimated worklets to call JavaScript functions. IfonLongPressis not a worklet, usingrunOnJSis unnecessary and may cause confusion.Consider removing
runOnJSto directly call the functions:-runOnJS(triggerHapticFeedback)(); -runOnJS(showContextMenu)(); +triggerHapticFeedback(); +showContextMenu();📝 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.triggerHapticFeedback(); showContextMenu();android/app/src/main/java/com/converse/PushNotificationsService.kt (2)
123-135:
⚠️ Potential issueEnsure proper comparison of
NotificationDataResultinstancesComparing
resultwith a new instance ofNotificationDataResult()using!=may not reliably determine ifresultcontains meaningful data. This comparison checks for instance equality rather than content equality.Consider implementing an
isEmpty()method or overriding theequals()method inNotificationDataResultto provide a more accurate comparison.
103-107:
⚠️ Potential issuePotential issue with timestamp calculation and incorrect comment
Timestamp conversion concern: The assignment
timestampNs = notificationData.timestampNs.toLong() / 1_000_000divides the timestamp by 1,000,000, converting nanoseconds to milliseconds. However, iftimestampNsexpects nanoseconds, this division might lead to incorrect timestamp values. Verify whether this conversion is necessary or if the original nanosecond value should be used.Incorrect comment correction: The comment
// Convert ByteString to byte arrayis misleading. The code actually converts aByteArrayto aByteStringusing.toByteString(). Update the comment to accurately describe the operation.Apply this diff to correct the comment:
- message = encryptedMessageData.toByteString()// Convert ByteString to byte array + message = encryptedMessageData.toByteString() // Convert ByteArray to ByteStringCommittable suggestion skipped: line range outside the PR's diff.
components/Chat/Frame/FramePreview.tsx (1)
7-415: 🛠️ Refactor suggestion
Consider removing commented-out code to improve maintainability.
The entire code from lines 7 to 415 is commented out, effectively disabling the
FramePreviewcomponent. Keeping large blocks of commented code can clutter the codebase and hinder readability. If this code is no longer needed, consider removing it entirely since version control systems preserve the history. If it's temporarily commented out during refactoring, add a comment explaining its purpose and anticipated reintroduction.android/app/src/main/java/com/converse/xmtp/Messages.kt (5)
99-99:
⚠️ Potential issueVerify parameters in
handleMessageByContentTypecallThe function call at line 99 seems to have an extra trailing comma or may be missing parameters. This could lead to a syntax error or unintended behavior.
Ensure that the parameters are correctly specified:
val decodedMessageResult = handleMessageByContentType( appContext, decodedMessage, xmtpClient - , )Committable suggestion skipped: line range outside the PR's diff.
62-62:
⚠️ Potential issueAdd exception handling around
conversation.sync()The call to
conversation.sync()may throw exceptions due to network issues or other errors. Without handling these exceptions, the app may crash unexpectedly.Wrap the
conversation.sync()call in a try-catch block to handle potential exceptions gracefully:try { conversation.sync() +} catch (e: Exception) { + Log.e(TAG, "Error syncing conversation: ${e.message}", e) + // Consider returning an appropriate NotificationDataResult or rethrowing the exception + return NotificationDataResult() }Committable suggestion skipped: line range outside the PR's diff.
369-370:
⚠️ Potential issueAdd exception handling when synchronizing consent
Calls to
xmtpClient.syncConsent()and accessingxmtpClient.preferences.consentListmay throw exceptions due to network errors or data issues.Wrap these calls in a try-catch block to handle potential exceptions and maintain application stability:
try { xmtpClient.syncConsent() val consentList = xmtpClient.preferences.consentList +} catch (e: Exception) { + Log.e(TAG, "Error syncing consent: ${e.message}", e) + // Handle the exception or return an appropriate result + return NotificationDataResult(shouldShowNotification = false) }📝 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.try { xmtpClient.syncConsent() val consentList = xmtpClient.preferences.consentList } catch (e: Exception) { Log.e(TAG, "Error syncing consent: ${e.message}", e) // Handle the exception or return an appropriate result return NotificationDataResult(shouldShowNotification = false) }
92-94:
⚠️ Potential issuePrevent potential
IndexOutOfBoundsExceptionwhen accessing addressesWhen accessing
addresses.get(0), there is a risk of anIndexOutOfBoundsExceptionifaddressesis empty. To ensure robustness, usefirstOrNull()instead.Apply this diff to safely access the first address:
dm.members().firstOrNull { it.inboxId == decodedMessage.senderAddress }?.addresses - ?.get(0) + ?.firstOrNull() ?.let { senderAddress -> decodedMessage.senderAddress = senderAddress }📝 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.dm.members().firstOrNull { it.inboxId == decodedMessage.senderAddress }?.addresses?.firstOrNull()?.let { senderAddress -> decodedMessage.senderAddress = senderAddress }
142-143:
⚠️ Potential issuePrevent potential
NullPointerExceptionwhen accessing group membersIn
handleGroupMessage, when accessinggroup.members(), there is a risk of aNullPointerExceptionif the group or its members arenull.Ensure that you safely access the group members:
val group = convoGroup.group +if (group == null) { + Log.e(TAG, "Group is null in handleGroupMessage") + return NotificationDataResult() +} group.members()?.firstOrNull { it.inboxId == decodedMessage.senderAddress }?.addresses ?.firstOrNull() ?.let { senderAddress -> decodedMessage.senderAddress = senderAddress }Committable suggestion skipped: line range outside the PR's diff.
components/Chat/Message/V3Message.tsx (3)
272-273:
⚠️ Potential issueRemove unreachable code after throw statement
The
return null;statement afterthrow new Error("Unknown message type");is unreachable because thethrowwill exit the function. This line should be removed to clean up the code.Apply this diff to remove the unreachable code:
throw new Error("Unknown message type"); - return null;📝 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.throw new Error("Unknown message type");🧰 Tools
🪛 Biome (1.9.4)
[error] 273-273: This code is unreachable
... because this statement will throw an exception beforehand
(lint/correctness/noUnreachable)
95-111: 🛠️ Refactor suggestion
Remove unnecessary empty useEffect
An empty
useEffectwith dependencies does nothing and can be safely removed to clean up the code. If you plan to add functionality later, consider adding a TODO comment to indicate future implementation.Apply this diff to remove the empty
useEffect:- useEffect(() => { - // const unsubscribe = subscribeToGroupMessages({ - // account: currentAccount, - // topic, - // callback: ({ data }) => { - // if (data) { - // const message = data.byId[messageId]; - // const nextMessage = data.byId[nextMessageId]; - // const previousMessage = data.byId[previousMessageId]; - // // If the updated at changed, update the message - // } - // }, - // }); - // return () => { - // unsubscribe(); - // }; - }, [currentAccount, topic, messageId, nextMessageId, previousMessageId]);📝 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.
330-331:
⚠️ Potential issueUse dynamic 'fromMe' value instead of hardcoded true
In
RemoteAttachmentMessage, thefromMeprop is hardcoded totrue, which may not accurately represent whether the message is from the current user. You should use thefromMevalue derived from the message context to ensure correct rendering.Apply this diff to use the dynamic
fromMevalue:- <MessageContainer fromMe={true}> - <MessageContentContainer fromMe={true}> + <MessageContainer fromMe={fromMe}> + <MessageContentContainer fromMe={fromMe}>Ensure you obtain
fromMefrom the message context or pass it as a prop toRemoteAttachmentMessage.Committable suggestion skipped: line range outside the PR's diff.
Added context menu store Added handling for context menus with pinned conversations Fixed dark mode messages Fixed timestamps Fixed Unread
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Outside diff range and nitpick comments (19)
components/PinnedConversations/PinnedV3GroupConversation.tsx (3)
24-26: Consider moving utility function to shared location.If
closeContextMenuis used across multiple components, consider moving it to a shared utilities file for better reusability.
125-133: Improve title fallback handling.Consider using a more meaningful default for the title when it's undefined.
- title={title ?? ""} + title={title ?? translate("unnamed_group")}
28-134: Consider adding error boundary.As this component is part of the V3 migration and handles complex group conversation logic, consider wrapping it with an error boundary to gracefully handle potential runtime errors.
This would prevent the entire conversation list from crashing if there are issues with a single pinned conversation.
components/PinnedConversations/PinnedV3DMConversation.tsx (2)
25-28: Consider memoizing the closeContextMenu function.Since this function is used in multiple context menu item callbacks, consider memoizing it with
useCallbackto prevent unnecessary recreations.-const closeContextMenu = () => { +const closeContextMenu = useCallback(() => { resetConversationListContextMenuStore(); -}; +}, []);
113-133: Consider a more meaningful fallback for the title prop.Instead of falling back to an empty string when the title is undefined, consider using a more meaningful placeholder.
- title={title ?? ""} + title={title ?? translate("unnamed_conversation")}components/Conversation/V3Conversation.tsx (3)
7-7: Remove commented out importThe commented import
DmChatPlaceholderappears to be unused. Either remove it or use it in the TODO placeholders at lines 119 and 124.
180-181: Address performance optimization TODOs and type issuesSeveral performance optimizations are commented out:
- The
@ts-ignorecomment lacks explanation- The
estimatedItemSizeis TODO'dmaintainVisibleContentPositionis disabled for AndroidConsider implementing these optimizations or document why they're disabled.
Also applies to: 189-199
301-303: Consider making the greeting message configurableThe hardcoded "👋" emoji might not be appropriate for all contexts or cultures. Consider making this configurable or using a translation key.
sendMessage({ - text: "👋", + text: translate("default_greeting"), });components/V3DMListItem.tsx (4)
175-178: Remove empty callbacks to clean up the code.The callbacks
onWillRightSwipeandonRightSwipeare defined but contain empty functions. If these callbacks are optional inConversationListItemDumb, you can omit them to reduce clutter.Apply this diff to remove unnecessary callbacks:
- const onWillRightSwipe = useCallback(() => {}, []); - const onRightSwipe = useCallback(() => {}, []);And remove them from the component props:
<ConversationListItemDumb ref={ref} onPress={onPress} onLongPress={onLongPress} onLeftSwipe={onLeftSwipe} - onRightSwipe={onRightSwipe} onWillLeftSwipe={onWillLeftSwipe} - onWillRightSwipe={onWillRightSwipe} leftActionIcon={leftActionIcon} // ... other props />
128-134: Includetranslatein the dependency array ofuseMemo.The
translatefunction is used within theuseMemohook but is not included in the dependency array. Iftranslatecan change (e.g., when changing the app's language), the memoized value may not update correctly. Ensure all dependencies are included to maintain consistent behavior.Apply this diff to update the dependency array:
const contextMenuItems = useMemo( () => [ { title: translate("pin"), // ... other properties }, // ... other menu items ], [ topic, setPinnedConversations, handleDelete, closeContextMenu, isUnread, toggleReadStatus, + translate, ] );
184-184: Remove commented-out proponRightActionPress.The prop
onRightActionPressis commented out in the JSX. If it's no longer needed, consider removing it to keep the code clean. If it will be used later, add a TODO comment explaining its future implementation.Apply this diff to remove the commented line:
<ConversationListItemDumb ref={ref} onPress={onPress} - // onRightActionPress={onRightPress} onLongPress={onLongPress} // ... other props />
67-72: Optimize parameters passed touseConversationIsUnread.Both
conversationandconversation.lastMessageare passed touseConversationIsUnread. IflastMessagecan be accessed fromconversationwithin the hook, consider passing onlyconversationto reduce redundancy.components/ConversationContextMenu.tsx (3)
41-44: Consider the impact of using context instead of props on component reusabilityBy fetching
isVisible,conversationTopic, andcontextMenuItemsdirectly from context hooks withinConversationContextMenuComponent, the component becomes tightly coupled to the context implementation. This tight coupling may reduce reusability and make unit testing more challenging, as the component now depends on the presence of specific context providers.Consider passing these values as props to maintain loose coupling. This approach enhances the component's reusability in different contexts and makes it easier to mock data during testing.
92-97: AddresetConversationListContextMenuStoreto the dependency arrayIn the
closeMenufunction, you're usingresetConversationListContextMenuStoreinside auseCallbackhook. To ensure that the callback updates correctly ifresetConversationListContextMenuStorechanges, it's best practice to include it in the dependency array.Consider modifying the dependency array as follows:
}, [height, translateY]); +}, [height, translateY, resetConversationListContextMenuStore]);
143-145: Avoid unnecessary type assertions withas ConversationTopicUsing a type assertion like
conversationTopic as ConversationTopiccan mask potential type mismatches and may lead to runtime errors ifconversationTopicisundefinedor of an unexpected type. Instead, consider ensuring thatconversationTopicis correctly typed when it's obtained from the context.For example, you can adjust the context hook to provide a default value or handle the case when
conversationTopicmight beundefined:const conversationTopic = useConversationListContextMenuConversationTopic()!;Or add conditional rendering to handle
undefinedvalues:{conversationTopic && ( <ConversationReadOnly topic={conversationTopic} /> )}This approach enhances type safety and reduces the likelihood of runtime errors.
components/V3GroupConversationListItem.tsx (1)
205-213: Update theidof the context menu item to match the actionIn the context menu items, the
idfor marking as read/unread is statically set to"markAsUnread", even though the title and action change based on theisUnreadstate. For clarity and consistency, update theidto reflect the actual action.Apply this change:
{ title: isUnread ? translate("mark_as_read") : translate("mark_as_unread"), action: () => { toggleReadStatus(); closeContextMenu(); }, - id: "markAsUnread", + id: isUnread ? "markAsRead" : "markAsUnread", },This modification ensures that the
idaccurately represents the action being performed.components/ConversationListItem/ConversationListItemDumb.tsx (3)
237-246: Remove unuseditemRectandcontainerRefThe shared value
itemRectand thecontainerRefare not used elsewhere in the code. TheonLayoutViewfunction updatesitemRect, but since it's unused, this code can be safely removed to simplify the component.Apply this diff to remove the unused code:
- const itemRect = useSharedValue({ x: 0, y: 0, width: 0, height: 0 }); - const containerRef = useAnimatedRef<View>(); - - const onLayoutView = useCallback( - (event: LayoutChangeEvent) => { - const { x, y, width, height } = event.nativeEvent.layout; - itemRect.value = { x, y, width, height }; - }, - [itemRect] - ); ... - <Animated.View ref={containerRef} onLayout={onLayoutView}> + <View>Also applies to: 331-331
386-386: Disable left swipe actions properlySetting
leftThreshold={10000}to prevent left swipe actions is a workaround that may not be reliable. If left swipe actions are not intended, consider removing therenderLeftActionsprop to disable them completely.Apply this diff to remove left swipe actions:
<Swipeable - renderLeftActions={renderLeftActions} - leftThreshold={10000} // Never trigger opening overshootFriction={4} ref={swipeableRef} onSwipeableWillClose={onSwipeableWillClose} onSwipeableClose={onSwipeableClose} hitSlop={{ left: -6 }} >Also, remove the
renderLeftActionsfunction and any related props likeonLeftSwipeandonWillLeftSwipeif they are no longer needed.
39-39: MakeleftActionIconprop optionalSince the left swipe actions are disabled, the
leftActionIconprop is no longer used and can be made optional in theConversationListItemDumbPropsto reflect its usage.Apply this diff to update the prop type:
- leftActionIcon: IIconName; + leftActionIcon?: IIconName;Also applies to: 216-216
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
components/Conversation/V3Conversation.tsx(1 hunks)components/ConversationContextMenu.tsx(5 hunks)components/ConversationListItem/ConversationListItemDumb.tsx(1 hunks)components/PinnedConversations/PinnedV3DMConversation.tsx(1 hunks)components/PinnedConversations/PinnedV3GroupConversation.tsx(1 hunks)components/V3DMListItem.tsx(1 hunks)components/V3GroupConversationListItem.tsx(1 hunks)
🔇 Additional comments (6)
components/PinnedConversations/PinnedV3GroupConversation.tsx (3)
1-22: LGTM! Well-organized imports and clear type definition.
The imports are logically grouped and the props interface is well-defined.
61-101: LGTM! Well-implemented context menu with proper memoization.
The context menu implementation follows React best practices with appropriate use of useMemo and useCallback. Dependencies are correctly specified and actions are well-organized.
103-105: Verify the hardcoded showUnread flag.
The showUnread flag is hardcoded to false despite having an isUnread state. This might be preventing unread status indicators from appearing.
components/PinnedConversations/PinnedV3DMConversation.tsx (3)
1-23: LGTM! Well-organized imports and clear type definition.
The imports are logically grouped and the props interface is well-defined with proper typing.
70-100: LGTM! Well-implemented context menu with proper memoization.
The context menu items are well-structured with unique IDs, clear actions, and proper translation usage. The memoization dependencies are correctly specified.
102-112: Verify the hardcoded showUnread value.
The showUnread flag is hardcoded to false while there's an isUnread state being calculated. This seems inconsistent with the component's purpose.
✅ Verification successful
Hardcoded showUnread is intentional, no changes needed
The hardcoded showUnread = false is correct. While there is an isUnread state being calculated, it's used for different purposes - specifically for context menu actions (marking conversations as read/unread) and not for visual indication in pinned conversations. This is evident from the codebase where:
- The
isUnreadstate is used in the context menu to toggle read status - Pinned conversations consistently set
showUnreadto false, as seen in bothPinnedV3DMConversationandPinnedV3GroupConversation - The
showUnreadOnConversationutility is used elsewhere in the application for non-pinned conversations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other usages of showUnread prop in PinnedConversation
rg -A 5 "showUnread.*?=.*?(true|false)" --type typescript
Length of output: 94
Script:
#!/bin/bash
# Search for showUnread usage in the codebase
rg "showUnread" --type ts --type tsx -A 5
# Also search for the component definition to understand its props
ast-grep --pattern 'interface $_ {
$$$
showUnread$_
$$$
}'
Length of output: 136
Script:
#!/bin/bash
# Search for showUnread usage without file type restrictions
rg "showUnread" -A 5
# Look for the component's interface/type definition
rg "PinnedV3DMConversation" -A 10
Length of output: 6815
Script:
#!/bin/bash
# Search for isUnread usage in the component and its utility function
rg "isUnread" -A 5 components/PinnedConversations/PinnedV3DMConversation.tsx
# Look for the showUnreadOnConversation utility implementation
cat utils/conversation/showUnreadOnConversation.ts
Length of output: 1706
| export const PinnedV3GroupConversation = ({ | ||
| group, | ||
| }: PinnedV3GroupConversationProps) => { | ||
| const currentAccount = useCurrentAccount()!; |
There was a problem hiding this comment.
Replace non-null assertion with proper type guard.
The non-null assertion (!) on currentAccount could lead to runtime errors if the account is not available.
Consider adding a type guard:
-const currentAccount = useCurrentAccount()!;
+const currentAccount = useCurrentAccount();
+if (!currentAccount) {
+ return null; // or handle the no-account case appropriately
+}📝 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 currentAccount = useCurrentAccount()!; | |
| const currentAccount = useCurrentAccount(); | |
| if (!currentAccount) { | |
| return null; // or handle the no-account case appropriately | |
| } |
| export const PinnedV3DMConversation = ({ | ||
| conversation, | ||
| }: PinnedV3DMConversationProps) => { | ||
| const currentAccount = useCurrentAccount()!; | ||
|
|
||
| const topic = conversation.topic; | ||
|
|
||
| const { data: peerInboxId } = useDmPeerInboxOnConversationList( | ||
| currentAccount, | ||
| conversation | ||
| ); | ||
|
|
||
| const preferredName = usePreferredInboxName(peerInboxId); | ||
|
|
||
| const preferredAvatar = usePreferredInboxAvatar(peerInboxId); | ||
|
|
||
| const { setPinnedConversations } = useChatStore( | ||
| useSelect(["setPinnedConversations"]) | ||
| ); | ||
|
|
||
| const timestamp = conversation?.lastMessage?.sentNs ?? 0; | ||
|
|
||
| const isUnread = useConversationIsUnread({ | ||
| topic, | ||
| lastMessage: conversation.lastMessage, | ||
| conversation: conversation, | ||
| timestamp, | ||
| }); | ||
|
|
||
| const toggleReadStatus = useToggleReadStatus({ | ||
| topic, | ||
| isUnread, | ||
| currentAccount, | ||
| }); | ||
|
|
||
| const handleDelete = useHandleDeleteDm({ | ||
| topic, | ||
| preferredName, | ||
| conversation, | ||
| }); | ||
|
|
There was a problem hiding this comment.
Consider safer alternatives to non-null assertion.
The non-null assertion on currentAccount could lead to runtime errors if the account is not available.
-const currentAccount = useCurrentAccount()!;
+const currentAccount = useCurrentAccount();
+if (!currentAccount) {
+ return null; // or handle the no-account case appropriately
+}Simplify useConversationIsUnread props.
The hook receives redundant information since lastMessage is already part of the conversation object.
const isUnread = useConversationIsUnread({
topic,
- lastMessage: conversation.lastMessage,
conversation: conversation,
timestamp,
});📝 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 PinnedV3DMConversation = ({ | |
| conversation, | |
| }: PinnedV3DMConversationProps) => { | |
| const currentAccount = useCurrentAccount()!; | |
| const topic = conversation.topic; | |
| const { data: peerInboxId } = useDmPeerInboxOnConversationList( | |
| currentAccount, | |
| conversation | |
| ); | |
| const preferredName = usePreferredInboxName(peerInboxId); | |
| const preferredAvatar = usePreferredInboxAvatar(peerInboxId); | |
| const { setPinnedConversations } = useChatStore( | |
| useSelect(["setPinnedConversations"]) | |
| ); | |
| const timestamp = conversation?.lastMessage?.sentNs ?? 0; | |
| const isUnread = useConversationIsUnread({ | |
| topic, | |
| lastMessage: conversation.lastMessage, | |
| conversation: conversation, | |
| timestamp, | |
| }); | |
| const toggleReadStatus = useToggleReadStatus({ | |
| topic, | |
| isUnread, | |
| currentAccount, | |
| }); | |
| const handleDelete = useHandleDeleteDm({ | |
| topic, | |
| preferredName, | |
| conversation, | |
| }); | |
| export const PinnedV3DMConversation = ({ | |
| conversation, | |
| }: PinnedV3DMConversationProps) => { | |
| const currentAccount = useCurrentAccount(); | |
| if (!currentAccount) { | |
| return null; // or handle the no-account case appropriately | |
| } | |
| const topic = conversation.topic; | |
| const { data: peerInboxId } = useDmPeerInboxOnConversationList( | |
| currentAccount, | |
| conversation | |
| ); | |
| const preferredName = usePreferredInboxName(peerInboxId); | |
| const preferredAvatar = usePreferredInboxAvatar(peerInboxId); | |
| const { setPinnedConversations } = useChatStore( | |
| useSelect(["setPinnedConversations"]) | |
| ); | |
| const timestamp = conversation?.lastMessage?.sentNs ?? 0; | |
| const isUnread = useConversationIsUnread({ | |
| topic, | |
| conversation: conversation, | |
| timestamp, | |
| }); | |
| const toggleReadStatus = useToggleReadStatus({ | |
| topic, | |
| isUnread, | |
| currentAccount, | |
| }); | |
| const handleDelete = useHandleDeleteDm({ | |
| topic, | |
| preferredName, | |
| conversation, | |
| }); |
| navigation.setOptions({ | ||
| headerTitle: () => <NewConversationTitle peerAddress={peerAddress!} />, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove non-null assertions in header hooks
The header hooks use non-null assertions (!) for peerAddress and topic. Consider adding proper type guards or error handling.
- headerTitle: () => <NewConversationTitle peerAddress={peerAddress!} />,
+ headerTitle: () => peerAddress ? <NewConversationTitle peerAddress={peerAddress} /> : null,Also applies to: 259-260, 271-272
| const messages = getCurrentConversationMessages()!; | ||
|
|
||
| return ( |
There was a problem hiding this comment.
Add error handling for getCurrentConversationMessages
The code assumes getCurrentConversationMessages() will return a non-null value but doesn't handle the error case despite the non-null assertion.
- const messages = getCurrentConversationMessages()!;
+ const messages = getCurrentConversationMessages();
+ if (!messages) {
+ return null;
+ }📝 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 messages = getCurrentConversationMessages()!; | |
| return ( | |
| const messages = getCurrentConversationMessages(); | |
| if (!messages) { | |
| return null; | |
| } | |
| return ( |
| if (conversationNotFound) { | ||
| // TODO: Add DM placeholder | ||
| return null; | ||
| } | ||
|
|
||
| if (messages?.ids.length === 0 && !messagesLoading) { | ||
| // TODO: Add DM placeholder | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Implement DM placeholders and loading states
The DM content has two TODO comments for placeholders but returns null in both cases. This could lead to a poor user experience. Consider:
- Implementing proper loading states
- Using appropriate placeholder components
if (conversationNotFound) {
- // TODO: Add DM placeholder
- return null;
+ return (
+ <Center>
+ <Text>{translate("dm_not_found")}</Text>
+ </Center>
+ );
}
if (messages?.ids.length === 0 && !messagesLoading) {
- // TODO: Add DM placeholder
- return null;
+ return (
+ <Center>
+ <Text>{translate("dm_empty")}</Text>
+ </Center>
+ );
}Committable suggestion skipped: line range outside the PR's diff.
| async () => { | ||
| await group.updateConsent("allowed"); | ||
| }, | ||
| async () => { | ||
| await group.updateConsent("allowed"); | ||
| await consentToInboxIdsOnProtocolByAccount({ | ||
| account: currentAccount, | ||
| inboxIds: [group.addedByInboxId], | ||
| consent: "allow", | ||
| }); | ||
| setInboxIdPeerStatus({ | ||
| [group.addedByInboxId]: "allowed", | ||
| }); | ||
| }, | ||
| ]; | ||
| showActionSheetWithOptions( | ||
| { | ||
| options, | ||
| cancelButtonIndex: options.length - 1, | ||
| title, | ||
| ...actionSheetColors(colorScheme), | ||
| }, | ||
| async (selectedIndex?: number) => { | ||
| if (selectedIndex !== undefined && selectedIndex < actions.length) { | ||
| actions[selectedIndex](); | ||
| } | ||
| } | ||
| ); | ||
| }, [colorScheme, currentAccount, group, setInboxIdPeerStatus]); |
There was a problem hiding this comment.
Properly await asynchronous actions in handleRestore
In the handleRestore function, the actions array consists of asynchronous functions. When invoking actions[selectedIndex](), you should await the promises to ensure proper execution flow and error handling.
Modify the callback to await the asynchronous actions:
showActionSheetWithOptions(
{
options,
cancelButtonIndex: options.length - 1,
title,
...actionSheetColors(colorScheme),
},
async (selectedIndex?: number) => {
if (selectedIndex !== undefined && selectedIndex < actions.length) {
- actions[selectedIndex]();
+ await actions[selectedIndex]().catch((error) => {
+ // Handle error appropriately
+ console.error("Action failed:", error);
+ });
}
}
);This ensures that any asynchronous operations complete before the function exits and errors are properly handled.
📝 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.
| async () => { | |
| await group.updateConsent("allowed"); | |
| }, | |
| async () => { | |
| await group.updateConsent("allowed"); | |
| await consentToInboxIdsOnProtocolByAccount({ | |
| account: currentAccount, | |
| inboxIds: [group.addedByInboxId], | |
| consent: "allow", | |
| }); | |
| setInboxIdPeerStatus({ | |
| [group.addedByInboxId]: "allowed", | |
| }); | |
| }, | |
| ]; | |
| showActionSheetWithOptions( | |
| { | |
| options, | |
| cancelButtonIndex: options.length - 1, | |
| title, | |
| ...actionSheetColors(colorScheme), | |
| }, | |
| async (selectedIndex?: number) => { | |
| if (selectedIndex !== undefined && selectedIndex < actions.length) { | |
| actions[selectedIndex](); | |
| } | |
| } | |
| ); | |
| }, [colorScheme, currentAccount, group, setInboxIdPeerStatus]); | |
| async () => { | |
| await group.updateConsent("allowed"); | |
| }, | |
| async () => { | |
| await group.updateConsent("allowed"); | |
| await consentToInboxIdsOnProtocolByAccount({ | |
| account: currentAccount, | |
| inboxIds: [group.addedByInboxId], | |
| consent: "allow", | |
| }); | |
| setInboxIdPeerStatus({ | |
| [group.addedByInboxId]: "allowed", | |
| }); | |
| }, | |
| ]; | |
| showActionSheetWithOptions( | |
| { | |
| options, | |
| cancelButtonIndex: options.length - 1, | |
| title, | |
| ...actionSheetColors(colorScheme), | |
| }, | |
| async (selectedIndex?: number) => { | |
| if (selectedIndex !== undefined && selectedIndex < actions.length) { | |
| await actions[selectedIndex]().catch((error) => { | |
| // Handle error appropriately | |
| console.error("Action failed:", error); | |
| }); | |
| } | |
| } | |
| ); | |
| }, [colorScheme, currentAccount, group, setInboxIdPeerStatus]); |
🛠️ Refactor suggestion
Add error handling to asynchronous actions in handleRestore
Similarly, the asynchronous actions in the actions array of handleRestore lack error handling. This could lead to unhandled promise rejections if any operation fails.
Include error handling using try-catch blocks:
async () => {
+ try {
await group.updateConsent("allowed");
+ } catch (error) {
+ // Handle error appropriately
+ console.error("Failed to restore:", error);
+ }
},
async () => {
+ try {
await group.updateConsent("allowed");
await consentToInboxIdsOnProtocolByAccount({
account: currentAccount,
inboxIds: [group.addedByInboxId],
consent: "allow",
});
setInboxIdPeerStatus({
[group.addedByInboxId]: "allowed",
});
+ } catch (error) {
+ // Handle error appropriately
+ console.error("Failed to restore and unblock inviter:", error);
+ }
},This ensures robustness by handling any errors that may occur during the asynchronous operations.
Committable suggestion skipped: line range outside the PR's diff.
| const actions = [ | ||
| () => { | ||
| saveTopicsData(currentAccount, { | ||
| [topic]: { | ||
| status: "deleted", | ||
| timestamp: new Date().getTime(), | ||
| }, | ||
| }), | ||
| setTopicsData({ | ||
| [topic]: { | ||
| status: "deleted", | ||
| timestamp: new Date().getTime(), | ||
| }, | ||
| }); | ||
| }, | ||
| async () => { | ||
| saveTopicsData(currentAccount, { | ||
| [topic]: { status: "deleted" }, | ||
| }); | ||
| setTopicsData({ | ||
| [topic]: { | ||
| status: "deleted", | ||
| timestamp: new Date().getTime(), | ||
| }, | ||
| }); | ||
| await group.updateConsent("denied"); | ||
| await consentToInboxIdsOnProtocolByAccount({ | ||
| account: currentAccount, | ||
| inboxIds: [group.addedByInboxId], | ||
| consent: "deny", | ||
| }); | ||
| setInboxIdPeerStatus({ | ||
| [group.addedByInboxId]: "denied", | ||
| }); | ||
| }, | ||
| ]; | ||
| // TODO: Implement | ||
| showActionSheetWithOptions( | ||
| { | ||
| options, | ||
| cancelButtonIndex: options.length - 1, | ||
| destructiveButtonIndex: [0, 1], | ||
| title, | ||
| ...actionSheetColors(colorScheme), | ||
| }, | ||
| async (selectedIndex?: number) => { | ||
| if (selectedIndex !== undefined && selectedIndex < actions.length) { | ||
| actions[selectedIndex](); | ||
| } | ||
| } | ||
| ); |
There was a problem hiding this comment.
Ensure proper handling of asynchronous actions in handleDelete
In the handleDelete function, the actions array contains both synchronous and asynchronous functions. When invoking actions[selectedIndex](), the asynchronous functions are not awaited, which may lead to unhandled promise rejections or unexpected behavior.
Consider modifying the callback to properly handle asynchronous actions:
showActionSheetWithOptions(
{
options,
cancelButtonIndex: options.length - 1,
destructiveButtonIndex: [0, 1],
title,
...actionSheetColors(colorScheme),
},
- async (selectedIndex?: number) => {
+ (selectedIndex?: number) => {
if (selectedIndex !== undefined && selectedIndex < actions.length) {
- actions[selectedIndex]();
+ const result = actions[selectedIndex]();
+ if (result instanceof Promise) {
+ result.catch((error) => {
+ // Handle error appropriately
+ console.error("Action failed:", error);
+ });
+ }
}
}
);Alternatively, ensure all actions are asynchronous and await them:
showActionSheetWithOptions(
{
options,
cancelButtonIndex: options.length - 1,
destructiveButtonIndex: [0, 1],
title,
...actionSheetColors(colorScheme),
},
async (selectedIndex?: number) => {
if (selectedIndex !== undefined && selectedIndex < actions.length) {
- actions[selectedIndex]();
+ await actions[selectedIndex]().catch((error) => {
+ // Handle error appropriately
+ console.error("Action failed:", error);
+ });
}
}
);Ensure that all functions in the actions array are asynchronous and handle any errors appropriately.
📝 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 actions = [ | |
| () => { | |
| saveTopicsData(currentAccount, { | |
| [topic]: { | |
| status: "deleted", | |
| timestamp: new Date().getTime(), | |
| }, | |
| }), | |
| setTopicsData({ | |
| [topic]: { | |
| status: "deleted", | |
| timestamp: new Date().getTime(), | |
| }, | |
| }); | |
| }, | |
| async () => { | |
| saveTopicsData(currentAccount, { | |
| [topic]: { status: "deleted" }, | |
| }); | |
| setTopicsData({ | |
| [topic]: { | |
| status: "deleted", | |
| timestamp: new Date().getTime(), | |
| }, | |
| }); | |
| await group.updateConsent("denied"); | |
| await consentToInboxIdsOnProtocolByAccount({ | |
| account: currentAccount, | |
| inboxIds: [group.addedByInboxId], | |
| consent: "deny", | |
| }); | |
| setInboxIdPeerStatus({ | |
| [group.addedByInboxId]: "denied", | |
| }); | |
| }, | |
| ]; | |
| // TODO: Implement | |
| showActionSheetWithOptions( | |
| { | |
| options, | |
| cancelButtonIndex: options.length - 1, | |
| destructiveButtonIndex: [0, 1], | |
| title, | |
| ...actionSheetColors(colorScheme), | |
| }, | |
| async (selectedIndex?: number) => { | |
| if (selectedIndex !== undefined && selectedIndex < actions.length) { | |
| actions[selectedIndex](); | |
| } | |
| } | |
| ); | |
| const actions = [ | |
| () => { | |
| saveTopicsData(currentAccount, { | |
| [topic]: { | |
| status: "deleted", | |
| timestamp: new Date().getTime(), | |
| }, | |
| }), | |
| setTopicsData({ | |
| [topic]: { | |
| status: "deleted", | |
| timestamp: new Date().getTime(), | |
| }, | |
| }); | |
| }, | |
| async () => { | |
| saveTopicsData(currentAccount, { | |
| [topic]: { status: "deleted" }, | |
| }); | |
| setTopicsData({ | |
| [topic]: { | |
| status: "deleted", | |
| timestamp: new Date().getTime(), | |
| }, | |
| }); | |
| await group.updateConsent("denied"); | |
| await consentToInboxIdsOnProtocolByAccount({ | |
| account: currentAccount, | |
| inboxIds: [group.addedByInboxId], | |
| consent: "deny", | |
| }); | |
| setInboxIdPeerStatus({ | |
| [group.addedByInboxId]: "denied", | |
| }); | |
| }, | |
| ]; | |
| // TODO: Implement | |
| showActionSheetWithOptions( | |
| { | |
| options, | |
| cancelButtonIndex: options.length - 1, | |
| destructiveButtonIndex: [0, 1], | |
| title, | |
| ...actionSheetColors(colorScheme), | |
| }, | |
| async (selectedIndex?: number) => { | |
| if (selectedIndex !== undefined && selectedIndex < actions.length) { | |
| await actions[selectedIndex]().catch((error) => { | |
| // Handle error appropriately | |
| console.error("Action failed:", error); | |
| }); | |
| } | |
| } | |
| ); |
| () => { | ||
| saveTopicsData(currentAccount, { | ||
| [topic]: { | ||
| status: "deleted", | ||
| timestamp: new Date().getTime(), | ||
| }, | ||
| }), | ||
| setTopicsData({ | ||
| [topic]: { | ||
| status: "deleted", | ||
| timestamp: new Date().getTime(), | ||
| }, | ||
| }); | ||
| }, | ||
| async () => { | ||
| saveTopicsData(currentAccount, { | ||
| [topic]: { status: "deleted" }, | ||
| }); | ||
| setTopicsData({ | ||
| [topic]: { | ||
| status: "deleted", | ||
| timestamp: new Date().getTime(), | ||
| }, | ||
| }); | ||
| await group.updateConsent("denied"); | ||
| await consentToInboxIdsOnProtocolByAccount({ | ||
| account: currentAccount, | ||
| inboxIds: [group.addedByInboxId], | ||
| consent: "deny", | ||
| }); | ||
| setInboxIdPeerStatus({ | ||
| [group.addedByInboxId]: "denied", | ||
| }); | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to asynchronous actions in handleDelete
The asynchronous functions within the actions array lack error handling. If any promise rejects, it could lead to unhandled promise rejections.
Wrap the asynchronous code in try-catch blocks to handle potential errors gracefully:
async () => {
+ try {
saveTopicsData(currentAccount, {
[topic]: { status: "deleted" },
});
setTopicsData({
[topic]: {
status: "deleted",
timestamp: new Date().getTime(),
},
});
await group.updateConsent("denied");
await consentToInboxIdsOnProtocolByAccount({
account: currentAccount,
inboxIds: [group.addedByInboxId],
consent: "deny",
});
setInboxIdPeerStatus({
[group.addedByInboxId]: "denied",
});
+ } catch (error) {
+ // Handle error appropriately
+ console.error("Failed to delete and block:", error);
+ }
},Ensure that each asynchronous action within the actions array includes error handling.
Committable suggestion skipped: line range outside the PR's diff.
|
|
||
| const renderLeftActions = useCallback(() => { | ||
| return ( | ||
| <RectButton style={styles.leftAction}> |
There was a problem hiding this comment.
Attach onLeftActionPress to the left action button
The left action button in renderLeftActions lacks an onPress handler, so user interactions won't trigger any action. Since onLeftActionPress is provided as a prop, it should be connected to the button to handle left swipe actions.
Apply this diff to attach the onPress handler:
<RectButton
style={styles.leftAction}
+ onPress={onLeftActionPress}
>📝 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.
| <RectButton style={styles.leftAction}> | |
| <RectButton style={styles.leftAction} onPress={onLeftActionPress}> |
| if (rightIsDestructive) { | ||
| return ( | ||
| <RectButton style={styles.rightAction} onPress={onRightActionPress}> | ||
| <Icon | ||
| icon="checkmark" | ||
| color={themedInversePrimaryColor} | ||
| size={PictoSizes.swipableItem} | ||
| /> | ||
| </RectButton> | ||
| ); | ||
| } else { | ||
| return ( | ||
| <RectButton | ||
| style={styles.rightActionRed} | ||
| onPress={onRightActionPress} | ||
| > | ||
| <Icon icon="trash" color="white" size={PictoSizes.swipableItem} /> | ||
| </RectButton> | ||
| ); | ||
| } | ||
| }, [ | ||
| rightIsDestructive, | ||
| styles.rightAction, | ||
| styles.rightActionRed, | ||
| onRightActionPress, | ||
| themedInversePrimaryColor, |
There was a problem hiding this comment.
Correct the rightIsDestructive logic to display appropriate swipe actions
The logic in renderRightActions appears inverted. When rightIsDestructive is true, it should display a destructive action (e.g., a trash icon with a red background). Currently, it shows a checkmark icon with a non-destructive style, which might confuse users.
Apply this diff to fix the logic:
const renderRightActions = useCallback(() => {
- if (rightIsDestructive) {
+ if (!rightIsDestructive) {
return (
- <RectButton style={styles.rightAction} onPress={onRightActionPress}>
+ <RectButton style={styles.rightActionRed} onPress={onRightActionPress}>
<Icon
- icon="checkmark"
+ icon="trash"
color={themedInversePrimaryColor}
size={PictoSizes.swipableItem}
/>
</RectButton>
);
- } else {
+ } else {
return (
- <RectButton style={styles.rightActionRed} onPress={onRightActionPress}>
+ <RectButton style={styles.rightAction} onPress={onRightActionPress}>
<Icon
- icon="trash"
+ icon="checkmark"
color="white"
size={PictoSizes.swipableItem}
/>
</RectButton>
);
}
}, [
rightIsDestructive,
styles.rightAction,
styles.rightActionRed,
onRightActionPress,
themedInversePrimaryColor,
]);Committable suggestion skipped: line range outside the PR's diff.
Added notification subscribing for V3 only
* feat: V3 Search Added search for V3 Only Added tests Updated Snapshots * remove checks Removed async Removed await * remove inboxId check
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor
Documentation