fix: updated people tab section #6359
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
No significant changes currently retry |
Our Pull Request Approval ProcessThis PR will be reviewed according to our: Your PR may be automatically closed if:
Thanks for contributing! |
Contributor
WalkthroughThis PR refactors the user profile page by relocating the ProfileForm component from shared-components to a new screens/MemberDetail directory and restructures it into a tabbed interface with separate components for contact details, organizations, events, and tags. It includes new GraphQL queries, updated password validation logic, and comprehensive test coverage for the new components. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 3 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 26
🤖 Fix all issues with AI agents
In @src/components/UserDetails/UserEvents.tsx:
- Around line 49-120: The dummyEvents array contains repeated identical "Node.js
Seminar" entries (eventName: 'Node.js Seminar') which appear to be accidental
duplicates; update the dummyEvents constant to remove the duplicate objects (or
replace them with unique events) so each array element is distinct and only one
Node.js Seminar entry remains, making sure to edit the dummyEvents definition
accordingly.
- Around line 123-141: The memo for filteredEvents is ineffective because
dummyEvents is recreated each render; move the dummyEvents array out of the
component (module-level constant) or memoize it with useMemo/useRef so its
identity is stable, then keep useMemo([... , searchValue, sortOption]) for
filteredEvents; update references to dummyEvents inside the component and ensure
only stable values are in the dependency list for useMemo that computes
filteredEvents (referencing filteredEvents, dummyEvents, sortOption,
searchValue).
- Around line 40-42: The component defines an inline prop type
PeopleTabUserEventsProps and lives in the wrong folder; extract the props into a
standalone exported interface (e.g., InterfaceUserEventsProps with id?: string)
in a new types/interface file for the correct portal (AdminPortal, UserPortal or
shared-components depending on usage), update the UserEvents component signature
to React.FC<InterfaceUserEventsProps> by importing that interface, move the
UserEvents file into the appropriate portal directory
(AdminPortal/UserPortal/shared-components) and update any imports and TSDoc
accordingly.
In @src/components/UserDetails/UserOrganizations.tsx:
- Line 264: The hardcoded action label actionName="Edit" in the
UserOrganizations component should be replaced with the i18n string; update the
component to use the translation key via the project's i18n helper (e.g.,
useTranslation/t function) and pass actionName={t('edit') or t('users.edit')}
instead of the literal "Edit", ensuring you import and call the translator where
the actionName prop is set so the label is localized.
- Around line 49-79: Move the inline type definitions out of the component and
into a dedicated interface file: create exported types/interfaces for
MemberDetailProps (or InterfaceUserOrganizationsProps), OrgRelationType, UserOrg
(or InterfaceUserOrg), InterfaceJoinedOrgEdge, and
InterfaceJoinedOrganizationsData; replace the inline declarations in
UserOrganizations.tsx with imports of those exported types and update any local
names to match the exported identifiers (e.g., use InterfaceUserOrg where
UserOrg was used) so the component no longer defines types inline but imports
the shared interfaces.
In @src/components/UserDetails/UserTags.spec.tsx:
- Around line 48-55: Update the test in UserTags.spec.tsx so its assertions
match the translation keys returned by the mock i18n function: render(<UserTags
/>) and replace the expectation for 'title' with 'peopleTabTagName' (since the
component uses tCommon('peopleTabTagName')); keep the other expectations as
their actual keys ('assignedTo', 'createdOn', 'createdBy') so assertions align
with the mocked t: (key) => key behavior.
In @src/components/UserDetails/UserTags.tsx:
- Around line 195-197: The anchor element rendering tag.createdBy in
UserTags.tsx (class styles.peopleTabUserTagCreatedByButton) lacks an href and
should be made accessible: if it navigates, give it a proper href (or use a Link
component) that points to the destination; if it performs an action, replace the
<a> with a <button> (preserve the className and add type="button"); if it’s
purely presentational, replace it with a non-interactive element like <span> and
remove interactive styling or add appropriate ARIA/keyboard handling. Update the
element accordingly and ensure any event handlers, keyboard support, and styling
still reference styles.peopleTabUserTagCreatedByButton.
- Around line 1-208: UserTags is placed under src/components/UserDetails which
violates portal separation; move the UserTags component to the correct portal
(likely src/components/AdminPortal/UserTags based on MemberDetail usage) and
update all imports that reference the UserTags component (e.g., in MemberDetail)
to the new path; ensure any companion test files (UserTags.spec.tsx) and
index/export files are moved/updated, keep the component's export default
UserTags unchanged, and run the build/tests to verify no broken imports remain.
- Around line 145-163: Replace the hardcoded user strings in the UserTags
component with i18n keys: change the menu title 'Sort By' to use the translator
(e.g., t('sortBy')) and change the search placeholder 'Search tags' to
t('searchTags'); keep the existing handlers and props (selected: sortBy,
onChange: setSortBy, onSearch: setSearchTerm, inputTestId/buttonTestId)
unchanged and add the corresponding keys ("sortBy" and "searchTags") to all
locale translation.json files.
In @src/screens/MemberDetail/MemberDetail.spec.tsx:
- Around line 107-113: Update the "switches to tags tab" test to also assert the
tab's active state: after firing click on the 'tab-tags' test id, assert that
the element with test id 'tab-tags' has attribute data-active="true" (in
addition to asserting 'user-tags' is in the document) so it matches the other
tab tests' behavior.
- Around line 63-70: The mock PeopleTabNavButton renders a <button> without an
explicit type, which defaults to "submit" and can cause unintended form
submissions; update the mock JSX (the button with data-testid={`tab-${title}`}
and data-active={isActive}) to include type="button" on the element so it
behaves as a non-submitting control.
- Around line 76-82: Remove the redundant vi.clearAllMocks() call in the
beforeEach hook in MemberDetail.spec.tsx: keep the existing afterEach(() => {
vi.clearAllMocks(); }) for test isolation and delete the entire beforeEach block
that only calls vi.clearAllMocks(), ensuring no other setup logic is lost.
In @src/screens/MemberDetail/MemberDetail.tsx:
- Line 41: MemberDetailProps is defined inline; extract it to a dedicated
interface file by creating and exporting InterfaceMemberDetailProps (with
optional id?: string) and then replace the inline type alias in the MemberDetail
component to import and use InterfaceMemberDetailProps instead of the local
MemberDetailProps; update the component signature (e.g., MemberDetail) and any
references to the inline type to use the new interface name and ensure the
import/export are correctly added.
In @src/screens/MemberDetail/UserContactDetails.spec.tsx:
- Around line 238-241: The test fails because UserContactDetails renders plain
text when loading (if (loading) return <div>{tCommon('loading')}</div>) but the
spec expects an element with data-testid="loader"; either update
UserContactDetails to return the Loader component with a test id (replace the
loading branch to return <Loader data-testid="loader" /> or similar) or change
the spec in UserContactDetails.spec.tsx to assert the rendered text (use
screen.getByText(tCommon('loading')) or toMatchInlineSnapshot) instead of
screen.getByTestId('loader'); update the symbol UserContactDetails and the
test's assertion accordingly so implementation and test align.
- Around line 92-94: The test mock for validatePassword is returning true but
the real function returns null on successful validation; update the vi.mock for
validatePassword so its validatePassword mock returns null (or resolves to null
if async) to mirror production behavior and ensure tests exercise the same
branches as the real validator.
- Around line 578-599: The tests currently assert NotificationToast.error is
called for invalid passwords but do not assert the member update mutation was
not invoked; add an assertion that the mocked member update mutation (e.g., the
updateMember.mutate or updateMemberMutation mock returned by your
useUpdateMember/useMutation hook) was not called after clicking save by using
expect(<mockedMutation>).not.toHaveBeenCalled(); apply this additional assertion
to both failing-password tests (the one around lines 578-599 and the one around
385-413) immediately after the existing NotificationToast.error assertion.
In @src/screens/MemberDetail/UserContactDetails.tsx:
- Around line 119-122: The handler handleUserUpdate shows a password validation
toast when passwordError is truthy but does not stop execution; modify
handleUserUpdate so that after computing passwordError (from formState.password
and validatePassword) and calling NotificationToast.error(passwordError) it
immediately returns (or otherwise aborts submission) to prevent continuing with
an invalid password. Ensure you update the block referencing formState.password,
validatePassword, passwordError, and NotificationToast.error inside
handleUserUpdate to short-circuit on invalid input.
- Line 50: The direct assignment document.title = t('title') inside the
UserContactDetails component should be moved into a React useEffect to avoid
running on every render; import useEffect if missing and add useEffect(() => {
document.title = t('title'); }, [t]) (or an appropriate dependency such as i18n
language) so the title is set as a side effect only when the translation/value
changes.
- Around line 219-230: Replace the non-semantic <i> with a semantic <button> for
the avatar edit control: keep the same className string (including
styles.userContactDetailContactAvatarEditIcon), data-testid="uploadImageBtn",
title and aria-label using tCommon('userEditProfilePicture'), set type="button",
keep onClick={() => fileInputRef.current?.click()}, remove the custom
onKeyDown/tabIndex because a native button already supports keyboard activation,
and preserve position/styling so tests and behavior (fileInputRef click) remain
unchanged.
- Around line 581-583: The Button currently uses an inline style on the Button
element; remove the inline style prop and replace it with a CSS class or
Bootstrap utilities: add a class (e.g., saveChangesBtn) to your CSS module
(e.g., app-fixed.module.css) with the background-color #A8C7FA and color #555,
then set the Button's className to that module class (or use equivalent
Bootstrap utility classes) so the inline style is eliminated; update the Button
element in UserContactDetails.tsx accordingly.
- Around line 75-81: The GraphQL query in UserContactDetails currently
destructures only { data, loading } from useQuery(GET_USER_BY_ID) and ignores
the error state; update the useQuery call to also destructure error (e.g., const
{ data, loading, error } = useQuery(...)) and add an early return or error UI
branch in the component to handle errors (for example return an error
message/component when error is truthy) so the component doesn't hang on loading
or render with undefined data; ensure any existing loading check remains but is
preceded by the error check and reference GET_USER_BY_ID and useQuery in your
fix.
- Line 38: Move the inline type MemberDetailProps into a dedicated interface
file and update the component to import it: create
src/types/MemberDetail/interface.ts exporting export interface
InterfaceMemberDetailProps { id?: string; }, then in UserContactDetails.tsx
remove the inline type declaration and import InterfaceMemberDetailProps and use
it as the component props type (replace MemberDetailProps with
InterfaceMemberDetailProps wherever referenced, e.g., in the UserContactDetails
component signature).
In @src/utils/passwordValidator.ts:
- Around line 1-20: The validatePassword function currently returns hardcoded
English strings and lacks TSDoc; change it to return standardized error codes
(e.g. "PASSWORD_TOO_SHORT", "PASSWORD_NO_UPPERCASE", "PASSWORD_NO_LOWERCASE",
"PASSWORD_NO_NUMBER", "PASSWORD_NO_SPECIAL") or null instead of message text,
add TSDoc to the exported validatePassword explaining return values and codes,
and update callers to call validatePassword(password) and pass the returned code
to the i18n translator (e.g. t(`passwordValidation.${code}`)); keep the existing
regex checks and function signature shape but replace each returned string with
the corresponding error code constant or literal.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (33)
docs/docs/auto-docs/App/functions/default.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/GET_COMMUNITY_DATA_PG.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/GET_COMMUNITY_SESSION_TIMEOUT_DATA_PG.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/GET_ORGANIZATION_BASIC_DATA.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/GET_ORGANIZATION_BLOCKED_USERS_COUNT.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/GET_ORGANIZATION_BLOCKED_USERS_PG.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/GET_ORGANIZATION_DATA_PG.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/GET_ORGANIZATION_EVENTS_PG.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/GET_ORGANIZATION_EVENTS_USER_PORTAL_PG.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/GET_ORGANIZATION_MEMBERS_PG.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/GET_ORGANIZATION_POSTS_PG.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/GET_ORGANIZATION_VENUES_COUNT.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/GET_ORGANIZATION_VENUES_PG.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/GET_USER_BY_ID.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/MEMBERSHIP_REQUEST_PG.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/MEMBERS_LIST.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/MEMBERS_LIST_PG.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/ORGANIZATIONS_LIST.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/ORGANIZATIONS_LIST_BASIC.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/ORGANIZATIONS_MEMBER_CONNECTION_LIST.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/ORGANIZATION_DONATION_CONNECTION_LIST.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/ORGANIZATION_EVENT_CONNECTION_LIST.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/ORGANIZATION_FIELDS.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/SIGNIN_QUERY.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/USERS_CONNECTION_LIST.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/USER_DETAILS.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Queries/Queries/variables/USER_ORGANIZATION_LIST.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/components/UserDetails/UserEvents/variables/default.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/components/UserDetails/UserOrganizations/variables/default.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/components/UserDetails/UserTags/functions/default.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/screens/MemberDetail/MemberDetail/variables/default.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/screens/MemberDetail/UserContactDetails/variables/default.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/utils/passwordValidator/functions/validatePassword.mdis excluded by!**/docs/docs/**
📒 Files selected for processing (14)
src/App.tsxsrc/GraphQl/Queries/Queries.tssrc/components/UserDetails/UserEvents.spec.tsxsrc/components/UserDetails/UserEvents.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/components/UserDetails/UserOrganizations.tsxsrc/components/UserDetails/UserTags.spec.tsxsrc/components/UserDetails/UserTags.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsxsrc/screens/MemberDetail/MemberDetail.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/screens/MemberDetail/UserContactDetails.tsxsrc/utils/passwordValidator.spec.tssrc/utils/passwordValidator.ts
🧰 Additional context used
📓 Path-based instructions (4)
src/**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
All components must follow reusable component policy: No inline interfaces, correct portal placement, wrapper usage
Files:
src/components/UserDetails/UserTags.tsxsrc/components/UserDetails/UserEvents.tsxsrc/components/UserDetails/UserOrganizations.tsxsrc/components/UserDetails/UserEvents.spec.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/components/UserDetails/UserTags.spec.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Zero TypeScript errors: Run pnpm run typecheck and must pass without errors or warnings
Files:
src/components/UserDetails/UserTags.tsxsrc/utils/passwordValidator.tssrc/App.tsxsrc/GraphQl/Queries/Queries.tssrc/components/UserDetails/UserEvents.tsxsrc/screens/MemberDetail/UserContactDetails.tsxsrc/components/UserDetails/UserOrganizations.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsxsrc/components/UserDetails/UserEvents.spec.tsxsrc/screens/MemberDetail/MemberDetail.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/components/UserDetails/UserTags.spec.tsxsrc/utils/passwordValidator.spec.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Conduct a comprehensive review and reevaluate all resolved and dismissed items.VALIDATION FRAMEWORK
1. Test Coverage & Quality
1.1 Test File Requirements
For each modified source file with logic or UI:
- ✅ Test file exists:
*.spec.tsx,*.spec.ts,*.test.tsx, or*.test.ts- ✅ Test file colocated with component (same directory)
- ✅ Test file updated to reflect source changes
- ✅ Target ≥95% code coverage for new/modified code
- ✅ List specific line numbers lacking test coverage
1.2 Test Quality Standards
✅ Framework Compliance (Vitest + React Testing Library):
- Use
vi.mock()for mocking (notjest.mock())- Use
describe(),it(),test()for structure- Use
@testing-library/reactqueries (getByRole, getByText, etc.)- Use
@testing-library/user-eventfor interactions✅ Coverage Completeness - All scenarios tested:
- Success paths with valid data
- Error scenarios (network failures, GraphQL errors, validation errors)
- Edge cases (empty data, null/undefined, boundary values)
- Loading and pending states
- User interactions and state changes
✅ Mock Quality:
- GraphQL:
MockedProviderorStaticMockLink- Realistic, comprehensive mock data
- Proper cleanup (reset mocks, clear timers)
✅ Test Structure:
- Isolated tests (no execution order dependency)
- No disabled tests (
*.skip) without documented reason + linked issue- Proper async utilities (
waitFor,act)- Shard-compatible (12-shard strategy, no shared state, <30s timeout)
2. Code Quality & Implementation
2.1 TypeScript Standards
- ✅ No
anytype without JSDoc justification- ✅ Proper typing for:
- Component props (interface declarations)
- Function parameters and return types
- GraphQL variables and responses
- State and context
- ✅ No
@ts-ignorewithout explanation- ✅ Zero TypeScript compilation errors
2.2 React & GraphQL Implementation
- ✅ React Best Practices:
- Fu...
Files:
src/components/UserDetails/UserTags.tsxsrc/utils/passwordValidator.tssrc/App.tsxsrc/GraphQl/Queries/Queries.tssrc/components/UserDetails/UserEvents.tsxsrc/screens/MemberDetail/UserContactDetails.tsxsrc/components/UserDetails/UserOrganizations.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsxsrc/components/UserDetails/UserEvents.spec.tsxsrc/screens/MemberDetail/MemberDetail.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/components/UserDetails/UserTags.spec.tsxsrc/utils/passwordValidator.spec.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Test coverage must be ≥95% for modified files. Run: pnpm run test:coverage and verify coverage/coverage-summary.json shows no files below threshold
Files:
src/screens/MemberDetail/UserContactDetails.spec.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsxsrc/components/UserDetails/UserEvents.spec.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/components/UserDetails/UserTags.spec.tsxsrc/utils/passwordValidator.spec.ts
🧠 Learnings (10)
📚 Learning: 2025-12-26T20:34:53.912Z
Learnt from: MuhammadUmar7195
Repo: PalisadoesFoundation/talawa-admin PR: 5847
File: src/screens/UserPortal/Volunteer/Invitations/Invitations.tsx:243-243
Timestamp: 2025-12-26T20:34:53.912Z
Learning: In PalisadoesFoundation/talawa-admin, whenever you modify a file (e.g., for migrations, tests, or feature changes), replace inline color attributes like color="grey" with Bootstrap utility classes (e.g., className="text-secondary") to satisfy the CSS-Policy-Check CI. This is a general compliance guideline, not scope creep, and should be applied to all affected TSX files during reviews. Steps: 1) Find inline color attributes in JSX/TSX. 2) Replace with appropriate Bootstrap text/color utility classes. 3) Ensure the visual outcome remains the same. 4) Run CI checks to confirm the policy passes.
Applied to files:
src/components/UserDetails/UserTags.tsxsrc/App.tsxsrc/components/UserDetails/UserEvents.tsxsrc/screens/MemberDetail/UserContactDetails.tsxsrc/components/UserDetails/UserOrganizations.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsxsrc/components/UserDetails/UserEvents.spec.tsxsrc/screens/MemberDetail/MemberDetail.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/components/UserDetails/UserTags.spec.tsx
📚 Learning: 2026-01-06T16:39:41.033Z
Learnt from: Jashan32
Repo: PalisadoesFoundation/talawa-admin PR: 6199
File: src/shared-components/ProfileForm/ProfileForm.tsx:0-0
Timestamp: 2026-01-06T16:39:41.033Z
Learning: When refactoring or consolidating components (e.g., talawa-admin), remove helper functions that were copied from old files but are not used in the new implementation. Before exporting utilities, verify there are real usages across the codebase (search usages, remove dead code, and keep only necessary exports) to improve maintainability and reduce bundle size.
Applied to files:
src/components/UserDetails/UserTags.tsxsrc/App.tsxsrc/components/UserDetails/UserEvents.tsxsrc/screens/MemberDetail/UserContactDetails.tsxsrc/components/UserDetails/UserOrganizations.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsxsrc/components/UserDetails/UserEvents.spec.tsxsrc/screens/MemberDetail/MemberDetail.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/components/UserDetails/UserTags.spec.tsx
📚 Learning: 2026-01-07T09:08:09.150Z
Learnt from: adityai0
Repo: PalisadoesFoundation/talawa-admin PR: 6202
File: src/components/EventManagement/EventAgendaItems/EventAgendaItems.tsx:4-13
Timestamp: 2026-01-07T09:08:09.150Z
Learning: In the talawa-admin repository, document comments with TSDoc in all TSX files. Do not use JSDoc. Write non-standard tags like component and requires as plain text (no leading @). Use standard TSDoc tags such as param, remarks, returns, and example with the leading @. This guideline applies to all TSX components (e.g., src/components/.../*.tsx).
Applied to files:
src/components/UserDetails/UserTags.tsxsrc/App.tsxsrc/components/UserDetails/UserEvents.tsxsrc/screens/MemberDetail/UserContactDetails.tsxsrc/components/UserDetails/UserOrganizations.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsxsrc/components/UserDetails/UserEvents.spec.tsxsrc/screens/MemberDetail/MemberDetail.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/components/UserDetails/UserTags.spec.tsx
📚 Learning: 2026-01-08T16:45:49.201Z
Learnt from: adityai0
Repo: PalisadoesFoundation/talawa-admin PR: 6241
File: src/components/AdminPortal/OrgPeopleListCard/OrgPeopleListCard.tsx:12-16
Timestamp: 2026-01-08T16:45:49.201Z
Learning: In TSX components, prefer flat JSDoc parameter documentation format (e.g., param id, param toggleRemoveModal) over the nested format (param props.id). The nested form can break editor parsing. When a component accepts a props object, destructure the props in the function signature to align with the flat JSDoc style (e.g., function MyComp({ id, toggleRemoveModal }: Props) { ... }). Ensure all param tags correspond to the destructured props and reflect actual component props to improve editor support and readability.
Applied to files:
src/components/UserDetails/UserTags.tsxsrc/components/UserDetails/UserEvents.tsxsrc/components/UserDetails/UserOrganizations.tsxsrc/components/UserDetails/UserEvents.spec.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/components/UserDetails/UserTags.spec.tsx
📚 Learning: 2026-01-06T21:27:50.880Z
Learnt from: Jashan32
Repo: PalisadoesFoundation/talawa-admin PR: 6222
File: src/utils/userUpdateUtils.ts:22-22
Timestamp: 2026-01-06T21:27:50.880Z
Learning: In the Talawa Admin codebase, the i18n extraction tool can misclassify TypeScript generic syntax (e.g., (obj: T): Partial) as translatable content. Suppress these false positives by adding a // i18n-ignore-next-line comment directly above the line with the type declaration or generic function signature. Apply this only to TS code where misclassification is confirmed; avoid blanket suppression.
Applied to files:
src/utils/passwordValidator.tssrc/GraphQl/Queries/Queries.tssrc/utils/passwordValidator.spec.ts
📚 Learning: 2026-01-07T14:21:19.711Z
Learnt from: cotishq
Repo: PalisadoesFoundation/talawa-admin PR: 6198
File: src/setup/setup.ts:123-124
Timestamp: 2026-01-07T14:21:19.711Z
Learning: Use TSDoc (eslint-plugin-tsdoc) instead of JSDoc. Do not wrap type names in curly braces in throws and returns. Write as "throws Error" and "returns Promise<void>" (no braces). Curly-brace usage causes tsdoc-malformed-inline-tag and tsdoc-escape-right-brace lint errors. Apply this rule to all TypeScript source files (e.g., src/**/*.ts) to ensure consistency.
Applied to files:
src/utils/passwordValidator.tssrc/GraphQl/Queries/Queries.tssrc/utils/passwordValidator.spec.ts
📚 Learning: 2025-12-21T08:59:37.942Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5287
File: src/shared-components/Recurrence/CustomRecurrenceModal.spec.tsx:78-84
Timestamp: 2025-12-21T08:59:37.942Z
Learning: In talawa-admin test files, ensure each spec file uses an explicit vi.clearAllMocks() in an afterEach block to guarantee test isolation. This should be present even if a global cleanup exists in vitest.setup.ts, as the linter enforces per-file hygiene. Apply this guideline to all spec files (e.g., src/**/*.spec.tsx) to satisfy ESLint/Vitest requirements and prevent bleed-over between tests.
Applied to files:
src/screens/MemberDetail/UserContactDetails.spec.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsxsrc/components/UserDetails/UserEvents.spec.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/components/UserDetails/UserTags.spec.tsxsrc/utils/passwordValidator.spec.ts
📚 Learning: 2025-12-27T11:22:41.191Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5782
File: src/screens/Leaderboard/Leaderboard.spec.tsx:342-370
Timestamp: 2025-12-27T11:22:41.191Z
Learning: In test files that render screens with shared components (e.g., src/screens/Leaderboard/Leaderboard.spec.tsx), verify integration behavior by asserting the screen renders correctly in different scenarios (e.g., search results) without asserting the internal ARIA attributes, text, or icons of the shared component. Rely on the shared component's own tests to cover accessibility and presentation details. This keeps tests focused on higher-level behavior and interactions.
Applied to files:
src/screens/MemberDetail/UserContactDetails.spec.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsxsrc/components/UserDetails/UserEvents.spec.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/components/UserDetails/UserTags.spec.tsxsrc/utils/passwordValidator.spec.ts
📚 Learning: 2026-01-08T18:48:10.364Z
Learnt from: abhishekblue
Repo: PalisadoesFoundation/talawa-admin PR: 6260
File: src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.spec.tsx:337-339
Timestamp: 2026-01-08T18:48:10.364Z
Learning: In tests that exercise Apollo Client queries (e.g., talawa-admin tests under src/screens/.../OrganizationPeople.spec.tsx), avoid using vi.useFakeTimers() because Apollo's internal promise resolution can be disrupted by mocked timers, causing timeouts. Use real timers with explicit waits (for example, await act(async () => { await new Promise(r => setTimeout(r, DURATION)); })) when testing components that fetch data via Apollo. If you must use timers, prefer real timers for the relevant test and flush promises deterministically. Replace or remove useFakeTimers in such tests and rely on actual time passage or proper promise flushing to ensure reliable Apollo-driven data fetching tests.
Applied to files:
src/screens/MemberDetail/UserContactDetails.spec.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsxsrc/components/UserDetails/UserEvents.spec.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/components/UserDetails/UserTags.spec.tsxsrc/utils/passwordValidator.spec.ts
📚 Learning: 2025-12-27T11:22:19.556Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5782
File: src/screens/Leaderboard/Leaderboard.spec.tsx:299-329
Timestamp: 2025-12-27T11:22:19.556Z
Learning: In talawa-admin test files (e.g., src/screens/Leaderboard/Leaderboard.spec.tsx), prefer separate explicit test cases for keyboard navigation interactions (Enter key, Space key) instead of consolidating them with test.each()/parameterization. This improves readability, documents each keyboard interaction path clearly, and makes failures easier to diagnose when specific key handling regresses. Apply this pattern to similarly structured spec files by keeping individual tests for each key interaction rather than combining them into a single parameterized test.
Applied to files:
src/screens/MemberDetail/UserContactDetails.spec.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsxsrc/components/UserDetails/UserEvents.spec.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/components/UserDetails/UserTags.spec.tsx
🧬 Code graph analysis (8)
src/components/UserDetails/UserTags.tsx (1)
src/shared-components/PeopleTabNavbar/PeopleTabNavbar.tsx (1)
PeopleTabNavbar(52-130)
src/components/UserDetails/UserEvents.tsx (1)
src/shared-components/PeopleTabNavbar/PeopleTabNavbar.tsx (1)
PeopleTabNavbar(52-130)
src/screens/MemberDetail/UserContactDetails.spec.tsx (3)
src/GraphQl/Queries/Queries.ts (1)
GET_USER_BY_ID(452-477)src/GraphQl/Mutations/mutations.ts (1)
UPDATE_CURRENT_USER_MUTATION(66-96)src/utils/passwordValidator.ts (1)
validatePassword(1-20)
src/screens/MemberDetail/MemberDetail.spec.tsx (1)
src/shared-components/ErrorBoundaryWrapper/ErrorBoundaryWrapper.tsx (1)
render(129-179)
src/components/UserDetails/UserEvents.spec.tsx (1)
src/types/PeopleTab/interface.ts (2)
InterfacePeopleTabNavbarProps(24-42)InterfacePeopletabUserEventsProps(62-71)
src/components/UserDetails/UserOrganizations.spec.tsx (2)
src/types/PeopleTab/interface.ts (2)
InterfacePeopleTabUserOrganizationProps(74-82)InterfacePeopleTabNavbarProps(24-42)src/GraphQl/Queries/Queries.ts (2)
USER_DETAILS(923-973)USER_JOINED_ORGANIZATIONS_NO_MEMBERS(107-123)
src/components/UserDetails/UserTags.spec.tsx (1)
src/types/PeopleTab/interface.ts (1)
InterfacePeopleTabNavbarProps(24-42)
src/utils/passwordValidator.spec.ts (1)
src/utils/passwordValidator.ts (1)
validatePassword(1-20)
🪛 Biome (2.1.2)
src/components/UserDetails/UserTags.tsx
[error] 195-195: Provide a href attribute for the a element.
An anchor element should always have a href
Check this thorough explanation to better understand the context.
(lint/a11y/useValidAnchor)
src/screens/MemberDetail/UserContactDetails.tsx
[error] 224-224: The HTML element i is non-interactive and should not have an interactive role.
Replace i with a div or a span.
Unsafe fix: Remove the role attribute.
(lint/a11y/noNoninteractiveElementToInteractiveRole)
src/screens/MemberDetail/MemberDetail.spec.tsx
[error] 63-68: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Performs linting, formatting, type-checking, unused file detection, checking for different source...
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (13)
src/GraphQl/Queries/Queries.ts (1)
452-478: LGTM! Well-structured GraphQL query.The GET_USER_BY_ID query is properly formed, follows existing patterns in the file, and exposes appropriate user profile fields. The query structure and export are correct.
src/App.tsx (1)
36-36: LGTM! Import path correctly updated for MemberDetail refactor.The lazy import path change from ProfileForm to MemberDetail aligns with the PR objectives to refactor the user profile page. The component is used consistently across admin and user routes (lines 214, 251, 309).
src/utils/passwordValidator.spec.ts (1)
1-36: Add afterEach cleanup for consistency.The tests comprehensively cover all validation scenarios and correctly assert the new string/null return values. However, based on coding guidelines, include an explicit
afterEachcleanup block for consistency across test files, even though this file doesn't currently use mocks.♻️ Add afterEach cleanup block
+import { describe, it, expect, afterEach, vi } from 'vitest'; -import { describe, it, expect } from 'vitest'; import { validatePassword } from './passwordValidator'; describe('validatePassword', () => { + afterEach(() => { + vi.clearAllMocks(); + }); + it('returns error if password is shorter than 8 characters', () => {Based on learnings, all spec files should include explicit cleanup to ensure test isolation and satisfy ESLint/Vitest requirements.
Likely an incorrect or invalid review comment.
src/screens/MemberDetail/MemberDetail.tsx (1)
39-39: Update import path after UserTags is moved.Line 39 imports
UserTagsfrom'components/UserDetails/UserTags', but this path will need updating once UserTags is moved to the correct portal directory (see review comment on UserTags.tsx).After moving UserTags to the correct directory (likely
src/components/AdminPortal/UserTags/), update this import:-import UserTags from 'components/UserDetails/UserTags'; +import UserTags from 'components/AdminPortal/UserTags/UserTags';Similarly verify and update the imports for
UserOrganizations(line 37) andUserEvents(line 38).src/components/UserDetails/UserEvents.spec.tsx (1)
1-135: Test structure follows best practices.The test suite demonstrates good practices:
- Proper mock cleanup with
vi.clearAllMocks()inafterEach(compliant with learnings)- Comprehensive mocking of dependencies (Apollo, router, localStorage, i18n)
- Tests cover key scenarios: rendering, filtering, empty state, sorting
- Follows Vitest + React Testing Library patterns
src/components/UserDetails/UserOrganizations.spec.tsx (4)
5-5: UnusedReactimport.The
Reactimport on line 5 is not directly used in this file. The JSX in the mocks uses the automatic JSX runtime, so the import can be removed unless you needReact.ChangeEventtypes (which are used inline in the mock). However, since the types are used inline in the mock function parameters, this import is actually needed for TypeScript.
150-212: Well-structured mock implementation for ApollouseQuery.The mock implementation correctly handles multiple queries (
USER_DETAILSandUSER_JOINED_ORGANIZATIONS_NO_MEMBERS) with proper type checking and returns appropriate mock data. The pattern of dynamically importing and configuring the mock inbeforeEachis effective for this use case.
225-241: Test coverage looks good for organization rendering.The test verifies all three organization types (Created, Belong, Joined) are rendered correctly after data loads.
342-366: Good coverage for sort order functionality.The test verifies both ascending and descending sort orders by checking the order of rendered organization cards.
src/screens/MemberDetail/UserContactDetails.tsx (1)
126-131: Silent failure whenurlToFilefails.When
urlToFilefails,avatarFileis set tonulland the update continues. While an error toast is shown, the user might not realize their avatar wasn't updated. Consider whether this should block the update or at least log for debugging.Verify this is the intended behavior. If the existing avatar URL can't be converted to a file, should the update proceed without the avatar, or should it be blocked?
src/screens/MemberDetail/UserContactDetails.spec.tsx (3)
618-635: Error handling test doesn't verify error handler is called.The test for GraphQL query failure (lines 618-635) only checks that the loader remains visible. It should also verify that
errorHandleris called with the error, or acknowledge that the component currently doesn't handle query errors (which is a bug).This test documents current behavior but the component doesn't actually call
errorHandleron query failure. Once the component is fixed to handle errors, this test should be updated to verify proper error handling.
717-738: Good keyboard accessibility test coverage.The test properly verifies both click and Enter key interactions for the file upload trigger, and correctly asserts that Space key doesn't trigger the action. This aligns with the per-learnings guidance for separate keyboard interaction tests.
200-212: Render helper correctly sets up mocks for URL object methods.The helper properly mocks
URL.createObjectURLandURL.revokeObjectURLwhich are needed for avatar preview functionality.
Comment on lines
+40
to
+42
| type PeopleTabUserEventsProps = { id?: string }; | ||
|
|
||
| const UserEvents: React.FC<PeopleTabUserEventsProps> = () => { |
Contributor
There was a problem hiding this comment.
Architecture violation: Inline props type and incorrect component location.
This component has two critical architecture issues:
-
Inline type definition: Line 40 defines
PeopleTabUserEventsPropsinline, violating the reusable component policy. Per guidelines: "NO inline prop interfaces in component files." -
Incorrect directory: The component is in
src/components/UserDetails/which doesn't follow the required portal structure:- Admin-only:
src/components/AdminPortal/** - User-only:
src/components/UserPortal/** - Shared:
src/shared-components/**
- Admin-only:
📁 Required refactoring steps
Step 1: Determine portal placement
- If used only in admin context → move to
src/components/AdminPortal/UserEvents/ - If used in user context → move to
src/components/UserPortal/UserEvents/ - If shared → move to
src/shared-components/UserEvents/
Step 2: Create interface file
Create src/types/[AdminPortal|UserPortal|shared-components]/UserEvents/interface.ts:
/**
* Props for UserEvents component.
*/
export interface InterfaceUserEventsProps {
/** Optional user ID for future data fetching support */
id?: string;
}Step 3: Update component imports
+import type { InterfaceUserEventsProps } from 'types/[portal]/UserEvents/interface';
-type PeopleTabUserEventsProps = { id?: string };
-const UserEvents: React.FC<PeopleTabUserEventsProps> = () => {
+const UserEvents: React.FC<InterfaceUserEventsProps> = () => {Step 4: Update TSDoc
* @param props - Component props.
- * Optional {@link PeopleTabUserEventsProps.id | id} may be provided and is
+ * Optional {@link InterfaceUserEventsProps.id | id} may be provided and isAs per coding guidelines §3 (Reusable Component Architecture), this must be fixed before approval.
🤖 Prompt for AI Agents
In @src/components/UserDetails/UserEvents.tsx around lines 40 - 42, The
component defines an inline prop type PeopleTabUserEventsProps and lives in the
wrong folder; extract the props into a standalone exported interface (e.g.,
InterfaceUserEventsProps with id?: string) in a new types/interface file for the
correct portal (AdminPortal, UserPortal or shared-components depending on
usage), update the UserEvents component signature to
React.FC<InterfaceUserEventsProps> by importing that interface, move the
UserEvents file into the appropriate portal directory
(AdminPortal/UserPortal/shared-components) and update any imports and TSDoc
accordingly.
Comment on lines
+49
to
+120
| const dummyEvents = [ | ||
| { | ||
| startTime: '10:00', | ||
| endTime: '12:00', | ||
| startDate: '2025-12-10', | ||
| endDate: '2025-12-10', | ||
| eventName: 'React Workshop', | ||
| eventDescription: 'Learn React basics and advanced patterns.', | ||
| actionIcon: '⭐', | ||
| actionName: 'Join', | ||
| }, | ||
| { | ||
| startTime: '14:00', | ||
| endTime: '16:00', | ||
| startDate: '2025-12-12', | ||
| endDate: '2025-12-12', | ||
| eventName: 'Node.js Seminar', | ||
| eventDescription: 'Deep dive into Node.js performance.', | ||
| actionIcon: '🔗', | ||
| actionName: 'Register', | ||
| }, | ||
| { | ||
| startTime: '14:00', | ||
| endTime: '16:00', | ||
| startDate: '2025-12-12', | ||
| endDate: '2025-12-12', | ||
| eventName: 'Node.js Seminar', | ||
| eventDescription: 'Deep dive into Node.js performance.', | ||
| actionIcon: '🔗', | ||
| actionName: 'Register', | ||
| }, | ||
| { | ||
| startTime: '14:00', | ||
| endTime: '16:00', | ||
| startDate: '2025-12-12', | ||
| endDate: '2025-12-12', | ||
| eventName: 'Node.js Seminar', | ||
| eventDescription: 'Deep dive into Node.js performance.', | ||
| actionIcon: '🔗', | ||
| actionName: 'Register', | ||
| }, | ||
| { | ||
| startTime: '14:00', | ||
| endTime: '16:00', | ||
| startDate: '2025-12-12', | ||
| endDate: '2025-12-12', | ||
| eventName: 'Node.js Seminar', | ||
| eventDescription: 'Deep dive into Node.js performance.', | ||
| actionIcon: '🔗', | ||
| actionName: 'Register', | ||
| }, | ||
| { | ||
| startTime: '14:00', | ||
| endTime: '16:00', | ||
| startDate: '2025-12-12', | ||
| endDate: '2025-12-12', | ||
| eventName: 'Node.js Seminar', | ||
| eventDescription: 'Deep dive into Node.js performance.', | ||
| actionIcon: '🔗', | ||
| actionName: 'Register', | ||
| }, | ||
| { | ||
| startTime: '14:00', | ||
| endTime: '16:00', | ||
| startDate: '2025-12-12', | ||
| endDate: '2025-12-12', | ||
| eventName: 'Node.js Seminar', | ||
| eventDescription: 'Deep dive into Node.js performance.', | ||
| actionIcon: '🔗', | ||
| actionName: 'Register', | ||
| }, | ||
| ]; |
Contributor
There was a problem hiding this comment.
Remove duplicate dummy data.
Lines 59-119 contain the exact same "Node.js Seminar" event repeated 6 times. This appears to be accidental copy-paste and should be removed or diversified.
🧹 Suggested cleanup
const dummyEvents = [
{
startTime: '10:00',
endTime: '12:00',
startDate: '2025-12-10',
endDate: '2025-12-10',
eventName: 'React Workshop',
eventDescription: 'Learn React basics and advanced patterns.',
actionIcon: '⭐',
actionName: 'Join',
},
{
startTime: '14:00',
endTime: '16:00',
startDate: '2025-12-12',
endDate: '2025-12-12',
eventName: 'Node.js Seminar',
eventDescription: 'Deep dive into Node.js performance.',
actionIcon: '🔗',
actionName: 'Register',
},
- {
- startTime: '14:00',
- endTime: '16:00',
- startDate: '2025-12-12',
- endDate: '2025-12-12',
- eventName: 'Node.js Seminar',
- eventDescription: 'Deep dive into Node.js performance.',
- actionIcon: '🔗',
- actionName: 'Register',
- },
- // ... (remove 5 duplicates)
];🤖 Prompt for AI Agents
In @src/components/UserDetails/UserEvents.tsx around lines 49 - 120, The
dummyEvents array contains repeated identical "Node.js Seminar" entries
(eventName: 'Node.js Seminar') which appear to be accidental duplicates; update
the dummyEvents constant to remove the duplicate objects (or replace them with
unique events) so each array element is distinct and only one Node.js Seminar
entry remains, making sure to edit the dummyEvents definition accordingly.
Comment on lines
+123
to
+141
| const filteredEvents = useMemo(() => { | ||
| let filtered = dummyEvents.filter( | ||
| (event) => | ||
| event.eventName.toLowerCase().includes(searchValue.toLowerCase()) || | ||
| event.eventDescription | ||
| .toLowerCase() | ||
| .includes(searchValue.toLowerCase()), | ||
| ); | ||
|
|
||
| filtered.sort((a, b) => { | ||
| const nameA = a.eventName.toLowerCase(); | ||
| const nameB = b.eventName.toLowerCase(); | ||
| return sortOption === 'ASC' | ||
| ? nameA.localeCompare(nameB) | ||
| : nameB.localeCompare(nameA); | ||
| }); | ||
|
|
||
| return filtered; | ||
| }, [dummyEvents, searchValue, sortOption]); |
Contributor
There was a problem hiding this comment.
Fix inefficient memoization.
The useMemo hook lists dummyEvents as a dependency (line 141), but dummyEvents is defined inside the component body and recreated on every render. This makes the memoization ineffective.
⚡ Recommended fix
Move dummyEvents outside the component to prevent recreation:
+// Example dummy events (outside component to prevent recreation)
+const DUMMY_EVENTS = [
+ {
+ startTime: '10:00',
+ endTime: '12:00',
+ startDate: '2025-12-10',
+ endDate: '2025-12-10',
+ eventName: 'React Workshop',
+ eventDescription: 'Learn React basics and advanced patterns.',
+ actionIcon: '⭐',
+ actionName: 'Join',
+ },
+ // ... rest of events
+];
+
const UserEvents: React.FC<PeopleTabUserEventsProps> = () => {
const { t } = useTranslation('translation', { keyPrefix: 'memberDetail' });
const [searchValue, setSearchValue] = useState('');
const [sortOption, setSortOption] = useState('Sort');
- // Example dummy events
- const dummyEvents = [
- // ...
- ];
// ===== Filter & Sort Before Rendering =====
const filteredEvents = useMemo(() => {
- let filtered = dummyEvents.filter(
+ let filtered = DUMMY_EVENTS.filter(
(event) =>
event.eventName.toLowerCase().includes(searchValue.toLowerCase()) ||
event.eventDescription
.toLowerCase()
.includes(searchValue.toLowerCase()),
);
filtered.sort((a, b) => {
const nameA = a.eventName.toLowerCase();
const nameB = b.eventName.toLowerCase();
return sortOption === 'ASC'
? nameA.localeCompare(nameB)
: nameB.localeCompare(nameA);
});
return filtered;
- }, [dummyEvents, searchValue, sortOption]);
+ }, [searchValue, sortOption]);🤖 Prompt for AI Agents
In @src/components/UserDetails/UserEvents.tsx around lines 123 - 141, The memo
for filteredEvents is ineffective because dummyEvents is recreated each render;
move the dummyEvents array out of the component (module-level constant) or
memoize it with useMemo/useRef so its identity is stable, then keep useMemo([...
, searchValue, sortOption]) for filteredEvents; update references to dummyEvents
inside the component and ensure only stable values are in the dependency list
for useMemo that computes filteredEvents (referencing filteredEvents,
dummyEvents, sortOption, searchValue).
Comment on lines
+143
to
+197
| return ( | ||
| <div> | ||
| <div className={styles.peopleTabUserEventsContainer}> | ||
| {/* ===== Page Header with Search & Sort ===== */} | ||
| <PeopleTabNavbar | ||
| search={{ | ||
| placeholder: 'Search created events', | ||
| onSearch: setSearchValue, | ||
| }} | ||
| sorting={[ | ||
| { | ||
| title: 'Sort By Name', | ||
| options: [ | ||
| { label: 'A → Z', value: 'ASC' }, | ||
| { label: 'Z → A', value: 'DESC' }, | ||
| ], | ||
| icon: '/images/svg/ri_arrow-up-down-line.svg', | ||
| selected: sortOption, | ||
| onChange: (value: string | number) => | ||
| setSortOption(value as 'ASC' | 'DESC'), | ||
| testIdPrefix: 'eventsSort', | ||
| }, | ||
| ]} | ||
| /> | ||
|
|
||
| <Card.Body className={`${styles.peoplePageUserEventCardBody}`}> | ||
| {filteredEvents.length === 0 ? ( | ||
| <div | ||
| className={`${styles.emptyContainer} w-100 h-100 d-flex justify-content-center align-items-center fw-semibold text-secondary`} | ||
| > | ||
| {t('noeventsAttended')} | ||
| </div> | ||
| ) : ( | ||
| filteredEvents.map((event, index) => ( | ||
| <PeopleTabUserEvents | ||
| key={index} | ||
| startTime={event.startTime} | ||
| endTime={event.endTime} | ||
| startDate={event.startDate} | ||
| endDate={event.endDate} | ||
| eventName={event.eventName} | ||
| eventDescription={event.eventDescription} | ||
| actionIcon={ | ||
| <IconButton size="small"> | ||
| <EditIcon /> | ||
| </IconButton> | ||
| } | ||
| actionName={'Edit'} | ||
| /> | ||
| )) | ||
| )} | ||
| </Card.Body> | ||
| </div> | ||
| </div> | ||
| ); |
Contributor
There was a problem hiding this comment.
Critical: Replace all hardcoded strings with i18n keys.
Multiple hardcoded user-facing strings violate i18n guidelines. All text must use translation keys.
🌐 Required i18n updates
Update component code:
<PeopleTabNavbar
search={{
- placeholder: 'Search created events',
+ placeholder: t('searchEventsPlaceholder'),
onSearch: setSearchValue,
}}
sorting={[
{
- title: 'Sort By Name',
+ title: t('sortByName'),
options: [
- { label: 'A → Z', value: 'ASC' },
- { label: 'Z → A', value: 'DESC' },
+ { label: t('sortAscending'), value: 'ASC' },
+ { label: t('sortDescending'), value: 'DESC' },
],
icon: '/images/svg/ri_arrow-up-down-line.svg',
selected: sortOption,
onChange: (value: string | number) =>
setSortOption(value as 'ASC' | 'DESC'),
testIdPrefix: 'eventsSort',
},
]}
/>
<Card.Body className={`${styles.peoplePageUserEventCardBody}`}>
{filteredEvents.length === 0 ? (
<div
className={`${styles.emptyContainer} w-100 h-100 d-flex justify-content-center align-items-center fw-semibold text-secondary`}
>
{t('noeventsAttended')}
</div>
) : (
filteredEvents.map((event, index) => (
<PeopleTabUserEvents
key={index}
startTime={event.startTime}
endTime={event.endTime}
startDate={event.startDate}
endDate={event.endDate}
eventName={event.eventName}
eventDescription={event.eventDescription}
actionIcon={
<IconButton size="small">
<EditIcon />
</IconButton>
}
- actionName={'Edit'}
+ actionName={t('edit')}
/>
))
)}Add to all locale files (public/locales/{en,es,fr,hi,zh}/translation.json):
{
"memberDetail": {
"searchEventsPlaceholder": "Search created events",
"sortByName": "Sort By Name",
"sortAscending": "A → Z",
"sortDescending": "Z → A",
"edit": "Edit",
"noeventsAttended": "No events attended"
}
}Per guidelines §2.3, zero hardcoded user-facing strings are allowed in UI components.
Comment on lines
+1
to
+273
| /** | ||
| * UserOrganizations | ||
| * | ||
| * Displays a list of organizations associated with a user. | ||
| * | ||
| * The list includes organizations the user has created, belongs to, | ||
| * or has joined. The component supports searching, sorting, | ||
| * and filtering to help users navigate large organization lists. | ||
| * | ||
| * Organization data is fetched via GraphQL queries and normalized | ||
| * into a unified structure before being filtered and rendered. | ||
| * | ||
| * @param props - Component props. | ||
| * Optional {@link MemberDetailProps.id | id} may be provided to fetch | ||
| * organizations for a specific user. If not provided, the ID is resolved | ||
| * from route state or local storage. | ||
| * | ||
| * @returns The rendered UserOrganizations component. | ||
| * | ||
| * @remarks | ||
| * - Uses Apollo Client queries to fetch user and organization data. | ||
| * - Merges created, belonging, and joined organizations into one list. | ||
| * - Removes duplicate organizations by ID. | ||
| * - Supports client-side search, sorting, and filtering by organization type. | ||
| * - Uses memoization to avoid unnecessary recalculations. | ||
| * - Integrates PeopleTabNavbar for search, sort, and filter controls. | ||
| * - Displays loading and empty states when applicable. | ||
| * | ||
| * @example | ||
| * ```tsx | ||
| * <UserOrganizations id="12345" /> | ||
| * ``` | ||
| */ | ||
| import React, { useMemo, useState } from 'react'; | ||
| import { useQuery } from '@apollo/client'; | ||
| import PeopleTabUserOrganizations from 'shared-components/PeopleTabUserOrganization/PeopleTabUserOrganizations'; | ||
| import PeopleTabNavbar from 'shared-components/PeopleTabNavbar/PeopleTabNavbar'; | ||
| import styles from 'style/app-fixed.module.css'; | ||
| import { IconButton } from '@mui/material'; | ||
| import EditIcon from '@mui/icons-material/Edit'; | ||
| import useLocalStorage from 'utils/useLocalstorage'; | ||
| import { useLocation } from 'react-router'; | ||
| import { | ||
| USER_JOINED_ORGANIZATIONS_NO_MEMBERS, | ||
| USER_DETAILS, | ||
| } from 'GraphQl/Queries/Queries'; | ||
| import { useTranslation } from 'react-i18next'; | ||
|
|
||
| type MemberDetailProps = { id?: string }; | ||
| type OrgRelationType = 'CREATED' | 'BELONG_TO' | 'JOINED'; | ||
|
|
||
| type UserOrg = { | ||
| id: string; | ||
| name: string; | ||
| relation: OrgRelationType; | ||
| adminsCount: number; | ||
| membersCount: number; | ||
| description?: string; | ||
| avatarURL?: string; | ||
| }; | ||
|
|
||
| interface InterfaceJoinedOrgEdge { | ||
| node: { | ||
| id: string; | ||
| name: string; | ||
| adminsCount: number; | ||
| membersCount: number; | ||
| description?: string; | ||
| avatarURL?: string; | ||
| }; | ||
| } | ||
|
|
||
| interface InterfaceJoinedOrganizationsData { | ||
| user: { | ||
| organizationsWhereMember?: { | ||
| edges?: InterfaceJoinedOrgEdge[]; | ||
| }; | ||
| }; | ||
| } | ||
|
|
||
| const UserOrganizations: React.FC<MemberDetailProps> = ({ | ||
| id, | ||
| }): JSX.Element => { | ||
| const { t: tCommon } = useTranslation('common'); | ||
| const [filterName] = useState(''); | ||
| const location = useLocation(); | ||
| const { getItem } = useLocalStorage(); | ||
| const [rowsPerPage] = React.useState(5); | ||
| const [orgFilter, setOrgFilter] = useState<'ALL' | OrgRelationType>('ALL'); | ||
|
|
||
| const currentId = location.state?.id || getItem('id') || id; | ||
|
|
||
| const { data: joinedOrganizationsData } = | ||
| useQuery<InterfaceJoinedOrganizationsData>( | ||
| USER_JOINED_ORGANIZATIONS_NO_MEMBERS, | ||
| { | ||
| variables: { id: currentId, first: rowsPerPage, filter: filterName }, | ||
| }, | ||
| ); | ||
|
|
||
| const { data: userData } = useQuery(USER_DETAILS, { | ||
| variables: { input: { id: currentId } }, | ||
| }); | ||
|
|
||
| const allUserOrgs: UserOrg[] = useMemo(() => { | ||
| const created: UserOrg[] = | ||
| userData?.user?.createdOrganizations?.map( | ||
| (org: { | ||
| id: string; | ||
| name: string; | ||
| adminsCount: number; | ||
| membersCount: number; | ||
| description?: string; | ||
| avatarURL?: string; | ||
| }) => ({ | ||
| id: org.id, | ||
| name: org.name, | ||
| relation: 'CREATED' as const, | ||
| adminsCount: org.adminsCount ?? 0, | ||
| membersCount: org.membersCount ?? 0, | ||
| description: org.description || 'No Description', | ||
| avatarURL: org.avatarURL || '', | ||
| }), | ||
| ) || []; | ||
|
|
||
| const belongTo: UserOrg[] = | ||
| userData?.user?.organizationsWhereMember?.edges?.map( | ||
| (edge: { | ||
| node: { | ||
| id: string; | ||
| name: string; | ||
| adminsCount: number; | ||
| membersCount: number; | ||
| description?: string; | ||
| avatarURL?: string; | ||
| }; | ||
| }) => ({ | ||
| id: edge.node.id, | ||
| name: edge.node.name, | ||
| relation: 'BELONG_TO' as const, | ||
| adminsCount: edge.node.adminsCount ?? 0, | ||
| membersCount: edge.node.membersCount ?? 0, | ||
| description: edge.node.description || 'No Description', | ||
| avatarURL: edge.node.avatarURL || '', | ||
| }), | ||
| ) || []; | ||
|
|
||
| const joined: UserOrg[] = | ||
| joinedOrganizationsData?.user?.organizationsWhereMember?.edges?.map( | ||
| (edge: { | ||
| node: { | ||
| id: string; | ||
| name: string; | ||
| adminsCount: number; | ||
| membersCount: number; | ||
| description?: string; | ||
| avatarURL?: string; | ||
| }; | ||
| }) => ({ | ||
| id: edge.node.id, | ||
| name: edge.node.name, | ||
| relation: 'JOINED' as const, | ||
| adminsCount: edge.node.adminsCount ?? 0, | ||
| membersCount: edge.node.membersCount ?? 0, | ||
| description: edge.node.description || 'No Description', | ||
| avatarURL: edge.node.avatarURL || '', | ||
| }), | ||
| ) || []; | ||
|
|
||
| // Merge and remove duplicates | ||
| const allOrgs = [...created, ...belongTo, ...joined]; | ||
| const uniqueOrgs = allOrgs.filter( | ||
| (org, index, self) => index === self.findIndex((o) => o.id === org.id), | ||
| ); | ||
|
|
||
| return uniqueOrgs; | ||
| }, [userData, joinedOrganizationsData]); | ||
|
|
||
| const [searchValue, setSearchValue] = useState(''); | ||
| const [sortOption, setSortOption] = useState<'ASC' | 'DESC'>('ASC'); | ||
|
|
||
| const filteredOrgs = useMemo(() => { | ||
| let list = [...allUserOrgs]; | ||
|
|
||
| if (searchValue) { | ||
| list = list.filter((org) => | ||
| org.name.toLowerCase().includes(searchValue.toLowerCase()), | ||
| ); | ||
| } | ||
|
|
||
| if (orgFilter !== 'ALL') { | ||
| list = list.filter((org) => org.relation === orgFilter); | ||
| } | ||
|
|
||
| list.sort((a, b) => { | ||
| const nameA = a.name.toLowerCase(); | ||
| const nameB = b.name.toLowerCase(); | ||
| return sortOption === 'ASC' | ||
| ? nameA.localeCompare(nameB) | ||
| : nameB.localeCompare(nameA); | ||
| }); | ||
|
|
||
| return list; | ||
| }, [allUserOrgs, searchValue, sortOption, orgFilter]); | ||
|
|
||
| return ( | ||
| <div className={styles.peopleTabUserOrganizationsContainer}> | ||
| {/* ===== Page Header with Search & Sort ===== */} | ||
| <PeopleTabNavbar | ||
| search={{ | ||
| placeholder: 'Search created organizations', | ||
| onSearch: setSearchValue, | ||
| }} | ||
| sorting={[ | ||
| { | ||
| title: 'Sort By Name', | ||
| options: [ | ||
| { label: 'A → Z', value: 'ASC' }, | ||
| { label: 'Z → A', value: 'DESC' }, | ||
| ], | ||
| icon: '/images/svg/ion_options-outline.svg', | ||
| selected: sortOption, | ||
| onChange: (value: string | number) => | ||
| setSortOption(value as 'ASC' | 'DESC'), | ||
| testIdPrefix: 'orgSort', | ||
| }, | ||
| { | ||
| title: 'Filter By Type', | ||
| options: [ | ||
| { label: 'All', value: 'ALL' }, | ||
| { label: 'Created Organizations', value: 'CREATED' }, | ||
| { label: 'Organizations user belong To', value: 'BELONG_TO' }, | ||
| { label: 'Joined Organizations', value: 'JOINED' }, | ||
| ], | ||
| icon: '/images/svg/ri_arrow-up-down-line.svg', | ||
| selected: orgFilter, | ||
| onChange: (value: string | number) => | ||
| setOrgFilter(value as 'ALL' | OrgRelationType), | ||
| testIdPrefix: 'orgFilter', | ||
| }, | ||
| ]} | ||
| /> | ||
|
|
||
| {/* ===== Organizations Grid ===== */} | ||
| <div className={styles.peopleTabUserOrganizationsGrid}> | ||
| {!userData?.user && !joinedOrganizationsData?.user ? ( | ||
| <p>{tCommon('loadingOrganizations')}</p> | ||
| ) : filteredOrgs.length === 0 ? ( | ||
| <p>{tCommon('noOrganizationsFound')}</p> | ||
| ) : ( | ||
| filteredOrgs.map((org) => ( | ||
| <PeopleTabUserOrganizations | ||
| key={org.id} | ||
| title={org.name} | ||
| description={org.description || 'No Description'} | ||
| adminCount={org.adminsCount} | ||
| membersCount={org.membersCount} | ||
| img={org.avatarURL || ''} | ||
| actionIcon={ | ||
| <IconButton size="small"> | ||
| <EditIcon /> | ||
| </IconButton> | ||
| } | ||
| actionName="Edit" | ||
| /> | ||
| )) | ||
| )} | ||
| </div> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| export default UserOrganizations; |
Contributor
There was a problem hiding this comment.
Critical: Component placement violates architecture policy.
This component is located at src/components/UserDetails/ which does not follow the required portal separation structure (§3.1). All components must be in AdminPortal/, UserPortal/, or shared-components/.
Based on context (member profile tabs in MemberDetail screen), this likely belongs in src/components/AdminPortal/UserOrganizations/.
📁 Recommended fix: Move to correct portal directory
# If admin-only:
mkdir -p src/components/AdminPortal/UserOrganizations
mv src/components/UserDetails/UserOrganizations.tsx src/components/AdminPortal/UserOrganizations/
mv src/components/UserDetails/UserOrganizations.spec.tsx src/components/AdminPortal/UserOrganizations/Update imports in MemberDetail.tsx:
-import UserOrganizations from 'components/UserDetails/UserOrganizations';
+import UserOrganizations from 'components/AdminPortal/UserOrganizations/UserOrganizations';
Comment on lines
578
to
599
| it('blocks update when password is invalid', async () => { | ||
| const { validatePassword } = await import('utils/passwordValidator'); | ||
| vi.mocked(validatePassword).mockReturnValue( | ||
| 'Password must be at least 8 characters', | ||
| ); | ||
|
|
||
| renderComponent(); | ||
|
|
||
| await screen.findByTestId('inputName'); | ||
|
|
||
| fireEvent.change(screen.getByTestId('inputPassword'), { | ||
| target: { value: '123' }, | ||
| }); | ||
|
|
||
| fireEvent.click(screen.getByTestId('saveChangesBtn')); | ||
|
|
||
| await waitFor(() => { | ||
| expect(NotificationToast.error).toHaveBeenCalledWith( | ||
| 'Password must be at least 8 characters', | ||
| ); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
Tests verify error toast but don't verify update is blocked.
The tests for invalid password (lines 578-599 and 385-413) verify that NotificationToast.error is called, but they don't verify that the mutation was NOT called. Given the critical bug in the component where password validation doesn't block the update, these tests give false confidence.
Add assertions to verify the mutation isn't called when validation fails:
await waitFor(() => {
expect(NotificationToast.error).toHaveBeenCalledWith(
'Password must be at least 8 characters',
);
});
+
+ // Verify mutation was NOT called
+ // This requires tracking mutation calls in the mockNote: This test will fail once the component bug is fixed (password validation should block the update). The test should be updated alongside the component fix.
🤖 Prompt for AI Agents
In @src/screens/MemberDetail/UserContactDetails.spec.tsx around lines 578 - 599,
The tests currently assert NotificationToast.error is called for invalid
passwords but do not assert the member update mutation was not invoked; add an
assertion that the mocked member update mutation (e.g., the updateMember.mutate
or updateMemberMutation mock returned by your useUpdateMember/useMutation hook)
was not called after clicking save by using
expect(<mockedMutation>).not.toHaveBeenCalled(); apply this additional assertion
to both failing-password tests (the one around lines 578-599 and the one around
385-413) immediately after the existing NotificationToast.error assertion.
| import DynamicDropDown from 'components/DynamicDropDown/DynamicDropDown'; | ||
| import { urlToFile } from 'utils/urlToFile'; | ||
| import { validatePassword } from 'utils/passwordValidator'; | ||
| type MemberDetailProps = { id?: string }; |
Contributor
There was a problem hiding this comment.
Props type should be defined in an interface file.
Per coding guidelines, component props must be defined in src/types/<Portal or shared-components>/<ComponentName>/interface.ts, not inline. The MemberDetailProps type should be moved to src/types/MemberDetail/interface.ts (or appropriate location) and named InterfaceMemberDetailProps.
Recommended structure
Create src/types/MemberDetail/interface.ts:
/**
* Props for UserContactDetails component.
*/
export interface InterfaceUserContactDetailsProps {
id?: string;
}Then import in the component:
-type MemberDetailProps = { id?: string };
-const UserContactDetails: React.FC<MemberDetailProps> = ({
+import type { InterfaceUserContactDetailsProps } from 'types/MemberDetail/interface';
+const UserContactDetails: React.FC<InterfaceUserContactDetailsProps> = ({🤖 Prompt for AI Agents
In @src/screens/MemberDetail/UserContactDetails.tsx at line 38, Move the inline
type MemberDetailProps into a dedicated interface file and update the component
to import it: create src/types/MemberDetail/interface.ts exporting export
interface InterfaceMemberDetailProps { id?: string; }, then in
UserContactDetails.tsx remove the inline type declaration and import
InterfaceMemberDetailProps and use it as the component props type (replace
MemberDetailProps with InterfaceMemberDetailProps wherever referenced, e.g., in
the UserContactDetails component signature).
Comment on lines
+1
to
20
| export const validatePassword = (password: string): string | null => { | ||
| if (password.length < 8) { | ||
| return 'Password must be at least 8 characters long.'; | ||
| } | ||
| if (!/[A-Z]/.test(password)) { | ||
| return 'Password must contain at least one uppercase letter.'; | ||
| } | ||
| if (!/[a-z]/.test(password)) { | ||
| return 'Password must contain at least one lowercase letter.'; | ||
| } | ||
| if (!/\d/.test(password)) { | ||
| return 'Password must contain at least one number.'; | ||
| } | ||
| if (!/[!@#$%^&*(),.?":{}|<>]/.test(password)) { | ||
| return 'Password must contain at least one special character.'; | ||
| } | ||
|
|
||
| // If all checks pass, return null (password is valid) | ||
| return null; | ||
| }; |
Contributor
There was a problem hiding this comment.
Critical: Replace hardcoded error messages with i18n keys.
The function returns user-facing error messages as hardcoded English strings, violating i18n guidelines. All error messages must use translation keys via useTranslation() or be returned as error codes that the UI translates.
Additionally, the exported function is missing TSDoc documentation.
🌐 Recommended approach: Return error codes instead of messages
Since this is a utility function (not a React component), it cannot use useTranslation() directly. Instead, return error codes that the calling component can translate:
+/**
+ * Validates a password against security requirements.
+ *
+ * @param password - The password string to validate.
+ * @returns An error code string if validation fails, or null if valid.
+ *
+ * @remarks
+ * Error codes returned:
+ * - 'PASSWORD_TOO_SHORT': Less than 8 characters
+ * - 'PASSWORD_NO_UPPERCASE': Missing uppercase letter
+ * - 'PASSWORD_NO_LOWERCASE': Missing lowercase letter
+ * - 'PASSWORD_NO_NUMBER': Missing number
+ * - 'PASSWORD_NO_SPECIAL': Missing special character
+ */
-export const validatePassword = (password: string): string | null => {
+export const validatePassword = (password: string): string | null => {
if (password.length < 8) {
- return 'Password must be at least 8 characters long.';
+ return 'PASSWORD_TOO_SHORT';
}
if (!/[A-Z]/.test(password)) {
- return 'Password must contain at least one uppercase letter.';
+ return 'PASSWORD_NO_UPPERCASE';
}
if (!/[a-z]/.test(password)) {
- return 'Password must contain at least one lowercase letter.';
+ return 'PASSWORD_NO_LOWERCASE';
}
if (!/\d/.test(password)) {
- return 'Password must contain at least one number.';
+ return 'PASSWORD_NO_NUMBER';
}
if (!/[!@#$%^&*(),.?":{}|<>]/.test(password)) {
- return 'Password must contain at least one special character.';
+ return 'PASSWORD_NO_SPECIAL';
}
// If all checks pass, return null (password is valid)
return null;
};Then in the calling component, translate these codes:
const errorCode = validatePassword(password);
if (errorCode) {
const errorMessage = t(`passwordValidation.${errorCode}`);
// Display errorMessage to user
}And add to translation files (public/locales/*/translation.json):
{
"passwordValidation": {
"PASSWORD_TOO_SHORT": "Password must be at least 8 characters long.",
"PASSWORD_NO_UPPERCASE": "Password must contain at least one uppercase letter.",
"PASSWORD_NO_LOWERCASE": "Password must contain at least one lowercase letter.",
"PASSWORD_NO_NUMBER": "Password must contain at least one number.",
"PASSWORD_NO_SPECIAL": "Password must contain at least one special character."
}
}🤖 Prompt for AI Agents
In @src/utils/passwordValidator.ts around lines 1 - 20, The validatePassword
function currently returns hardcoded English strings and lacks TSDoc; change it
to return standardized error codes (e.g. "PASSWORD_TOO_SHORT",
"PASSWORD_NO_UPPERCASE", "PASSWORD_NO_LOWERCASE", "PASSWORD_NO_NUMBER",
"PASSWORD_NO_SPECIAL") or null instead of message text, add TSDoc to the
exported validatePassword explaining return values and codes, and update callers
to call validatePassword(password) and pass the returned code to the i18n
translator (e.g. t(`passwordValidation.${code}`)); keep the existing regex
checks and function signature shape but replace each returned string with the
corresponding error code constant or literal.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 Fix all issues with AI agents
In @src/components/UserDetails/UserOrganizations.spec.tsx:
- Around line 133-212: The current complex mockUseQuery implementation inside
beforeEach should be extracted into a reusable helper to improve readability and
allow per-test customization; create a function (e.g.,
createMockUseQueryImplementation) that accepts optional overrides
(userDetailsData, joinedOrgsData) and returns the generic <TData, TVariables>
(query, options?) => { ... } implementation that checks query === USER_DETAILS
and query === USER_JOINED_ORGANIZATIONS_NO_MEMBERS and returns the appropriate
data/refetch/loading shape, then call
mockUseQuery.mockImplementation(createMockUseQueryImplementation()) in
beforeEach and override with createMockUseQueryImplementation(customData,
customJoinedData) in tests that need different responses.
- Around line 1-368: UserOrganizations component and its spec violate the portal
separation policy by living under the generic UserDetails area; move the
UserOrganizations component and UserOrganizations.spec.tsx (identify by
component name UserOrganizations and test file UserOrganizations.spec.tsx) into
the AdminPortal's UserDetails area and update imports accordingly, and relocate
the related interface (currently referenced as InterfacePeopleTabNavbarProps and
InterfacePeopleTabUserOrganizationProps from the PeopleTab interface) into the
AdminPortal types location and update any import paths to point to the new
interface module so the component and tests reference the AdminPortal-specific
interfaces.
- Around line 22-77: The mocks use the wrong field name for avatars; update all
occurrences of avatarUrl to avatarURL in the mock objects (specifically in
mockUserData within createdOrganizations and
organizationsWhereMember.edges[].node, and in
mockJoinedOrganizationsData.organizationsWhereMember.edges[].node) so the test
data matches the GraphQL schema and the component can render avatar images.
- Line 294: The test is asserting the translation key string instead of the
rendered translated text; update the test in UserOrganizations.spec.tsx so it
either mocks useTranslation to return the key (e.g., mock useTranslation to
return { t: (k)=>k, tCommon: (k)=>k }) making assertions like
expect(screen.getByText('noOrganizationsFound')).toBeInTheDocument(), or change
the assertions to check for the actual translated message produced by
tCommon('noOrganizationsFound') (e.g., expect(screen.getByText('No organizations
found.')).toBeInTheDocument()); locate usages of useTranslation and
tCommon('noOrganizationsFound') and adjust the test setup or expected string
accordingly.
- Around line 147-368: The component currently only destructures data from
useQuery and lacks error/loading handling; update the two useQuery calls to
destructure error and loading as well (e.g., const { data:
joinedOrganizationsData, error: joinedError, loading: joinedLoading } =
useQuery(USER_JOINED_ORGANIZATIONS_NO_MEMBERS, ... ) and const { data: userData,
error: userError, loading: userLoading } = useQuery(USER_DETAILS, ...)), add JSX
conditional rendering to show the loading UI when either joinedLoading or
userLoading is true and render an error state when joinedError or userError is
present (display a suitable error message like "errorLoadingOrganizations"), and
extend UserOrganizations.spec.tsx to mock useQuery to return loading: true and
assert "Loading organizations..." is shown, and to return error objects for
USER_DETAILS and USER_JOINED_ORGANIZATIONS_NO_MEMBERS separately and assert the
component renders the error UI.
In @src/screens/MemberDetail/MemberDetail.spec.tsx:
- Around line 52-73: The mock for PeopleTabNavbarButton in MemberDetail.spec.tsx
renders a <button> without an explicit type, causing it to default to "submit";
update the mock component (the default export function in the vi.mock block for
'shared-components/PeopleTabNavbarButton/PeopleTabNavbarButton') to add
type="button" to the rendered button element so it won't trigger form
submissions during tests.
- Around line 78-84: The test file registers vi.clearAllMocks() in both
beforeEach and afterEach which is redundant; remove the beforeEach block that
calls vi.clearAllMocks() and keep only the afterEach(() => { vi.clearAllMocks();
}) to satisfy ESLint and maintain test isolation, ensuring no other setup logic
in beforeEach is lost (if there is, only remove the vi.clearAllMocks() call from
beforeEach instead).
In @src/screens/MemberDetail/UserContactDetails.spec.tsx:
- Line 121: The mocked validatePassword currently returns true but the real
function returns string|null (null for valid, error message string for invalid);
update the mock in UserContactDetails.spec.tsx to vi.fn(() => null) to match the
actual signature, and keep the existing overrides in specific tests that return
error strings (they already use validatePassword.mockReturnValueOnce or similar)
so type checks and test behavior remain correct.
- Around line 289-308: Tests currently call vi.clearAllMocks() in both afterEach
and beforeEach, which is redundant; remove the vi.clearAllMocks() call from the
beforeEach so cleanup is centralized in afterEach, keeping the other beforeEach
setup (mocking window.location.reload via Object.defineProperty and
document.title) intact; update the beforeEach block to only perform the
window.location and document.title mocks and leave vi.clearAllMocks only in
afterEach.
In @src/screens/MemberDetail/UserContactDetails.tsx:
- Around line 1-7: Update the TSDoc for the UserContactDetails component to
fully document the props parameter and the return type: add a @param entry
describing the shape or type name of props (e.g., props:
UserContactDetailsProps) and include a @returns tag stating the component
returns a JSX.Element (or React.ReactElement), and ensure the comment sits
immediately above the UserContactDetails function/component declaration.
- Line 50: The component UserContactDetails currently sets document.title
directly in the render body; move that side-effect into a useEffect hook: remove
the direct assignment document.title = t('title') and add import/useEffect if
missing, then add useEffect(() => { document.title = t('title'); }, [t]) so the
title is set on mount and updates only when the translation function changes.
Ensure you reference the document.title change only inside the useEffect and
leave the rest of the component unchanged.
- Line 583: Replace the inline style object style={{ backgroundColor: '#A8C7FA',
color: '#555' }} in the JSX (in UserContactDetails.tsx) with Bootstrap utility
classes or a module CSS class: remove the style prop, add appropriate Bootstrap
classes (e.g., bg-*, text-*) that approximate the colors, or create a CSS module
rule (e.g., .customContactBg { background-color: #A8C7FA; color: #555; }) and
use that className on the same element to satisfy the CSS-Policy-Check; ensure
the element that currently uses the style prop is updated to reference the new
className.
- Around line 219-230: Replace the non-interactive <i> element used as a
clickable avatar edit control with a semantic <button> element: locate the
element that uses className including
styles.userContactDetailContactAvatarEditIcon, data-testid="uploadImageBtn",
title/aria-label set via tCommon('userEditProfilePicture') and handlers
onClick/onKeyDown that call fileInputRef.current?.click(); change it to a
visually equivalent <button> (preserving classes, data-testid, title,
aria-label, tabIndex, and the same onClick/onKeyDown behavior) so it no longer
relies on role="button" and improves accessibility while keeping fileInputRef
usage intact.
- Line 127: Replace the hardcoded message in the UserContactDetails component
with a t() lookup and add the key memberDetail.avatarProcessingFailed to all
locale translation files (public/locales/en/translation.json, es, fr, hi, zh)
containing the English string; update the component to call
t('memberDetail.avatarProcessingFailed') wherever the string "Failed to process
profile picture. Please try uploading again." is used so user-facing text is
fully internationalized.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (9)
docs/docs/auto-docs/shared-components/ProfileForm/ProfileForm/functions/getLanguageName.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/shared-components/ProfileForm/ProfileForm/variables/default.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/shared-components/ProfileForm/ProfileFormWrapper/variables/default.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/shared-components/ProfileForm/ProfileHeader/variables/default.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/shared-components/ProfileForm/profileForm.mock/variables/MOCKS1.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/shared-components/ProfileForm/profileForm.mock/variables/MOCKS2.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/shared-components/ProfileForm/profileForm.mock/variables/MOCK_FILE.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/shared-components/ProfileForm/profileForm.mock/variables/UPDATE_MOCK.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/shared-components/ProfileForm/profileForm.mock/variables/UPDATE_USER_ERROR_MOCKS.mdis excluded by!**/docs/docs/**
📒 Files selected for processing (14)
.github/workflows/config/countline_excluded_file_list.txtsrc/App.spec.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/components/UserDetails/UserTags.spec.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/screens/MemberDetail/UserContactDetails.tsxsrc/shared-components/ProfileForm/ProfileForm.spec.tsxsrc/shared-components/ProfileForm/ProfileForm.tsxsrc/shared-components/ProfileForm/ProfileFormWrapper.tsxsrc/shared-components/ProfileForm/ProfileHeader.spec.tsxsrc/shared-components/ProfileForm/ProfileHeader.tsxsrc/shared-components/ProfileForm/profileForm.mock.tssrc/shared-components/ProfileForm/profileForm.module.css
💤 Files with no reviewable changes (8)
- src/shared-components/ProfileForm/ProfileForm.tsx
- src/shared-components/ProfileForm/ProfileFormWrapper.tsx
- src/shared-components/ProfileForm/profileForm.mock.ts
- src/shared-components/ProfileForm/profileForm.module.css
- src/shared-components/ProfileForm/ProfileForm.spec.tsx
- src/shared-components/ProfileForm/ProfileHeader.spec.tsx
- src/shared-components/ProfileForm/ProfileHeader.tsx
- .github/workflows/config/countline_excluded_file_list.txt
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Test coverage must be ≥95% for modified files. Run: pnpm run test:coverage and verify coverage/coverage-summary.json shows no files below threshold
Files:
src/screens/MemberDetail/MemberDetail.spec.tsxsrc/components/UserDetails/UserTags.spec.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/App.spec.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Zero TypeScript errors: Run pnpm run typecheck and must pass without errors or warnings
Files:
src/screens/MemberDetail/MemberDetail.spec.tsxsrc/components/UserDetails/UserTags.spec.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/App.spec.tsxsrc/screens/MemberDetail/UserContactDetails.tsx
**/*
⚙️ CodeRabbit configuration file
**/*: Conduct a comprehensive review and reevaluate all resolved and dismissed items.VALIDATION FRAMEWORK
1. Test Coverage & Quality
1.1 Test File Requirements
For each modified source file with logic or UI:
- ✅ Test file exists:
*.spec.tsx,*.spec.ts,*.test.tsx, or*.test.ts- ✅ Test file colocated with component (same directory)
- ✅ Test file updated to reflect source changes
- ✅ Target ≥95% code coverage for new/modified code
- ✅ List specific line numbers lacking test coverage
1.2 Test Quality Standards
✅ Framework Compliance (Vitest + React Testing Library):
- Use
vi.mock()for mocking (notjest.mock())- Use
describe(),it(),test()for structure- Use
@testing-library/reactqueries (getByRole, getByText, etc.)- Use
@testing-library/user-eventfor interactions✅ Coverage Completeness - All scenarios tested:
- Success paths with valid data
- Error scenarios (network failures, GraphQL errors, validation errors)
- Edge cases (empty data, null/undefined, boundary values)
- Loading and pending states
- User interactions and state changes
✅ Mock Quality:
- GraphQL:
MockedProviderorStaticMockLink- Realistic, comprehensive mock data
- Proper cleanup (reset mocks, clear timers)
✅ Test Structure:
- Isolated tests (no execution order dependency)
- No disabled tests (
*.skip) without documented reason + linked issue- Proper async utilities (
waitFor,act)- Shard-compatible (12-shard strategy, no shared state, <30s timeout)
2. Code Quality & Implementation
2.1 TypeScript Standards
- ✅ No
anytype without JSDoc justification- ✅ Proper typing for:
- Component props (interface declarations)
- Function parameters and return types
- GraphQL variables and responses
- State and context
- ✅ No
@ts-ignorewithout explanation- ✅ Zero TypeScript compilation errors
2.2 React & GraphQL Implementation
- ✅ React Best Practices:
- Fu...
Files:
src/screens/MemberDetail/MemberDetail.spec.tsxsrc/components/UserDetails/UserTags.spec.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/App.spec.tsxsrc/screens/MemberDetail/UserContactDetails.tsx
src/**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
All components must follow reusable component policy: No inline interfaces, correct portal placement, wrapper usage
Files:
src/components/UserDetails/UserTags.spec.tsxsrc/components/UserDetails/UserOrganizations.spec.tsx
🧠 Learnings (8)
📚 Learning: 2025-12-21T08:59:37.942Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5287
File: src/shared-components/Recurrence/CustomRecurrenceModal.spec.tsx:78-84
Timestamp: 2025-12-21T08:59:37.942Z
Learning: In talawa-admin test files, ensure each spec file uses an explicit vi.clearAllMocks() in an afterEach block to guarantee test isolation. This should be present even if a global cleanup exists in vitest.setup.ts, as the linter enforces per-file hygiene. Apply this guideline to all spec files (e.g., src/**/*.spec.tsx) to satisfy ESLint/Vitest requirements and prevent bleed-over between tests.
Applied to files:
src/screens/MemberDetail/MemberDetail.spec.tsxsrc/components/UserDetails/UserTags.spec.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/App.spec.tsx
📚 Learning: 2025-12-27T11:22:41.191Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5782
File: src/screens/Leaderboard/Leaderboard.spec.tsx:342-370
Timestamp: 2025-12-27T11:22:41.191Z
Learning: In test files that render screens with shared components (e.g., src/screens/Leaderboard/Leaderboard.spec.tsx), verify integration behavior by asserting the screen renders correctly in different scenarios (e.g., search results) without asserting the internal ARIA attributes, text, or icons of the shared component. Rely on the shared component's own tests to cover accessibility and presentation details. This keeps tests focused on higher-level behavior and interactions.
Applied to files:
src/screens/MemberDetail/MemberDetail.spec.tsxsrc/components/UserDetails/UserTags.spec.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/App.spec.tsx
📚 Learning: 2026-01-08T18:48:10.364Z
Learnt from: abhishekblue
Repo: PalisadoesFoundation/talawa-admin PR: 6260
File: src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.spec.tsx:337-339
Timestamp: 2026-01-08T18:48:10.364Z
Learning: In tests that exercise Apollo Client queries (e.g., talawa-admin tests under src/screens/.../OrganizationPeople.spec.tsx), avoid using vi.useFakeTimers() because Apollo's internal promise resolution can be disrupted by mocked timers, causing timeouts. Use real timers with explicit waits (for example, await act(async () => { await new Promise(r => setTimeout(r, DURATION)); })) when testing components that fetch data via Apollo. If you must use timers, prefer real timers for the relevant test and flush promises deterministically. Replace or remove useFakeTimers in such tests and rely on actual time passage or proper promise flushing to ensure reliable Apollo-driven data fetching tests.
Applied to files:
src/screens/MemberDetail/MemberDetail.spec.tsxsrc/components/UserDetails/UserTags.spec.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/App.spec.tsx
📚 Learning: 2025-12-26T20:34:53.912Z
Learnt from: MuhammadUmar7195
Repo: PalisadoesFoundation/talawa-admin PR: 5847
File: src/screens/UserPortal/Volunteer/Invitations/Invitations.tsx:243-243
Timestamp: 2025-12-26T20:34:53.912Z
Learning: In PalisadoesFoundation/talawa-admin, whenever you modify a file (e.g., for migrations, tests, or feature changes), replace inline color attributes like color="grey" with Bootstrap utility classes (e.g., className="text-secondary") to satisfy the CSS-Policy-Check CI. This is a general compliance guideline, not scope creep, and should be applied to all affected TSX files during reviews. Steps: 1) Find inline color attributes in JSX/TSX. 2) Replace with appropriate Bootstrap text/color utility classes. 3) Ensure the visual outcome remains the same. 4) Run CI checks to confirm the policy passes.
Applied to files:
src/screens/MemberDetail/MemberDetail.spec.tsxsrc/components/UserDetails/UserTags.spec.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/App.spec.tsxsrc/screens/MemberDetail/UserContactDetails.tsx
📚 Learning: 2026-01-06T16:39:41.033Z
Learnt from: Jashan32
Repo: PalisadoesFoundation/talawa-admin PR: 6199
File: src/shared-components/ProfileForm/ProfileForm.tsx:0-0
Timestamp: 2026-01-06T16:39:41.033Z
Learning: When refactoring or consolidating components (e.g., talawa-admin), remove helper functions that were copied from old files but are not used in the new implementation. Before exporting utilities, verify there are real usages across the codebase (search usages, remove dead code, and keep only necessary exports) to improve maintainability and reduce bundle size.
Applied to files:
src/screens/MemberDetail/MemberDetail.spec.tsxsrc/components/UserDetails/UserTags.spec.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/App.spec.tsxsrc/screens/MemberDetail/UserContactDetails.tsx
📚 Learning: 2026-01-07T09:08:09.150Z
Learnt from: adityai0
Repo: PalisadoesFoundation/talawa-admin PR: 6202
File: src/components/EventManagement/EventAgendaItems/EventAgendaItems.tsx:4-13
Timestamp: 2026-01-07T09:08:09.150Z
Learning: In the talawa-admin repository, document comments with TSDoc in all TSX files. Do not use JSDoc. Write non-standard tags like component and requires as plain text (no leading @). Use standard TSDoc tags such as param, remarks, returns, and example with the leading @. This guideline applies to all TSX components (e.g., src/components/.../*.tsx).
Applied to files:
src/screens/MemberDetail/MemberDetail.spec.tsxsrc/components/UserDetails/UserTags.spec.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/App.spec.tsxsrc/screens/MemberDetail/UserContactDetails.tsx
📚 Learning: 2025-12-27T11:22:19.556Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5782
File: src/screens/Leaderboard/Leaderboard.spec.tsx:299-329
Timestamp: 2025-12-27T11:22:19.556Z
Learning: In talawa-admin test files (e.g., src/screens/Leaderboard/Leaderboard.spec.tsx), prefer separate explicit test cases for keyboard navigation interactions (Enter key, Space key) instead of consolidating them with test.each()/parameterization. This improves readability, documents each keyboard interaction path clearly, and makes failures easier to diagnose when specific key handling regresses. Apply this pattern to similarly structured spec files by keeping individual tests for each key interaction rather than combining them into a single parameterized test.
Applied to files:
src/screens/MemberDetail/MemberDetail.spec.tsxsrc/components/UserDetails/UserTags.spec.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/components/UserDetails/UserOrganizations.spec.tsxsrc/App.spec.tsx
📚 Learning: 2026-01-08T16:45:49.201Z
Learnt from: adityai0
Repo: PalisadoesFoundation/talawa-admin PR: 6241
File: src/components/AdminPortal/OrgPeopleListCard/OrgPeopleListCard.tsx:12-16
Timestamp: 2026-01-08T16:45:49.201Z
Learning: In TSX components, prefer flat JSDoc parameter documentation format (e.g., param id, param toggleRemoveModal) over the nested format (param props.id). The nested form can break editor parsing. When a component accepts a props object, destructure the props in the function signature to align with the flat JSDoc style (e.g., function MyComp({ id, toggleRemoveModal }: Props) { ... }). Ensure all param tags correspond to the destructured props and reflect actual component props to improve editor support and readability.
Applied to files:
src/components/UserDetails/UserTags.spec.tsxsrc/components/UserDetails/UserOrganizations.spec.tsx
🧬 Code graph analysis (3)
src/components/UserDetails/UserTags.spec.tsx (1)
src/types/PeopleTab/interface.ts (1)
InterfacePeopleTabNavbarProps(24-42)
src/screens/MemberDetail/UserContactDetails.spec.tsx (4)
src/GraphQl/Queries/Queries.ts (1)
GET_USER_BY_ID(452-477)src/GraphQl/Mutations/mutations.ts (1)
UPDATE_CURRENT_USER_MUTATION(66-96)src/utils/passwordValidator.ts (1)
validatePassword(1-20)src/components/NotificationToast/NotificationToast.tsx (1)
NotificationToast(76-82)
src/components/UserDetails/UserOrganizations.spec.tsx (2)
src/types/PeopleTab/interface.ts (2)
InterfacePeopleTabUserOrganizationProps(74-82)InterfacePeopleTabNavbarProps(24-42)src/GraphQl/Queries/Queries.ts (2)
USER_DETAILS(923-973)USER_JOINED_ORGANIZATIONS_NO_MEMBERS(107-123)
🪛 Biome (2.1.2)
src/screens/MemberDetail/MemberDetail.spec.tsx
[error] 64-68: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
src/screens/MemberDetail/UserContactDetails.tsx
[error] 224-224: The HTML element i is non-interactive and should not have an interactive role.
Replace i with a div or a span.
Unsafe fix: Remove the role attribute.
(lint/a11y/noNoninteractiveElementToInteractiveRole)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Performs linting, formatting, type-checking, unused file detection, checking for different source...
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (4)
src/App.spec.tsx (1)
54-56: Mock path and component location are correct—no changes needed.The component
MemberDetailis intentionally shared across multiple routes (/admin/profile,/member/:orgId/:userId, and/user/settings), so its placement atscreens/MemberDetail/MemberDetail.tsxis appropriate per §3.1 guidelines for shared components. The mock path'screens/MemberDetail/MemberDetail'correctly reflects this location.The testid
'mock-profile-form'is a legacy name from a prior component rename, but since this is an internal test mock used only within the test file, renaming it is optional and not required for functionality.Likely an incorrect or invalid review comment.
src/components/UserDetails/UserOrganizations.spec.tsx (1)
214-216: Proper test cleanup implementation.The explicit
vi.clearAllMocks()in theafterEachblock ensures proper test isolation, preventing mock bleed-over between tests. This follows the required pattern for talawa-admin spec files.Based on learnings about test isolation requirements.
src/screens/MemberDetail/UserContactDetails.spec.tsx (1)
1-806: Excellent test coverage and structure.This test suite demonstrates high-quality testing practices:
- Comprehensive coverage of success paths, error scenarios, and edge cases
- Proper use of
waitForfor async operations- Realistic mocks with MockedProvider for GraphQL
- Tests for accessibility interactions (keyboard navigation)
- Validation of state changes, form updates, and error handling
- Good organization with clear test descriptions
The tests align well with the ≥95% coverage target and follow Vitest + React Testing Library patterns.
src/screens/MemberDetail/UserContactDetails.tsx (1)
38-38: This component is a screen-level component and not subject to the reusable component architecture policy.UserContactDetails is located in
src/screens/MemberDetail/and is only used locally within MemberDetail.tsx. The reusable component architecture policy (guidelines §3) applies exclusively to components insrc/components/AdminPortal/,src/components/UserPortal/, andsrc/shared-components/directories. Screen-level components insrc/screens/are not required to follow the interface.ts pattern for prop definitions.Likely an incorrect or invalid review comment.
Comment on lines
+1
to
+368
| import { describe, it, expect, vi, beforeEach } from 'vitest'; | ||
| import { render, screen, fireEvent, waitFor } from '@testing-library/react'; | ||
| import UserOrganizations from './UserOrganizations'; | ||
| import { MemoryRouter } from 'react-router'; | ||
| import React from 'react'; | ||
| import { | ||
| InterfacePeopleTabNavbarProps, | ||
| InterfacePeopleTabUserOrganizationProps, | ||
| } from 'types/PeopleTab/interface'; | ||
| import { | ||
| USER_DETAILS, | ||
| USER_JOINED_ORGANIZATIONS_NO_MEMBERS, | ||
| } from 'GraphQl/Queries/Queries'; | ||
| import { DocumentNode } from 'graphql'; | ||
| import { OperationVariables } from '@apollo/client/core/types'; | ||
| import { QueryHookOptions } from '@apollo/client/react/types/types'; | ||
|
|
||
| // ---- DATA ---- // | ||
|
|
||
| const USER_ID = 'user-1'; | ||
|
|
||
| const mockUserData = { | ||
| user: { | ||
| __typename: 'User', | ||
| createdOrganizations: [ | ||
| { | ||
| __typename: 'Organization', | ||
| id: '1', | ||
| name: 'Created Org', | ||
| adminsCount: 1, | ||
| membersCount: 5, | ||
| description: 'Created organization description', | ||
| avatarUrl: 'https://example.com/avatar1.png', | ||
| }, | ||
| ], | ||
| organizationsWhereMember: { | ||
| __typename: 'OrganizationConnection', | ||
| edges: [ | ||
| { | ||
| __typename: 'OrganizationEdge', | ||
| node: { | ||
| __typename: 'Organization', | ||
| id: '2', | ||
| name: 'Belong Org', | ||
| adminsCount: 2, | ||
| membersCount: 3, | ||
| description: 'Belong organization description', | ||
| avatarUrl: 'https://example.com/avatar2.png', | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| const mockJoinedOrganizationsData = { | ||
| user: { | ||
| __typename: 'User', | ||
| organizationsWhereMember: { | ||
| __typename: 'OrganizationConnection', | ||
| edges: [ | ||
| { | ||
| __typename: 'OrganizationEdge', | ||
| node: { | ||
| __typename: 'Organization', | ||
| id: '3', | ||
| name: 'Joined Org', | ||
| adminsCount: 1, | ||
| membersCount: 4, | ||
| description: 'Joined organization description', | ||
| avatarUrl: 'https://example.com/avatar3.png', | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| // ---- MOCKS ---- // | ||
|
|
||
| vi.mock('utils/useLocalstorage', () => ({ | ||
| default: () => ({ | ||
| getItem: vi.fn(() => 'user-1'), | ||
| }), | ||
| })); | ||
|
|
||
| vi.mock( | ||
| 'shared-components/PeopleTabUserOrganizations/PeopleTabUserOrganizations', | ||
| () => ({ | ||
| default: (props: InterfacePeopleTabUserOrganizationProps) => ( | ||
| <div data-testid="org-card"> | ||
| <h3>{props.title}</h3> | ||
| <p>{props.description}</p> | ||
| </div> | ||
| ), | ||
| }), | ||
| ); | ||
|
|
||
| vi.mock('shared-components/PeopleTabNavbar/PeopleTabNavbar', () => ({ | ||
| default: (props: InterfacePeopleTabNavbarProps) => ( | ||
| <div> | ||
| {props.search && ( | ||
| <input | ||
| data-testid={props.search.inputTestId ?? 'search-input'} | ||
| placeholder={props.search.placeholder} | ||
| onChange={(e: React.ChangeEvent<HTMLInputElement>) => | ||
| props.search?.onSearch(e.target.value) | ||
| } | ||
| /> | ||
| )} | ||
| {props.sorting && | ||
| props.sorting.map((s) => ( | ||
| <select | ||
| key={s.title} | ||
| data-testid={s.testIdPrefix} | ||
| value={s.selected} | ||
| onChange={(e: React.ChangeEvent<HTMLSelectElement>) => | ||
| s.onChange(e.target.value) | ||
| } | ||
| > | ||
| {s.options.map((o) => ( | ||
| <option key={o.value} value={o.value}> | ||
| {o.label} | ||
| </option> | ||
| ))} | ||
| </select> | ||
| ))} | ||
| </div> | ||
| ), | ||
| })); | ||
|
|
||
| // Mock @apollo/client with factory function | ||
| vi.mock('@apollo/client', async (importOriginal) => { | ||
| const actual = await importOriginal<typeof import('@apollo/client')>(); | ||
|
|
||
| // Create mock function inside the factory | ||
| const mockUseQuery = vi.fn(); | ||
|
|
||
| return { | ||
| ...actual, | ||
| useQuery: mockUseQuery, | ||
| }; | ||
| }); | ||
|
|
||
| // ---- TESTS ---- // | ||
|
|
||
| describe('UserOrganizations', () => { | ||
| let mockUseQuery: ReturnType<typeof vi.fn>; | ||
|
|
||
| beforeEach(async () => { | ||
| vi.clearAllMocks(); | ||
|
|
||
| // Get the mocked useQuery function | ||
| const apolloClient = await import('@apollo/client'); | ||
| mockUseQuery = apolloClient.useQuery as ReturnType<typeof vi.fn>; | ||
|
|
||
| // Define interface for query variables | ||
| interface InterfaceUserDetailsVariables { | ||
| input: { | ||
| id: string; | ||
| }; | ||
| } | ||
|
|
||
| interface InterfaceJoinedOrgsVariables { | ||
| id: string; | ||
| first: number; | ||
| filter: string; | ||
| } | ||
|
|
||
| // Default mock implementation with proper typing | ||
| mockUseQuery.mockImplementation( | ||
| <TData, TVariables extends OperationVariables>( | ||
| query: DocumentNode, | ||
| options?: QueryHookOptions<TData, TVariables>, | ||
| ) => { | ||
| if (query === USER_DETAILS) { | ||
| const vars = options?.variables as | ||
| | InterfaceUserDetailsVariables | ||
| | undefined; | ||
| if (vars?.input?.id === USER_ID) { | ||
| return { | ||
| data: mockUserData, | ||
| loading: false, | ||
| error: undefined, | ||
| refetch: vi.fn(), | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| if (query === USER_JOINED_ORGANIZATIONS_NO_MEMBERS) { | ||
| const vars = options?.variables as | ||
| | InterfaceJoinedOrgsVariables | ||
| | undefined; | ||
| if (vars?.id === USER_ID) { | ||
| return { | ||
| data: mockJoinedOrganizationsData, | ||
| loading: false, | ||
| error: undefined, | ||
| refetch: vi.fn(), | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| data: undefined, | ||
| loading: false, | ||
| error: undefined, | ||
| refetch: vi.fn(), | ||
| }; | ||
| }, | ||
| ); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| const renderComponent = () => | ||
| render( | ||
| <MemoryRouter> | ||
| <UserOrganizations /> | ||
| </MemoryRouter>, | ||
| ); | ||
|
|
||
| it('renders all organization types', async () => { | ||
| renderComponent(); | ||
|
|
||
| // Wait for loading to complete | ||
| await waitFor(() => { | ||
| expect( | ||
| screen.queryByText('Loading organizations...'), | ||
| ).not.toBeInTheDocument(); | ||
| }); | ||
|
|
||
| // Check all organizations are rendered | ||
| await waitFor(() => { | ||
| expect(screen.getByText('Created Org')).toBeInTheDocument(); | ||
| expect(screen.getByText('Belong Org')).toBeInTheDocument(); | ||
| expect(screen.getByText('Joined Org')).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
|
||
| it('filters organizations by search', async () => { | ||
| renderComponent(); | ||
|
|
||
| // Wait for data to load | ||
| await waitFor(() => { | ||
| expect(screen.getByText('Created Org')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| fireEvent.change(screen.getByTestId('search-input'), { | ||
| target: { value: 'created' }, | ||
| }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(screen.getByText('Created Org')).toBeInTheDocument(); | ||
| expect(screen.queryByText('Belong Org')).not.toBeInTheDocument(); | ||
| expect(screen.queryByText('Joined Org')).not.toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
|
||
| it('filters organizations by type', async () => { | ||
| renderComponent(); | ||
|
|
||
| // Wait for data to load | ||
| await waitFor(() => { | ||
| expect(screen.getByText('Joined Org')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| fireEvent.change(screen.getByTestId('orgFilter'), { | ||
| target: { value: 'JOINED' }, | ||
| }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(screen.getByText('Joined Org')).toBeInTheDocument(); | ||
| expect(screen.queryByText('Created Org')).not.toBeInTheDocument(); | ||
| expect(screen.queryByText('Belong Org')).not.toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
|
||
| it('shows empty state when no orgs match filter', async () => { | ||
| renderComponent(); | ||
|
|
||
| // Wait for data to load | ||
| await waitFor(() => { | ||
| expect(screen.getByText('Created Org')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| fireEvent.change(screen.getByTestId('search-input'), { | ||
| target: { value: 'xyz' }, | ||
| }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(screen.getByText('noOrganizationsFound')).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
|
||
| it('handles missing user data safely', async () => { | ||
| // Mock empty data - return empty objects instead of null user | ||
| mockUseQuery.mockImplementation((query: DocumentNode) => { | ||
| if (query === USER_DETAILS) { | ||
| return { | ||
| data: { | ||
| user: { | ||
| createdOrganizations: [], | ||
| organizationsWhereMember: { edges: [] }, | ||
| }, | ||
| }, | ||
| loading: false, | ||
| error: undefined, | ||
| refetch: vi.fn(), | ||
| }; | ||
| } | ||
| if (query === USER_JOINED_ORGANIZATIONS_NO_MEMBERS) { | ||
| return { | ||
| data: { | ||
| user: { | ||
| organizationsWhereMember: { edges: [] }, | ||
| }, | ||
| }, | ||
| loading: false, | ||
| error: undefined, | ||
| refetch: vi.fn(), | ||
| }; | ||
| } | ||
| return { | ||
| data: undefined, | ||
| loading: false, | ||
| error: undefined, | ||
| refetch: vi.fn(), | ||
| }; | ||
| }); | ||
|
|
||
| renderComponent(); | ||
|
|
||
| // Wait for the empty state | ||
| await waitFor(() => { | ||
| expect(screen.getByText('noOrganizationsFound')).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
|
||
| it('changes sort order when sort option is changed', async () => { | ||
| renderComponent(); | ||
|
|
||
| await waitFor(() => { | ||
| const orgsAsc = screen.getAllByRole('heading', { level: 3 }); | ||
|
|
||
| expect(orgsAsc).toHaveLength(3); | ||
| // Default sort is ASC, so order should be: Belong, Created, Joined | ||
| expect(orgsAsc[0]).toHaveTextContent('Belong Org'); | ||
| expect(orgsAsc[1]).toHaveTextContent('Created Org'); | ||
| expect(orgsAsc[2]).toHaveTextContent('Joined Org'); | ||
| }); | ||
|
|
||
| fireEvent.change(screen.getByTestId('orgSort'), { | ||
| target: { value: 'DESC' }, | ||
| }); | ||
|
|
||
| await waitFor(() => { | ||
| const orgsDesc = screen.getAllByRole('heading', { level: 3 }); | ||
| expect(orgsDesc).toHaveLength(3); | ||
| // DESC sort order should be: Joined, Created, Belong | ||
| expect(orgsDesc[0]).toHaveTextContent('Joined Org'); | ||
| expect(orgsDesc[1]).toHaveTextContent('Created Org'); | ||
| expect(orgsDesc[2]).toHaveTextContent('Belong Org'); | ||
| }); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
Critical architecture violation: Component location violates reusable component policy.
The test file and its component are located at src/components/UserDetails/, which does not follow the required portal separation structure defined in §3.1 of the coding guidelines.
Required directory structure:
- Admin-only:
src/components/AdminPortal/** - User-only:
src/components/UserPortal/** - Shared (both portals):
src/shared-components/**
Based on the PR context (People Tab feature for viewing user profiles in the admin portal), this component should be relocated to:
src/components/AdminPortal/UserDetails/UserOrganizations.tsxsrc/components/AdminPortal/UserDetails/UserOrganizations.spec.tsx
Additionally, the corresponding interface should be moved from types/PeopleTab/interface.ts to src/types/AdminPortal/UserOrganizations/interface.ts to follow the src/types/<Portal>/<ComponentName>/interface.ts pattern per §3.3.
This is a fundamental architectural requirement that affects maintainability and must be corrected before approval.
As per coding guidelines §3 (Reusable Component Architecture).
🤖 Prompt for AI Agents
In @src/components/UserDetails/UserOrganizations.spec.tsx around lines 1 - 368,
UserOrganizations component and its spec violate the portal separation policy by
living under the generic UserDetails area; move the UserOrganizations component
and UserOrganizations.spec.tsx (identify by component name UserOrganizations and
test file UserOrganizations.spec.tsx) into the AdminPortal's UserDetails area
and update imports accordingly, and relocate the related interface (currently
referenced as InterfacePeopleTabNavbarProps and
InterfacePeopleTabUserOrganizationProps from the PeopleTab interface) into the
AdminPortal types location and update any import paths to point to the new
interface module so the component and tests reference the AdminPortal-specific
interfaces.
Comment on lines
+22
to
+77
| const mockUserData = { | ||
| user: { | ||
| __typename: 'User', | ||
| createdOrganizations: [ | ||
| { | ||
| __typename: 'Organization', | ||
| id: '1', | ||
| name: 'Created Org', | ||
| adminsCount: 1, | ||
| membersCount: 5, | ||
| description: 'Created organization description', | ||
| avatarUrl: 'https://example.com/avatar1.png', | ||
| }, | ||
| ], | ||
| organizationsWhereMember: { | ||
| __typename: 'OrganizationConnection', | ||
| edges: [ | ||
| { | ||
| __typename: 'OrganizationEdge', | ||
| node: { | ||
| __typename: 'Organization', | ||
| id: '2', | ||
| name: 'Belong Org', | ||
| adminsCount: 2, | ||
| membersCount: 3, | ||
| description: 'Belong organization description', | ||
| avatarUrl: 'https://example.com/avatar2.png', | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| const mockJoinedOrganizationsData = { | ||
| user: { | ||
| __typename: 'User', | ||
| organizationsWhereMember: { | ||
| __typename: 'OrganizationConnection', | ||
| edges: [ | ||
| { | ||
| __typename: 'OrganizationEdge', | ||
| node: { | ||
| __typename: 'Organization', | ||
| id: '3', | ||
| name: 'Joined Org', | ||
| adminsCount: 1, | ||
| membersCount: 4, | ||
| description: 'Joined organization description', | ||
| avatarUrl: 'https://example.com/avatar3.png', | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| }; |
Contributor
There was a problem hiding this comment.
Fix field name inconsistency: avatarUrl should be avatarURL.
The mock data uses avatarUrl (lowercase 'url') on lines 33, 48, and 71, but the GraphQL schema in src/GraphQl/Queries/Queries.ts defines this field as avatarURL (capital 'URL'). This mismatch will cause the component to fail to display avatar images when connected to the real API.
🔧 Proposed fix
description: 'Created organization description',
- avatarUrl: 'https://example.com/avatar1.png',
+ avatarURL: 'https://example.com/avatar1.png',
},
],
organizationsWhereMember: {
...
description: 'Belong organization description',
- avatarUrl: 'https://example.com/avatar2.png',
+ avatarURL: 'https://example.com/avatar2.png',
},
},
],
...
description: 'Joined organization description',
- avatarUrl: 'https://example.com/avatar3.png',
+ avatarURL: 'https://example.com/avatar3.png',
},
},
],🤖 Prompt for AI Agents
In @src/components/UserDetails/UserOrganizations.spec.tsx around lines 22 - 77,
The mocks use the wrong field name for avatars; update all occurrences of
avatarUrl to avatarURL in the mock objects (specifically in mockUserData within
createdOrganizations and organizationsWhereMember.edges[].node, and in
mockJoinedOrganizationsData.organizationsWhereMember.edges[].node) so the test
data matches the GraphQL schema and the component can render avatar images.
Comment on lines
+133
to
+212
| vi.mock('@apollo/client', async (importOriginal) => { | ||
| const actual = await importOriginal<typeof import('@apollo/client')>(); | ||
|
|
||
| // Create mock function inside the factory | ||
| const mockUseQuery = vi.fn(); | ||
|
|
||
| return { | ||
| ...actual, | ||
| useQuery: mockUseQuery, | ||
| }; | ||
| }); | ||
|
|
||
| // ---- TESTS ---- // | ||
|
|
||
| describe('UserOrganizations', () => { | ||
| let mockUseQuery: ReturnType<typeof vi.fn>; | ||
|
|
||
| beforeEach(async () => { | ||
| vi.clearAllMocks(); | ||
|
|
||
| // Get the mocked useQuery function | ||
| const apolloClient = await import('@apollo/client'); | ||
| mockUseQuery = apolloClient.useQuery as ReturnType<typeof vi.fn>; | ||
|
|
||
| // Define interface for query variables | ||
| interface InterfaceUserDetailsVariables { | ||
| input: { | ||
| id: string; | ||
| }; | ||
| } | ||
|
|
||
| interface InterfaceJoinedOrgsVariables { | ||
| id: string; | ||
| first: number; | ||
| filter: string; | ||
| } | ||
|
|
||
| // Default mock implementation with proper typing | ||
| mockUseQuery.mockImplementation( | ||
| <TData, TVariables extends OperationVariables>( | ||
| query: DocumentNode, | ||
| options?: QueryHookOptions<TData, TVariables>, | ||
| ) => { | ||
| if (query === USER_DETAILS) { | ||
| const vars = options?.variables as | ||
| | InterfaceUserDetailsVariables | ||
| | undefined; | ||
| if (vars?.input?.id === USER_ID) { | ||
| return { | ||
| data: mockUserData, | ||
| loading: false, | ||
| error: undefined, | ||
| refetch: vi.fn(), | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| if (query === USER_JOINED_ORGANIZATIONS_NO_MEMBERS) { | ||
| const vars = options?.variables as | ||
| | InterfaceJoinedOrgsVariables | ||
| | undefined; | ||
| if (vars?.id === USER_ID) { | ||
| return { | ||
| data: mockJoinedOrganizationsData, | ||
| loading: false, | ||
| error: undefined, | ||
| refetch: vi.fn(), | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| data: undefined, | ||
| loading: false, | ||
| error: undefined, | ||
| refetch: vi.fn(), | ||
| }; | ||
| }, | ||
| ); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting query mock logic into a helper function.
The Apollo useQuery mock implementation (lines 171-211) is sophisticated but complex. To improve test maintainability and readability, consider extracting the mock logic into a dedicated helper function.
♻️ Proposed refactor
// Helper function at the top of the test file
const createMockUseQueryImplementation = (
userDetailsData = mockUserData,
joinedOrgsData = mockJoinedOrganizationsData,
) => {
return <TData, TVariables extends OperationVariables>(
query: DocumentNode,
options?: QueryHookOptions<TData, TVariables>,
) => {
if (query === USER_DETAILS) {
const vars = options?.variables as InterfaceUserDetailsVariables | undefined;
if (vars?.input?.id === USER_ID) {
return {
data: userDetailsData,
loading: false,
error: undefined,
refetch: vi.fn(),
};
}
}
if (query === USER_JOINED_ORGANIZATIONS_NO_MEMBERS) {
const vars = options?.variables as InterfaceJoinedOrgsVariables | undefined;
if (vars?.id === USER_ID) {
return {
data: joinedOrgsData,
loading: false,
error: undefined,
refetch: vi.fn(),
};
}
}
return {
data: undefined,
loading: false,
error: undefined,
refetch: vi.fn(),
};
};
};
// In beforeEach:
mockUseQuery.mockImplementation(createMockUseQueryImplementation());
// In individual tests that need different data:
mockUseQuery.mockImplementation(createMockUseQueryImplementation(customData, customJoinedData));This makes it easier to customize mock data per test while keeping the logic reusable.
🤖 Prompt for AI Agents
In @src/components/UserDetails/UserOrganizations.spec.tsx around lines 133 -
212, The current complex mockUseQuery implementation inside beforeEach should be
extracted into a reusable helper to improve readability and allow per-test
customization; create a function (e.g., createMockUseQueryImplementation) that
accepts optional overrides (userDetailsData, joinedOrgsData) and returns the
generic <TData, TVariables> (query, options?) => { ... } implementation that
checks query === USER_DETAILS and query === USER_JOINED_ORGANIZATIONS_NO_MEMBERS
and returns the appropriate data/refetch/loading shape, then call
mockUseQuery.mockImplementation(createMockUseQueryImplementation()) in
beforeEach and override with createMockUseQueryImplementation(customData,
customJoinedData) in tests that need different responses.
Comment on lines
+147
to
+368
| describe('UserOrganizations', () => { | ||
| let mockUseQuery: ReturnType<typeof vi.fn>; | ||
|
|
||
| beforeEach(async () => { | ||
| vi.clearAllMocks(); | ||
|
|
||
| // Get the mocked useQuery function | ||
| const apolloClient = await import('@apollo/client'); | ||
| mockUseQuery = apolloClient.useQuery as ReturnType<typeof vi.fn>; | ||
|
|
||
| // Define interface for query variables | ||
| interface InterfaceUserDetailsVariables { | ||
| input: { | ||
| id: string; | ||
| }; | ||
| } | ||
|
|
||
| interface InterfaceJoinedOrgsVariables { | ||
| id: string; | ||
| first: number; | ||
| filter: string; | ||
| } | ||
|
|
||
| // Default mock implementation with proper typing | ||
| mockUseQuery.mockImplementation( | ||
| <TData, TVariables extends OperationVariables>( | ||
| query: DocumentNode, | ||
| options?: QueryHookOptions<TData, TVariables>, | ||
| ) => { | ||
| if (query === USER_DETAILS) { | ||
| const vars = options?.variables as | ||
| | InterfaceUserDetailsVariables | ||
| | undefined; | ||
| if (vars?.input?.id === USER_ID) { | ||
| return { | ||
| data: mockUserData, | ||
| loading: false, | ||
| error: undefined, | ||
| refetch: vi.fn(), | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| if (query === USER_JOINED_ORGANIZATIONS_NO_MEMBERS) { | ||
| const vars = options?.variables as | ||
| | InterfaceJoinedOrgsVariables | ||
| | undefined; | ||
| if (vars?.id === USER_ID) { | ||
| return { | ||
| data: mockJoinedOrganizationsData, | ||
| loading: false, | ||
| error: undefined, | ||
| refetch: vi.fn(), | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| data: undefined, | ||
| loading: false, | ||
| error: undefined, | ||
| refetch: vi.fn(), | ||
| }; | ||
| }, | ||
| ); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| const renderComponent = () => | ||
| render( | ||
| <MemoryRouter> | ||
| <UserOrganizations /> | ||
| </MemoryRouter>, | ||
| ); | ||
|
|
||
| it('renders all organization types', async () => { | ||
| renderComponent(); | ||
|
|
||
| // Wait for loading to complete | ||
| await waitFor(() => { | ||
| expect( | ||
| screen.queryByText('Loading organizations...'), | ||
| ).not.toBeInTheDocument(); | ||
| }); | ||
|
|
||
| // Check all organizations are rendered | ||
| await waitFor(() => { | ||
| expect(screen.getByText('Created Org')).toBeInTheDocument(); | ||
| expect(screen.getByText('Belong Org')).toBeInTheDocument(); | ||
| expect(screen.getByText('Joined Org')).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
|
||
| it('filters organizations by search', async () => { | ||
| renderComponent(); | ||
|
|
||
| // Wait for data to load | ||
| await waitFor(() => { | ||
| expect(screen.getByText('Created Org')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| fireEvent.change(screen.getByTestId('search-input'), { | ||
| target: { value: 'created' }, | ||
| }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(screen.getByText('Created Org')).toBeInTheDocument(); | ||
| expect(screen.queryByText('Belong Org')).not.toBeInTheDocument(); | ||
| expect(screen.queryByText('Joined Org')).not.toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
|
||
| it('filters organizations by type', async () => { | ||
| renderComponent(); | ||
|
|
||
| // Wait for data to load | ||
| await waitFor(() => { | ||
| expect(screen.getByText('Joined Org')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| fireEvent.change(screen.getByTestId('orgFilter'), { | ||
| target: { value: 'JOINED' }, | ||
| }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(screen.getByText('Joined Org')).toBeInTheDocument(); | ||
| expect(screen.queryByText('Created Org')).not.toBeInTheDocument(); | ||
| expect(screen.queryByText('Belong Org')).not.toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
|
||
| it('shows empty state when no orgs match filter', async () => { | ||
| renderComponent(); | ||
|
|
||
| // Wait for data to load | ||
| await waitFor(() => { | ||
| expect(screen.getByText('Created Org')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| fireEvent.change(screen.getByTestId('search-input'), { | ||
| target: { value: 'xyz' }, | ||
| }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(screen.getByText('noOrganizationsFound')).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
|
||
| it('handles missing user data safely', async () => { | ||
| // Mock empty data - return empty objects instead of null user | ||
| mockUseQuery.mockImplementation((query: DocumentNode) => { | ||
| if (query === USER_DETAILS) { | ||
| return { | ||
| data: { | ||
| user: { | ||
| createdOrganizations: [], | ||
| organizationsWhereMember: { edges: [] }, | ||
| }, | ||
| }, | ||
| loading: false, | ||
| error: undefined, | ||
| refetch: vi.fn(), | ||
| }; | ||
| } | ||
| if (query === USER_JOINED_ORGANIZATIONS_NO_MEMBERS) { | ||
| return { | ||
| data: { | ||
| user: { | ||
| organizationsWhereMember: { edges: [] }, | ||
| }, | ||
| }, | ||
| loading: false, | ||
| error: undefined, | ||
| refetch: vi.fn(), | ||
| }; | ||
| } | ||
| return { | ||
| data: undefined, | ||
| loading: false, | ||
| error: undefined, | ||
| refetch: vi.fn(), | ||
| }; | ||
| }); | ||
|
|
||
| renderComponent(); | ||
|
|
||
| // Wait for the empty state | ||
| await waitFor(() => { | ||
| expect(screen.getByText('noOrganizationsFound')).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
|
||
| it('changes sort order when sort option is changed', async () => { | ||
| renderComponent(); | ||
|
|
||
| await waitFor(() => { | ||
| const orgsAsc = screen.getAllByRole('heading', { level: 3 }); | ||
|
|
||
| expect(orgsAsc).toHaveLength(3); | ||
| // Default sort is ASC, so order should be: Belong, Created, Joined | ||
| expect(orgsAsc[0]).toHaveTextContent('Belong Org'); | ||
| expect(orgsAsc[1]).toHaveTextContent('Created Org'); | ||
| expect(orgsAsc[2]).toHaveTextContent('Joined Org'); | ||
| }); | ||
|
|
||
| fireEvent.change(screen.getByTestId('orgSort'), { | ||
| target: { value: 'DESC' }, | ||
| }); | ||
|
|
||
| await waitFor(() => { | ||
| const orgsDesc = screen.getAllByRole('heading', { level: 3 }); | ||
| expect(orgsDesc).toHaveLength(3); | ||
| // DESC sort order should be: Joined, Created, Belong | ||
| expect(orgsDesc[0]).toHaveTextContent('Joined Org'); | ||
| expect(orgsDesc[1]).toHaveTextContent('Created Org'); | ||
| expect(orgsDesc[2]).toHaveTextContent('Belong Org'); | ||
| }); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and examine the UserOrganizations component
find . -name "UserOrganizations.tsx" -not -path "*/node_modules/*" -not -path "*/.git/*" | head -5Repository: PalisadoesFoundation/talawa-admin
Length of output: 127
🏁 Script executed:
#!/bin/bash
# Get more context about the component implementation
if [ -f "src/components/UserDetails/UserOrganizations.tsx" ]; then
echo "=== Component file exists ==="
wc -l src/components/UserDetails/UserOrganizations.tsx
echo "=== Checking for loading state rendering ==="
rg -n "loading" src/components/UserDetails/UserOrganizations.tsx | head -10
echo "=== Checking for error state rendering ==="
rg -n "error" src/components/UserDetails/UserOrganizations.tsx | head -10
fiRepository: PalisadoesFoundation/talawa-admin
Length of output: 361
🏁 Script executed:
#!/bin/bash
# Check test file to see what assertions exist for loading/error
if [ -f "src/components/UserDetails/UserOrganizations.spec.tsx" ]; then
echo "=== Test file size ==="
wc -l src/components/UserDetails/UserOrganizations.spec.tsx
echo "=== Search for loading assertions in test ==="
rg -n "loading\|Loading" src/components/UserDetails/UserOrganizations.spec.tsx
echo "=== Search for error assertions in test ==="
rg -n "error\|Error" src/components/UserDetails/UserOrganizations.spec.tsx
fiRepository: PalisadoesFoundation/talawa-admin
Length of output: 247
🏁 Script executed:
#!/bin/bash
# Read the UserOrganizations component to see the full loading/error handling
head -100 src/components/UserDetails/UserOrganizations.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 3267
🏁 Script executed:
#!/bin/bash
# Read more of the component, especially around the loading state
sed -n '220,273p' src/components/UserDetails/UserOrganizations.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 1898
🏁 Script executed:
#!/bin/bash
# Check the useQuery hook calls to see if error is being used
rg -n -A 5 "useQuery" src/components/UserDetails/UserOrganizations.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 901
🏁 Script executed:
#!/bin/bash
# Search for any error handling pattern in the component
rg -n "error" src/components/UserDetails/UserOrganizations.tsx -A 2 -B 2Repository: PalisadoesFoundation/talawa-admin
Length of output: 59
🏁 Script executed:
#!/bin/bash
# Check the full render section to understand the loading/error logic
sed -n '200,273p' src/components/UserDetails/UserOrganizations.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 2482
🏁 Script executed:
#!/bin/bash
# Verify what the test file actually does - check full test structure
head -150 src/components/UserDetails/UserOrganizations.spec.tsx | tail -50Repository: PalisadoesFoundation/talawa-admin
Length of output: 1358
🏁 Script executed:
#!/bin/bash
# Check if useQuery hooks destructure the 'error' and 'loading' properties
sed -n '94,110p' src/components/UserDetails/UserOrganizations.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 568
🏁 Script executed:
#!/bin/bash
# Get the exact useQuery lines to see the complete destructuring
sed -n '91,104p' src/components/UserDetails/UserOrganizations.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 485
🏁 Script executed:
#!/bin/bash
# Verify the test assertions - check if any test validates loading UI appears
rg -n "loadingOrganizations" src/components/UserDetails/UserOrganizations.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 59
🏁 Script executed:
#!/bin/bash
# Comprehensive check - see entire test describe block structure
cat src/components/UserDetails/UserOrganizations.spec.tsx | grep -E "^\s*(it|describe|test)\(" | head -20Repository: PalisadoesFoundation/talawa-admin
Length of output: 463
🏁 Script executed:
#!/bin/bash
# Check if component uses error anywhere at all
cat src/components/UserDetails/UserOrganizations.tsx | grep -i "error"Repository: PalisadoesFoundation/talawa-admin
Length of output: 59
Add error handling and improve test coverage for loading/error states.
The component has two critical issues:
-
Missing error handling in component: Both
useQuerycalls (lines 94-99 and 101-103) destructure onlydata, nevererrororloadingproperties. The component shows "loadingOrganizations" only when both data objects are undefined, but doesn't handle GraphQL errors at all. The useQuery calls should be updated to:const { data: joinedOrganizationsData, error: joinedError, loading: joinedLoading } = useQuery(...); const { data: userData, error: userError, loading: userLoading } = useQuery(...);
Then add conditional rendering for error states in the JSX (around line 247).
-
Missing test coverage for error and loading states: Per §1.2 of the coding guidelines, test coverage must include error scenarios and loading states. Currently, the test suite has no assertions that verify:
- The loading UI ("Loading organizations...") is displayed during data fetching
- Error states are handled when queries fail
Add tests for:
- Loading state display while queries are in-flight
- Error handling when USER_DETAILS query fails
- Error handling when USER_JOINED_ORGANIZATIONS_NO_MEMBERS query fails
🤖 Prompt for AI Agents
In @src/components/UserDetails/UserOrganizations.spec.tsx around lines 147 -
368, The component currently only destructures data from useQuery and lacks
error/loading handling; update the two useQuery calls to destructure error and
loading as well (e.g., const { data: joinedOrganizationsData, error:
joinedError, loading: joinedLoading } =
useQuery(USER_JOINED_ORGANIZATIONS_NO_MEMBERS, ... ) and const { data: userData,
error: userError, loading: userLoading } = useQuery(USER_DETAILS, ...)), add JSX
conditional rendering to show the loading UI when either joinedLoading or
userLoading is true and render an error state when joinedError or userError is
present (display a suitable error message like "errorLoadingOrganizations"), and
extend UserOrganizations.spec.tsx to mock useQuery to return loading: true and
assert "Loading organizations..." is shown, and to return error objects for
USER_DETAILS and USER_JOINED_ORGANIZATIONS_NO_MEMBERS separately and assert the
component renders the error UI.
| }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(screen.getByText('noOrganizationsFound')).toBeInTheDocument(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify i18n implementation in UserOrganizations component
# 1. Check UserOrganizations component for useTranslation usage
echo "=== Checking UserOrganizations.tsx for useTranslation ==="
cat src/components/UserDetails/UserOrganizations.tsx | head -100
# 2. Check for 'noOrganizationsFound' usage
echo -e "\n=== Searching for 'noOrganizationsFound' in component ==="
rg -n "noOrganizationsFound" src/components/UserDetails/UserOrganizations.tsx
# 3. Check test file around lines 294 and 338
echo -e "\n=== Test assertions at lines 290-300 ==="
sed -n '290,300p' src/components/UserDetails/UserOrganizations.spec.tsx
echo -e "\n=== Test assertions at lines 334-344 ==="
sed -n '334,344p' src/components/UserDetails/UserOrganizations.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 4132
🏁 Script executed:
#!/bin/bash
# Check locale files for 'noOrganizationsFound' translation key
echo "=== Searching for translation key in locale files ==="
rg -n "noOrganizationsFound" public/locales/Repository: PalisadoesFoundation/talawa-admin
Length of output: 556
Fix test assertions: Check for translated text, not translation keys.
The component correctly uses useTranslation() at line 39 and renders translated text via tCommon('noOrganizationsFound') at line 249. However, the test assertions at lines 294 and 338 check for the literal string 'noOrganizationsFound' (the translation key), when they should assert on the actual translated output (e.g., "No organizations found." in English).
These tests will fail in non-English locales. Either:
- Mock
useTranslationto return the key for predictable testing, or - Change assertions to check for the expected translated string in the default locale
🤖 Prompt for AI Agents
In @src/components/UserDetails/UserOrganizations.spec.tsx at line 294, The test
is asserting the translation key string instead of the rendered translated text;
update the test in UserOrganizations.spec.tsx so it either mocks useTranslation
to return the key (e.g., mock useTranslation to return { t: (k)=>k, tCommon:
(k)=>k }) making assertions like
expect(screen.getByText('noOrganizationsFound')).toBeInTheDocument(), or change
the assertions to check for the actual translated message produced by
tCommon('noOrganizationsFound') (e.g., expect(screen.getByText('No organizations
found.')).toBeInTheDocument()); locate usages of useTranslation and
tCommon('noOrganizationsFound') and adjust the test setup or expected string
accordingly.
Comment on lines
1
to
116
| import { render, screen, fireEvent } from '@testing-library/react'; | ||
| import { describe, it, expect, vi } from 'vitest'; | ||
| import UserTags from './UserTags'; | ||
| import { InterfacePeopleTabNavbarProps } from 'types/PeopleTab/interface'; | ||
|
|
||
| /** | ||
| * Mock PageHeader because: | ||
| * - We only want to test UserTags logic | ||
| * - PageHeader is already tested elsewhere | ||
| */ | ||
| vi.mock('shared-components/PeopleTabNavbar/PeopleTabNavbar', () => ({ | ||
| default: (props: InterfacePeopleTabNavbarProps) => ( | ||
| <div> | ||
| {/* Mock search input */} | ||
| {props.search && ( | ||
| <input | ||
| data-testid={props.search.inputTestId || 'search-input'} | ||
| placeholder={props.search.placeholder} | ||
| onChange={(e) => props.search?.onSearch(e.target.value)} | ||
| /> | ||
| )} | ||
|
|
||
| {/* Mock sorting select */} | ||
| {props.sorting && | ||
| props.sorting.map((sort) => ( | ||
| <select | ||
| key={sort.title} | ||
| data-testid={`${sort.testIdPrefix}-select`} | ||
| value={sort.selected} | ||
| onChange={(e) => sort.onChange(e.target.value)} | ||
| > | ||
| {sort.options.map((opt) => ( | ||
| <option key={opt.value} value={opt.value}> | ||
| {opt.label} | ||
| </option> | ||
| ))} | ||
| </select> | ||
| ))} | ||
| </div> | ||
| ), | ||
| })); | ||
|
|
||
| describe('UserTags Component', () => { | ||
| afterEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('renders table headers correctly', () => { | ||
| render(<UserTags />); | ||
|
|
||
| expect(screen.getByText('peopleTabTagName')).toBeInTheDocument(); | ||
| expect(screen.getByText('assignedTo')).toBeInTheDocument(); | ||
| expect(screen.getByText('createdOn')).toBeInTheDocument(); | ||
| expect(screen.getByText('createdBy')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('renders all tags initially', () => { | ||
| render(<UserTags />); | ||
|
|
||
| // One known tag | ||
| expect(screen.getByText('Marketing Campaign')).toBeInTheDocument(); | ||
| expect(screen.getByText('Product Launch')).toBeInTheDocument(); | ||
| expect(screen.getByText('Security Audit')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('filters tags by search term (tag name)', () => { | ||
| render(<UserTags />); | ||
|
|
||
| const searchInput = screen.getByTestId('tagsSearchInput'); | ||
|
|
||
| fireEvent.change(searchInput, { | ||
| target: { value: 'Marketing' }, | ||
| }); | ||
|
|
||
| expect(screen.getByText('Marketing Campaign')).toBeInTheDocument(); | ||
| expect(screen.queryByText('Product Launch')).not.toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('filters tags by createdBy field', () => { | ||
| render(<UserTags />); | ||
|
|
||
| const searchInput = screen.getByTestId('tagsSearchInput'); | ||
|
|
||
| fireEvent.change(searchInput, { | ||
| target: { value: 'John Doe' }, | ||
| }); | ||
|
|
||
| expect(screen.getByText('Marketing Campaign')).toBeInTheDocument(); | ||
| expect(screen.getByText('Sales Q2')).toBeInTheDocument(); | ||
| expect(screen.queryByText('Design Review')).not.toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('changes sorting order when sort option changes', () => { | ||
| render(<UserTags />); | ||
|
|
||
| const sortSelect = screen.getByTestId('tagsSort-select'); | ||
|
|
||
| // Change to oldest | ||
| fireEvent.change(sortSelect, { | ||
| target: { value: 'oldest' }, | ||
| }); | ||
|
|
||
| // First row should now be the first dummy tag | ||
| const rows = screen.getAllByRole('row'); | ||
| expect(rows[1]).toHaveTextContent('Marketing Campaign'); | ||
| }); | ||
|
|
||
| it('shows correct created by links', () => { | ||
| render(<UserTags />); | ||
|
|
||
| const createdByLinks = screen.getAllByText( | ||
| /John Doe|Sarah Smith|Mike Johnson/, | ||
| ); | ||
| expect(createdByLinks.length).toBeGreaterThan(0); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find UserTags component and related files
echo "=== Finding UserTags files ==="
fd -t f "UserTags" --extension tsx --extension ts 2>/dev/null | head -20
# Check if interface file exists for UserTags
echo -e "\n=== Checking for UserTags interface files ==="
fd -t f "interface.ts" 2>/dev/null | xargs rg -l "UserTags" 2>/dev/null || echo "No interface.ts found with UserTags"
# Check the actual directory structure of components
echo -e "\n=== Directory structure of src/components ==="
fd -t d "^UserDetails$|^AdminPortal$|^UserPortal$" src/components 2>/dev/null
# Check where UserTags is imported from
echo -e "\n=== Where UserTags is imported from ==="
rg "from.*UserTags|import.*UserTags" --type tsx --type ts 2>/dev/null | head -10
# Verify test file imports
echo -e "\n=== Analyzing UserTags.spec.tsx imports ==="
head -10 src/components/UserDetails/UserTags.spec.tsx 2>/dev/nullRepository: PalisadoesFoundation/talawa-admin
Length of output: 833
🏁 Script executed:
#!/bin/bash
# Check where UserTags is actually used/imported from
echo "=== Searching for all imports/usages of UserTags ==="
rg "UserTags" --type tsx --type ts -B 2 -A 2 2>/dev/null | head -50
# Check the Tag interface to see if it relates to UserTags
echo -e "\n=== Content of src/types/Tag/interface.ts ==="
cat src/types/Tag/interface.ts 2>/dev/null | head -30
# Check what's actually in UserTags.tsx to understand its purpose
echo -e "\n=== First 40 lines of UserTags.tsx ==="
head -40 src/components/UserDetails/UserTags.tsx 2>/dev/nullRepository: PalisadoesFoundation/talawa-admin
Length of output: 2154
🏁 Script executed:
#!/bin/bash
# Check if UserDetails is a special directory or legacy component
echo "=== Contents of src/components/UserDetails directory ==="
ls -la src/components/UserDetails/ 2>/dev/null
# Check if there's a corresponding types/UserDetails directory
echo -e "\n=== Looking for UserDetails in types directory ==="
find src/types -type d -name "*UserDetails*" 2>/dev/null
# Check if UserTags component exports props interface
echo -e "\n=== Full UserTags.tsx file to check for inline interfaces ==="
wc -l src/components/UserDetails/UserTags.tsx
# Check if there's any types/PeopleTab/interface.ts since test imports from there
echo -e "\n=== Check types/PeopleTab/interface.ts ==="
cat src/types/PeopleTab/interface.ts 2>/dev/null | head -50
# Verify if UserTags should actually be in a portal or shared
echo -e "\n=== Check if UserTags is referenced elsewhere in config/tsconfig ==="
rg "UserTags" src/types/ 2>/dev/null || echo "No UserTags in types/"Repository: PalisadoesFoundation/talawa-admin
Length of output: 2488
🏁 Script executed:
#!/bin/bash
# Get full UserTags.tsx content to check for inline prop interfaces
echo "=== Full UserTags.tsx file ==="
cat src/components/UserDetails/UserTags.tsx 2>/dev/null
# Also check if these components are used anywhere
echo -e "\n=== Check if UserDetails components are imported elsewhere ==="
rg "from.*UserDetails|from.*UserTags|from.*UserEvents|from.*UserOrganizations" --type tsx --type ts 2>/dev/nullRepository: PalisadoesFoundation/talawa-admin
Length of output: 5989
🏁 Script executed:
#!/bin/bash
# Check the git history to see if UserDetails is an established directory
echo "=== Check git log for UserDetails directory ==="
git log --oneline -- src/components/UserDetails/ 2>/dev/null | head -10
# Verify if there's any documentation about UserDetails structure
echo -e "\n=== Check for README or docs in UserDetails ==="
find src/components/UserDetails -name "README*" -o -name "*.md" 2>/dev/null
# Check if this is a multi-component feature directory or if components should be elsewhere
echo -e "\n=== Check all directories at src/components level ==="
ls -d src/components/*/ 2>/dev/null | head -20
# Verify the purpose by checking if these are truly user profile detail components
echo -e "\n=== Check ESLint config for restricted imports or path patterns ==="
rg "no-restricted-imports|paths|alias" .eslintrc* tsconfig* 2>/dev/null | head -20Repository: PalisadoesFoundation/talawa-admin
Length of output: 990
🏁 Script executed:
#!/bin/bash
# Check if UserDetails components are actually used by both portals or just one
echo "=== Check where UserDetails components are imported ==="
rg "UserEvents|UserOrganizations|UserTags" src/components/ --type tsx --type ts -l 2>/dev/null
# Check if UserDetails might be under a portal indirectly
echo -e "\n=== Check if UserDetails is imported within AdminPortal or UserPortal ==="
rg "UserDetails" src/components/AdminPortal/ src/components/UserPortal/ --type tsx --type ts 2>/dev/null | head -20
# Verify the purpose of UserDetails directory
echo -e "\n=== Check if other top-level component directories are meant to be utilities ==="
ls -la src/components/ | grep "^d" | awk '{print $NF}' | grep -v "^$" | sortRepository: PalisadoesFoundation/talawa-admin
Length of output: 1039
Component directory structure violates reusable component policy §3.1.
UserTags is located at src/components/UserDetails/ which does not follow the required strict portal separation. When this component is integrated, move it to the appropriate location:
- Admin-only:
src/components/AdminPortal/UserTags/ - User-only:
src/components/UserPortal/UserTags/ - Shared:
src/shared-components/UserTags/
Additionally required when integrating:
- Create props interface in
src/types/[portal or shared-components]/UserTags/interface.ts(currently missing) - Add TSDoc to component interface (currently none exists)
Note: The component is currently unused in the codebase, so directory and interface structure should be addressed when actually integrated into a portal or marked as shared.
| !selectedAvatar && formState.avatarURL | ||
| ? await urlToFile(formState.avatarURL).catch(() => { | ||
| NotificationToast.error( | ||
| 'Failed to process profile picture. Please try uploading again.', |
Contributor
There was a problem hiding this comment.
Hardcoded user-facing string violates i18n requirement.
Line 127 contains a hardcoded English string: "Failed to process profile picture. Please try uploading again." This violates the zero-hardcoded-strings policy for user-facing text.
Required fix:
-
Add translation key to all locale files:
public/locales/en/translation.jsonpublic/locales/es/translation.jsonpublic/locales/fr/translation.jsonpublic/locales/hi/translation.jsonpublic/locales/zh/translation.json
-
Replace hardcoded string with translation key using
t().
🌐 Proposed fix
? await urlToFile(formState.avatarURL).catch(() => {
NotificationToast.error(
- 'Failed to process profile picture. Please try uploading again.',
+ t('avatarProcessingFailed')
);
return null;
})Add to translation files:
{
"memberDetail": {
"avatarProcessingFailed": "Failed to process profile picture. Please try uploading again."
}
}As per coding guidelines §2.3.
🤖 Prompt for AI Agents
In @src/screens/MemberDetail/UserContactDetails.tsx at line 127, Replace the
hardcoded message in the UserContactDetails component with a t() lookup and add
the key memberDetail.avatarProcessingFailed to all locale translation files
(public/locales/en/translation.json, es, fr, hi, zh) containing the English
string; update the component to call t('memberDetail.avatarProcessingFailed')
wherever the string "Failed to process profile picture. Please try uploading
again." is used so user-facing text is fully internationalized.
| </Button> | ||
| <Button | ||
| variant="outline" | ||
| style={{ backgroundColor: '#A8C7FA', color: '#555' }} |
Contributor
There was a problem hiding this comment.
Replace inline style colors with Bootstrap utility classes.
Line 583 uses inline style prop with hardcoded backgroundColor and color values. This violates the CSS-Policy-Check CI requirement.
🎨 Proposed fix
<Button
variant="outline"
- style={{ backgroundColor: '#A8C7FA', color: '#555' }}
+ className="bg-primary-subtle text-secondary"
onClick={handleUserUpdate}
data-testid="saveChangesBtn"
>Or define a CSS class in the module stylesheet if the exact color is required.
Based on learnings and coding guidelines compliance.
🤖 Prompt for AI Agents
In @src/screens/MemberDetail/UserContactDetails.tsx at line 583, Replace the
inline style object style={{ backgroundColor: '#A8C7FA', color: '#555' }} in the
JSX (in UserContactDetails.tsx) with Bootstrap utility classes or a module CSS
class: remove the style prop, add appropriate Bootstrap classes (e.g., bg-*,
text-*) that approximate the colors, or create a CSS module rule (e.g.,
.customContactBg { background-color: #A8C7FA; color: #555; }) and use that
className on the same element to satisfy the CSS-Policy-Check; ensure the
element that currently uses the style prop is updated to reference the new
className.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/GraphQl/Queries/Queries.ts:
- Around line 452-477: Add a JSDoc single-line comment above the exported
GET_USER_BY_ID constant describing its purpose and usage (following the pattern
used for CURRENT_USER and ORGANIZATION_LIST); specifically note that
GET_USER_BY_ID is a GraphQL query to fetch a user by ID and return profile and
contact fields (id, name, emailAddress, addressLine1/2, avatarURL, phones,
birthDate, city, state, postalCode, countryCode, description, educationGrade,
employmentStatus, maritalStatus, natalSex, naturalLanguageCode, etc.), so
callers know when to use it.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/GraphQl/Queries/Queries.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Zero TypeScript errors: Run pnpm run typecheck and must pass without errors or warnings
Files:
src/GraphQl/Queries/Queries.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Conduct a comprehensive review and reevaluate all resolved and dismissed items.VALIDATION FRAMEWORK
1. Test Coverage & Quality
1.1 Test File Requirements
For each modified source file with logic or UI:
- ✅ Test file exists:
*.spec.tsx,*.spec.ts,*.test.tsx, or*.test.ts- ✅ Test file colocated with component (same directory)
- ✅ Test file updated to reflect source changes
- ✅ Target ≥95% code coverage for new/modified code
- ✅ List specific line numbers lacking test coverage
1.2 Test Quality Standards
✅ Framework Compliance (Vitest + React Testing Library):
- Use
vi.mock()for mocking (notjest.mock())- Use
describe(),it(),test()for structure- Use
@testing-library/reactqueries (getByRole, getByText, etc.)- Use
@testing-library/user-eventfor interactions✅ Coverage Completeness - All scenarios tested:
- Success paths with valid data
- Error scenarios (network failures, GraphQL errors, validation errors)
- Edge cases (empty data, null/undefined, boundary values)
- Loading and pending states
- User interactions and state changes
✅ Mock Quality:
- GraphQL:
MockedProviderorStaticMockLink- Realistic, comprehensive mock data
- Proper cleanup (reset mocks, clear timers)
✅ Test Structure:
- Isolated tests (no execution order dependency)
- No disabled tests (
*.skip) without documented reason + linked issue- Proper async utilities (
waitFor,act)- Shard-compatible (12-shard strategy, no shared state, <30s timeout)
2. Code Quality & Implementation
2.1 TypeScript Standards
- ✅ No
anytype without JSDoc justification- ✅ Proper typing for:
- Component props (interface declarations)
- Function parameters and return types
- GraphQL variables and responses
- State and context
- ✅ No
@ts-ignorewithout explanation- ✅ Zero TypeScript compilation errors
2.2 React & GraphQL Implementation
- ✅ React Best Practices:
- Fu...
Files:
src/GraphQl/Queries/Queries.ts
🧠 Learnings (2)
📚 Learning: 2026-01-06T21:27:50.880Z
Learnt from: Jashan32
Repo: PalisadoesFoundation/talawa-admin PR: 6222
File: src/utils/userUpdateUtils.ts:22-22
Timestamp: 2026-01-06T21:27:50.880Z
Learning: In the Talawa Admin codebase, the i18n extraction tool can misclassify TypeScript generic syntax (e.g., (obj: T): Partial) as translatable content. Suppress these false positives by adding a // i18n-ignore-next-line comment directly above the line with the type declaration or generic function signature. Apply this only to TS code where misclassification is confirmed; avoid blanket suppression.
Applied to files:
src/GraphQl/Queries/Queries.ts
📚 Learning: 2026-01-07T14:21:19.711Z
Learnt from: cotishq
Repo: PalisadoesFoundation/talawa-admin PR: 6198
File: src/setup/setup.ts:123-124
Timestamp: 2026-01-07T14:21:19.711Z
Learning: Use TSDoc (eslint-plugin-tsdoc) instead of JSDoc. Do not wrap type names in curly braces in throws and returns. Write as "throws Error" and "returns Promise<void>" (no braces). Curly-brace usage causes tsdoc-malformed-inline-tag and tsdoc-escape-right-brace lint errors. Apply this rule to all TypeScript source files (e.g., src/**/*.ts) to ensure consistency.
Applied to files:
src/GraphQl/Queries/Queries.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Performs linting, formatting, type-checking, unused file detection, checking for different source...
- GitHub Check: Analyse Code With CodeQL (javascript)
Comment on lines
+452
to
+477
| export const GET_USER_BY_ID = gql` | ||
| query GetUserById($input: QueryUserInput!) { | ||
| user(input: $input) { | ||
| id | ||
| name | ||
| emailAddress | ||
| addressLine1 | ||
| addressLine2 | ||
| birthDate | ||
| city | ||
| avatarURL | ||
| countryCode | ||
| description | ||
| educationGrade | ||
| employmentStatus | ||
| homePhoneNumber | ||
| maritalStatus | ||
| mobilePhoneNumber | ||
| natalSex | ||
| naturalLanguageCode | ||
| postalCode | ||
| state | ||
| workPhoneNumber | ||
| } | ||
| } | ||
| `; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the query exists at the specified lines and check for TSDoc documentation
echo "=== Checking file structure ==="
wc -l src/GraphQl/Queries/Queries.ts
echo -e "\n=== Content at lines 450-480 ==="
sed -n '450,480p' src/GraphQl/Queries/Queries.ts
echo -e "\n=== Checking for other GraphQL query documentation patterns ==="
echo "First 10 exported GraphQL queries/their documentation:"
rg -B2 'export const [A-Z_]+ = gql`' src/GraphQl/Queries/Queries.ts | head -40
echo -e "\n=== Search for test files using GET_USER_BY_ID ==="
rg -l "GET_USER_BY_ID" --type ts --type tsx
echo -e "\n=== Search for GET_USER_BY_ID in all test files ==="
rg -n "GET_USER_BY_ID" src/ -g "*.spec.*" -g "*.test.*"Repository: PalisadoesFoundation/talawa-admin
Length of output: 2000
Add JSDoc documentation for the exported GET_USER_BY_ID query.
Per coding guidelines, all exported GraphQL queries must have JSDoc comments. While test coverage exists in UserContactDetails.spec.tsx, the query is missing documentation to describe its purpose and usage context.
Follow the existing pattern in this file—use a simple single-line comment like other queries (e.g., CURRENT_USER, ORGANIZATION_LIST):
📝 Suggested documentation
+// Query to fetch user's profile and contact details by ID
export const GET_USER_BY_ID = gql`
query GetUserById($input: QueryUserInput!) {
user(input: $input) {🤖 Prompt for AI Agents
In @src/GraphQl/Queries/Queries.ts around lines 452 - 477, Add a JSDoc
single-line comment above the exported GET_USER_BY_ID constant describing its
purpose and usage (following the pattern used for CURRENT_USER and
ORGANIZATION_LIST); specifically note that GET_USER_BY_ID is a GraphQL query to
fetch a user by ID and return profile and contact fields (id, name,
emailAddress, addressLine1/2, avatarURL, phones, birthDate, city, state,
postalCode, countryCode, description, educationGrade, employmentStatus,
maritalStatus, natalSex, naturalLanguageCode, etc.), so callers know when to use
it.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #6359 +/- ##
===========================================
- Coverage 96.74% 96.69% -0.05%
===========================================
Files 456 458 +2
Lines 16598 16686 +88
Branches 5118 5237 +119
===========================================
+ Hits 16057 16134 +77
- Misses 528 539 +11
Partials 13 13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @src/components/UserDetails/UserEvents.spec.tsx:
- Line 1: Tests in UserEvents.spec.tsx use fireEvent which is less realistic;
replace fireEvent usages with @testing-library/user-event by importing userEvent
and using userEvent.setup() in tests (e.g., inside the 'filters events by search
input' test) and await user.type on the search input (test id 'events-search')
to simulate typing; update test functions to be async where needed and remove
fireEvent imports and calls, ensuring assertions remain the same (e.g., checking
for 'React Workshop' present and 'Node.js Seminar' absent).
- Around line 125-134: The test "changes sorting order when sort option changes"
is weakly asserting only that event cards exist after changing the sort; either
delete this redundant test (since the later sorting test covers behavior) or
strengthen it: locate the test using the UserEvents component and the
'eventsSort-select' test id, assert the select's initial value (e.g., 'ASC'),
fireEvent.change to 'DESC' and assert the select now has value 'DESC', or
alternatively assert a visible ordering change by inspecting the text/content of
the first/last 'event-card' before and after the change to confirm sort applied.
In @src/screens/MemberDetail/MemberDetail.spec.tsx:
- Around line 78-84: Remove the redundant beforeEach that only calls
vi.clearAllMocks(): keep the existing afterEach(() => { vi.clearAllMocks(); })
for test isolation and delete the beforeEach block that also calls
vi.clearAllMocks(); if the beforeEach contains other necessary setup, move those
lines into a shared setup helper or the existing beforeEach but ensure
vi.clearAllMocks() is only invoked in the afterEach.
- Around line 64-68: The mock button element rendered with data-testid
`tab-${title}` is missing an explicit type and should include type="button" to
avoid implicit "submit" behavior; update the JSX for the button inside the
MemberDetail test (the element with data-testid `tab-${title}` and props
isActive/onClick/action) to add type="button".
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
src/components/UserDetails/UserEvents.spec.tsxsrc/components/UserDetails/UserTags.spec.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsx
🧰 Additional context used
📓 Path-based instructions (4)
src/**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
All components must follow reusable component policy: No inline interfaces, correct portal placement, wrapper usage
Files:
src/components/UserDetails/UserTags.spec.tsxsrc/components/UserDetails/UserEvents.spec.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Test coverage must be ≥95% for modified files. Run: pnpm run test:coverage and verify coverage/coverage-summary.json shows no files below threshold
Files:
src/components/UserDetails/UserTags.spec.tsxsrc/components/UserDetails/UserEvents.spec.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Zero TypeScript errors: Run pnpm run typecheck and must pass without errors or warnings
Files:
src/components/UserDetails/UserTags.spec.tsxsrc/components/UserDetails/UserEvents.spec.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsx
**/*
⚙️ CodeRabbit configuration file
**/*: Conduct a comprehensive review and reevaluate all resolved and dismissed items.VALIDATION FRAMEWORK
1. Test Coverage & Quality
1.1 Test File Requirements
For each modified source file with logic or UI:
- ✅ Test file exists:
*.spec.tsx,*.spec.ts,*.test.tsx, or*.test.ts- ✅ Test file colocated with component (same directory)
- ✅ Test file updated to reflect source changes
- ✅ Target ≥95% code coverage for new/modified code
- ✅ List specific line numbers lacking test coverage
1.2 Test Quality Standards
✅ Framework Compliance (Vitest + React Testing Library):
- Use
vi.mock()for mocking (notjest.mock())- Use
describe(),it(),test()for structure- Use
@testing-library/reactqueries (getByRole, getByText, etc.)- Use
@testing-library/user-eventfor interactions✅ Coverage Completeness - All scenarios tested:
- Success paths with valid data
- Error scenarios (network failures, GraphQL errors, validation errors)
- Edge cases (empty data, null/undefined, boundary values)
- Loading and pending states
- User interactions and state changes
✅ Mock Quality:
- GraphQL:
MockedProviderorStaticMockLink- Realistic, comprehensive mock data
- Proper cleanup (reset mocks, clear timers)
✅ Test Structure:
- Isolated tests (no execution order dependency)
- No disabled tests (
*.skip) without documented reason + linked issue- Proper async utilities (
waitFor,act)- Shard-compatible (12-shard strategy, no shared state, <30s timeout)
2. Code Quality & Implementation
2.1 TypeScript Standards
- ✅ No
anytype without JSDoc justification- ✅ Proper typing for:
- Component props (interface declarations)
- Function parameters and return types
- GraphQL variables and responses
- State and context
- ✅ No
@ts-ignorewithout explanation- ✅ Zero TypeScript compilation errors
2.2 React & GraphQL Implementation
- ✅ React Best Practices:
- Fu...
Files:
src/components/UserDetails/UserTags.spec.tsxsrc/components/UserDetails/UserEvents.spec.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsx
🧠 Learnings (8)
📚 Learning: 2025-12-21T08:59:37.942Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5287
File: src/shared-components/Recurrence/CustomRecurrenceModal.spec.tsx:78-84
Timestamp: 2025-12-21T08:59:37.942Z
Learning: In talawa-admin test files, ensure each spec file uses an explicit vi.clearAllMocks() in an afterEach block to guarantee test isolation. This should be present even if a global cleanup exists in vitest.setup.ts, as the linter enforces per-file hygiene. Apply this guideline to all spec files (e.g., src/**/*.spec.tsx) to satisfy ESLint/Vitest requirements and prevent bleed-over between tests.
Applied to files:
src/components/UserDetails/UserTags.spec.tsxsrc/components/UserDetails/UserEvents.spec.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsx
📚 Learning: 2025-12-27T11:22:41.191Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5782
File: src/screens/Leaderboard/Leaderboard.spec.tsx:342-370
Timestamp: 2025-12-27T11:22:41.191Z
Learning: In test files that render screens with shared components (e.g., src/screens/Leaderboard/Leaderboard.spec.tsx), verify integration behavior by asserting the screen renders correctly in different scenarios (e.g., search results) without asserting the internal ARIA attributes, text, or icons of the shared component. Rely on the shared component's own tests to cover accessibility and presentation details. This keeps tests focused on higher-level behavior and interactions.
Applied to files:
src/components/UserDetails/UserTags.spec.tsxsrc/components/UserDetails/UserEvents.spec.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsx
📚 Learning: 2026-01-08T18:48:10.364Z
Learnt from: abhishekblue
Repo: PalisadoesFoundation/talawa-admin PR: 6260
File: src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.spec.tsx:337-339
Timestamp: 2026-01-08T18:48:10.364Z
Learning: In tests that exercise Apollo Client queries (e.g., talawa-admin tests under src/screens/.../OrganizationPeople.spec.tsx), avoid using vi.useFakeTimers() because Apollo's internal promise resolution can be disrupted by mocked timers, causing timeouts. Use real timers with explicit waits (for example, await act(async () => { await new Promise(r => setTimeout(r, DURATION)); })) when testing components that fetch data via Apollo. If you must use timers, prefer real timers for the relevant test and flush promises deterministically. Replace or remove useFakeTimers in such tests and rely on actual time passage or proper promise flushing to ensure reliable Apollo-driven data fetching tests.
Applied to files:
src/components/UserDetails/UserTags.spec.tsxsrc/components/UserDetails/UserEvents.spec.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsx
📚 Learning: 2025-12-26T20:34:53.912Z
Learnt from: MuhammadUmar7195
Repo: PalisadoesFoundation/talawa-admin PR: 5847
File: src/screens/UserPortal/Volunteer/Invitations/Invitations.tsx:243-243
Timestamp: 2025-12-26T20:34:53.912Z
Learning: In PalisadoesFoundation/talawa-admin, whenever you modify a file (e.g., for migrations, tests, or feature changes), replace inline color attributes like color="grey" with Bootstrap utility classes (e.g., className="text-secondary") to satisfy the CSS-Policy-Check CI. This is a general compliance guideline, not scope creep, and should be applied to all affected TSX files during reviews. Steps: 1) Find inline color attributes in JSX/TSX. 2) Replace with appropriate Bootstrap text/color utility classes. 3) Ensure the visual outcome remains the same. 4) Run CI checks to confirm the policy passes.
Applied to files:
src/components/UserDetails/UserTags.spec.tsxsrc/components/UserDetails/UserEvents.spec.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsx
📚 Learning: 2026-01-06T16:39:41.033Z
Learnt from: Jashan32
Repo: PalisadoesFoundation/talawa-admin PR: 6199
File: src/shared-components/ProfileForm/ProfileForm.tsx:0-0
Timestamp: 2026-01-06T16:39:41.033Z
Learning: When refactoring or consolidating components (e.g., talawa-admin), remove helper functions that were copied from old files but are not used in the new implementation. Before exporting utilities, verify there are real usages across the codebase (search usages, remove dead code, and keep only necessary exports) to improve maintainability and reduce bundle size.
Applied to files:
src/components/UserDetails/UserTags.spec.tsxsrc/components/UserDetails/UserEvents.spec.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsx
📚 Learning: 2026-01-07T09:08:09.150Z
Learnt from: adityai0
Repo: PalisadoesFoundation/talawa-admin PR: 6202
File: src/components/EventManagement/EventAgendaItems/EventAgendaItems.tsx:4-13
Timestamp: 2026-01-07T09:08:09.150Z
Learning: In the talawa-admin repository, document comments with TSDoc in all TSX files. Do not use JSDoc. Write non-standard tags like component and requires as plain text (no leading @). Use standard TSDoc tags such as param, remarks, returns, and example with the leading @. This guideline applies to all TSX components (e.g., src/components/.../*.tsx).
Applied to files:
src/components/UserDetails/UserTags.spec.tsxsrc/components/UserDetails/UserEvents.spec.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsx
📚 Learning: 2025-12-27T11:22:19.556Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5782
File: src/screens/Leaderboard/Leaderboard.spec.tsx:299-329
Timestamp: 2025-12-27T11:22:19.556Z
Learning: In talawa-admin test files (e.g., src/screens/Leaderboard/Leaderboard.spec.tsx), prefer separate explicit test cases for keyboard navigation interactions (Enter key, Space key) instead of consolidating them with test.each()/parameterization. This improves readability, documents each keyboard interaction path clearly, and makes failures easier to diagnose when specific key handling regresses. Apply this pattern to similarly structured spec files by keeping individual tests for each key interaction rather than combining them into a single parameterized test.
Applied to files:
src/components/UserDetails/UserTags.spec.tsxsrc/components/UserDetails/UserEvents.spec.tsxsrc/screens/MemberDetail/MemberDetail.spec.tsx
📚 Learning: 2026-01-08T16:45:49.201Z
Learnt from: adityai0
Repo: PalisadoesFoundation/talawa-admin PR: 6241
File: src/components/AdminPortal/OrgPeopleListCard/OrgPeopleListCard.tsx:12-16
Timestamp: 2026-01-08T16:45:49.201Z
Learning: In TSX components, prefer flat JSDoc parameter documentation format (e.g., param id, param toggleRemoveModal) over the nested format (param props.id). The nested form can break editor parsing. When a component accepts a props object, destructure the props in the function signature to align with the flat JSDoc style (e.g., function MyComp({ id, toggleRemoveModal }: Props) { ... }). Ensure all param tags correspond to the destructured props and reflect actual component props to improve editor support and readability.
Applied to files:
src/components/UserDetails/UserTags.spec.tsxsrc/components/UserDetails/UserEvents.spec.tsx
🧬 Code graph analysis (2)
src/components/UserDetails/UserTags.spec.tsx (1)
src/types/PeopleTab/interface.ts (1)
InterfacePeopleTabNavbarProps(24-42)
src/components/UserDetails/UserEvents.spec.tsx (1)
src/types/PeopleTab/interface.ts (2)
InterfacePeopleTabNavbarProps(24-42)InterfacePeopletabUserEventsProps(62-71)
🪛 Biome (2.1.2)
src/screens/MemberDetail/MemberDetail.spec.tsx
[error] 64-68: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Performs linting, formatting, type-checking, unused file detection, checking for different source...
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (4)
src/components/UserDetails/UserTags.spec.tsx (2)
1-46: LGTM! Well-structured test file with proper isolation.The imports, mock setup, and test isolation pattern are correctly implemented. The
afterEachblock withvi.clearAllMocks()ensures proper test hygiene per repository guidelines. The mock forPageHeadercorrectly exposes search and sorting controls to enable testing UserTags logic in isolation.Based on learnings, the explicit
vi.clearAllMocks()inafterEachsatisfies the per-file hygiene requirement.
48-134: LGTM! Comprehensive test coverage for UserTags.The test suite covers:
- Rendering verification (headers, initial data)
- Search filtering by tag name and createdBy field
- Sorting behavior (oldest/latest)
- Link rendering verification
Tests are well-isolated and follow the recommended pattern of explicit test cases rather than parameterization.
src/screens/MemberDetail/MemberDetail.spec.tsx (1)
86-147: LGTM! Tab navigation tests are well-structured.The test suite covers:
- Default tab rendering (overview)
- Tab switching to all sections (organizations, events, tags)
- Round-trip navigation back to overview
- Active state verification
Each test is explicit and isolated, following the recommended pattern for keyboard/click navigation tests per repository guidelines.
src/components/UserDetails/UserEvents.spec.tsx (1)
92-95: Good test isolation withafterEachcleanup.Proper use of
vi.clearAllMocks()inafterEachto ensure test isolation. This aligns with the project's testing hygiene guidelines.
| @@ -0,0 +1,159 @@ | |||
| import { render, screen, fireEvent } from '@testing-library/react'; | |||
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using @testing-library/user-event for more realistic interactions.
The coding guidelines specify using @testing-library/user-event for user interactions instead of fireEvent. While fireEvent works, userEvent simulates real user behavior more accurately (e.g., focus, keystrokes, event bubbling).
♻️ Suggested refactor
-import { render, screen, fireEvent } from '@testing-library/react';
+import { render, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
import { describe, it, expect, vi } from 'vitest';Then update tests to use async patterns:
it('filters events by search input', async () => {
const user = userEvent.setup();
render(<UserEvents />);
const searchInput = screen.getByTestId('events-search');
await user.type(searchInput, 'react');
expect(screen.getByText('React Workshop')).toBeInTheDocument();
expect(screen.queryByText('Node.js Seminar')).not.toBeInTheDocument();
});🤖 Prompt for AI Agents
In @src/components/UserDetails/UserEvents.spec.tsx at line 1, Tests in
UserEvents.spec.tsx use fireEvent which is less realistic; replace fireEvent
usages with @testing-library/user-event by importing userEvent and using
userEvent.setup() in tests (e.g., inside the 'filters events by search input'
test) and await user.type on the search input (test id 'events-search') to
simulate typing; update test functions to be async where needed and remove
fireEvent imports and calls, ensuring assertions remain the same (e.g., checking
for 'React Workshop' present and 'Node.js Seminar' absent).
Comment on lines
+42
to
+59
| // Mock Apollo useQuery | ||
| vi.mock('@apollo/client', async () => { | ||
| const actual = await vi.importActual('@apollo/client'); | ||
| return { | ||
| ...actual, | ||
| useQuery: vi.fn(() => ({ | ||
| data: { | ||
| user: { | ||
| createdOrganizations: [], | ||
| organizationsWhereMember: { edges: [] }, | ||
| joinedOrganizations: { edges: [] }, | ||
| }, | ||
| }, | ||
| loading: false, | ||
| error: undefined, | ||
| })), | ||
| }; | ||
| }); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add tests for loading and error states.
The useQuery mock only returns a success state. Per testing guidelines, tests should cover error scenarios (network failures, GraphQL errors) and loading states. Consider adding test cases that verify the component's behavior when loading: true or when error is defined.
♻️ Suggested approach
import { useQuery } from '@apollo/client';
import { vi, Mock } from 'vitest';
// In your tests:
it('displays loading state', () => {
(useQuery as Mock).mockReturnValueOnce({
data: undefined,
loading: true,
error: undefined,
});
render(<UserEvents />);
// Assert loading indicator is shown
});
it('displays error state', () => {
(useQuery as Mock).mockReturnValueOnce({
data: undefined,
loading: false,
error: new Error('Failed to fetch'),
});
render(<UserEvents />);
// Assert error message is shown
});
Comment on lines
+125
to
+134
| it('changes sorting order when sort option changes', () => { | ||
| render(<UserEvents />); | ||
|
|
||
| const sortSelect = screen.getByTestId('eventsSort-select'); | ||
|
|
||
| fireEvent.change(sortSelect, { target: { value: 'DESC' } }); | ||
|
|
||
| const events = screen.getAllByTestId('event-card'); | ||
| expect(events.length).toBeGreaterThan(0); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Strengthen sorting test assertion.
This test only verifies that event cards exist after changing sort order. It doesn't confirm that the sorting actually took effect. The test at lines 136-158 does verify actual sorting order, making this test somewhat redundant with a weaker assertion.
Consider either removing this test (since lines 136-158 cover sorting more thoroughly) or strengthening the assertion to verify something distinct.
♻️ Option: Remove redundant test or add distinct assertion
- it('changes sorting order when sort option changes', () => {
- render(<UserEvents />);
-
- const sortSelect = screen.getByTestId('eventsSort-select');
-
- fireEvent.change(sortSelect, { target: { value: 'DESC' } });
-
- const events = screen.getAllByTestId('event-card');
- expect(events.length).toBeGreaterThan(0);
- });Or keep it and verify that the select's value actually changed:
it('changes sorting order when sort option changes', () => {
render(<UserEvents />);
const sortSelect = screen.getByTestId('eventsSort-select');
expect(sortSelect).toHaveValue('ASC'); // verify initial state
fireEvent.change(sortSelect, { target: { value: 'DESC' } });
expect(sortSelect).toHaveValue('DESC'); // verify state changed
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('changes sorting order when sort option changes', () => { | |
| render(<UserEvents />); | |
| const sortSelect = screen.getByTestId('eventsSort-select'); | |
| fireEvent.change(sortSelect, { target: { value: 'DESC' } }); | |
| const events = screen.getAllByTestId('event-card'); | |
| expect(events.length).toBeGreaterThan(0); | |
| }); |
🤖 Prompt for AI Agents
In @src/components/UserDetails/UserEvents.spec.tsx around lines 125 - 134, The
test "changes sorting order when sort option changes" is weakly asserting only
that event cards exist after changing the sort; either delete this redundant
test (since the later sorting test covers behavior) or strengthen it: locate the
test using the UserEvents component and the 'eventsSort-select' test id, assert
the select's initial value (e.g., 'ASC'), fireEvent.change to 'DESC' and assert
the select now has value 'DESC', or alternatively assert a visible ordering
change by inspecting the text/content of the first/last 'event-card' before and
after the change to confirm sort applied.
Contributor
|
Please:
|
Contributor
|
Please also attach a video of your work |
Contributor
Author
Not able to send the video here. Sending it to your WhatsApp. |
Contributor
Author
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 21
🤖 Fix all issues with AI agents
In @src/screens/MemberDetail/MemberDetail.spec.tsx:
- Around line 2-3: Update the import in MemberDetail.spec.tsx to remove
fireEvent and import userEvent from '@testing-library/user-event'; then in the
tests "switches to organizations tab", "switches to tags tab", "switches to
events tab", and "switches back to overview tab" replace fireEvent.click(...)
calls with await userEvent.click(...) calls, marking those test functions async
so you can await the click calls (the "switches back to overview tab" test has
two click calls to convert). Ensure all references to fireEvent are removed and
use await userEvent.click on the tab elements to match project testing
guidelines.
- Around line 52-74: Add the missing active-state assertion for the Tags tab: in
the tags tab test (the block that already verifies the tags content around lines
106–112) add an assertion that the element with test id "tab-tags" has the
attribute data-active equal to "true" (to match the
overview/organizations/events tests that check active state). Locate the test
referencing "tab-tags" and append the active-state check using your test
framework's attribute assertion.
- Around line 1-5: Replace use of fireEvent with @testing-library/user-event:
change the import to pull userEvent from '@testing-library/user-event' instead
of fireEvent, then update every fireEvent.click(...) call in
MemberDetail.spec.tsx (the calls around the interactions in the tests
referencing the MemberDetail component) to await userEvent.click(...). Ensure
the tests containing those interactions are declared async so you can use await
(update the relevant it/describe callbacks to async where needed) and remove the
unused fireEvent import.
In @src/screens/MemberDetail/UserContactDetails.spec.tsx:
- Around line 280-296: The test for MemberDetail uses BrowserRouter which can
yield different routing context (empty useParams); update the test to wrap
MemberDetail with MemoryRouter providing appropriate initialEntries (and
initialIndex if needed) so the component receives expected route params/path;
locate the test block 'Should display dicebear image if image is null' and
replace BrowserRouter with MemoryRouter configured to the route used by
MemberDetail (so useParams and path matching work consistently).
- Around line 211-243: Replace weak truthy checks that use
expect(getAllByText(...)).toBeTruthy() with stronger assertions: for elements
where you expect a single match use expect(screen.getAllByText(/Profile
Information/i)).toHaveLength(1) or prefer expect(screen.getByText(/Profile
Information/i)).toBeInTheDocument(), and for other fields assert exact counts
with toHaveLength() or assert presence with getByText + toBeInTheDocument();
update the assertions in the 'should render the elements for member profile'
test (and the similar assertions referenced at lines ~245-277) that call
getAllByText for Email, name, Birth Date, Gender, Profile Information, Contact
Information to use either getByText + toBeInTheDocument or getAllByText +
toHaveLength with the expected count.
- Around line 594-605: The test is non-deterministic because it wraps the
assertion in an if-check; remove the conditional and make a deterministic
assertion by awaiting a finder or asserting directly: replace the conditional
block that uses screen.queryByText('No events attended') with an awaited finder
like await screen.findByText('No events attended') and then expect(...)
.toBeInTheDocument(), or assert expect(screen.queryByText('No events
attended')).toBeInTheDocument() without any if, using renderMemberDetailScreen
and the existing wait/async flow.
- Around line 580-592: The test "handles birth date picker changes" is fragile
because it uses birthDatePicker.querySelector('input') and a conditional that
makes the assertion optional; instead, locate the date input deterministically
(e.g., use screen.getByTestId('birthDate') as the input itself or use
getByRole/getByLabelText) and assert it exists before interacting, then
fireEvent/change (or userEvent.type) and add an explicit expectation (for value
or a mock handler call). Update the test to remove the if-check, use a
guaranteed query for the input (reference: renderMemberDetailScreen,
screen.getByTestId('birthDate'), fireEvent.change), assert the input is present,
perform the change, and assert the new value or that the onChange handler was
invoked.
- Around line 891-929: The test's final assertion is wrong: after clicking reset
the form should revert to the original value, not remain as 'Changed Name'.
Update the assertion in the 'resets form to original data when reset is clicked'
test to expect the original mock name (derive it from your mock data, e.g., from
MOCKS1 or the initial response used by StaticMockLink) instead of 'Changed
Name'; locate the test by its name and change the expect on nameInput (the one
retrieved by getByTestId('inputName')) after clicking resetButton
(getByTestId('resetChangesBtn')) to assert the original value.
In @src/screens/MemberDetail/UserContactDetails.tsx:
- Line 50: The component UserContactDetails currently performs a side effect by
assigning document.title directly during render (document.title = t('title'));
remove that direct assignment and rely solely on the existing useEffect that
sets document.title (the useEffect referencing t('title')) so the title update
runs as an effect rather than during render; ensure no other direct
document.title writes remain in the component.
- Around line 480-481: In UserContactDetails update the three label htmlFor
attributes that currently use the duplicate value "address" so they match their
corresponding input IDs: change the label for the first address field to
htmlFor="addressLine1", the second to htmlFor="addressLine2", and the postal
code label to htmlFor="postalCode" so each label correctly associates with
inputs id="addressLine1", id="addressLine2", and id="postalCode".
- Around line 445-446: The labels currently use a duplicate
htmlFor="phoneNumber" which breaks accessibility; update each label in
UserContactDetails (the three phone number label elements near the
mobile/work/home phone inputs) to use the matching input ID values (e.g.,
htmlFor="mobilePhoneNumber", htmlFor="workPhoneNumber",
htmlFor="homePhoneNumber") so each label correctly associates with its
corresponding input element.
- Around line 38-41: Move the inline prop type out of the component into a new
interface file and update the component to use that named interface: create
src/types/AdminPortal/UserContactDetails/interface.ts exporting
InterfaceUserContactDetailsProps { id?: string } (with the provided JSDoc), then
replace the inline MemberDetailProps type and the UserContactDetails signature
so it imports and uses InterfaceUserContactDetailsProps (e.g., const
UserContactDetails: React.FC<InterfaceUserContactDetailsProps> = ({ id }) =>
...) and remove the old MemberDetailProps declaration.
- Around line 1-7: The TSDoc for the exported component UserContactDetails is
missing the required @returns tag; update the TSDoc block for UserContactDetails
to include "@returns JSX.Element" so the component's documentation conforms to
the coding guidelines and clearly indicates the rendered type.
- Around line 85-92: The fetchPolicy option is mistakenly nested inside the
variables object in the useQuery call, so move fetchPolicy out to be a sibling
of variables in the options object for useQuery; update the useQuery invocation
that references GET_USER_BY_ID and currentId so it passes variables: { input: {
id: currentId } } and fetchPolicy: 'no-cache' at the same level as variables
(not inside it).
- Around line 140-142: Replace the hardcoded English message passed to
NotificationToast.error in UserContactDetails (the NotificationToast.error(...)
call) with a translation lookup using your i18n/t helper and the new locale key;
add "avatarProcessingFailed": "Failed to process profile picture. Please try
uploading again." under the memberDetail namespace in your locale files and call
the translator (e.g., t('memberDetail.avatarProcessingFailed')) when invoking
NotificationToast.error so the message is localized.
- Around line 192-195: resetChanges currently spreads data.user directly into
formState, so birthDate loses the YYYY-MM-DD format applied in the initial
useEffect; update resetChanges to setFormState using data.user but override
birthDate by formatting data.user.birthDate with dayjs(...).format('YYYY-MM-DD')
(use the same formatting logic as in the useEffect) and keep setisUpdated(false)
and the conditional on data.user.
- Around line 603-610: Replace the inline style on the Button in
UserContactDetails (the Button with onClick={handleUserUpdate} and
data-testid="saveChangesBtn") by adding a CSS module class (e.g.,
saveChangesBtn) in style/app-fixed.module.css with the background-color:#A8C7FA
and color:#555, import that module into the component, and assign the className
to the Button instead of using the style prop so the inline color attributes are
removed and the CI CSS-Policy-Check passes.
- Around line 240-251: The interactive avatar edit control currently uses an <i>
with role="button", which is inaccessible; replace that element with a semantic
<button> (e.g., change the <i> element rendered in UserContactDetails to a
<button>), keep the same className, data-testid, title and aria-label, add
type="button", remove role and tabIndex and the onKeyDown handler (the native
button provides keyboard activation), and preserve the onClick handler that
calls fileInputRef.current?.click(); ensure
styles.userContactDetailContactAvatarEditIcon and other Bootstrap positioning
classes remain applied.
- Around line 121-129: The local duplicate helper removeEmptyFields inside
handleUserUpdate should be removed and the shared implementation imported from
the existing utility; delete the inline const removeEmptyFields declaration in
UserContactDetails.tsx (the one nested in handleUserUpdate) and add an import of
removeEmptyFields from src/utils/userUpdateUtils.ts near the top of the file,
then use the imported removeEmptyFields wherever the local version was used
(ensure handleUserUpdate still calls removeEmptyFields as before).
In @src/style/app-fixed.module.css:
- Around line 2861-2890: Ensure these utilities are used as CSS-module-scoped
classes (e.g., consumers should reference styles.avatarImage,
styles.avatarPlaceholder, styles.avatarPlaceholderSize, styles.flexCenter,
styles.flexColumn) rather than global class names, and update .avatarImage to
include display: block to prevent inline-image baseline spacing when applied to
<img> elements; verify all call sites import the module and reference the
classes via the module object and adjust any DOM usage expecting global classes.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (35)
docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/ACCEPT_EVENT_INVITATION.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/ADD_ADMIN_MUTATION.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/CREATE_MEMBER_PG.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/CREATE_ORGANIZATION_MEMBERSHIP_MUTATION_PG.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/CREATE_ORGANIZATION_MUTATION.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/CREATE_ORGANIZATION_MUTATION_PG.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/CREATE_POST_MUTATION.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/DELETE_ORGANIZATION_MUTATION.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/DELETE_POST_MUTATION.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/DONATE_TO_ORGANIZATION.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/FORGOT_PASSWORD_MUTATION.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/GENERATE_OTP_MUTATION.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/GET_FILE_PRESIGNEDURL.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/LOGOUT_MUTATION.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/PRESIGNED_URL.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/REFRESH_TOKEN_MUTATION.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/REMOVE_ADMIN_MUTATION.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/REMOVE_MEMBER_MUTATION.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/REMOVE_MEMBER_MUTATION_PG.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/RESET_COMMUNITY.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/REVOKE_REFRESH_TOKEN.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/SEND_EVENT_INVITATIONS.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/SIGNUP_MUTATION.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/UPDATE_COMMUNITY_PG.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/UPDATE_EVENT_MUTATION.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/UPDATE_POST_MUTATION.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/UPDATE_POST_VOTE.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/UPDATE_SESSION_TIMEOUT_PG.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/UPDATE_USER_PASSWORD_MUTATION.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/GraphQl/Mutations/mutations/variables/VERIFY_EVENT_INVITATION.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/screens/MemberDetail/mock/variables/MOCKS1.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/screens/MemberDetail/mock/variables/MOCKS2.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/screens/MemberDetail/mock/variables/MOCK_FILE.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/screens/MemberDetail/mock/variables/UPDATE_MOCK.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/screens/MemberDetail/mock/variables/UPDATE_USER_ERROR_MOCKS.mdis excluded by!**/docs/docs/**
📒 Files selected for processing (7)
.github/workflows/config/countline_excluded_file_list.txtsrc/GraphQl/Mutations/mutations.tssrc/screens/MemberDetail/MemberDetail.spec.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/screens/MemberDetail/UserContactDetails.tsxsrc/screens/MemberDetail/mock.tssrc/style/app-fixed.module.css
💤 Files with no reviewable changes (1)
- src/GraphQl/Mutations/mutations.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Test coverage must be ≥95% for modified files. Run: pnpm run test:coverage and verify coverage/coverage-summary.json shows no files below threshold
Files:
src/screens/MemberDetail/MemberDetail.spec.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Zero TypeScript errors: Run pnpm run typecheck and must pass without errors or warnings
Files:
src/screens/MemberDetail/MemberDetail.spec.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/screens/MemberDetail/UserContactDetails.tsx
**/*
⚙️ CodeRabbit configuration file
**/*: Conduct a comprehensive review and reevaluate all resolved and dismissed items.VALIDATION FRAMEWORK
1. Test Coverage & Quality
1.1 Test File Requirements
For each modified source file with logic or UI:
- ✅ Test file exists:
*.spec.tsx,*.spec.ts,*.test.tsx, or*.test.ts- ✅ Test file colocated with component (same directory)
- ✅ Test file updated to reflect source changes
- ✅ Target ≥95% code coverage for new/modified code
- ✅ List specific line numbers lacking test coverage
1.2 Test Quality Standards
✅ Framework Compliance (Vitest + React Testing Library):
- Use
vi.mock()for mocking (notjest.mock())- Use
describe(),it(),test()for structure- Use
@testing-library/reactqueries (getByRole, getByText, etc.)- Use
@testing-library/user-eventfor interactions✅ Coverage Completeness - All scenarios tested:
- Success paths with valid data
- Error scenarios (network failures, GraphQL errors, validation errors)
- Edge cases (empty data, null/undefined, boundary values)
- Loading and pending states
- User interactions and state changes
✅ Mock Quality:
- GraphQL:
MockedProviderorStaticMockLink- Realistic, comprehensive mock data
- Proper cleanup (reset mocks, clear timers)
✅ Test Structure:
- Isolated tests (no execution order dependency)
- No disabled tests (
*.skip) without documented reason + linked issue- Proper async utilities (
waitFor,act)- Shard-compatible (12-shard strategy, no shared state, <30s timeout)
2. Code Quality & Implementation
2.1 TypeScript Standards
- ✅ No
anytype without JSDoc justification- ✅ Proper typing for:
- Component props (interface declarations)
- Function parameters and return types
- GraphQL variables and responses
- State and context
- ✅ No
@ts-ignorewithout explanation- ✅ Zero TypeScript compilation errors
2.2 React & GraphQL Implementation
- ✅ React Best Practices:
- Fu...
Files:
src/screens/MemberDetail/MemberDetail.spec.tsxsrc/style/app-fixed.module.csssrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/screens/MemberDetail/UserContactDetails.tsx
🧠 Learnings (9)
📚 Learning: 2025-12-21T08:59:37.942Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5287
File: src/shared-components/Recurrence/CustomRecurrenceModal.spec.tsx:78-84
Timestamp: 2025-12-21T08:59:37.942Z
Learning: In talawa-admin test files, ensure each spec file uses an explicit vi.clearAllMocks() in an afterEach block to guarantee test isolation. This should be present even if a global cleanup exists in vitest.setup.ts, as the linter enforces per-file hygiene. Apply this guideline to all spec files (e.g., src/**/*.spec.tsx) to satisfy ESLint/Vitest requirements and prevent bleed-over between tests.
Applied to files:
src/screens/MemberDetail/MemberDetail.spec.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsx
📚 Learning: 2025-12-27T11:22:41.191Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5782
File: src/screens/Leaderboard/Leaderboard.spec.tsx:342-370
Timestamp: 2025-12-27T11:22:41.191Z
Learning: In test files that render screens with shared components (e.g., src/screens/Leaderboard/Leaderboard.spec.tsx), verify integration behavior by asserting the screen renders correctly in different scenarios (e.g., search results) without asserting the internal ARIA attributes, text, or icons of the shared component. Rely on the shared component's own tests to cover accessibility and presentation details. This keeps tests focused on higher-level behavior and interactions.
Applied to files:
src/screens/MemberDetail/MemberDetail.spec.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsx
📚 Learning: 2026-01-08T18:48:10.364Z
Learnt from: abhishekblue
Repo: PalisadoesFoundation/talawa-admin PR: 6260
File: src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.spec.tsx:337-339
Timestamp: 2026-01-08T18:48:10.364Z
Learning: In tests that exercise Apollo Client queries (e.g., talawa-admin tests under src/screens/.../OrganizationPeople.spec.tsx), avoid using vi.useFakeTimers() because Apollo's internal promise resolution can be disrupted by mocked timers, causing timeouts. Use real timers with explicit waits (for example, await act(async () => { await new Promise(r => setTimeout(r, DURATION)); })) when testing components that fetch data via Apollo. If you must use timers, prefer real timers for the relevant test and flush promises deterministically. Replace or remove useFakeTimers in such tests and rely on actual time passage or proper promise flushing to ensure reliable Apollo-driven data fetching tests.
Applied to files:
src/screens/MemberDetail/MemberDetail.spec.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsx
📚 Learning: 2025-12-26T20:34:53.912Z
Learnt from: MuhammadUmar7195
Repo: PalisadoesFoundation/talawa-admin PR: 5847
File: src/screens/UserPortal/Volunteer/Invitations/Invitations.tsx:243-243
Timestamp: 2025-12-26T20:34:53.912Z
Learning: In PalisadoesFoundation/talawa-admin, whenever you modify a file (e.g., for migrations, tests, or feature changes), replace inline color attributes like color="grey" with Bootstrap utility classes (e.g., className="text-secondary") to satisfy the CSS-Policy-Check CI. This is a general compliance guideline, not scope creep, and should be applied to all affected TSX files during reviews. Steps: 1) Find inline color attributes in JSX/TSX. 2) Replace with appropriate Bootstrap text/color utility classes. 3) Ensure the visual outcome remains the same. 4) Run CI checks to confirm the policy passes.
Applied to files:
src/screens/MemberDetail/MemberDetail.spec.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/screens/MemberDetail/UserContactDetails.tsx
📚 Learning: 2026-01-06T16:39:41.033Z
Learnt from: Jashan32
Repo: PalisadoesFoundation/talawa-admin PR: 6199
File: src/shared-components/ProfileForm/ProfileForm.tsx:0-0
Timestamp: 2026-01-06T16:39:41.033Z
Learning: When refactoring or consolidating components (e.g., talawa-admin), remove helper functions that were copied from old files but are not used in the new implementation. Before exporting utilities, verify there are real usages across the codebase (search usages, remove dead code, and keep only necessary exports) to improve maintainability and reduce bundle size.
Applied to files:
src/screens/MemberDetail/MemberDetail.spec.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/screens/MemberDetail/UserContactDetails.tsx
📚 Learning: 2026-01-07T09:08:09.150Z
Learnt from: adityai0
Repo: PalisadoesFoundation/talawa-admin PR: 6202
File: src/components/EventManagement/EventAgendaItems/EventAgendaItems.tsx:4-13
Timestamp: 2026-01-07T09:08:09.150Z
Learning: In the talawa-admin repository, document comments with TSDoc in all TSX files. Do not use JSDoc. Write non-standard tags like component and requires as plain text (no leading @). Use standard TSDoc tags such as param, remarks, returns, and example with the leading @. This guideline applies to all TSX components (e.g., src/components/.../*.tsx).
Applied to files:
src/screens/MemberDetail/MemberDetail.spec.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsxsrc/screens/MemberDetail/UserContactDetails.tsx
📚 Learning: 2025-12-27T11:22:19.556Z
Learnt from: rawadhossain
Repo: PalisadoesFoundation/talawa-admin PR: 5782
File: src/screens/Leaderboard/Leaderboard.spec.tsx:299-329
Timestamp: 2025-12-27T11:22:19.556Z
Learning: In talawa-admin test files (e.g., src/screens/Leaderboard/Leaderboard.spec.tsx), prefer separate explicit test cases for keyboard navigation interactions (Enter key, Space key) instead of consolidating them with test.each()/parameterization. This improves readability, documents each keyboard interaction path clearly, and makes failures easier to diagnose when specific key handling regresses. Apply this pattern to similarly structured spec files by keeping individual tests for each key interaction rather than combining them into a single parameterized test.
Applied to files:
src/screens/MemberDetail/MemberDetail.spec.tsxsrc/screens/MemberDetail/UserContactDetails.spec.tsx
📚 Learning: 2026-01-10T20:29:21.541Z
Learnt from: Piyush-Goenka
Repo: PalisadoesFoundation/talawa-admin PR: 6315
File: src/screens/AdminPortal/ManageTag/editModal/EditUserTagModal.module.css:24-30
Timestamp: 2026-01-10T20:29:21.541Z
Learning: For CSS review guidance, apply changes to CSS files across the project (not TSX unless specifically asked). When suggesting improvements to CSS selectors, prefer CSS-only solutions that leverage existing DOM attributes (e.g., form ids, data attributes) and avoid proposing changes that require modifying TSX files. This aligns with the PR scope (CSS file changes and how they’re imported in TSX) and ensures changes remain isolated to styles rather than TSX logic.
Applied to files:
src/style/app-fixed.module.css
📚 Learning: 2026-01-12T18:08:53.979Z
Learnt from: Piyush-Goenka
Repo: PalisadoesFoundation/talawa-admin PR: 6373
File: src/screens/AdminPortal/OrganizationFunds/OrganizationFunds.module.css:12-18
Timestamp: 2026-01-12T18:08:53.979Z
Learning: In CSS migration PRs (e.g., the CSS Streamlining phases), limit the scope to moving styles from global stylesheets to colocated .module.css files and adopting design tokens. Do not attempt layout refactors or general CSS implementation pattern changes unless the PR explicitly states this is the goal. Preserve existing CSS behavior during migration unless a refactor is explicitly intended.
Applied to files:
src/style/app-fixed.module.css
🧬 Code graph analysis (2)
src/screens/MemberDetail/UserContactDetails.spec.tsx (5)
src/utils/StaticMockLink.ts (1)
StaticMockLink(42-186)src/screens/MemberDetail/mock.ts (5)
MOCKS1(13-163)MOCKS2(165-257)UPDATE_USER_ERROR_MOCKS(303-369)MOCK_FILE(371-435)UPDATE_MOCK(259-301)src/components/AdminPortal/Advertisements/AdvertisementsMocks.ts (2)
wait(196-202)link(189-189)src/state/store.ts (1)
store(4-6)src/components/NotificationToast/NotificationToast.tsx (1)
NotificationToast(76-82)
src/screens/MemberDetail/UserContactDetails.tsx (9)
src/utils/useLocalstorage.ts (2)
useLocalStorage(82-92)getItem(30-34)src/GraphQl/Mutations/mutations.ts (1)
UPDATE_USER_MUTATION(98-128)src/GraphQl/Queries/Queries.ts (1)
GET_USER_BY_ID(452-477)src/components/NotificationToast/NotificationToast.tsx (1)
NotificationToast(76-82)src/utils/SanitizeInput.tsx (1)
sanitizeInput(8-52)src/utils/userUpdateUtils.ts (1)
removeEmptyFields(23-35)src/utils/passwordValidator.ts (1)
validatePassword(1-20)src/utils/errorHandler.tsx (1)
errorHandler(9-88)src/utils/formEnumFields.ts (5)
genderEnum(349-349)educationGradeEnum(347-347)employmentStatusEnum(350-350)maritalStatusEnum(348-348)countryOptions(346-346)
🪛 Biome (2.1.2)
src/screens/MemberDetail/UserContactDetails.tsx
[error] 245-245: The HTML element i is non-interactive and should not have an interactive role.
Replace i with a div or a span.
Unsafe fix: Remove the role attribute.
(lint/a11y/noNoninteractiveElementToInteractiveRole)
🔇 Additional comments (6)
.github/workflows/config/countline_excluded_file_list.txt (1)
14-15: Exclusion is justified—countline.py is a code complexity check, not a coverage gate.This script enforces maximum line count thresholds (600 lines) to maintain code maintainability. Both files legitimately exceed this limit:
EventForm.tsx: 654 linesUserContactDetails.tsx: 618 linesExcluding large but necessary components is the appropriate way to handle this constraint. This list does not affect test coverage or quality gates; it only exempts files from the line count validation check.
src/screens/MemberDetail/UserContactDetails.spec.tsx (3)
1-30: LGTM on imports and setup.The test file correctly uses Vitest (
vi) for mocking, React Testing Library for rendering/queries, and proper Apollo mocking withStaticMockLink. The import ofNotificationToastfor spying is appropriate.
62-104: Inline type definition in mock should be extracted.Per coding guidelines, prop interfaces should be defined in
src/types/.../interface.ts, not inline. The mock's prop type (lines 70-75) should be extracted.However, since this is a test file mock and not a reusable component, this is acceptable for test isolation purposes.
196-209: Good test isolation withvi.clearAllMocks()inafterEach.This follows the repository's test hygiene guidelines requiring explicit mock cleanup per spec file, even with global cleanup in
vitest.setup.ts. Based on learnings, this ensures test isolation.src/screens/MemberDetail/UserContactDetails.tsx (2)
199-614: Overall component structure is well-organized.The component properly:
- Uses Apollo
useQueryanduseMutationwith refetch- Implements form state management with
useState- Handles file upload validation (type, size)
- Provides accessibility attributes on most form controls
- Uses Bootstrap layout components effectively
- Conditionally renders save/reset buttons based on
isUpdatedstate
107-109: No action required — code already uses translation keys correctly.The code at lines 107-109 already uses
t('invalidFileType')andt('fileTooLarge'), not hardcoded strings. Both translation keys exist in all 5 locale files (en,es,fr,hi,zh) with proper translations inpublic/locales/{locale}/translation.json. The test expectations correctly validate thatNotificationToast.erroris called with the translated message strings.Likely an incorrect or invalid review comment.
Comment on lines
+1
to
+5
| import { describe, it, expect, vi } from 'vitest'; | ||
| import { render, screen, fireEvent } from '@testing-library/react'; | ||
| import MemberDetail from './MemberDetail'; | ||
| import { ReactNode } from 'react'; | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the test file to see imports and afterEach usage
head -n 20 src/screens/MemberDetail/MemberDetail.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 210
🏁 Script executed:
# Check lines around 78-82 where afterEach is referenced
sed -n '70,85p' src/screens/MemberDetail/MemberDetail.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 352
🏁 Script executed:
# Find Vitest config
fd -t f "(vitest|vite)\\.config" -e ts -e jsRepository: PalisadoesFoundation/talawa-admin
Length of output: 142
🏁 Script executed:
# Check the Vitest config for globals setting
cat vite.config.ts 2>/dev/null || cat vitest.config.ts 2>/dev/null || echo "No config found"Repository: PalisadoesFoundation/talawa-admin
Length of output: 2734
🏁 Script executed:
# Search for fireEvent usage in the test file to verify the concern
rg "fireEvent" src/screens/MemberDetail/MemberDetail.spec.tsx -nRepository: PalisadoesFoundation/talawa-admin
Length of output: 447
🏁 Script executed:
# Verify full extent of fireEvent usage and check if userEvent is imported anywhere
rg "userEvent|fireEvent" src/screens/MemberDetail/MemberDetail.spec.tsx -nRepository: PalisadoesFoundation/talawa-admin
Length of output: 447
🏁 Script executed:
# Check coding guidelines requirement - confirm userEvent is the expected pattern
rg "userEvent|@testing-library/user-event" src/components -A 2 -B 2 | head -50Repository: PalisadoesFoundation/talawa-admin
Length of output: 4314
🏁 Script executed:
# Look at the PeopleTabNavbarButton mock to understand title values
sed -n '50,75p' src/screens/MemberDetail/MemberDetail.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 507
Replace fireEvent with @testing-library/user-event per framework compliance standards.
The test uses fireEvent.click() at lines 97, 109, 117, 130, and 134. Coding guidelines (§1.2 Framework Compliance) require using @testing-library/user-event for interactions. Update the import and convert all fireEvent.click() calls to await userEvent.click().
Proposed fix
-import { render, screen, fireEvent } from '@testing-library/react';
+import { render, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';Then replace each instance:
- fireEvent.click(screen.getByTestId('tab-organizations'));
+ await userEvent.click(screen.getByTestId('tab-organizations'));🤖 Prompt for AI Agents
In @src/screens/MemberDetail/MemberDetail.spec.tsx around lines 1 - 5, Replace
use of fireEvent with @testing-library/user-event: change the import to pull
userEvent from '@testing-library/user-event' instead of fireEvent, then update
every fireEvent.click(...) call in MemberDetail.spec.tsx (the calls around the
interactions in the tests referencing the MemberDetail component) to await
userEvent.click(...). Ensure the tests containing those interactions are
declared async so you can use await (update the relevant it/describe callbacks
to async where needed) and remove the unused fireEvent import.
Comment on lines
+2
to
+3
| import { render, screen, fireEvent } from '@testing-library/react'; | ||
| import MemberDetail from './MemberDetail'; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd MemberDetail.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 210
🏁 Script executed:
cat -n src/screens/MemberDetail/MemberDetail.spec.tsx | head -150Repository: PalisadoesFoundation/talawa-admin
Length of output: 4798
🏁 Script executed:
rg "fireEvent|user-event" src/screens/MemberDetail/MemberDetail.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 612
🏁 Script executed:
grep -i "@testing-library/user-event" package.json pnpm-lock.yaml 2>/dev/null | head -20Repository: PalisadoesFoundation/talawa-admin
Length of output: 414
Replace fireEvent with @testing-library/user-event for all tab click interactions.
Project test guidelines prefer user-event for interactions (closer to real user behavior and async-safe). This requires changes to the import statement and all tab-switching tests in this file.
Scope of changes needed
- Line 2: Update import statement (remove
fireEvent, adduserEvent) - Line 94-104: "switches to organizations tab" test
- Line 106-112: "switches to tags tab" test
- Line 114-124: "switches to events tab" test
- Line 126-144: "switches back to overview tab" test (contains 2
fireEvent.clickcalls at lines 130 and 134)
🤖 Prompt for AI Agents
In @src/screens/MemberDetail/MemberDetail.spec.tsx around lines 2 - 3, Update
the import in MemberDetail.spec.tsx to remove fireEvent and import userEvent
from '@testing-library/user-event'; then in the tests "switches to organizations
tab", "switches to tags tab", "switches to events tab", and "switches back to
overview tab" replace fireEvent.click(...) calls with await userEvent.click(...)
calls, marking those test functions async so you can await the click calls (the
"switches back to overview tab" test has two click calls to convert). Ensure all
references to fireEvent are removed and use await userEvent.click on the tab
elements to match project testing guidelines.
Comment on lines
+52
to
+74
| vi.mock( | ||
| 'shared-components/PeopleTabNavbarButton/PeopleTabNavbarButton', | ||
| () => ({ | ||
| default: ({ | ||
| title, | ||
| action, | ||
| isActive, | ||
| }: { | ||
| title: string; | ||
| action: () => void; | ||
| isActive: boolean; | ||
| }) => ( | ||
| <button | ||
| data-testid={`tab-${title}`} | ||
| type="button" | ||
| data-active={isActive} | ||
| onClick={action} | ||
| > | ||
| {title} | ||
| </button> | ||
| ), | ||
| }), | ||
| ); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the MemberDetail test file and examine the relevant lines
find . -type f -name "MemberDetail.spec.tsx" | head -5Repository: PalisadoesFoundation/talawa-admin
Length of output: 125
🏁 Script executed:
# Read the test file to see lines 52-74 and 83-144
cat -n src/screens/MemberDetail/MemberDetail.spec.tsx | sed -n '50,150p'Repository: PalisadoesFoundation/talawa-admin
Length of output: 3158
🏁 Script executed:
# Also find and examine the actual MemberDetail component to see how it uses PeopleTabNavbarButton
find . -type f -name "MemberDetail.tsx" | grep -v spec | head -5Repository: PalisadoesFoundation/talawa-admin
Length of output: 120
🏁 Script executed:
# Read the MemberDetail.tsx component to see how it uses PeopleTabNavbarButton
cat -n src/screens/MemberDetail/MemberDetail.tsx | head -200Repository: PalisadoesFoundation/talawa-admin
Length of output: 3994
🏁 Script executed:
# Search for PeopleTabNavbarButton usage in MemberDetail.tsx
grep -n "PeopleTabNavbarButton\|title=" src/screens/MemberDetail/MemberDetail.tsx | head -20Repository: PalisadoesFoundation/talawa-admin
Length of output: 493
🏁 Script executed:
# Get more context around PeopleTabNavbarButton usage
grep -B5 -A5 "PeopleTabNavbarButton" src/screens/MemberDetail/MemberDetail.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 1734
Add missing active-state assertion for Tags tab to match other tab tests.
The tags tab test (line 106–112) only verifies that the content appears but doesn't assert that tab-tags has data-active='true', unlike the overview, organizations, and events tabs which all include this check. This should be added for consistency:
Recommended change
it('switches to tags tab', () => {
render(<MemberDetail />);
fireEvent.click(screen.getByTestId('tab-tags'));
expect(screen.getByTestId('user-tags')).toBeInTheDocument();
+ expect(screen.getByTestId('tab-tags')).toHaveAttribute('data-active', 'true');
});(The concern about test IDs being brittle due to i18n keys is not applicable here—the component passes the translated string result from tCommon(), not the translation key itself, so test IDs remain stable.)
🤖 Prompt for AI Agents
In @src/screens/MemberDetail/MemberDetail.spec.tsx around lines 52 - 74, Add the
missing active-state assertion for the Tags tab: in the tags tab test (the block
that already verifies the tags content around lines 106–112) add an assertion
that the element with test id "tab-tags" has the attribute data-active equal to
"true" (to match the overview/organizations/events tests that check active
state). Locate the test referencing "tab-tags" and append the active-state check
using your test framework's attribute assertion.
Comment on lines
+211
to
+243
| describe('Member Profile View (Admin viewing member)', () => { | ||
| test('should render the elements for member profile', async () => { | ||
| renderMemberDetailScreen(link1); | ||
| await wait(); | ||
|
|
||
| expect(screen.queryByText('Loading data...')).not.toBeInTheDocument(); | ||
| expect(screen.getAllByText(/Email/i)).toBeTruthy(); | ||
| expect(screen.getAllByText(/name/i)).toBeTruthy(); | ||
| expect(screen.getAllByText(/Birth Date/i)).toBeTruthy(); | ||
| expect(screen.getAllByText(/Gender/i)).toBeTruthy(); | ||
| expect(screen.getAllByText(/Profile Information/i)).toBeTruthy(); | ||
| expect(screen.getAllByText(/Profile Information/i)).toHaveLength(1); | ||
| expect(screen.getAllByText(/Contact Information/i)).toHaveLength(1); | ||
| }); | ||
|
|
||
| test('handles member profile update success', async () => { | ||
| renderMemberDetailScreen(link1); | ||
|
|
||
| await wait(); | ||
|
|
||
| const nameInput = screen.getByTestId('inputName'); | ||
| fireEvent.change(nameInput, { target: { value: 'New Name' } }); | ||
| expect(nameInput).toHaveValue('New Name'); | ||
|
|
||
| const saveButton = screen.getByTestId('saveChangesBtn'); | ||
| expect(saveButton).toBeInTheDocument(); | ||
| fireEvent.click(saveButton); | ||
|
|
||
| await waitFor(() => { | ||
| expect(nameInput).toHaveValue('New Name'); | ||
| }); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Weak assertions using toBeTruthy() on getAllByText.
Using toBeTruthy() on getAllByText results only checks that the array is non-empty but doesn't verify the expected count or content. Consider using toHaveLength() or expect(...).toBeInTheDocument() for stronger assertions.
♻️ Suggested improvement
- expect(screen.getAllByText(/Email/i)).toBeTruthy();
- expect(screen.getAllByText(/name/i)).toBeTruthy();
- expect(screen.getAllByText(/Birth Date/i)).toBeTruthy();
- expect(screen.getAllByText(/Gender/i)).toBeTruthy();
+ expect(screen.getByText(/Email/i)).toBeInTheDocument();
+ expect(screen.getByLabelText(/name/i)).toBeInTheDocument();
+ expect(screen.getByText(/Birth Date/i)).toBeInTheDocument();
+ expect(screen.getByText(/Gender/i)).toBeInTheDocument();Also applies to: 245-277
🤖 Prompt for AI Agents
In @src/screens/MemberDetail/UserContactDetails.spec.tsx around lines 211 - 243,
Replace weak truthy checks that use expect(getAllByText(...)).toBeTruthy() with
stronger assertions: for elements where you expect a single match use
expect(screen.getAllByText(/Profile Information/i)).toHaveLength(1) or prefer
expect(screen.getByText(/Profile Information/i)).toBeInTheDocument(), and for
other fields assert exact counts with toHaveLength() or assert presence with
getByText + toBeInTheDocument(); update the assertions in the 'should render the
elements for member profile' test (and the similar assertions referenced at
lines ~245-277) that call getAllByText for Email, name, Birth Date, Gender,
Profile Information, Contact Information to use either getByText +
toBeInTheDocument or getAllByText + toHaveLength with the expected count.
Comment on lines
+280
to
+296
| test('Should display dicebear image if image is null', async () => { | ||
| const dicebearUrl = 'mocked-data-uri'; | ||
|
|
||
| render( | ||
| <MockedProvider link={link1}> | ||
| <BrowserRouter> | ||
| <MemberDetail /> | ||
| </BrowserRouter> | ||
| </MockedProvider>, | ||
| ); | ||
|
|
||
| const userImage = await waitFor(() => | ||
| screen.getByTestId('profile-picture'), | ||
| ); | ||
|
|
||
| expect(userImage.getAttribute('src')).toBe(dicebearUrl); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Test renders without routing context.
Using BrowserRouter without a specific route pattern may cause the component to behave differently than in the actual app (e.g., useParams returns empty, path detection fails). Consider using MemoryRouter with initialEntries for consistent behavior.
🤖 Prompt for AI Agents
In @src/screens/MemberDetail/UserContactDetails.spec.tsx around lines 280 - 296,
The test for MemberDetail uses BrowserRouter which can yield different routing
context (empty useParams); update the test to wrap MemberDetail with
MemoryRouter providing appropriate initialEntries (and initialIndex if needed)
so the component receives expected route params/path; locate the test block
'Should display dicebear image if image is null' and replace BrowserRouter with
MemoryRouter configured to the route used by MemberDetail (so useParams and path
matching work consistently).
Comment on lines
+240
to
+251
| <i | ||
| className={`fas fa-edit position-absolute bottom-0 right-0 p-2 bg-white rounded-circle ${styles.userContactDetailContactAvatarEditIcon}`} | ||
| onClick={() => fileInputRef.current?.click()} | ||
| data-testid="uploadImageBtn" | ||
| title={tCommon('userEditProfilePicture')} | ||
| role="button" | ||
| aria-label={tCommon('userEditProfilePicture')} | ||
| tabIndex={0} | ||
| onKeyDown={(e) => | ||
| e.key === 'Enter' && fileInputRef.current?.click() | ||
| } | ||
| /> |
Contributor
There was a problem hiding this comment.
Accessibility issue: <i> element should not have interactive role.
Static analysis correctly identifies that using <i> with role="button" violates accessibility guidelines. Replace with a <button> element for proper semantics and keyboard handling.
🐛 Proposed fix
- <i
- className={`fas fa-edit position-absolute bottom-0 right-0 p-2 bg-white rounded-circle ${styles.userContactDetailContactAvatarEditIcon}`}
+ <button
+ type="button"
+ className={`fas fa-edit position-absolute bottom-0 right-0 p-2 bg-white rounded-circle border-0 ${styles.userContactDetailContactAvatarEditIcon}`}
onClick={() => fileInputRef.current?.click()}
data-testid="uploadImageBtn"
title={tCommon('userEditProfilePicture')}
- role="button"
aria-label={tCommon('userEditProfilePicture')}
- tabIndex={0}
- onKeyDown={(e) =>
- e.key === 'Enter' && fileInputRef.current?.click()
- }
- />
+ >
+ <span className="visually-hidden">{tCommon('userEditProfilePicture')}</span>
+ </button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <i | |
| className={`fas fa-edit position-absolute bottom-0 right-0 p-2 bg-white rounded-circle ${styles.userContactDetailContactAvatarEditIcon}`} | |
| onClick={() => fileInputRef.current?.click()} | |
| data-testid="uploadImageBtn" | |
| title={tCommon('userEditProfilePicture')} | |
| role="button" | |
| aria-label={tCommon('userEditProfilePicture')} | |
| tabIndex={0} | |
| onKeyDown={(e) => | |
| e.key === 'Enter' && fileInputRef.current?.click() | |
| } | |
| /> | |
| <button | |
| type="button" | |
| className={`position-absolute bottom-0 right-0 p-2 bg-white rounded-circle border-0 ${styles.userContactDetailContactAvatarEditIcon}`} | |
| onClick={() => fileInputRef.current?.click()} | |
| data-testid="uploadImageBtn" | |
| title={tCommon('userEditProfilePicture')} | |
| aria-label={tCommon('userEditProfilePicture')} | |
| > | |
| <i className="fas fa-edit" aria-hidden="true"></i> | |
| </button> |
🧰 Tools
🪛 Biome (2.1.2)
[error] 245-245: The HTML element i is non-interactive and should not have an interactive role.
Replace i with a div or a span.
Unsafe fix: Remove the role attribute.
(lint/a11y/noNoninteractiveElementToInteractiveRole)
🤖 Prompt for AI Agents
In @src/screens/MemberDetail/UserContactDetails.tsx around lines 240 - 251, The
interactive avatar edit control currently uses an <i> with role="button", which
is inaccessible; replace that element with a semantic <button> (e.g., change the
<i> element rendered in UserContactDetails to a <button>), keep the same
className, data-testid, title and aria-label, add type="button", remove role and
tabIndex and the onKeyDown handler (the native button provides keyboard
activation), and preserve the onClick handler that calls
fileInputRef.current?.click(); ensure
styles.userContactDetailContactAvatarEditIcon and other Bootstrap positioning
classes remain applied.
Comment on lines
+445
to
+446
| <Col md={12}> | ||
| <label htmlFor="phoneNumber" className="form-label"> |
Contributor
There was a problem hiding this comment.
Duplicate htmlFor attributes: multiple labels use "phoneNumber".
Lines 429, 446, and 463 all use htmlFor="phoneNumber" but the inputs have different IDs (mobilePhoneNumber, workPhoneNumber, homePhoneNumber). This breaks the label-input association for accessibility.
🐛 Proposed fix
- <label htmlFor="phoneNumber" className="form-label">
+ <label htmlFor="workPhoneNumber" className="form-label">
{t('workPhoneNumber')}
</label>
...
- <label htmlFor="phoneNumber" className="form-label">
+ <label htmlFor="homePhoneNumber" className="form-label">
{t('homePhoneNumber')}
</label>Also applies to: 462-463
🤖 Prompt for AI Agents
In @src/screens/MemberDetail/UserContactDetails.tsx around lines 445 - 446, The
labels currently use a duplicate htmlFor="phoneNumber" which breaks
accessibility; update each label in UserContactDetails (the three phone number
label elements near the mobile/work/home phone inputs) to use the matching input
ID values (e.g., htmlFor="mobilePhoneNumber", htmlFor="workPhoneNumber",
htmlFor="homePhoneNumber") so each label correctly associates with its
corresponding input element.
Comment on lines
+480
to
+481
| <label htmlFor="address" className="form-label"> | ||
| {t('addressLine1')} |
Contributor
There was a problem hiding this comment.
More duplicate htmlFor="address" attributes.
Labels at lines 480, 497, and 514 all use htmlFor="address" but the inputs have IDs addressLine1, addressLine2, and postalCode. Fix the label associations.
🐛 Proposed fix
- <label htmlFor="address" className="form-label">
+ <label htmlFor="addressLine1" className="form-label">
{t('addressLine1')}
</label>
...
- <label htmlFor="address" className="form-label">
+ <label htmlFor="addressLine2" className="form-label">
{t('addressLine2')}
</label>
...
- <label htmlFor="address" className="form-label">
+ <label htmlFor="postalCode" className="form-label">
{t('postalCode')}
</label>Also applies to: 496-497, 513-514
🤖 Prompt for AI Agents
In @src/screens/MemberDetail/UserContactDetails.tsx around lines 480 - 481, In
UserContactDetails update the three label htmlFor attributes that currently use
the duplicate value "address" so they match their corresponding input IDs:
change the label for the first address field to htmlFor="addressLine1", the
second to htmlFor="addressLine2", and the postal code label to
htmlFor="postalCode" so each label correctly associates with inputs
id="addressLine1", id="addressLine2", and id="postalCode".
Comment on lines
+603
to
+610
| <Button | ||
| variant="outline" | ||
| style={{ backgroundColor: '#A8C7FA', color: '#555' }} | ||
| onClick={handleUserUpdate} | ||
| data-testid="saveChangesBtn" | ||
| > | ||
| {tCommon('saveChanges')} | ||
| </Button> |
Contributor
There was a problem hiding this comment.
Inline style violates CSS policy.
Per learnings, inline color attributes should be replaced with Bootstrap utility classes or CSS module classes to satisfy the CSS-Policy-Check CI.
🐛 Proposed fix
Add a class to your CSS module (style/app-fixed.module.css):
.saveChangesBtn {
background-color: #A8C7FA;
color: #555;
}Then update the component:
<Button
variant="outline"
- style={{ backgroundColor: '#A8C7FA', color: '#555' }}
+ className={styles.saveChangesBtn}
onClick={handleUserUpdate}
data-testid="saveChangesBtn"
>🤖 Prompt for AI Agents
In @src/screens/MemberDetail/UserContactDetails.tsx around lines 603 - 610,
Replace the inline style on the Button in UserContactDetails (the Button with
onClick={handleUserUpdate} and data-testid="saveChangesBtn") by adding a CSS
module class (e.g., saveChangesBtn) in style/app-fixed.module.css with the
background-color:#A8C7FA and color:#555, import that module into the component,
and assign the className to the Button instead of using the style prop so the
inline color attributes are removed and the CI CSS-Policy-Check passes.
Comment on lines
+2861
to
+2890
| .avatarImage { | ||
| width: 40px; | ||
| height: 40px; | ||
| border-radius: 50%; | ||
| } | ||
| .flexCenter { | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| } | ||
| .flexColumn { | ||
| display: flex; | ||
| flex-direction: column; | ||
| align-items: center; | ||
| justify-content: center; | ||
| } | ||
|
|
||
| .avatarPlaceholder { | ||
| width: 40px; | ||
| height: 40px; | ||
| border-radius: 50%; | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| margin-top: 6px; | ||
| } | ||
| .avatarPlaceholderSize { | ||
| width: 40px; | ||
| height: 40px; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
CSS module utilities: ensure they’re consumed as module-scoped classes (and consider display: block for images).
Since this is app-fixed.module.css, these helpers won’t be global unless explicitly treated as such; verify callers use styles.avatarImage, etc. Also consider display: block; on .avatarImage if it’s applied to <img> to avoid baseline/inline spacing quirks. Based on learnings, keeping this CSS-only.
Proposed minimal CSS-only tweak
.avatarImage {
width: 40px;
height: 40px;
border-radius: 50%;
+ display: block;
}🤖 Prompt for AI Agents
In @src/style/app-fixed.module.css around lines 2861 - 2890, Ensure these
utilities are used as CSS-module-scoped classes (e.g., consumers should
reference styles.avatarImage, styles.avatarPlaceholder,
styles.avatarPlaceholderSize, styles.flexCenter, styles.flexColumn) rather than
global class names, and update .avatarImage to include display: block to prevent
inline-image baseline spacing when applied to <img> elements; verify all call
sites import the module and reference the classes via the module object and
adjust any DOM usage expecting global classes.
Contributor
Author
|
@palisadoes This file needs to be deleted: src/shared-components/ProfileForm/ProfileForm.tsx, as it conflicts with my updates. I am not able to remove it because if I run git merge upstream/develop or git rebase upstream/develop, it ends up undoing all the changes I made. We have two options:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What kind of change does this PR introduce?
feature, refactoring
Issue Number:
Fixes #4637
Snapshots/Videos:
If relevant, did you update the documentation?
Summary
Updated the People Tab section to exactly match the interface provided in Figma.
The entire UI is now consistent with the design.
Each user profile contains four tabs: Overview, Organizations, Events, and Tags.
The Overview and Organizations tabs are dynamic and fetch data from the backend.
Since the Talawa API currently does not provide queries for Events and Tags, these tabs are using dummy data.
This will be addressed and corrected in a separate PR once the required backend support is available. For now, all components, text, styling, and overall structure are complete; once the API provides the necessary queries, only data fetching and integration will remain.
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
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.