Fix Search Functionality for Full Name and Partial Matches in People Section (#3388) #3520
Conversation
WalkthroughThis pull request enhances localization and search functionalities. Several translation files have been updated with a new key, Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant P as People Component
participant S as GraphQL Server
U->>P: Enters search input (full name)
P->>P: Splits input into firstName & lastName
P->>S: Sends query with { firstName_contains, lastName_contains }
S-->>P: Returns matching members
P->>U: Displays results or shows "nothingToShow" if no match found
Assessment against linked issues
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
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/screens/UserPortal/People/People.tsx (1)
Line range hint
1-24: Consider enhancing error handling for network failures.While the component handles loading and empty states well, consider adding explicit error handling for network failures to improve user experience.
const { data, loading, refetch } = useQuery( ORGANIZATIONS_MEMBER_CONNECTION_LIST, { variables: { orgId: organizationId, firstName_contains: '', lastName_contains: '', }, + onError: (error) => { + // Handle network/query errors + console.error('Search failed:', error); + // Show error message to user + } }, );
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
public/locales/en/translation.json(1 hunks)public/locales/fr/translation.json(1 hunks)public/locales/hi/translation.json(1 hunks)public/locales/sp/translation.json(1 hunks)public/locales/zh/translation.json(1 hunks)src/screens/UserPortal/People/People.spec.tsx(11 hunks)src/screens/UserPortal/People/People.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test Application
🔇 Additional comments (8)
src/screens/UserPortal/People/People.tsx (2)
91-91: LGTM! Added lastName parameter for full name search support.The addition of
lastName_containsparameter enables filtering by last name in the GraphQL query.
116-121: Well-implemented full name search logic!The search implementation:
- Properly handles whitespace trimming
- Correctly splits full name into first and last components
- Gracefully handles partial inputs using default empty strings
src/screens/UserPortal/People/People.spec.tsx (1)
724-762: Excellent test coverage for the new search functionality!The test suite thoroughly covers:
- Full name search scenarios
- Edge cases with empty inputs
- Mock data setup with lastName parameter
public/locales/zh/translation.json (1)
983-984: LGTM! Clear and natural Chinese translation added.The translation for "nothingToShow" is grammatically correct and maintains consistency with other language versions.
public/locales/en/translation.json (1)
1009-1010: LGTM! Clear and concise English message added.The "Nothing to show here" message is user-friendly and serves as a good base for translations.
public/locales/hi/translation.json (1)
969-970: New "nothingToShow" Key AdditionThe new key
"nothingToShow": "यहां दिखाने के लिए कुछ नहीं है।"has been added to the "people" section to provide a clear empty state message. The text is concise and appears consistent with similar updates in the other localization files.public/locales/fr/translation.json (1)
969-970: New "nothingToShow" Key AdditionThe added key
"nothingToShow": "Rien à afficher ici."in the "people" section correctly reflects the intended message for an empty state in French. It aligns well with similar messages in other locales.public/locales/sp/translation.json (1)
984-985: Localized Message Added Correctly.
The new key"nothingToShow"with the value"Nada que mostrar aquí."is correctly inserted in the"people"section. It is consistent with similar localization changes in other languages and meets the objective of providing a clearer empty state message for the People section.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/screens/UserPortal/People/People.spec.tsx (1)
1511-1511: Consider adding test cases for special character handling.While the test coverage is comprehensive, consider adding test cases for:
- Names with special characters (e.g., "O'Connor", "García")
- Names with multiple spaces
- Names with hyphens (e.g., "Jean-Pierre")
This would ensure the search functionality handles all possible name formats correctly.
Would you like me to help generate these additional test cases?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/screens/UserPortal/People/People.spec.tsx(12 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test Application
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (3)
src/screens/UserPortal/People/People.spec.tsx (3)
168-168: LGTM! Search query enhancement.The addition of
lastName_containsparameter to the search query aligns with the PR objective to support full name searches.
256-256: LGTM! Test cases updated for full name search.The test cases have been updated to verify full name search functionality by:
- Using 'j c' as input to test first and last name matching
- Verifying that 'John Cena' is found and 'Noble Mittal' is not shown
Also applies to: 281-281
724-1511: LGTM! Comprehensive test coverage for edge cases.Excellent addition of test cases covering:
- Full name search with exact matches
- Error handling for failed GraphQL queries
- Empty search results
- Rapid mode switching
- Pagination with changing data
- Empty and whitespace-only search inputs
This significantly improves the test coverage and ensures the robustness of the search functionality.
|
@palisadoes I’ve made the required changes. Please review them |
What kind of change does this PR introduce?
Bugfix
Issue Number:
Fixes #3388
Snapshots/Videos:
20250202034155.mp4
If relevant, did you update the documentation?
No
Summary
This PR addresses the issue where the search functionality in the "People" section fails to return results when searching with a full name, partial matches beyond the first name, or queries with extra spaces. The current behavior shows "people.nothingToShow," which is not helpful or accurate. This fix improves the search functionality to support full names, partial matches, and handles spaces correctly, ensuring that users can find a person even with incomplete or extended queries.
Does this PR introduce a breaking change?
No
Checklist
CodeRabbit AI Review
Test Coverage
Other information
Have you read the contributing guide?
Yes
Summary by CodeRabbit
New Features
Bug Fixes