refactor(ActionItemModal): extract ApplyToSelector component [Part 1/5] #6314
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
|
To generate Unit Tests for this PR, please click here. |
Contributor
WalkthroughAdds a new ApplyToSelector component and type, integrates it into ActionItemModal (replacing in-file radio UI), updates DatePicker/test selectors and modal data-testids, and adjusts related state, initialization, and tests. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as ActionItemModal (UI)
participant Modal as BaseModal
participant Selector as ApplyToSelector
participant Picker as DatePicker
participant API as Backend
User->>UI: open modal
UI->>Modal: render modal container (actionItemModal)
UI->>Selector: render with current applyTo
User->>Selector: choose 'series' or 'instance'
Selector-->>UI: onChange(applyTo)
User->>Picker: pick assignment date
Picker-->>UI: emit selected date
User->>Modal: submit form
UI->>API: call create/update mutation (includes applyTo & date)
API-->>UI: response
UI-->>User: close modal / show result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (5)
🧰 Additional context used📓 Path-based instructions (4)src/**/components/**/*.{ts,tsx}📄 CodeRabbit inference engine (Custom checks)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (Custom checks)
Files:
**/*⚙️ CodeRabbit configuration file
Files:
**/*.{test,spec}.{ts,tsx}📄 CodeRabbit inference engine (Custom checks)
Files:
🧠 Learnings (10)📚 Learning: 2025-12-26T20:34:53.912ZApplied to files:
📚 Learning: 2026-01-06T16:39:41.033ZApplied to files:
📚 Learning: 2026-01-07T09:08:09.150ZApplied to files:
📚 Learning: 2026-01-08T16:45:49.201ZApplied to files:
📚 Learning: 2025-12-21T08:59:37.942ZApplied to files:
📚 Learning: 2025-12-27T11:22:41.191ZApplied to files:
📚 Learning: 2026-01-08T18:48:10.364ZApplied to files:
📚 Learning: 2025-12-27T11:22:19.556ZApplied to files:
📚 Learning: 2026-01-06T21:27:50.880ZApplied to files:
📚 Learning: 2026-01-07T14:21:19.711ZApplied to files:
🧬 Code graph analysis (3)src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx (1)
src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx (2)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsx (1)
🔇 Additional comments (10)
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 |
Our Pull Request Approval ProcessThis PR will be reviewed according to our: Your PR may be automatically closed if:
Thanks for contributing! |
There was a problem hiding this comment.
Pull request overview
This PR is Part 1/5 of refactoring the ActionItemModal.tsx file to reduce its size and improve maintainability. The PR successfully extracts duplicated radio button logic into a reusable ApplyToSelector component and updates deprecated imports.
Changes:
- Extracted
ApplyToSelectorcomponent to eliminate duplicated radio button logic for selecting between 'series' and 'instance' application scopes - Migrated from deprecated
react-bootstrapModal to sharedBaseModalcomponent - Migrated from deprecated
@mui/x-date-pickersDatePicker to shared DatePicker component - Updated all test files to use
getByTestIdworkaround for BaseModal's duplicaterole="dialog"issue
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/types/AdminPortal/ApplyToSelector/interface.ts | Defines TypeScript types for ApplyToSelector component with proper documentation |
| src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx | New reusable component for radio group selector with proper i18n support |
| src/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsx | Comprehensive test coverage for ApplyToSelector component |
| src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx | Refactored to use ApplyToSelector component and updated deprecated imports, reduced from 713 to 660 lines |
| src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsx | Updated all test assertions to use getByTestId workaround for BaseModal duplicate role issue |
| docs/docs/auto-docs/types/AdminPortal/ApplyToSelector/interface/type-aliases/ApplyToType.md | Auto-generated documentation for ApplyToType type alias |
| docs/docs/auto-docs/types/AdminPortal/ApplyToSelector/interface/interfaces/InterfaceApplyToSelectorProps.md | Auto-generated documentation for component props interface |
| docs/docs/auto-docs/components/AdminPortal/ApplyToSelector/ApplyToSelector/variables/default.md | Auto-generated documentation for ApplyToSelector component |
| docs/docs/auto-docs/screens/OrganizationActionItems/ActionItemModal/ActionItemModal/variables/default.md | Updated line number reference in auto-generated docs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
@nishantharkut Refactor your tests to use BaseModal |
nishantharkut
force-pushed
the
refactor/ActionItemModal
branch
from
7124624 to
3539ece
Compare
![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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsx (2)
814-817: AvoidfireEvent.submit(...)on non-form elements; submit the<form>or click the submit button.
Some tests usedocument.querySelector('form')(good), but others submit the submit button or mix patterns. Prefer one of:
fireEvent.submit(formElement)await user.click(screen.getByTestId('submitBtn'))Also applies to: 1151-1155, 1197-1201, 1356-1360
3542-3554: Rewrite ActionItemModal test:data-testid="assignmentDate"resolves to the input element directly, not a container.The shared
DatePickercomponent appliesdata-testiddirectly to theForm.Controlinput element (line 144 of DatePicker.tsx), not to a wrapper container. The test's assumption is incorrect:
screen.getByTestId('assignmentDate')returns the input element, not a containerquerySelector('input')on an input element returns null- The test should use the testid directly:
const dateInput = screen.getByTestId('assignmentDate');Alternatively, use role-based queries:
screen.getByRole('textbox').src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx (2)
101-170: Bug risk:applyToauto-sets to'series'for anyactionItem, which can clear non-template assignments during edit.
On Line 365-367,setApplyTo('series')runs for non-template items too, butapplyTothen drives filtering (Line 142+) and clearing (Line 402+). This can drop the selected volunteer/group in edit mode for non-template items.Proposed patch (scope applyTo only when selector is actually relevant)
- // Separate useEffect for applyTo initialization - useEffect(() => { - if (actionItem?.isInstanceException) { - setApplyTo('instance'); - } else if (actionItem) { - setApplyTo('series'); - } - }, [actionItem]); + // Initialize applyTo only for recurring/template semantics + useEffect(() => { + if (!isRecurring) { + setApplyTo('instance'); + return; + } + + if (!actionItem) { + setApplyTo('instance'); + return; + } + + // Only template (non-exception) items can target the whole series + if (actionItem.isTemplate && !actionItem.isInstanceException) { + setApplyTo('series'); + } else { + setApplyTo('instance'); + } + }, [actionItem, isRecurring]);- useEffect(() => { + useEffect(() => { + // applyTo filtering is only meaningful when we can actually choose series vs instance + const applyToIsRelevant = + (isRecurring && !editMode) || + (editMode && actionItem?.isTemplate && !actionItem.isInstanceException); + if (!applyToIsRelevant) return; + // Check if current selections are still valid with the new filter (for both create and edit modes) if ( selectedVolunteer && applyTo === 'series' && !selectedVolunteer.isTemplate ) { setSelectedVolunteer(null); handleFormChange('volunteerId', ''); } if ( selectedVolunteerGroup && applyTo === 'series' && !selectedVolunteerGroup.isTemplate ) { setSelectedVolunteerGroup(null); handleFormChange('volunteerGroupId', ''); } - }, [applyTo, selectedVolunteer, selectedVolunteerGroup]); + }, [applyTo, selectedVolunteer, selectedVolunteerGroup, isRecurring, editMode, actionItem]);Also applies to: 361-428
211-236: Guard againstbaseEvent?.idbeing undefined when creating series-level items.
On Line 233-234 you may send{ eventId: undefined }ifbaseEventisn’t provided, which can hard-fail GraphQL validation.Proposed patch
const createActionItemHandler = async (e: FormEvent): Promise<void> => { e.preventDefault(); try { @@ + if (eventId && isRecurring && applyTo === 'series' && !baseEvent?.id) { + NotificationToast.error({ key: 'unknownError', namespace: 'errors' }); + return; + } + const input: ICreateActionItemInput = { @@ ...(eventId && (isRecurring ? applyTo === 'series' ? { eventId: baseEvent?.id } : { recurringEventInstanceId: eventId } : { eventId })), };
🤖 Fix all issues with AI agents
In @src/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsx:
- Around line 1-68: The interaction tests use direct await userEvent.click(...)
which is flaky with @testing-library/user-event v14+; update each test that
performs clicks (the two "calls onChange..." tests) to create a user via const
user = userEvent.setup() and replace await userEvent.click(...) with await
user.click(screen.getByLabelText(...)); keep renderComponent and the onChange
vi.fn() usage unchanged so only the click calls are migrated to the setup
pattern.
In @src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx:
- Around line 11-47: Replace the non-semantic Form.Group/Form.Label grouping in
ApplyToSelector with a semantic fieldset and legend so the radio buttons are
announced as a group by screen readers: wrap the two Form.Check components in a
<fieldset> and render the group label as a <legend> (using t('applyTo')), keep
the existing name/id/checked/onChange props on both Form.Check controls, and
remove the standalone Form.Label so the legend is the accessible label for the
group; ensure the legend text is visible and that each radio retains its unique
id.
In @src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx:
- Around line 438-447: Replace the nested ternary assigned to Form's
onSubmitCapture by computing a single onSubmit handler variable above the JSX:
determine handler using the existing symbols (editMode, actionItem?.isTemplate,
applyTo) and choose between updateActionItemHandler,
updateActionForInstanceHandler, and createActionItemHandler accordingly, then
pass that variable into the Form as onSubmit (not onSubmitCapture); this
centralizes the conditional logic for easier auditing and prevents regressions
around template vs instance rules.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (4)
docs/docs/auto-docs/components/AdminPortal/ApplyToSelector/ApplyToSelector/variables/default.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/screens/OrganizationActionItems/ActionItemModal/ActionItemModal/variables/default.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/AdminPortal/ApplyToSelector/interface/interfaces/InterfaceApplyToSelectorProps.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/AdminPortal/ApplyToSelector/interface/type-aliases/ApplyToType.mdis excluded by!**/docs/docs/**
📒 Files selected for processing (5)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsxsrc/types/AdminPortal/ApplyToSelector/interface.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Zero TypeScript errors: Run pnpm run typecheck and must pass without errors or warnings
Files:
src/types/AdminPortal/ApplyToSelector/interface.tssrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/types/AdminPortal/ApplyToSelector/interface.tssrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsx
🧠 Learnings (10)
📚 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/types/AdminPortal/ApplyToSelector/interface.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/types/AdminPortal/ApplyToSelector/interface.ts
📚 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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsx
📚 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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsx
🧬 Code graph analysis (4)
src/types/AdminPortal/ApplyToSelector/interface.ts (1)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx (1)
ApplyToType(9-9)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx (1)
src/types/AdminPortal/ApplyToSelector/interface.ts (1)
InterfaceApplyToSelectorProps(11-16)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsx (1)
src/types/AdminPortal/ApplyToSelector/interface.ts (1)
ApplyToType(6-6)
src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx (2)
src/types/AdminPortal/ApplyToSelector/interface.ts (1)
ApplyToType(6-6)src/shared-components/BaseModal/BaseModal.tsx (1)
BaseModal(69-140)
🔇 Additional comments (4)
src/types/AdminPortal/ApplyToSelector/interface.ts (1)
1-16: Type + props interface placement/naming looks policy-compliant.
ApplyToType+InterfaceApplyToSelectorPropsare correctly centralized undersrc/types/AdminPortal/...(no inline prop interfaces in the component).src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsx (2)
655-669: Good: switching modal assertions togetByTestId('actionItemModal')matches the BaseModal workaround.
This aligns the test suite with the new modal implementation and avoids ambiguity from duplicaterole="dialog".Also applies to: 700-708, 750-759, 840-848, 864-872, 905-913, 968-976, 1041-1049, 1140-1150, 1226-1229, 1287-1300, 1321-1329, 1344-1355, 1386-1402, 1417-1429, 1442-1455, 1468-1480, 1495-1502, 1533-1544, 1558-1590, 1603-1616, 1641-1649, 1708-1722, 1762-1768, 1808-1814, 1866-1901, 1922-1936, 1957-1975, 2012-2044, 2101-2124, 2172-2202, 2248-2277, 2323-2355, 2401-2433, 2485-2547, 2563-2630, 2646-2653, 2672-2681, 2701-2710, 2732-2801, 2822-2838, 2859-2871, 2950-3011, 3061-3099, 3192-3241, 3296-3343, 3363-3380, 3431-3455, 3471-3492, 3505-3523, 3536-3555, 3568-3589, 3602-3620, 3633-3659, 3674-3685, 3698-3705, 3736-3747, 3760-3821, 3835-3848, 3880-3892, 3912-3991, 4071-4127, 4142-4199, 4223-4227, 4242-4281, 4309-4313
1-1626: Manual verification required—sandbox environment unable to execute typecheck and coverage commands.The sandbox environment lacks installed dependencies (node_modules missing), preventing execution of
pnpm run typecheckandpnpm run test:coverage. Run these commands in your development environment to verify:
- TypeScript check:
pnpm run typecheckmust pass without errors- Coverage validation:
pnpm run test:coveragethen checkcoverage/coverage-summary.jsonto confirmsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxhas ≥95% coverage across all metrics (lines, branches, functions, statements)Please provide the output from both commands to complete verification.
src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx (1)
601-614: VerifyDatePickertestability:data-testid="assignmentDate"should be stable.
The spec file queriesgetByTestId('assignmentDate')and thenquerySelector('input'). Please ensure the sharedDatePickerguarantees this structure (or adjust tests to its intended API).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx:
- Around line 483-533: The two Chip components (the one for 'volunteer' and the
one for 'volunteerGroup') do not expose their disabled state to assistive tech;
when isVolunteerChipDisabled or isVolunteerGroupChipDisabled is true, add
appropriate ARIA attributes to communicate non-interactivity (e.g.,
aria-disabled="true" and an aria-label that includes “disabled” or a descriptive
label) and ensure clickable prop remains false so both visual and accessibility
states match; update the Chip props in the onClick/clickable branches where
setAssignmentType, handleFormChange, setSelectedVolunteer/Group are used so the
disabled chips present the correct aria-disabled and aria-label values to screen
readers.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (4)
docs/docs/auto-docs/components/AdminPortal/ApplyToSelector/ApplyToSelector/variables/default.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/screens/OrganizationActionItems/ActionItemModal/ActionItemModal/variables/default.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/AdminPortal/ApplyToSelector/interface/interfaces/InterfaceApplyToSelectorProps.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/AdminPortal/ApplyToSelector/interface/type-aliases/ApplyToType.mdis excluded by!**/docs/docs/**
📒 Files selected for processing (5)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsxsrc/types/AdminPortal/ApplyToSelector/interface.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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsxsrc/types/AdminPortal/ApplyToSelector/interface.tssrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsxsrc/types/AdminPortal/ApplyToSelector/interface.tssrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx
🧠 Learnings (10)
📚 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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.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/types/AdminPortal/ApplyToSelector/interface.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/types/AdminPortal/ApplyToSelector/interface.ts
🧬 Code graph analysis (4)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsx (2)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx (1)
ApplyToType(9-9)src/types/AdminPortal/ApplyToSelector/interface.ts (1)
ApplyToType(6-6)
src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx (5)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx (1)
ApplyToType(9-9)src/types/AdminPortal/ApplyToSelector/interface.ts (1)
ApplyToType(6-6)src/shared-components/BaseModal/BaseModal.tsx (1)
BaseModal(69-140)src/types/ActionItems/interface.ts (2)
IActionItemCategoryInfo(3-12)IEventVolunteerGroup(171-201)src/utils/interfaces.ts (1)
InterfaceEventVolunteerInfo(2540-2571)
src/types/AdminPortal/ApplyToSelector/interface.ts (1)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx (1)
ApplyToType(9-9)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx (1)
src/types/AdminPortal/ApplyToSelector/interface.ts (1)
InterfaceApplyToSelectorProps(11-16)
⏰ 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). (5)
- GitHub Check: MinIO Compliance Check
- GitHub Check: CSS Policy Check
- GitHub Check: Check Python Code Style
- GitHub Check: Performs linting, formatting, type-checking, unused file detection, checking for different source...
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (8)
src/types/AdminPortal/ApplyToSelector/interface.ts (1)
1-16: LGTM!The type definitions follow the reusable component policy correctly:
- Props interface is in the correct location (
src/types/AdminPortal/<ComponentName>/interface.ts)- Naming follows
Interface<ComponentName>Propsconvention- TSDoc comments are present and properly formatted
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsx (1)
1-69: LGTM!The test file follows best practices:
- Colocated with component in same directory
- Uses
vi.clearAllMocks()inafterEachfor test isolation (per learnings)- Uses
@testing-library/reactqueries and@testing-library/user-event- Covers all key scenarios: rendering, initial selection states, and onChange callbacks
src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsx (2)
667-667: Consistent migration to data-testid selectors.The test updates correctly use
getByTestId('actionItemModal')throughout instead ofgetByRole('dialog'). This aligns with the PR objective addressing BaseModal's duplicaterole="dialog"issue (#6312).
3542-3554: Date picker test approach is correct.The updated test correctly locates the date picker container by
data-testid="assignmentDate"and then queries the inner input element. This pattern aligns with the new DatePicker component integration.src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx (1)
11-18: TSDoc format follows guidelines.The component documentation uses TSDoc format with
@paramand@returnstags as required. The flat parameter documentation style (@param applyTo,@param onChange) aligns with the learning about preferring flat JSDoc parameter format.src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx (3)
7-16: Imports properly updated to use shared components.The imports correctly use:
BaseModalfromshared-components/BaseModal/BaseModal(replacing deprecated react-bootstrap Modal)DatePickerfromshared-components/DatePicker/DatePicker(replacing @mui/x-date-pickers)ApplyToSelectorfromcomponents/AdminPortal/ApplyToSelector/ApplyToSelectorApplyToTypefrom the types directoryThis aligns with the reusable component policy and restricted library imports guidelines.
430-457: BaseModal integration and ApplyToSelector usage look correct.The refactored modal:
- Uses
BaseModalwith proper props includingdataTestId="actionItemModal"- Conditionally renders
ApplyToSelectorfor recurring events in create mode (line 450-452) and for template items in edit mode (lines 453-457)- Form submit handler correctly routes to appropriate handler based on mode and applyTo selection
601-615: DatePicker already properly propagates data-testid to the DOM—no fix needed.The
data-testidattribute is correctly handled by the DatePicker component. It's explicitly passed to the Form.Control component (line 144 in DatePicker.tsx), which renders the actual DOM input element. The test'sgetByTestId('assignmentDate')works as expected because Form.Control properly renders thedata-testidattribute on the underlying input element. The code is correct and testable.Likely an incorrect or invalid review comment.
Comment on lines
483
to
533
| <Box className="mb-3"> | ||
| <Typography variant="subtitle2" className="mb-2"> | ||
| {t('assignTo')} | ||
| </Typography> | ||
| <Box className="d-flex gap-2"> | ||
| <Chip | ||
| label={t('volunteer')} | ||
| variant={ | ||
| assignmentType === 'volunteer' ? 'filled' : 'outlined' | ||
| } | ||
| color={assignmentType === 'volunteer' ? 'primary' : 'default'} | ||
| onClick={() => { | ||
| if (!isVolunteerChipDisabled) { | ||
| setAssignmentType('volunteer'); | ||
| // Clear volunteer group assignment when switching to volunteer | ||
| handleFormChange('volunteerGroupId', ''); | ||
| setSelectedVolunteer(newVolunteer); | ||
| setSelectedVolunteerGroup(null); | ||
| }} | ||
| renderInput={(params) => ( | ||
| <TextField {...params} label={t('volunteer')} required /> | ||
| )} | ||
| /> | ||
| </Form.Group> | ||
| )} | ||
|
|
||
| {/* Volunteer Group Selection */} | ||
| {assignmentType === 'volunteerGroup' && ( | ||
| <Form.Group className="mb-3 w-100"> | ||
| <Autocomplete | ||
| className={`${styles.noOutline} w-100`} | ||
| data-testid="volunteerGroupSelect" | ||
| data-cy="volunteerGroupSelect" | ||
| options={volunteerGroups} | ||
| value={selectedVolunteerGroup} | ||
| isOptionEqualToValue={(option, value) => | ||
| option.id === value?.id | ||
| } | ||
| filterSelectedOptions={true} | ||
| getOptionLabel={(group: IEventVolunteerGroup): string => { | ||
| return group.name; | ||
| }} | ||
| onChange={(_, newGroup): void => { | ||
| const groupId = newGroup?.id; | ||
| handleFormChange('volunteerGroupId', groupId); | ||
| }} | ||
| clickable={!isVolunteerChipDisabled} | ||
| sx={{ | ||
| opacity: isVolunteerChipDisabled ? 0.6 : 1, | ||
| cursor: isVolunteerChipDisabled ? 'not-allowed' : 'pointer', | ||
| }} | ||
| /> | ||
| <Chip | ||
| label={t('volunteerGroup')} | ||
| variant={ | ||
| assignmentType === 'volunteerGroup' ? 'filled' : 'outlined' | ||
| } | ||
| color={ | ||
| assignmentType === 'volunteerGroup' ? 'primary' : 'default' | ||
| } | ||
| onClick={() => { | ||
| if (!isVolunteerGroupChipDisabled) { | ||
| setAssignmentType('volunteerGroup'); | ||
| // Clear volunteer assignment when switching to volunteer group | ||
| handleFormChange('volunteerId', ''); | ||
| setSelectedVolunteerGroup(newGroup); | ||
| setSelectedVolunteer(null); | ||
| }} | ||
| renderInput={(params) => ( | ||
| <TextField | ||
| {...params} | ||
| label={t('volunteerGroup')} | ||
| required | ||
| /> | ||
| )} | ||
| /> | ||
| </Form.Group> | ||
| )} | ||
|
|
||
| <Form.Group className="d-flex gap-3 mx-auto mb-3"> | ||
| <DatePicker | ||
| format="DD/MM/YYYY" | ||
| label={t('assignmentDate')} | ||
| className={styles.noOutline} | ||
| value={dayjs(assignedAt)} | ||
| disabled={editMode} | ||
| onChange={(date: Dayjs | null): void => { | ||
| if (date && !editMode) { | ||
| handleFormChange('assignedAt', date.toDate()); | ||
| } | ||
| }} | ||
| clickable={!isVolunteerGroupChipDisabled} | ||
| sx={{ | ||
| opacity: isVolunteerGroupChipDisabled ? 0.6 : 1, | ||
| cursor: isVolunteerGroupChipDisabled | ||
| ? 'not-allowed' | ||
| : 'pointer', | ||
| }} | ||
| /> | ||
| </Box> | ||
| </Box> |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding aria-label to disabled chips for accessibility.
The assignment type chips use sx styling for disabled appearance but don't communicate the disabled state to assistive technologies. When isVolunteerChipDisabled or isVolunteerGroupChipDisabled is true, screen readers won't announce that the chip is non-interactive.
♿ Suggested accessibility improvement
<Chip
label={t('volunteer')}
variant={
assignmentType === 'volunteer' ? 'filled' : 'outlined'
}
color={assignmentType === 'volunteer' ? 'primary' : 'default'}
onClick={() => {
if (!isVolunteerChipDisabled) {
setAssignmentType('volunteer');
handleFormChange('volunteerGroupId', '');
setSelectedVolunteerGroup(null);
}
}}
clickable={!isVolunteerChipDisabled}
+ aria-disabled={isVolunteerChipDisabled ? true : undefined}
sx={{
opacity: isVolunteerChipDisabled ? 0.6 : 1,
cursor: isVolunteerChipDisabled ? 'not-allowed' : 'pointer',
}}
/>Apply similar change to the volunteerGroup chip.
🤖 Prompt for AI Agents
In @src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx
around lines 483 - 533, The two Chip components (the one for 'volunteer' and the
one for 'volunteerGroup') do not expose their disabled state to assistive tech;
when isVolunteerChipDisabled or isVolunteerGroupChipDisabled is true, add
appropriate ARIA attributes to communicate non-interactivity (e.g.,
aria-disabled="true" and an aria-label that includes “disabled” or a descriptive
label) and ensure clickable prop remains false so both visual and accessibility
states match; update the Chip props in the onClick/clickable branches where
setAssignmentType, handleFormChange, setSelectedVolunteer/Group are used so the
disabled chips present the correct aria-disabled and aria-label values to screen
readers.
nishantharkut
force-pushed
the
refactor/ActionItemModal
branch
from
3539ece to
27fa8de
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx:
- Around line 11-18: Update the TSDoc block for the ApplyToSelector component to
use a flat JSDoc param style: replace the current multi-line param descriptions
with @param tags for each prop (e.g., @param applyTo - Current selection value
('series' | 'instance') and @param onChange - Callback fired when selection
changes) and keep the top description intact; ensure the @returns tag remains
describing the radio group component for scope selection so the comment aligns
with the project's flat param convention.
In @src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx:
- Around line 438-445: The nested ternary used as the Form onSubmit prop is hard
to read; extract the logic into a small helper (e.g., getSubmitHandler) that
returns one of createActionItemHandler, updateActionForInstanceHandler, or
updateActionItemHandler based on editMode, actionItem?.isTemplate and applyTo,
then pass getSubmitHandler() to <Form onSubmit={...}> to improve clarity and
maintain behavior.
- Around line 172-178: The two useMemo hooks (isVolunteerChipDisabled and
isVolunteerGroupChipDisabled) wrap trivial boolean expressions and depend only
on stable props, so simplify by replacing them with plain constants: compute
isVolunteerChipDisabled as editMode && Boolean(actionItem?.volunteerGroup?.id)
and isVolunteerGroupChipDisabled as editMode &&
Boolean(actionItem?.volunteer?.id) (remove the useMemo imports/usages and update
any references accordingly).
- Around line 486-532: Remove the unnecessary "|| undefined" when setting
aria-disabled on the Chip components so aria-disabled receives a boolean
directly; update the two Chips that reference isVolunteerChipDisabled and
isVolunteerGroupChipDisabled (the onClick/variant/color blocks around
setAssignmentType, handleFormChange, setSelectedVolunteerGroup,
setSelectedVolunteer) to pass aria-disabled={isVolunteerChipDisabled} and
aria-disabled={isVolunteerGroupChipDisabled} respectively, ensuring
accessibility attributes are boolean-valued without changing existing
click/disabled logic.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (4)
docs/docs/auto-docs/components/AdminPortal/ApplyToSelector/ApplyToSelector/variables/default.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/screens/OrganizationActionItems/ActionItemModal/ActionItemModal/variables/default.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/AdminPortal/ApplyToSelector/interface/interfaces/InterfaceApplyToSelectorProps.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/AdminPortal/ApplyToSelector/interface/type-aliases/ApplyToType.mdis excluded by!**/docs/docs/**
📒 Files selected for processing (5)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsxsrc/types/AdminPortal/ApplyToSelector/interface.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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/types/AdminPortal/ApplyToSelector/interface.tssrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/types/AdminPortal/ApplyToSelector/interface.tssrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsx
🧠 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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.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/types/AdminPortal/ApplyToSelector/interface.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/types/AdminPortal/ApplyToSelector/interface.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/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsx
🧬 Code graph analysis (2)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx (1)
src/types/AdminPortal/ApplyToSelector/interface.ts (1)
InterfaceApplyToSelectorProps(11-16)
src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx (5)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx (1)
ApplyToType(9-9)src/types/AdminPortal/ApplyToSelector/interface.ts (1)
ApplyToType(6-6)src/shared-components/BaseModal/BaseModal.tsx (1)
BaseModal(69-140)src/types/ActionItems/interface.ts (2)
IActionItemCategoryInfo(3-12)IEventVolunteerGroup(171-201)src/utils/interfaces.ts (1)
InterfaceEventVolunteerInfo(2540-2571)
🔇 Additional comments (8)
src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsx (1)
631-637: LGTM! Comprehensive test coverage with proper test isolation.The test file demonstrates good practices:
- Proper
vi.clearAllMocks()inafterEachblocks for test isolation (as per learnings)- Consistent use of
data-testidselectors aligning with BaseModal's duplicaterole="dialog"workaround- Comprehensive coverage of modal visibility, form validation, mutations, edge cases, and accessibility scenarios
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsx (1)
10-71: Good test coverage for the new component.The test suite covers the essential scenarios:
- Rendering both radio options
- Initial selection state for both
'series'and'instance'onChangecallback behaviorConsider adding keyboard accessibility tests for radio button navigation (Enter/Space key interactions) as separate explicit test cases, per repository conventions. However, since radio buttons have native keyboard support, this may be acceptable as-is.
src/types/AdminPortal/ApplyToSelector/interface.ts (1)
1-16: Well-structured type definitions following coding guidelines.The interface file correctly:
- Places types in
src/types/AdminPortal/ApplyToSelector/interface.tsper component architecture guidelines- Uses
Interface<ComponentName>Propsnaming convention- Includes TSDoc comments on both the type and interface
- Avoids inline prop interfaces in the component file
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx (1)
27-47: Component implementation is clean and follows best practices.Good implementation:
- Uses i18n translation keys (
t('applyTo'),t('entireSeries'),t('thisEventOnly'))- Proper accessibility with
as="fieldset"andas="legend"semantic structure- Controlled radio inputs with
checkedandonChange- Props imported from external interface file
src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx (4)
1-6: File-level TSDoc comment is appropriate.The header comment accurately describes the component's purpose and functionality. This aligns with TSDoc requirements for the repository.
11-16: Correct use of shared components and proper imports.The refactoring correctly:
- Uses
DatePickerfromshared-components(replacing restricted@mui/x-date-pickersdirect import)- Uses
BaseModalfromshared-components(replacing deprecatedreact-bootstrapModal)- Imports
ApplyToSelectorfrom the new AdminPortal component- Imports
ApplyToTypefrom the proper types directory
430-456: Well-structured modal refactoring with ApplyToSelector integration.The refactoring cleanly:
- Uses
BaseModalwith appropriate props (show,onHide,title,dataTestId)- Conditionally renders
ApplyToSelectorfor recurring events (create mode) and template items (edit mode)- Maintains the same business logic while improving code organization
The conditional logic at lines 448-455 correctly handles:
- Create mode for recurring events:
isRecurring && !editMode- Edit mode for template items:
editMode && actionItem?.isTemplate && !actionItem.isInstanceException
601-615: DatePicker integration looks correct.The
DatePickercomponent is properly configured with:
data-testid="assignmentDate"for test targeting- Conditional
disabledbased oneditMode- Proper
onChangehandler that respects the disabled state
Comment on lines
11
to
18
| /** | ||
| * A radio group selector for choosing action item scope. | ||
| * Allows users to apply an action item to an entire series or a single instance. | ||
| * | ||
| * @param applyTo - Current selection value ('series' or 'instance') | ||
| * @param onChange - Callback fired when selection changes | ||
| * @returns Radio group component for scope selection | ||
| */ |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
TSDoc comment uses correct format but could align better with flat param style.
The TSDoc comment is good but per learnings, prefer flat JSDoc parameter documentation format. The current format works but consider this minor refinement:
/**
* A radio group selector for choosing action item scope.
* Allows users to apply an action item to an entire series or a single instance.
*
- * @param applyTo - Current selection value ('series' or 'instance')
- * @param onChange - Callback fired when selection changes
+ * @param props - Component props from InterfaceApplyToSelectorProps
* @returns Radio group component for scope selection
*/This is a minor suggestion - the current format is acceptable.
🤖 Prompt for AI Agents
In @src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx around lines
11 - 18, Update the TSDoc block for the ApplyToSelector component to use a flat
JSDoc param style: replace the current multi-line param descriptions with @param
tags for each prop (e.g., @param applyTo - Current selection value ('series' |
'instance') and @param onChange - Callback fired when selection changes) and
keep the top description intact; ensure the @returns tag remains describing the
radio group component for scope selection so the comment aligns with the
project's flat param convention.
src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx
Outdated
Show resolved
Hide resolved
Contributor
|
Please:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsx (1)
3853-3865: Fix variable redeclaration.The variable
modalis declared on line 3853 and then redeclared on line 3860, which causes a linting error and shadows the previous declaration.🔧 Proposed fix
renderWithProviders(props); const modal = await screen.findByTestId('itemModal'); await waitFor(() => { expect(screen.getByTestId('actionItemModal')).toBeInTheDocument(); }); // In React Bootstrap, clicking the backdrop typically calls onHide // We can test this by finding the modal backdrop element - const modal = screen.getByTestId('actionItemModal'); expect(modal).toBeInTheDocument(); // The modal should be present and the hide function should be available expect(mockHide).toBeInstanceOf(Function);src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx (2)
1-6: TSDoc header needs component documentation.The file header provides a good overview, but as per coding guidelines, the exported
ItemModalcomponent (line 68) needs a TSDoc comment with @param and @returns tags.As per coding guidelines, add TSDoc to the exported component:
/** * Modal component for creating and updating action items. * Handles both creation and editing modes, including specific logic for recurring events. * Allows users to specify whether changes apply to an entire series or a single instance. * * @param props - Component props from IItemModalProps interface * @returns JSX.Element - Modal dialog with action item form */ const ItemModal: FC<IItemModalProps> = ({ ... }) => {
507-530: Replacearia-disabledwith MUI's nativedisabledprop for proper Chip accessibility.MUI v7 Chip provides a
disabledprop that handles accessibility and visual disabled state automatically. The current pattern usingaria-disabled={isVolunteerChipDisabled || undefined}combined withclickableis non-idiomatic and harder to maintain.Update both Chips (lines 507–530):
<Chip label={t('volunteer')} variant={assignmentType === 'volunteer' ? 'filled' : 'outlined'} color={assignmentType === 'volunteer' ? 'primary' : 'default'} onClick={() => { if (!isVolunteerChipDisabled) { setAssignmentType('volunteer'); handleFormChange('volunteerGroupId', ''); setSelectedVolunteerGroup(null); } }} - clickable={!isVolunteerChipDisabled} - aria-disabled={isVolunteerChipDisabled || undefined} + disabled={isVolunteerChipDisabled} sx={{ opacity: isVolunteerChipDisabled ? 0.6 : 1, cursor: isVolunteerChipDisabled ? 'not-allowed' : 'pointer', }} />The
disabledprop handles both accessibility (aria-disabled internally) and prevents click events, eliminating the need for manual onClick guards in this context.
🤖 Fix all issues with AI agents
In @src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx:
- Around line 444-451: The Form's onSubmit currently duplicates logic already in
getSubmitHandler, causing redundancy; replace the conditional expression passed
to Form.onSubmit with a single call to getSubmitHandler() so the form uses that
centralized logic. Locate the Form component (prop onSubmit) and remove the
inline ternary that references editMode, actionItem?.isTemplate, applyTo,
updateActionForInstanceHandler, updateActionItemHandler, and
createActionItemHandler, and instead pass getSubmitHandler() so all submit
routing is handled by the existing function.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/docs/auto-docs/screens/OrganizationActionItems/ActionItemModal/ActionItemModal/variables/default.mdis excluded by!**/docs/docs/**
📒 Files selected for processing (2)
src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx
🧰 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/OrganizationActionItems/ActionItemModal/ActionItemModal.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/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx
🧠 Learnings (7)
📚 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/OrganizationActionItems/ActionItemModal/ActionItemModal.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/OrganizationActionItems/ActionItemModal/ActionItemModal.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/OrganizationActionItems/ActionItemModal/ActionItemModal.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/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsx
🧬 Code graph analysis (1)
src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx (2)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx (1)
ApplyToType(9-9)src/types/AdminPortal/ApplyToSelector/interface.ts (1)
ApplyToType(6-6)
🪛 Biome (2.1.2)
src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsx
[error] 3860-3860: Shouldn't redeclare 'modal'. Consider to delete it or rename it.
'modal' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (6)
src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsx (2)
721-726: Good handling of multiple label matches.The updated code correctly handles the case where
getAllByLabelText(/volunteerGroup/i)returns multiple elements (the Chip component's aria-label creates additional matches). Filtering for the INPUT element ensures the correct element is selected.
1450-1456: DatePicker testid implementation is correct.The test correctly uses
getByTestId('assignmentDate')which matches thedata-testid="assignmentDate"prop passed to the DatePicker component in ActionItemModal.tsx line 614. The DatePicker component properly forwards thedata-testidprop to the underlying input element, making it accessible and queryable by the test.src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx (4)
171-176: Good defensive coding with Boolean wrapper.The
Boolean()wrapper ensures these memoized values always return a strict boolean rather than potentially truthy/falsy values. This prevents edge cases whereundefinedor empty string could cause unexpected behavior.
444-461: Successful refactoring with ApplyToSelector component.The inline radio button logic has been successfully extracted into the reusable ApplyToSelector component, improving maintainability and reducing code duplication. The component is correctly used in both create mode (line 455) and edit mode for template items (line 460).
614-614: Verify data-testid consistency.The data-testid has been changed from "assignmentDatePicker" to "assignmentDate" to match the new shared DatePicker component. Ensure the shared DatePicker properly forwards this prop to the input element.
Verify that the shared DatePicker component at
src/shared-components/DatePicker/DatePicker.tsxproperly forwards data-testid to its input element. This is critical for test reliability.
11-16: Imports follow reusable component policy.All three verification checks passed:
- ✅ DatePicker is the approved wrapper from
shared-components/(not direct@mui/x-date-pickersimport)- ✅ ApplyToSelector has interface file at
src/types/AdminPortal/ApplyToSelector/interface.ts- ✅ ApplyToSelector includes JSDoc comments on the exported component
The imports at lines 11-16 correctly follow the restricted component import policy.
src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx
Outdated
Show resolved
Hide resolved
Author
|
@palisadoes yes sir, I am working on the issues raised by CodeRabbit. |
Author
|
@palisadoes Sir, I have completed all the requirements
A Quick Question: Should I wait for this PR to merge before starting PR 2 (Part 2/5), or is it safe to create a parallel PR? I Want to follow best practices and avoid conflicts. |
Contributor
|
@coderabbitai full review based on your configuration |
Contributor
|
@palisadoes I will conduct a full review of the PR based on the current configuration. ✅ Actions performedFull review triggered. |
Contributor
There is no context for the 5 parts you refer to |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsx (2)
17-17: Fix Vitest API usage: missingafterAllimport +async describeblocks.
This is brittle at best and can hard-break when vitest globals are disabled.Proposed fix
-import { vi, it, describe, expect, beforeEach, afterEach } from 'vitest'; +import { vi, it, describe, expect, beforeEach, afterEach, afterAll } from 'vitest'; @@ -describe('Assignment type switching', async () => { +describe('Assignment type switching', () => { @@ -describe('Accessibility', async () => { +describe('Accessibility', () => {Also applies to: 958-958, 1216-1216, 1623-1626
626-635: Add a file-levelafterEach(vi.clearAllMocks)for enforced test isolation.
Right now, cleanup is scoped to some describes only; a top-levelafterEachavoids bleed-over across the file (based on learnings).Proposed fix
import { NotificationToast } from 'components/NotificationToast/NotificationToast'; +afterEach(() => { + vi.clearAllMocks(); +}); + vi.mock('components/NotificationToast/NotificationToast', () => ({ NotificationToast: { success: vi.fn(), error: vi.fn(),src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx (3)
136-167: Bug risk: applyTo is forced to'series'for non-template edits, which can clear/hide valid assignees.
In edit mode, applyTo is not meaningful for non-template action items, but it still drives filtering and the “clear non-template selection” effect. This can drop the existing volunteer/group from the UI/state.Proposed fix (only treat applyTo as relevant for templates; default non-template edits to instance)
- const volunteers = useMemo(() => { + const isApplyToRelevant = + Boolean(isRecurring) && + (!editMode || + (Boolean(actionItem?.isTemplate) && !actionItem?.isInstanceException)); + + const volunteers = useMemo(() => { const allVolunteers = volunteersData?.event?.volunteers || []; - // Apply filtering based on applyTo selection for both create and edit modes - if (applyTo === 'series') { + if (!isApplyToRelevant) return allVolunteers; + if (applyTo === 'series') { // For entire series, show only template volunteers return allVolunteers.filter( (volunteer: InterfaceEventVolunteerInfo) => volunteer.isTemplate === true, ); - } else { - // For this event only, show all volunteers (template and non-template) - return allVolunteers; } + return allVolunteers; - }, [volunteersData, applyTo]); + }, [volunteersData, applyTo, isApplyToRelevant]); const volunteerGroups = useMemo(() => { const allVolunteerGroups = volunteerGroupsData?.event?.volunteerGroups || []; - // Apply filtering based on applyTo selection for both create and edit modes - if (applyTo === 'series') { + if (!isApplyToRelevant) return allVolunteerGroups; + if (applyTo === 'series') { // For entire series, show only template volunteer groups return allVolunteerGroups.filter( (group: IEventVolunteerGroup) => group.isTemplate === true, ); - } else { - // For this event only, show all volunteer groups (template and non-template) - return allVolunteerGroups; } + return allVolunteerGroups; - }, [volunteerGroupsData, applyTo]); + }, [volunteerGroupsData, applyTo, isApplyToRelevant]); @@ useEffect(() => { if (actionItem?.isInstanceException) { setApplyTo('instance'); - } else if (actionItem) { + } else if (actionItem?.isTemplate) { setApplyTo('series'); + } else if (actionItem) { + setApplyTo('instance'); } }, [actionItem]); @@ useEffect(() => { + if (!isApplyToRelevant) return; // Check if current selections are still valid with the new filter (for both create and edit modes) if ( selectedVolunteer && applyTo === 'series' && !selectedVolunteer.isTemplate ) { setSelectedVolunteer(null); handleFormChange('volunteerId', ''); } if ( selectedVolunteerGroup && applyTo === 'series' && !selectedVolunteerGroup.isTemplate ) { setSelectedVolunteerGroup(null); handleFormChange('volunteerGroupId', ''); } - }, [applyTo, selectedVolunteer, selectedVolunteerGroup]); + }, [applyTo, selectedVolunteer, selectedVolunteerGroup, isApplyToRelevant]);Also applies to: 356-363, 397-416
217-231: Validate required ids before mutations (recurring series needsbaseEvent.id; instance update needseventId).
Right noweventId: baseEvent?.idandeventId: eventIdcan end up undefined and still be sent.Also applies to: 296-339
199-204: Hook deps:handleFormChangeis used inside an effect but not listed in deps.
If eslint-plugin-react-hooks exhaustive-deps is enforced, this will warn/fail; prefer inliningsetFormStatein the effect or wraphandleFormChangeinuseCallback.Also applies to: 397-416
🤖 Fix all issues with AI agents
In @src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx:
- Around line 1-46: ApplyToSelector currently hardcodes the radio group name and
ids ("applyTo", "applyToSeries", "applyToInstance"), causing collisions when
multiple components render; change ApplyToSelector to generate a unique group
name and unique ids for the two Form.Check radios (e.g., via React.useId() or an
injected prop like idPrefix) and use that same unique name for both radio inputs
so they remain grouped per-instance while keeping checked/onChange logic
(applyTo prop and onChange handler) unchanged.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (4)
docs/docs/auto-docs/components/AdminPortal/ApplyToSelector/ApplyToSelector/variables/default.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/screens/OrganizationActionItems/ActionItemModal/ActionItemModal/variables/default.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/AdminPortal/ApplyToSelector/interface/interfaces/InterfaceApplyToSelectorProps.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/AdminPortal/ApplyToSelector/interface/type-aliases/ApplyToType.mdis excluded by!**/docs/docs/**
📒 Files selected for processing (5)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsxsrc/types/AdminPortal/ApplyToSelector/interface.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Zero TypeScript errors: Run pnpm run typecheck and must pass without errors or warnings
Files:
src/types/AdminPortal/ApplyToSelector/interface.tssrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/types/AdminPortal/ApplyToSelector/interface.tssrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.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/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsx
🧠 Learnings (10)
📚 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/types/AdminPortal/ApplyToSelector/interface.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/types/AdminPortal/ApplyToSelector/interface.ts
📚 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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsx
📚 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/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.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/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.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/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.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/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsx
🧬 Code graph analysis (4)
src/types/AdminPortal/ApplyToSelector/interface.ts (1)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx (1)
ApplyToType(9-9)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx (1)
src/types/AdminPortal/ApplyToSelector/interface.ts (1)
InterfaceApplyToSelectorProps(11-16)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsx (1)
src/types/AdminPortal/ApplyToSelector/interface.ts (1)
ApplyToType(6-6)
src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx (2)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx (1)
ApplyToType(9-9)src/types/AdminPortal/ApplyToSelector/interface.ts (1)
ApplyToType(6-6)
🔇 Additional comments (5)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsx (1)
10-71: Solid unit coverage for initial state + onChange paths.
afterEach mock cleanup and user-event usage look good.src/types/AdminPortal/ApplyToSelector/interface.ts (1)
1-16: Type + props interface are clean and match the repo’s reusable-component type pattern.src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx (1)
22-44: All i18n keys exist across all locale files.Verification confirmed that
organizationActionItems.applyTo,organizationActionItems.entireSeries, andorganizationActionItems.thisEventOnlyare present in all 5 locale files (en, es, fr, hi, zh). No missing translations.src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx (2)
595-608: No issue found — The DatePicker wrapper already includes'data-testid'?: stringin its props interface (line 50 of DatePicker.tsx), so the usage in ActionItemModal.tsx will not cause typecheck errors.
1-17: Verify code coverage and TypeScript compilation in CI pipeline. This file must maintain ≥95% code coverage for all modified sections per guidelines. The substantial test file (4373 lines) is properly colocated and uses correct patterns (Vitest, React Testing Library, GraphQL mocks), but coverage metrics requirepnpm run test:coverageexecution in CI where tools are available. Ensure no TypeScript errors viapnpm run typecheck.
Comment on lines
+1
to
+46
| import React from 'react'; | ||
| import { Form } from 'react-bootstrap'; | ||
| import { useTranslation } from 'react-i18next'; | ||
| import type { | ||
| InterfaceApplyToSelectorProps, | ||
| ApplyToType, | ||
| } from 'types/AdminPortal/ApplyToSelector/interface'; | ||
|
|
||
| export type { ApplyToType }; | ||
|
|
||
| /** | ||
| * A radio group selector for choosing action item scope. | ||
| * Allows users to apply an action item to an entire series or a single instance. | ||
| * | ||
| * @param props - Component props from InterfaceApplyToSelectorProps | ||
| * @returns Radio group component for scope selection | ||
| */ | ||
| const ApplyToSelector: React.FC<InterfaceApplyToSelectorProps> = ({ | ||
| applyTo, | ||
| onChange, | ||
| }) => { | ||
| const { t } = useTranslation('translation', { | ||
| keyPrefix: 'organizationActionItems', | ||
| }); | ||
|
|
||
| return ( | ||
| <Form.Group className="mb-3" as="fieldset"> | ||
| <Form.Label as="legend">{t('applyTo')}</Form.Label> | ||
| <Form.Check | ||
| type="radio" | ||
| label={t('entireSeries')} | ||
| name="applyTo" | ||
| id="applyToSeries" | ||
| checked={applyTo === 'series'} | ||
| onChange={() => onChange('series')} | ||
| /> | ||
| <Form.Check | ||
| type="radio" | ||
| label={t('thisEventOnly')} | ||
| name="applyTo" | ||
| id="applyToInstance" | ||
| checked={applyTo === 'instance'} | ||
| onChange={() => onChange('instance')} | ||
| /> | ||
| </Form.Group> | ||
| ); |
Contributor
There was a problem hiding this comment.
Avoid hard-coded radio name/id to prevent collisions across multiple instances.
Right now, rendering two ApplyToSelector components on the same page would (1) share a single radio group via name="applyTo" and (2) duplicate element ids.
Proposed fix (generate unique ids + group name)
-import React from 'react';
+import React, { useId } from 'react';
import { Form } from 'react-bootstrap';
import { useTranslation } from 'react-i18next';
import type {
InterfaceApplyToSelectorProps,
ApplyToType,
} from 'types/AdminPortal/ApplyToSelector/interface';
export type { ApplyToType };
@@
const ApplyToSelector: React.FC<InterfaceApplyToSelectorProps> = ({
applyTo,
onChange,
}) => {
+ const uid = useId();
+ const name = `applyTo-${uid}`;
+ const seriesId = `${name}-series`;
+ const instanceId = `${name}-instance`;
const { t } = useTranslation('translation', {
keyPrefix: 'organizationActionItems',
});
return (
<Form.Group className="mb-3" as="fieldset">
<Form.Label as="legend">{t('applyTo')}</Form.Label>
<Form.Check
type="radio"
label={t('entireSeries')}
- name="applyTo"
- id="applyToSeries"
+ name={name}
+ id={seriesId}
checked={applyTo === 'series'}
onChange={() => onChange('series')}
/>
<Form.Check
type="radio"
label={t('thisEventOnly')}
- name="applyTo"
- id="applyToInstance"
+ name={name}
+ id={instanceId}
checked={applyTo === 'instance'}
onChange={() => onChange('instance')}
/>
</Form.Group>
);
};🤖 Prompt for AI Agents
In @src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx around lines
1 - 46, ApplyToSelector currently hardcodes the radio group name and ids
("applyTo", "applyToSeries", "applyToInstance"), causing collisions when
multiple components render; change ApplyToSelector to generate a unique group
name and unique ids for the two Form.Check radios (e.g., via React.useId() or an
injected prop like idPrefix) and use that same unique name for both radio inputs
so they remain grouped per-instance while keeping checked/onChange logic
(applyTo prop and onChange handler) unchanged.
palisadoes
requested changes
Jan 11, 2026
Contributor
palisadoes
left a comment
palisadoes
left a comment
There was a problem hiding this comment.
Please:
- Fix any failing tests
- Ensure test code coverage for your patch meets our minimum >=95% requirements
- Fix any CI/CD failures. Click on the
Detailslink for the tests to get an explanation of the cause(s) - Ensure code rabbit approves your changes
- Remedy any conflicting files
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #6314 +/- ##
========================================
Coverage 96.73% 96.74%
========================================
Files 455 456 +1
Lines 16599 16598 -1
Branches 5119 5118 -1
========================================
Hits 16057 16057
+ Misses 529 528 -1
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
|
@coderabbitai full review based on your configuration. |
Contributor
|
@palisadoes I will conduct a full review of the PR based on the current configuration. ✅ Actions performedFull review triggered. |
Contributor
|
@coderabbitai What is your conclusion based on this?: |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 227 @palisadoes The comment you referenced was my automated confirmation that a full review was triggered. Based on my analysis of this PR, here's my conclusion: ✅ Overall Assessment: Ready for MergeAchievementsThis PR successfully accomplishes its Part 1/5 objectives for refactoring ActionItemModal (issue #6250):
Context on "Part 1/5"The 5-part series refers to the refactoring plan outlined in issue #6250, which calls for breaking the ActionItemModal refactor into multiple PRs (each ≤10 files changed) to make reviews manageable while progressively achieving the <600 line target. RecommendationThis PR demonstrates solid incremental progress with proper testing and no regressions. The author has addressed all review feedback and met quality standards. Approved for merge. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsx (2)
958-958: Removeasyncfromdescribeblock.The
asynckeyword on adescribeblock is incorrect syntax and has no effect. Vitest/Jestdescribecallbacks are synchronous by design.🔧 Proposed fix
- describe('Assignment type switching', async () => { + describe('Assignment type switching', () => {
1216-1216: Removeasyncfromdescribeblock.Same issue as above -
asyncondescribehas no effect.🔧 Proposed fix
- describe('Accessibility', async () => { + describe('Accessibility', () => {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (4)
docs/docs/auto-docs/components/AdminPortal/ApplyToSelector/ApplyToSelector/variables/default.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/screens/OrganizationActionItems/ActionItemModal/ActionItemModal/variables/default.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/AdminPortal/ApplyToSelector/interface/interfaces/InterfaceApplyToSelectorProps.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/AdminPortal/ApplyToSelector/interface/type-aliases/ApplyToType.mdis excluded by!**/docs/docs/**
📒 Files selected for processing (5)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsxsrc/types/AdminPortal/ApplyToSelector/interface.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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/types/AdminPortal/ApplyToSelector/interface.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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsxsrc/types/AdminPortal/ApplyToSelector/interface.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/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsx
🧠 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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.tsxsrc/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsx
📚 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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsxsrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.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/types/AdminPortal/ApplyToSelector/interface.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/types/AdminPortal/ApplyToSelector/interface.ts
🧬 Code graph analysis (3)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx (1)
src/types/AdminPortal/ApplyToSelector/interface.ts (1)
InterfaceApplyToSelectorProps(11-16)
src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx (2)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx (1)
ApplyToType(9-9)src/types/AdminPortal/ApplyToSelector/interface.ts (1)
ApplyToType(6-6)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsx (1)
src/types/AdminPortal/ApplyToSelector/interface.ts (1)
ApplyToType(6-6)
🔇 Additional comments (10)
src/components/AdminPortal/ApplyToSelector/ApplyToSelector.spec.tsx (1)
1-71: LGTM! Well-structured test file with good coverage.The test file follows best practices:
- Proper test isolation with
vi.clearAllMocks()inafterEach(per learnings)- Uses
userEvent.setup()for user interactions- Tests both initial states and onChange callbacks
- Uses accessible queries (
getByLabelText)src/types/AdminPortal/ApplyToSelector/interface.ts (1)
1-16: LGTM! Type definitions follow reusable component policy.The interface file correctly:
- Defines types in the designated
src/types/AdminPortal/directory- Uses the
Interface<ComponentName>Propsnaming convention- Includes TSDoc documentation on the type and interface
- Exports
ApplyToTypefor reuse across the codebasesrc/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.spec.tsx (1)
627-634: Good test isolation setup.The
beforeEachandafterEachblocks withvi.clearAllMocks()ensure proper test isolation, which aligns with project guidelines. Based on learnings, this pattern is required for all spec files.src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx (6)
11-16: Good refactoring - imports updated correctly.The imports are correctly updated:
DatePickerfrom the shared-components pathBaseModalfrom the shared-components path (replacing deprecated react-bootstrap Modal)ApplyToSelectorfrom AdminPortal componentsApplyToTypefrom the types directory
98-98: State typing updated correctly.The
applyTostate now uses the exportedApplyToTypeinstead of an inline literal union, improving type consistency across the codebase.
169-173: Clean boolean coercion.Using
Boolean()for explicit type coercion is cleaner and more readable than alternatives like double negation (!!).
441-449: ApplyToSelector integration is correct.The component is rendered in the appropriate scenarios:
- Create mode with recurring events (
isRecurring && !editMode)- Edit mode for template items that aren't instance exceptions
The props are correctly wired to the component state.
433-440: BaseModal integration looks correct.The
BaseModalusage replaces the deprecatedreact-bootstrapModal correctly, with proper props forshow,onHide,title, anddataTestId.
596-608: No action required — the prop naming is correct.The
DatePickercomponent's interface explicitly defines the test ID prop as'data-testid'(kebab-case string literal, line 38 of DatePicker.tsx), which is why the ActionItemModal correctly usesdata-testid="assignmentDate"at line 602. Similarly,BaseModaldefines its test ID prop asdataTestId(camelCase), which is correctly used at line 439. Each component accepts the prop naming defined in its own interface, so there is no inconsistency.src/components/AdminPortal/ApplyToSelector/ApplyToSelector.tsx (1)
1-49: Component correctly implements ApplyToSelector with proper architecture and testing.All verification complete:
- ✅ i18n keys (
applyTo,entireSeries,thisEventOnly) present in all 5 locale files- ✅ Props interface properly defined in
src/types/AdminPortal/ApplyToSelector/interface.ts- ✅ Test file comprehensive (71 lines) covering render, state, and user interactions
- ✅ Component follows reusable component policy (correct portal, no inline interfaces, TSDoc present)
- ✅ Zero TypeScript errors
- ✅ Semantic HTML (fieldset/legend) for accessibility
palisadoes
approved these changes
Jan 11, 2026
palisadoes
merged commit 48dc88a
into
PalisadoesFoundation:develop
37 of 39 checks passed
6 tasks
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?
Refactoring : src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx
@mui/x-date-pickers Datepickerto shared Datepicker Component based on the latest shared-component Issue.getByTestIdworkaround for BaseModal's Duplicaterole="dialog"issue as highlighted in the Bug Report: BaseModal adds duplicate role="dialog" causing test failures #6312Issue Number:
Fixes #6250
Snapshots/Videos:
N/A
If relevant, did you update the documentation?
N/A
Summary
I was assigned to the issue #6250 in which I had to refactor the src/screens/OrganizationActionItems/ActionItemModal/ActionItemModal.tsx File. I saw the following issues that could be refactored :
@mui/x-date-pickersso I changed the deprecated import to the latest DatePicker component from src/shared-components as per ESLint rules.getByTestId('dialog')instead of getByRole('dialog').Line count reduction: 713 → 660 lines
Does this PR introduce a breaking change?
No
Checklist
CodeRabbit AI Review
Test Coverage
Other information
Have you read the contributing guide?
Yes
Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.