feat: V3 Search #1261
feat: V3 Search #1261
Conversation
WalkthroughThe pull request introduces new unit tests for the Changes
Assessment against linked issues
Possibly related PRs
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (8)
features/conversations/utils/search.ts (2)
73-79: Add input validation to addressMatchesSearchQueryWhile the function is correct, it should validate inputs to prevent potential errors.
Consider this improvement:
const addressMatchesSearchQuery = ( account: string, searchQuery: string, address: string ) => { + if (!address || !searchQuery) return false; return address.toLowerCase().includes(searchQuery.toLowerCase()); };
1-79: Consider performance optimizationsThe search implementation makes multiple async calls and string operations per item, which could impact performance with large datasets. Consider these improvements:
- Cache profile data to reduce API calls
- Implement debouncing for search operations
- Consider using a more efficient search algorithm for large datasets (e.g., Trie data structure for prefix matching)
Would you like assistance in implementing any of these optimizations?
queries/useInboxProfileSocialsQuery.ts (1)
Line range hint
36-56: Consider improving type safety by handling undefined values explicitlyThe config uses non-null assertions (
!) with potentially undefinedinboxId. While theenabledcondition prevents execution wheninboxIdis undefined, it's better to handle these cases explicitly for improved type safety.Consider this safer approach:
const inboxProfileSocialsQueryConfig = ( account: string, inboxId: InboxId | undefined ) => ({ - queryKey: profileSocialsQueryKey(account, inboxId!), - queryFn: () => fetchInboxProfileSocials(inboxId!), + queryKey: profileSocialsQueryKey(account, inboxId ?? ''), + queryFn: () => { + if (!inboxId) throw new Error('inboxId is required'); + return fetchInboxProfileSocials(inboxId); + }, enabled: !!account && !!inboxId, // ... rest of the config });features/conversations/utils/__tests__/search.test.ts (2)
14-16: Consider using a separate mock implementation fileThe inline mock implementation for
getPreferredInboxNamemight be brittle as it's tightly coupled to the implementation details. Consider moving mock implementations to a separate mock file for better maintainability.- jest.mock("@/utils/profile", () => ({ - getPreferredInboxName: jest.fn((profiles) => profiles?.userNames?.[0] || ""), - })); + jest.mock("@/utils/profile", () => require("../__mocks__/profile"));
30-70: Add test cases for edge scenariosThe current test suite misses some important edge cases. Consider adding tests for:
- Empty search query
- Case sensitivity
- Special characters in search query
- Unicode/emoji in names
utils/xmtpRN/sync.ts (3)
Line range hint
120-134: LGTM! Consider enhancing error context.The error handling and retry strategy for conversation streaming are well-implemented. The non-fatal error approach is appropriate for stream failures.
Consider adding the error type to the logging context for better debugging:
}).catch((e) => { logger.error(e, { - context: `Failed to stream conversations for ${account} no longer retrying`, + context: `Failed to stream conversations for ${account} no longer retrying. Error type: ${e.constructor.name}`, }); });
Line range hint
134-146: Consider refactoring retry configuration and improving documentation.While the implementation is correct, there are opportunities for improvement:
- Extract the retry configuration to reduce duplication:
const RETRY_CONFIG = { retries: 5, delay: 1000, factor: 2, maxDelay: 30000, }; await retryWithBackoff({ ...RETRY_CONFIG, fn: () => streamAllMessages(account), context: `streaming all messages for ${account}`, });
- Enhance the comment about group message streaming:
- // Streaming all dm messages (not groups because buggy) + // Streaming all DM messages only. Group message streaming is temporarily disabled + // due to known synchronization issues (TODO: Link to issue tracker)
Line range hint
120-146: Consider adding rate limiting for message synchronization.While the retry mechanism and error handling are robust, the message synchronization process might benefit from rate limiting to prevent overwhelming the XMTP network or client resources, especially when dealing with large message histories.
Consider implementing rate limiting using a token bucket algorithm or similar approach. This could help prevent potential issues with:
- Network bandwidth consumption
- Client-side memory usage
- Server load during mass synchronization
Would you like me to provide an example implementation of rate limiting for the sync process?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
queries/__snapshots__/QueryKeys.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (5)
features/conversations/utils/__tests__/search.test.ts(1 hunks)features/conversations/utils/search.ts(1 hunks)queries/useInboxProfileSocialsQuery.ts(5 hunks)screens/ConversationList.tsx(2 hunks)utils/xmtpRN/sync.ts(2 hunks)
🔇 Additional comments (6)
features/conversations/utils/search.ts (1)
1-8: LGTM: Imports and type definitions are well-structured
The imports and type definitions are appropriate for the search functionality implementation.
queries/useInboxProfileSocialsQuery.ts (2)
Line range hint 57-93: LGTM! Clean implementation of React Query hooks and utilities
The query hooks and utility functions are well-structured, following React Query best practices. The batch processing implementation for multiple queries is particularly well done.
95-104: 🛠️ Refactor suggestion
Verify return type consistency and add parameter validation
The return type ProfileSocials[] | null might not match the actual data structure from queryClient.getQueryData(). Based on the fetchInboxProfileSocials implementation and other functions in the file, it seems the data should be ProfileSocials | null (single object, not array).
Let's verify the data structure usage:
Consider this safer implementation:
export const getInboxProfileSocialsQueryData = (
account: string,
inboxId: InboxId
-): ProfileSocials[] | null => {
+): ProfileSocials | null => {
+ if (!account || !inboxId) return null;
return (
queryClient.getQueryData(
inboxProfileSocialsQueryConfig(account, inboxId).queryKey
) ?? null
);
};features/conversations/utils/__tests__/search.test.ts (2)
21-29: LGTM! Well-structured test setup
The test suite setup follows best practices with proper test isolation through beforeEach and clear variable definitions.
1-1: Verify test coverage for all exported functions
Let's verify that all exported functions from the search module are properly tested.
✅ Verification successful
Test coverage is comprehensive for all exported functions
The test file thoroughly covers both exported functions dmMatchesSearchQuery and groupMatchesSearchQuery. Each function has multiple test cases covering different scenarios:
For dmMatchesSearchQuery:
- Matching by inboxId
- Matching by member's address
- No matches (negative case)
For groupMatchesSearchQuery:
- Matching by group name
- Matching by member's inboxId
- Matching by member's address
- No matches (negative case)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find all exported functions from search.ts and compare with test coverage
ast-grep --pattern 'export function $name($$$)' ../search.ts
Length of output: 122
Script:
#!/bin/bash
# Find the search.ts file first
fd "search.ts" --type f
# Then find all exported functions from search.ts
ast-grep --pattern 'export function $name($$$)'
# Also check for other export patterns
ast-grep --pattern 'export const $name = $$$'
Length of output: 234
Script:
#!/bin/bash
# Let's examine the content of the specific search.ts file and its test file
cat features/conversations/utils/search.ts
cat features/conversations/utils/__tests__/search.test.ts
Length of output: 6705
screens/ConversationList.tsx (1)
45-49: Added necessary imports for search functionality
The imports for ConversationVersion, dmMatchesSearchQuery, and groupMatchesSearchQuery are correctly added to support the new search feature.
| const mockDm = { | ||
| peerInboxId: jest.fn().mockResolvedValue(mockInboxId), | ||
| members: jest.fn().mockResolvedValue([]), | ||
| } as unknown as DmWithCodecsType; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety in test mocks
Instead of using type casting with as unknown as DmWithCodecsType, consider creating a proper mock factory function that returns a correctly typed mock object. This would catch type issues earlier and make the tests more maintainable.
function createMockDm(overrides?: Partial<DmWithCodecsType>): DmWithCodecsType {
return {
peerInboxId: jest.fn().mockResolvedValue("defaultId"),
members: jest.fn().mockResolvedValue([]),
...overrides
};
}Also applies to: 46-49
| describe("groupMatchesSearchQuery", () => { | ||
| it("returns true if group name matches the search query", async () => { | ||
| const mockGroup = { | ||
| name: "testGroupName", | ||
| members: jest.fn().mockResolvedValue([]), | ||
| } as unknown as GroupWithCodecsType; | ||
|
|
||
| const result = await groupMatchesSearchQuery( | ||
| account, | ||
| searchQuery, | ||
| mockGroup | ||
| ); | ||
| expect(result).toBe(true); | ||
| }); | ||
|
|
||
| it("returns true if a member's inboxId matches the search query", async () => { | ||
| const mockGroup = { | ||
| name: "nonMatchingName", | ||
| members: jest | ||
| .fn() | ||
| .mockResolvedValue([ | ||
| { inboxId: mockInboxId, addresses: ["nonMatchingAddress"] }, | ||
| ]), | ||
| } as unknown as GroupWithCodecsType; | ||
|
|
||
| mockGetInboxProfileSocialsQueryData.mockReturnValue([ | ||
| { userNames: ["testUser"] }, | ||
| ]); | ||
|
|
||
| const result = await groupMatchesSearchQuery( | ||
| account, | ||
| "mockInbox", | ||
| mockGroup | ||
| ); | ||
| expect(result).toBe(true); | ||
| }); | ||
|
|
||
| it("returns true if a member's address matches the search query", async () => { | ||
| const mockGroup = { | ||
| name: "nonMatchingName", | ||
| members: jest | ||
| .fn() | ||
| .mockResolvedValue([ | ||
| { inboxId: "nonMatchingId", addresses: ["testAddress"] }, | ||
| ]), | ||
| } as unknown as GroupWithCodecsType; | ||
|
|
||
| mockGetInboxProfileSocialsQueryData.mockReturnValue(null); | ||
|
|
||
| const result = await groupMatchesSearchQuery( | ||
| account, | ||
| searchQuery, | ||
| mockGroup | ||
| ); | ||
| expect(result).toBe(true); | ||
| }); | ||
|
|
||
| it("returns false if neither group name, inboxId, nor members match the search query", async () => { | ||
| const mockGroup = { | ||
| name: "nonMatchingName", | ||
| members: jest | ||
| .fn() | ||
| .mockResolvedValue([ | ||
| { inboxId: "nonMatchingId", addresses: ["nonMatchingAddress"] }, | ||
| ]), | ||
| } as unknown as GroupWithCodecsType; | ||
|
|
||
| mockGetInboxProfileSocialsQueryData.mockReturnValue(null); | ||
|
|
||
| const result = await groupMatchesSearchQuery( | ||
| account, | ||
| searchQuery, | ||
| mockGroup | ||
| ); | ||
| expect(result).toBe(false); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor test structure to reduce duplication
The group tests follow a similar pattern to DM tests with similar mock setups. Consider:
- Creating shared test utilities for common mock setups
- Using test.each for similar test cases with different inputs
- Adding the same edge cases as suggested for DM tests
Example refactor:
const testCases = [
['group name', { name: 'testGroupName' }, 'test', true],
['member inboxId', { name: 'other', members: [{ inboxId: 'testId' }] }, 'test', true],
// ... more cases
] as const;
test.each(testCases)(
'returns %s when %s matches search query',
async (scenario, mockData, query, expected) => {
const mockGroup = createMockGroup(mockData);
const result = await groupMatchesSearchQuery(account, query, mockGroup);
expect(result).toBe(expected);
}
);
Added search for V3 Only Added tests Updated Snapshots
alexrisch
force-pushed
the
ar/v3-search
branch
from
35adfaa to
f8ecc59
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (4)
features/conversations/utils/search.ts (3)
9-13: Add input validation for searchQuery parameterConsider adding runtime validation for the searchQuery parameter to ensure it's a non-empty string and within reasonable length limits.
type DmSearchParams = { account: string; - searchQuery: string; + searchQuery: string & { __brand: 'NonEmptyString' }; dm: DmWithCodecsType; }; + +const validateSearchQuery = (query: string): query is DmSearchParams['searchQuery'] => { + return typeof query === 'string' && + query.trim().length > 0 && + query.length <= 100; // adjust max length as needed +};
27-37: Potential memory leak in members iterationThe members array is held in memory while processing each member sequentially. For large groups, this could lead to memory pressure.
Consider using an async generator to process members in chunks:
- const members = await dm.members(); - for (const member of members) { + for await (const member of dm.members()) { if ( await addressMatchesSearchQuery({ searchQuery, address: member.addresses[0], }) ) { return true; } }
114-122: Remove unnecessary async keywordThe function doesn't perform any asynchronous operations, so it doesn't need to be marked as async.
-const addressMatchesSearchQuery = async ({ +const addressMatchesSearchQuery = ({ searchQuery, address, -}: AddressSearchParams): Promise<boolean> => { +}: AddressSearchParams): boolean => { if (!address) { return false; } return address.toLowerCase().includes(searchQuery.toLowerCase()); };screens/ConversationList.tsx (1)
117-141: Extract filtering logic into a custom hookThe filtering logic is complex enough to warrant its own custom hook, which would improve maintainability and reusability.
Consider creating a new hook
useFilteredConversations:function useFilteredConversations( items: (ConversationFlatListItem | ConversationWithCodecsType)[] | undefined, searchQuery: string, currentAccount: Account | null ) { const [filteredItems, setFilteredItems] = useState<{ items: FlatListItemType[]; searchQuery: string; }>({ items: [], searchQuery: "" }); useEffect(() => { // ... existing filtering logic ... }, [searchQuery, items, currentAccount]); return filteredItems; }This would simplify the main component and make the filtering logic more testable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
queries/__snapshots__/QueryKeys.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (5)
features/conversations/utils/__tests__/search.test.ts(1 hunks)features/conversations/utils/search.ts(1 hunks)queries/useInboxProfileSocialsQuery.ts(5 hunks)screens/ConversationList.tsx(2 hunks)utils/xmtpRN/sync.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- features/conversations/utils/tests/search.test.ts
- queries/useInboxProfileSocialsQuery.ts
- utils/xmtpRN/sync.ts
🔇 Additional comments (3)
features/conversations/utils/search.ts (1)
1-122: Ensure comprehensive test coverage
While the AI summary mentions tests were added, ensure the test coverage includes:
- Edge cases for empty/invalid inputs
- Performance testing with large member lists
- Mocking of async operations
- Error scenarios for profile data fetching
screens/ConversationList.tsx (2)
45-49: LGTM: Clean import structure
The new imports are well-organized and properly scoped for the search functionality.
142-148: LGTM: Clean search query handling
The search query validation and state updates are well implemented.
| const profiles = getInboxProfileSocialsQueryData(account, inboxId); | ||
| if (!profiles) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Fix missing await for async operation
The getInboxProfileSocialsQueryData call is likely asynchronous but isn't awaited, which could lead to race conditions.
- const profiles = getInboxProfileSocialsQueryData(account, inboxId);
+ const profiles = await getInboxProfileSocialsQueryData(account, inboxId);
if (!profiles) {
return false;
}Committable suggestion skipped: line range outside the PR's diff.
| if (group.name.toLowerCase().includes(searchQuery.toLowerCase())) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize case-insensitive string comparisons
Converting strings to lowercase on every comparison is inefficient. Consider normalizing the search query once at the start.
export const groupMatchesSearchQuery = async ({
account,
searchQuery,
group,
}: GroupSearchParams): Promise<boolean> => {
if (!group) {
return false;
}
+ const normalizedQuery = searchQuery.toLowerCase();
- if (group.name.toLowerCase().includes(searchQuery.toLowerCase())) {
+ if (group.name.toLowerCase().includes(normalizedQuery)) {
return true;
}Committable suggestion skipped: line range outside the PR's diff.
| const groupMatches = await groupMatchesSearchQuery({ | ||
| account: currentAccount!, | ||
| searchQuery, | ||
| group: item, | ||
| }); |
There was a problem hiding this comment.
Improve type safety by validating currentAccount
The code uses non-null assertions (currentAccount!) which could lead to runtime errors if currentAccount is null.
Add early return or validation:
const getFilteredItems = async () => {
+ if (!currentAccount) {
+ console.warn('No current account available for search');
+ return [];
+ }
const filteredItems: FlatListItemType[] = [];
for (const item of items ?? []) {
if (item.version === ConversationVersion.GROUP) {
const groupMatches = await groupMatchesSearchQuery({
- account: currentAccount!,
+ account: currentAccount,
searchQuery,
group: item,
});
// ... rest of the code
} else if (item.version === ConversationVersion.DM) {
const dmMatches = await dmMatchesSearchQuery({
- account: currentAccount!,
+ account: currentAccount,
searchQuery,
dm: item,
});
// ... rest of the code
}
}
return filteredItems ?? [];
};Also applies to: 130-134
| const getFilteredItems = async () => { | ||
| const filteredItems: FlatListItemType[] = []; | ||
| for (const item of items ?? []) { | ||
| if (item.version === ConversationVersion.GROUP) { | ||
| const groupMatches = await groupMatchesSearchQuery({ | ||
| account: currentAccount!, | ||
| searchQuery, | ||
| group: item, | ||
| }); | ||
| if (groupMatches) { | ||
| filteredItems.push(item); | ||
| } | ||
| } else if (item.version === ConversationVersion.DM) { | ||
| const dmMatches = await dmMatchesSearchQuery({ | ||
| account: currentAccount!, | ||
| searchQuery, | ||
| dm: item, | ||
| }); | ||
| if (dmMatches) { | ||
| filteredItems.push(item); | ||
| } | ||
| } | ||
| } | ||
| return filteredItems ?? []; | ||
| }; |
There was a problem hiding this comment.
Add error handling for async operations
The async filtering logic lacks error handling, which could lead to unhandled promise rejections if either dmMatchesSearchQuery or groupMatchesSearchQuery fails.
Add try-catch block:
const getFilteredItems = async () => {
+ try {
const filteredItems: FlatListItemType[] = [];
for (const item of items ?? []) {
if (item.version === ConversationVersion.GROUP) {
const groupMatches = await groupMatchesSearchQuery({
account: currentAccount!,
searchQuery,
group: item,
});
if (groupMatches) {
filteredItems.push(item);
}
} else if (item.version === ConversationVersion.DM) {
const dmMatches = await dmMatchesSearchQuery({
account: currentAccount!,
searchQuery,
dm: item,
});
if (dmMatches) {
filteredItems.push(item);
}
}
}
return filteredItems ?? [];
+ } catch (error) {
+ console.error('Search filtering failed:', error);
+ return [];
+ }
};📝 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 getFilteredItems = async () => { | |
| const filteredItems: FlatListItemType[] = []; | |
| for (const item of items ?? []) { | |
| if (item.version === ConversationVersion.GROUP) { | |
| const groupMatches = await groupMatchesSearchQuery({ | |
| account: currentAccount!, | |
| searchQuery, | |
| group: item, | |
| }); | |
| if (groupMatches) { | |
| filteredItems.push(item); | |
| } | |
| } else if (item.version === ConversationVersion.DM) { | |
| const dmMatches = await dmMatchesSearchQuery({ | |
| account: currentAccount!, | |
| searchQuery, | |
| dm: item, | |
| }); | |
| if (dmMatches) { | |
| filteredItems.push(item); | |
| } | |
| } | |
| } | |
| return filteredItems ?? []; | |
| }; | |
| const getFilteredItems = async () => { | |
| try { | |
| const filteredItems: FlatListItemType[] = []; | |
| for (const item of items ?? []) { | |
| if (item.version === ConversationVersion.GROUP) { | |
| const groupMatches = await groupMatchesSearchQuery({ | |
| account: currentAccount!, | |
| searchQuery, | |
| group: item, | |
| }); | |
| if (groupMatches) { | |
| filteredItems.push(item); | |
| } | |
| } else if (item.version === ConversationVersion.DM) { | |
| const dmMatches = await dmMatchesSearchQuery({ | |
| account: currentAccount!, | |
| searchQuery, | |
| dm: item, | |
| }); | |
| if (dmMatches) { | |
| filteredItems.push(item); | |
| } | |
| } | |
| } | |
| return filteredItems ?? []; | |
| } catch (error) { | |
| console.error('Search filtering failed:', error); | |
| return []; | |
| } | |
| }; |
Removed async Removed await
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
features/conversations/utils/search.ts (2)
105-113: Optimize string comparisons and consider async consistencyThe function could be improved for better performance and consistency:
Consider these improvements:
-const addressMatchesSearchQuery = ({ +const addressMatchesSearchQuery = async ({ searchQuery, address, -}: AddressSearchParams): boolean => { +}: AddressSearchParams): Promise<boolean> => { if (!address) { return false; } - return address.toLowerCase().includes(searchQuery.toLowerCase()); + const normalizedQuery = searchQuery.toLowerCase(); + return address.toLowerCase().includes(normalizedQuery); };
1-113: Consider caching normalized strings and search resultsWhile the implementation successfully meets the PR objectives for enhanced search functionality, consider these architectural improvements:
- Cache normalized strings (lowercase versions) at the entity level to avoid repeated transformations
- Consider implementing a caching mechanism for search results to improve performance for frequently searched terms
- Add rate limiting for search operations to prevent potential DoS attacks
- Consider implementing fuzzy search for better user experience
Would you like help implementing any of these improvements?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
features/conversations/utils/search.ts(1 hunks)
🔇 Additional comments (4)
features/conversations/utils/search.ts (4)
1-14: LGTM! Well-structured type definitions
The type definitions are clear, well-organized, and provide good type safety for the search parameters.
Also applies to: 38-42, 75-79, 100-104
81-98:
Fix async operation and optimize string comparisons
The function has several issues that need to be addressed:
Apply these improvements:
const inboxIdMatchesSearchQuery = async ({
account,
searchQuery,
inboxId,
}: InboxIdSearchParams): Promise<boolean> => {
+ if (!searchQuery?.trim()) {
+ return false;
+ }
+ try {
+ const normalizedQuery = searchQuery.toLowerCase();
- if (inboxId.toLowerCase().includes(searchQuery.toLowerCase())) {
+ if (inboxId.toLowerCase().includes(normalizedQuery)) {
return true;
}
- const profiles = getInboxProfileSocialsQueryData(account, inboxId);
+ const profiles = await getInboxProfileSocialsQueryData(account, inboxId);
if (!profiles) {
return false;
}
const name = getPreferredInboxName(profiles);
- if (name.toLowerCase().includes(searchQuery.toLowerCase())) {
+ if (name.toLowerCase().includes(normalizedQuery)) {
return true;
}
return false;
+ } catch (error) {
+ console.error('Error in inboxIdMatchesSearchQuery:', error);
+ return false;
+ }
};Likely invalid or redundant comment.
15-36:
Add error handling and safe array access
The function needs improvements in error handling and array access safety:
Apply these improvements:
export const dmMatchesSearchQuery = async ({
account,
searchQuery,
dm,
}: DmSearchParams): Promise<boolean> => {
+ if (!searchQuery?.trim()) {
+ return false;
+ }
+ try {
const inboxId = await dm.peerInboxId();
if (await inboxIdMatchesSearchQuery({ account, searchQuery, inboxId })) {
return true;
}
const members = await dm.members();
for (const member of members) {
+ const address = member.addresses?.[0];
+ if (!address) continue;
if (
addressMatchesSearchQuery({
searchQuery,
- address: member.addresses[0],
+ address,
})
) {
return true;
}
}
return false;
+ } catch (error) {
+ console.error('Error in dmMatchesSearchQuery:', error);
+ return false;
+ }
};Likely invalid or redundant comment.
44-73:
Add error handling and optimize string comparisons
Similar to dmMatchesSearchQuery, this function needs error handling improvements. Additionally, the string comparisons can be optimized.
Apply these improvements:
export const groupMatchesSearchQuery = async ({
account,
searchQuery,
group,
}: GroupSearchParams): Promise<boolean> => {
+ if (!searchQuery?.trim()) {
+ return false;
+ }
+ try {
+ const normalizedQuery = searchQuery.toLowerCase();
- if (group.name.toLowerCase().includes(searchQuery.toLowerCase())) {
+ if (group.name.toLowerCase().includes(normalizedQuery)) {
return true;
}
const members = await group.members();
for (const member of members) {
+ const address = member.addresses?.[0];
+ if (!address) continue;
if (
await inboxIdMatchesSearchQuery({
account,
searchQuery,
inboxId: member.inboxId,
})
) {
return true;
}
if (
addressMatchesSearchQuery({
searchQuery,
- address: member.addresses[0],
+ address,
})
) {
return true;
}
}
return false;
+ } catch (error) {
+ console.error('Error in groupMatchesSearchQuery:', error);
+ return false;
+ }
};Likely invalid or redundant comment.
* feat: V3 Search Added search for V3 Only Added tests Updated Snapshots * remove checks Removed async Removed await * remove inboxId check
Added search for V3 Only
Added tests
Updated Snapshots
Closes #1262
Closes #222
Summary by CodeRabbit
New Features
Bug Fixes
Tests