feat: implement crud modal template foundation Phase1 #6371
Conversation
|
No significant changes currently retry |
Our Pull Request Approval ProcessThis PR will be reviewed according to our: Your PR may be automatically closed if:
Thanks for contributing! |
WalkthroughIntroduces a comprehensive CRUD modal template system with a base modal component and four specialized templates (Create, Edit, Delete, View), complete with TypeScript interfaces, CSS styling, extensive test coverage, and internationalization support in five languages. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (7 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In @src/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsx:
- Around line 992-993: The test buttons lack explicit types which can cause them
to act as form submitters; update the JSX in the test where the Edit and Delete
buttons are rendered (the elements using onClick={mockEdit} and
onClick={mockDelete}) to include type="button" on both buttons so they do not
trigger form submission or other default submit behavior during tests.
- Line 212: The test renders CRUDModalTemplate with a customFooter button that
lacks an explicit type, which defaults to submit and can trigger unintended form
submissions; update the customFooter usage in CRUDModalTemplate.spec.tsx to
render the element as <button type="button">Custom Action</button> (or otherwise
ensure the element passed to the customFooter prop has type="button") so the
button no longer submits forms.
In @src/shared-components/CRUDModalTemplate/DeleteModal.tsx:
- Around line 1-145: The component calls tCommon('deleteEntityConfirmation') and
tCommon('deleteConfirmation') in DeleteModal but those keys are missing in all
locale files; add the two keys with the specified strings to
public/locales/{en,es,fr,hi,zh}/common.json so translations render correctly,
and create a colocated test file DeleteModal.spec.tsx that asserts: the warning
icon conditionally renders when showWarning is true, the message interpolates
entityName via tCommon('deleteEntityConfirmation') vs
tCommon('deleteConfirmation') when entityName is absent, recurringEventContent
is rendered when provided, both Cancel and Delete buttons are disabled when
loading, and onDelete is only invoked when loading is false (test DeleteModal’s
handleDelete behavior and button click effects).
In @src/shared-components/CRUDModalTemplate/index.ts:
- Around line 31-41: The exported types for CRUDModalTemplate are being imported
from the wrong path; update the export statement in
src/shared-components/CRUDModalTemplate/index.ts so the type re-exports come
from the shared-components types location (replace
'types/CRUDModalTemplate/interface' with
'types/shared-components/CRUDModalTemplate/interface'), keeping the same
exported symbols (InterfaceCrudModalBaseProps, InterfaceCRUDModalTemplateProps,
InterfaceCreateModalProps, InterfaceEditModalProps, InterfaceDeleteModalProps,
InterfaceViewModalProps, InterfaceModalFormState, InterfaceRecurringEventProps,
ModalSize).
In @src/types/CRUDModalTemplate/interface.ts:
- Around line 22-32: The interface InterfaceCrudModalBaseProps exposes two
competing visibility props (open and show) causing API confusion; either remove
show to simplify the API or mark show as deprecated and implement a migration
path: mark show as @deprecated in the InterfaceCrudModalBaseProps TSDoc, keep
only open as the canonical prop, and in the component that consumes these props
(where props.open/props.show are read) add a runtime warning when show is
provided and open is undefined (use show as fallback but log a deprecation
message), plus update docs and tests to assert the new behavior.
- Around line 1-10: The file containing the CRUD Modal Template type definitions
is in the wrong location; move the interface file from
src/types/CRUDModalTemplate/interface.ts into the shared-components tree at
src/types/shared-components/CRUDModalTemplate/interface.ts and keep the same
exported symbols (e.g., any exported interfaces or types referenced by
components like CRUDModalTemplate props). After moving, update all imports
across the codebase that reference 'src/types/CRUDModalTemplate/interface' to
the new path 'src/types/shared-components/CRUDModalTemplate/interface' (and
adjust any barrel/index exports or tsconfig path mappings if your project uses
them) so consumers of the types (e.g., components importing ReactNode-using
types) continue to resolve correctly.
- Line 9: The code imports only ReactNode from React but uses
React.FormEvent<HTMLFormElement> in type signatures (e.g., the handlers at the
CRUD modal component where form events are typed), causing missing namespace
errors; fix by importing FormEvent as a type (import type { ReactNode, FormEvent
} from 'react') and change occurrences of React.FormEvent<HTMLFormElement> to
FormEvent<HTMLFormElement> in the relevant function/method signatures (the form
submit/handler types referenced in this file).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (15)
docs/docs/auto-docs/UserPortalNavigationBar/interface/interfaces/InterfaceUserPortalNavbarProps.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/shared-components/CRUDModalTemplate/CRUDModalTemplate/variables/CRUDModalTemplate.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/shared-components/CRUDModalTemplate/CreateModal/variables/CreateModal.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/shared-components/CRUDModalTemplate/DeleteModal/variables/DeleteModal.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/shared-components/CRUDModalTemplate/EditModal/functions/EditModal.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/shared-components/CRUDModalTemplate/ViewModal/functions/ViewModal.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/CRUDModalTemplate/interface/interfaces/InterfaceCRUDModalTemplateProps.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/CRUDModalTemplate/interface/interfaces/InterfaceCreateModalProps.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/CRUDModalTemplate/interface/interfaces/InterfaceCrudModalBaseProps.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/CRUDModalTemplate/interface/interfaces/InterfaceDeleteModalProps.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/CRUDModalTemplate/interface/interfaces/InterfaceEditModalProps.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/CRUDModalTemplate/interface/interfaces/InterfaceModalFormState.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/CRUDModalTemplate/interface/interfaces/InterfaceRecurringEventProps.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/CRUDModalTemplate/interface/interfaces/InterfaceViewModalProps.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/CRUDModalTemplate/interface/type-aliases/ModalSize.mdis excluded by!**/docs/docs/**
📒 Files selected for processing (9)
src/shared-components/CRUDModalTemplate/CRUDModalTemplate.module.csssrc/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsxsrc/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsxsrc/shared-components/CRUDModalTemplate/CreateModal.tsxsrc/shared-components/CRUDModalTemplate/DeleteModal.tsxsrc/shared-components/CRUDModalTemplate/EditModal.tsxsrc/shared-components/CRUDModalTemplate/ViewModal.tsxsrc/shared-components/CRUDModalTemplate/index.tssrc/types/CRUDModalTemplate/interface.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*
⚙️ 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/shared-components/CRUDModalTemplate/CRUDModalTemplate.module.csssrc/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsxsrc/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsxsrc/shared-components/CRUDModalTemplate/ViewModal.tsxsrc/shared-components/CRUDModalTemplate/index.tssrc/shared-components/CRUDModalTemplate/DeleteModal.tsxsrc/shared-components/CRUDModalTemplate/CreateModal.tsxsrc/shared-components/CRUDModalTemplate/EditModal.tsxsrc/types/CRUDModalTemplate/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/shared-components/CRUDModalTemplate/CRUDModalTemplate.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/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsxsrc/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsxsrc/shared-components/CRUDModalTemplate/ViewModal.tsxsrc/shared-components/CRUDModalTemplate/index.tssrc/shared-components/CRUDModalTemplate/DeleteModal.tsxsrc/shared-components/CRUDModalTemplate/CreateModal.tsxsrc/shared-components/CRUDModalTemplate/EditModal.tsxsrc/types/CRUDModalTemplate/interface.ts
🧠 Learnings (11)
📚 Learning: 2026-01-10T20:29:21.541Z
Learnt from: Piyush-Goenka
Repo: PalisadoesFoundation/talawa-admin PR: 6315
File: src/screens/AdminPortal/ManageTag/editModal/EditUserTagModal.module.css:24-30
Timestamp: 2026-01-10T20:29:21.541Z
Learning: For CSS review guidance, apply changes to CSS files across the project (not TSX unless specifically asked). When suggesting improvements to CSS selectors, prefer CSS-only solutions that leverage existing DOM attributes (e.g., form ids, data attributes) and avoid proposing changes that require modifying TSX files. This aligns with the PR scope (CSS file changes and how they’re imported in TSX) and ensures changes remain isolated to styles rather than TSX logic.
Applied to files:
src/shared-components/CRUDModalTemplate/CRUDModalTemplate.module.css
📚 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/shared-components/CRUDModalTemplate/CRUDModalTemplate.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/shared-components/CRUDModalTemplate/CRUDModalTemplate.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/shared-components/CRUDModalTemplate/CRUDModalTemplate.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/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsxsrc/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsxsrc/shared-components/CRUDModalTemplate/ViewModal.tsxsrc/shared-components/CRUDModalTemplate/DeleteModal.tsxsrc/shared-components/CRUDModalTemplate/CreateModal.tsxsrc/shared-components/CRUDModalTemplate/EditModal.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/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsxsrc/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsxsrc/shared-components/CRUDModalTemplate/ViewModal.tsxsrc/shared-components/CRUDModalTemplate/DeleteModal.tsxsrc/shared-components/CRUDModalTemplate/CreateModal.tsxsrc/shared-components/CRUDModalTemplate/EditModal.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/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsxsrc/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsxsrc/shared-components/CRUDModalTemplate/ViewModal.tsxsrc/shared-components/CRUDModalTemplate/DeleteModal.tsxsrc/shared-components/CRUDModalTemplate/CreateModal.tsxsrc/shared-components/CRUDModalTemplate/EditModal.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/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsx
📚 Learning: 2026-01-03T17:02:07.250Z
Learnt from: Jashan32
Repo: PalisadoesFoundation/talawa-admin PR: 6130
File: src/shared-components/pinnedPosts/pinnedPostCard.tsx:111-113
Timestamp: 2026-01-03T17:02:07.250Z
Learning: When reviewing PRs that replace react-toastify with NotificationToast, limit the scope to the API replacement and fixing tests that break. Do not add new test coverage for NotificationToast calls in the same PR unless tests already assert on toast behavior; any new test coverage should be handled in separate PRs. This guideline applies to TypeScript React component files under src/shared-components (e.g., src/shared-components/**/*.tsx) where the replacement occurs.
Applied to files:
src/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsxsrc/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsxsrc/shared-components/CRUDModalTemplate/ViewModal.tsxsrc/shared-components/CRUDModalTemplate/DeleteModal.tsxsrc/shared-components/CRUDModalTemplate/CreateModal.tsxsrc/shared-components/CRUDModalTemplate/EditModal.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/shared-components/CRUDModalTemplate/index.tssrc/types/CRUDModalTemplate/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/shared-components/CRUDModalTemplate/index.tssrc/types/CRUDModalTemplate/interface.ts
🧬 Code graph analysis (6)
src/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsx (5)
src/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsx (1)
CRUDModalTemplate(42-149)src/shared-components/CRUDModalTemplate/CreateModal.tsx (1)
CreateModal(44-135)src/shared-components/CRUDModalTemplate/EditModal.tsx (1)
EditModal(45-138)src/shared-components/CRUDModalTemplate/DeleteModal.tsx (1)
DeleteModal(63-145)src/shared-components/CRUDModalTemplate/ViewModal.tsx (1)
ViewModal(58-107)
src/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsx (3)
src/shared-components/CRUDModalTemplate/index.ts (2)
CRUDModalTemplate(25-25)InterfaceCRUDModalTemplateProps(33-33)src/types/CRUDModalTemplate/interface.ts (1)
InterfaceCRUDModalTemplateProps(98-141)src/shared-components/BaseModal/BaseModal.tsx (1)
BaseModal(28-100)
src/shared-components/CRUDModalTemplate/DeleteModal.tsx (3)
src/shared-components/CRUDModalTemplate/index.ts (3)
DeleteModal(28-28)InterfaceDeleteModalProps(36-36)CRUDModalTemplate(25-25)src/types/CRUDModalTemplate/interface.ts (1)
InterfaceDeleteModalProps(215-245)src/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsx (1)
CRUDModalTemplate(42-149)
src/shared-components/CRUDModalTemplate/CreateModal.tsx (3)
src/shared-components/CRUDModalTemplate/index.ts (3)
CreateModal(26-26)InterfaceCreateModalProps(34-34)CRUDModalTemplate(25-25)src/types/CRUDModalTemplate/interface.ts (1)
InterfaceCreateModalProps(148-166)src/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsx (1)
CRUDModalTemplate(42-149)
src/shared-components/CRUDModalTemplate/EditModal.tsx (3)
src/shared-components/CRUDModalTemplate/index.ts (3)
EditModal(27-27)InterfaceEditModalProps(35-35)CRUDModalTemplate(25-25)src/types/CRUDModalTemplate/interface.ts (1)
InterfaceEditModalProps(176-208)src/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsx (1)
CRUDModalTemplate(42-149)
src/types/CRUDModalTemplate/interface.ts (1)
src/shared-components/CRUDModalTemplate/index.ts (8)
InterfaceCrudModalBaseProps(32-32)InterfaceCRUDModalTemplateProps(33-33)InterfaceCreateModalProps(34-34)InterfaceEditModalProps(35-35)InterfaceDeleteModalProps(36-36)InterfaceViewModalProps(37-37)InterfaceModalFormState(38-38)InterfaceRecurringEventProps(39-39)
🪛 Biome (2.1.2)
src/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsx
[error] 212-212: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 992-992: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 993-993: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Performs linting, formatting, type-checking, unused file detection, checking for different source...
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (13)
src/shared-components/CRUDModalTemplate/CRUDModalTemplate.module.css (1)
1-99: LGTM! Well-structured CSS module for CRUD modal templates.The styling is clean, follows standard patterns (e.g.,
visuallyHiddenfor accessibility), and aligns with Bootstrap conventions (z-index 1050). The utility classes provide consistent appearance across modal variants.Minor note: The
.viewModalFieldclass (lines 69-71) appears to be defined but not used in the current ViewModal implementation. Consider removing it if unused, or document its intended usage.src/shared-components/CRUDModalTemplate/ViewModal.tsx (1)
1-107: LGTM! Clean implementation of ViewModal component.The component correctly:
- Uses external interface for props (compliant with coding guidelines)
- Implements proper i18n with
useTranslation('common')- Provides comprehensive TSDoc documentation with examples
- Handles loading states and optional custom actions
- Follows consistent patterns with sibling modal components
src/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsx (2)
21-27: Good test hygiene withvi.clearAllMocks()inafterEach.This ensures proper test isolation as per project guidelines. The pattern is consistent across all test describe blocks.
525-596: Comprehensive focus management tests.Good coverage of auto-focus behavior including:
- Focus on first input when modal opens
- Skipping hidden/disabled inputs
- Graceful handling when no focusable inputs exist
The
rerenderpattern withwaitForand timeout is appropriate for testing thesetTimeoutin the focus logic.src/shared-components/CRUDModalTemplate/CreateModal.tsx (1)
1-135: LGTM! Well-implemented CreateModal with good UX features.The component correctly implements:
- Auto-focus on first focusable input with proper selector exclusions
- Ctrl/Cmd+Enter keyboard shortcut via
requestSubmit()- Proper form-button linkage using
idandformattributes- i18n for all user-facing text
- External props interface (compliant with guidelines)
- Comprehensive TSDoc documentation
src/shared-components/CRUDModalTemplate/EditModal.tsx (1)
1-138: LGTM! EditModal properly extends the modal pattern for edit workflows.Key strengths:
- Dual loading state support (
loadingfor save,loadingDatafor fetch)- Auto-focus correctly deferred until data is loaded
- Consistent keyboard shortcuts and form handling with CreateModal
- Proper i18n usage
- External props interface (compliant)
src/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsx (1)
42-149: LGTM! Solid foundation for the CRUD modal system.The base template correctly handles:
- Loading state with static backdrop, disabled keyboard/close button
- Escape key with proper cleanup and loading guard
- Flexible footer (custom or default buttons)
- Error display via Alert
- Integration with BaseModal and LoadingState (compliant with restricted imports)
Note: Default
primaryText='Save'andsecondaryText='Cancel'(lines 49-50) are English, but all specialized modals (CreateModal, EditModal, DeleteModal) override these with translated strings viacustomFooter, so this is acceptable.src/shared-components/CRUDModalTemplate/index.ts (1)
1-23: Well-documented module with helpful example.The TSDoc block provides clear documentation of the module's purpose and includes a practical usage example. This aligns with the documentation requirements.
src/types/CRUDModalTemplate/interface.ts (5)
143-166: CreateModal interface is well-designed.The interface correctly requires
childrenandonSubmitas mandatory props, with optionalsubmitDisabledfor form validation control. The async return type (void | Promise<void>) appropriately supports both sync and async handlers.
168-208: EditModal generic interface provides good type safety.Using a generic
<T = unknown>for thedataprop allows consumers to specify their entity type while providing a sensible default. TheloadingDataprop correctly separates data-fetching state from submission state.
210-245: DeleteModal interface correctly separates concerns.The
onDeletecallback (notonSubmit) clearly communicates the destructive intent. TherecurringEventContentprop provides extensibility for the recurring event use case mentioned in the PR objectives.
247-278: ViewModal requiresdataas mandatory prop.Unlike EditModal where
datais optional (T | null), ViewModal correctly requiresdata: Tsince a view modal without data to display is invalid. ThecustomActionsprop enables "Edit" or "Delete" buttons from the view context.
280-327: Helper interfaces provide useful abstractions.
InterfaceModalFormStateandInterfaceRecurringEventPropsencapsulate common patterns that can be composed with the modal props. This aligns with the PR objective of reducing repetitive implementations.
src/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsx
Outdated
Show resolved
Hide resolved
src/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsx
Outdated
Show resolved
Hide resolved
| import React from 'react'; | ||
| import { Button } from 'react-bootstrap'; | ||
| import { useTranslation } from 'react-i18next'; | ||
| import { CRUDModalTemplate } from './CRUDModalTemplate'; | ||
| import type { InterfaceDeleteModalProps } from 'types/CRUDModalTemplate/interface'; | ||
| import styles from './CRUDModalTemplate.module.css'; | ||
| import globalStyles from 'style/app-fixed.module.css'; | ||
|
|
||
| /** | ||
| * DeleteModal Component | ||
| * | ||
| * Specialized modal template for delete confirmations. | ||
| * Displays warning UI and handles delete operations. | ||
| * | ||
| * Features: | ||
| * - Warning icon for visual emphasis | ||
| * - Highlighted entity name in confirmation message | ||
| * - Support for recurring event deletion patterns | ||
| * - Danger-styled delete button | ||
| * - Loading state prevents duplicate delete requests | ||
| * | ||
| * @example | ||
| * ```tsx | ||
| * <DeleteModal | ||
| * open={showModal} | ||
| * title="Delete Campaign" | ||
| * onClose={handleClose} | ||
| * onDelete={handleDelete} | ||
| * loading={isDeleting} | ||
| * entityName="Summer Campaign 2024" | ||
| * confirmationMessage="Are you sure you want to delete this campaign?" | ||
| * /> | ||
| * ``` | ||
| * | ||
| * @example Recurring event deletion | ||
| * ```tsx | ||
| * <DeleteModal | ||
| * open={showModal} | ||
| * title="Delete Recurring Event" | ||
| * onClose={handleClose} | ||
| * onDelete={handleDelete} | ||
| * loading={isDeleting} | ||
| * entityName="Weekly Meeting" | ||
| * recurringEventContent={ | ||
| * <Form.Group> | ||
| * <Form.Check | ||
| * type="radio" | ||
| * label="Delete this instance only" | ||
| * checked={deleteMode === 'instance'} | ||
| * onChange={() => setDeleteMode('instance')} | ||
| * /> | ||
| * <Form.Check | ||
| * type="radio" | ||
| * label="Delete all future instances" | ||
| * checked={deleteMode === 'series'} | ||
| * onChange={() => setDeleteMode('series')} | ||
| * /> | ||
| * </Form.Group> | ||
| * } | ||
| * /> | ||
| * ``` | ||
| */ | ||
| export const DeleteModal: React.FC<InterfaceDeleteModalProps> = ({ | ||
| open, | ||
| show, | ||
| title, | ||
| onClose, | ||
| children, | ||
| onDelete, | ||
| loading = false, | ||
| error, | ||
| size, | ||
| className, | ||
| centered = true, | ||
| 'data-testid': dataTestId, | ||
| entityName, | ||
| showWarning = true, | ||
| recurringEventContent, | ||
| }) => { | ||
| const { t: tCommon } = useTranslation('common'); | ||
| const isOpen = open ?? show ?? false; | ||
|
|
||
| const handleDelete = () => { | ||
| if (!loading) { | ||
| onDelete(); | ||
| } | ||
| }; | ||
|
|
||
| const customFooter = ( | ||
| <div className={styles.footerButtons}> | ||
| <Button | ||
| variant="secondary" | ||
| onClick={onClose} | ||
| disabled={loading} | ||
| data-testid="modal-cancel-btn" | ||
| > | ||
| {tCommon('cancel')} | ||
| </Button> | ||
| <Button | ||
| variant="danger" | ||
| onClick={handleDelete} | ||
| disabled={loading} | ||
| className={globalStyles.removeButton} | ||
| data-testid="modal-delete-btn" | ||
| > | ||
| {tCommon('delete')} | ||
| </Button> | ||
| </div> | ||
| ); | ||
|
|
||
| return ( | ||
| <CRUDModalTemplate | ||
| open={isOpen} | ||
| title={title} | ||
| onClose={onClose} | ||
| loading={loading} | ||
| error={error} | ||
| size={size} | ||
| className={className} | ||
| centered={centered} | ||
| data-testid={dataTestId} | ||
| customFooter={customFooter} | ||
| > | ||
| {showWarning && ( | ||
| <div className={styles.deleteWarningIcon}> | ||
| <i className="fa fa-exclamation-triangle"></i> | ||
| </div> | ||
| )} | ||
|
|
||
| {children ? ( | ||
| children | ||
| ) : ( | ||
| <p className={styles.deleteMessage}> | ||
| {entityName | ||
| ? tCommon('deleteEntityConfirmation', { entityName }) | ||
| : tCommon('deleteConfirmation')} | ||
| </p> | ||
| )} | ||
|
|
||
| {recurringEventContent && ( | ||
| <div className={styles.recurringOptions}>{recurringEventContent}</div> | ||
| )} | ||
| </CRUDModalTemplate> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify translation keys exist in locale files
echo "=== Checking translation keys in common.json files ==="
for locale in en es fr hi zh; do
file="public/locales/$locale/common.json"
if [ -f "$file" ]; then
echo ""
echo "File: $file"
# Check for each key
grep -q '"cancel"' "$file" && echo "✓ 'cancel' found" || echo "✗ 'cancel' NOT found"
grep -q '"delete"' "$file" && echo "✓ 'delete' found" || echo "✗ 'delete' NOT found"
grep -q '"deleteEntityConfirmation"' "$file" && echo "✓ 'deleteEntityConfirmation' found" || echo "✗ 'deleteEntityConfirmation' NOT found"
grep -q '"deleteConfirmation"' "$file" && echo "✓ 'deleteConfirmation' found" || echo "✗ 'deleteConfirmation' NOT found"
else
echo "✗ $file NOT FOUND"
fi
doneRepository: PalisadoesFoundation/talawa-admin
Length of output: 846
🏁 Script executed:
# Check test file existence and type definition location
echo "=== Checking DeleteModal test and type files ==="
ls -la src/shared-components/CRUDModalTemplate/DeleteModal.spec.tsx 2>/dev/null || echo "Test file: NOT FOUND"
ls -la src/types/CRUDModalTemplate/interface.ts 2>/dev/null || echo "Interface file location check needed"
find src/types -name "*interface.ts" -type f | grep -i crud | head -5Repository: PalisadoesFoundation/talawa-admin
Length of output: 277
🏁 Script executed:
# Verify TypeScript compilation
echo "=== Checking TypeScript errors for DeleteModal ==="
cd src/shared-components/CRUDModalTemplate && ls -la DeleteModal.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 192
Add missing translation keys and test file.
Two critical issues prevent approval:
-
Missing i18n keys in all locales: The component uses
tCommon('deleteEntityConfirmation')andtCommon('deleteConfirmation')(lines 132–135), but these keys are absent from all five locale files (public/locales/{en,es,fr,hi,zh}/common.json). Users will see raw translation keys instead of confirmation messages. Add these keys to all locale files:{ "deleteEntityConfirmation": "Are you sure you want to delete {{entityName}}?", "deleteConfirmation": "Are you sure you want to delete this?" } -
Missing test file: DeleteModal.spec.tsx does not exist. All components with logic must have colocated test files with ≥95% coverage (§1.1). Create
src/shared-components/CRUDModalTemplate/DeleteModal.spec.tsxcovering:- Conditional warning icon display
- Entity-aware message interpolation
- Recurring event content rendering
- Loading state disables both buttons
- Delete action only fires when not loading
🤖 Prompt for AI Agents
In @src/shared-components/CRUDModalTemplate/DeleteModal.tsx around lines 1 -
145, The component calls tCommon('deleteEntityConfirmation') and
tCommon('deleteConfirmation') in DeleteModal but those keys are missing in all
locale files; add the two keys with the specified strings to
public/locales/{en,es,fr,hi,zh}/common.json so translations render correctly,
and create a colocated test file DeleteModal.spec.tsx that asserts: the warning
icon conditionally renders when showWarning is true, the message interpolates
entityName via tCommon('deleteEntityConfirmation') vs
tCommon('deleteConfirmation') when entityName is absent, recurringEventContent
is rendered when provided, both Cancel and Delete buttons are disabled when
loading, and onDelete is only invoked when loading is false (test DeleteModal’s
handleDelete behavior and button click effects).
| /** | ||
| * Type definitions for CRUD Modal Templates | ||
| * | ||
| * This file contains all TypeScript interfaces and types used across the | ||
| * CRUD modal template system. These types ensure consistency and type safety | ||
| * when creating, editing, viewing, and deleting entities. | ||
| */ | ||
|
|
||
| import type { ReactNode } from 'react'; | ||
|
|
There was a problem hiding this comment.
File location violates shared-components type directory structure.
Per coding guidelines §3.3, types for shared components must be in src/types/shared-components/<ComponentName>/interface.ts. This file should be moved from:
src/types/CRUDModalTemplate/interface.ts
to:
src/types/shared-components/CRUDModalTemplate/interface.ts
🤖 Prompt for AI Agents
In @src/types/CRUDModalTemplate/interface.ts around lines 1 - 10, The file
containing the CRUD Modal Template type definitions is in the wrong location;
move the interface file from src/types/CRUDModalTemplate/interface.ts into the
shared-components tree at
src/types/shared-components/CRUDModalTemplate/interface.ts and keep the same
exported symbols (e.g., any exported interfaces or types referenced by
components like CRUDModalTemplate props). After moving, update all imports
across the codebase that reference 'src/types/CRUDModalTemplate/interface' to
the new path 'src/types/shared-components/CRUDModalTemplate/interface' (and
adjust any barrel/index exports or tsconfig path mappings if your project uses
them) so consumers of the types (e.g., components importing ReactNode-using
types) continue to resolve correctly.
| * when creating, editing, viewing, and deleting entities. | ||
| */ | ||
|
|
||
| import type { ReactNode } from 'react'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and read its contents
cat -n src/types/CRUDModalTemplate/interface.tsRepository: PalisadoesFoundation/talawa-admin
Length of output: 9496
🏁 Script executed:
# Run typecheck to identify any TypeScript compilation errors
pnpm run typecheckRepository: PalisadoesFoundation/talawa-admin
Length of output: 321
Import FormEvent directly from React to resolve type reference errors.
The file uses React.FormEvent<HTMLFormElement> on lines 158 and 187, but only imports ReactNode from React using a type import. The React namespace is not available. Import FormEvent directly and update the type signatures:
Fix
-import type { ReactNode } from 'react';
+import type { ReactNode, FormEvent } from 'react';Then update lines 158 and 187:
- onSubmit: (event: React.FormEvent<HTMLFormElement>) => void | Promise<void>;
+ onSubmit: (event: FormEvent<HTMLFormElement>) => void | Promise<void>;🤖 Prompt for AI Agents
In @src/types/CRUDModalTemplate/interface.ts at line 9, The code imports only
ReactNode from React but uses React.FormEvent<HTMLFormElement> in type
signatures (e.g., the handlers at the CRUD modal component where form events are
typed), causing missing namespace errors; fix by importing FormEvent as a type
(import type { ReactNode, FormEvent } from 'react') and change occurrences of
React.FormEvent<HTMLFormElement> to FormEvent<HTMLFormElement> in the relevant
function/method signatures (the form submit/handler types referenced in this
file).
| export interface InterfaceCrudModalBaseProps { | ||
| /** | ||
| * Controls whether the modal is visible (defaults to false) | ||
| */ | ||
| open?: boolean; | ||
|
|
||
| /** | ||
| * Alternative prop name for modal visibility (for compatibility). | ||
| * If both `open` and `show` are provided, `open` takes precedence (defaults to false) | ||
| */ | ||
| show?: boolean; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Dual visibility props (open and show) may cause API confusion.
While the TSDoc documents that open takes precedence, having two props for the same purpose increases the API surface and cognitive load. Consider whether the compatibility requirement justifies this complexity, or if a migration path from show to open with deprecation warnings would be cleaner.
🤖 Prompt for AI Agents
In @src/types/CRUDModalTemplate/interface.ts around lines 22 - 32, The interface
InterfaceCrudModalBaseProps exposes two competing visibility props (open and
show) causing API confusion; either remove show to simplify the API or mark show
as deprecated and implement a migration path: mark show as @deprecated in the
InterfaceCrudModalBaseProps TSDoc, keep only open as the canonical prop, and in
the component that consumes these props (where props.open/props.show are read)
add a runtime warning when show is provided and open is undefined (use show as
fallback but log a deprecation message), plus update docs and tests to assert
the new behavior.
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In @src/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsx:
- Around line 584-596: The test mixes using renderWithI18n and a manual
I18nextProvider wrap for rerender which is inconsistent; update the test so
rerender is obtained from renderWithI18n and used for subsequent renders (or
implement a custom renderWithI18n that returns RTL's rerender already wrapped
with I18nextProvider), ensuring all calls use the same i18nForTest provider
rather than manually wrapping CreateModal with I18nextProvider; reference the
renderWithI18n helper, the rerender function, I18nextProvider and i18nForTest
when making the change.
- Around line 879-884: The ViewModal test suite is missing an afterEach cleanup
to enforce test isolation; add an afterEach block inside the
describe('ViewModal') that calls testing-library's cleanup (ensure cleanup is
imported from @testing-library/react if not already) so DOM is unmounted between
tests and keep vi.clearAllMocks() either in beforeEach or also call it in
afterEach for explicit mock reset (refer to the describe('ViewModal') block and
the mockOnClose test helper).
- Around line 742-748: The DeleteModal describe block is missing an afterEach
cleanup; add an afterEach inside the 'DeleteModal' describe that calls
vi.clearAllMocks() and the testing-library cleanup (imported from
'@testing-library/react' if not already) to mirror the parent CRUDModalTemplate
pattern and ensure mock/state teardown for mockOnClose and mockOnDelete between
tests.
- Around line 358-364: Add an afterEach block to the CreateModal test suite that
calls vi.clearAllMocks() to ensure test isolation; place it alongside the
existing beforeEach in the describe('CreateModal') block so mocks like
mockOnClose and mockOnSubmit are cleared after each test (use afterEach(() =>
vi.clearAllMocks()) referencing the existing beforeEach and vi.clearAllMocks).
In @src/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsx:
- Around line 48-49: The default props primaryText and secondaryText in
CRUDModalTemplate are hardcoded to English; update CRUDModalTemplate to use
useTranslation from react-i18next (e.g., const { t } = useTranslation()) and
derive localized defaults for primaryText and secondaryText (like
t('common.save') and t('common.cancel')) instead of literal 'Save'/'Cancel',
making sure to keep existing props override behavior (apply translation inside
the component body if props are undefined or set prop default values using the
translated strings).
In @src/shared-components/CRUDModalTemplate/DeleteModal.tsx:
- Around line 123-127: The warning icon rendered when showWarning is true is
missing accessibility attributes; since this Font Awesome <i> is decorative,
update the <i className="fa fa-exclamation-triangle"> element (inside the block
gated by showWarning and styled by deleteWarningIcon) to include
aria-hidden="true" so screen readers ignore it (alternatively use
role="presentation" if preferred).
- Around line 79-86: handleDelete currently invokes onDelete() but ignores its
return value; change handleDelete to return the result of onDelete (i.e., return
onDelete()) so callers can await or chain the promise—update the handleDelete
implementation in DeleteModal (and its type if needed) to forward the returned
Promise from onDelete, while preserving the loading check (e.g., if (!loading)
return onDelete()).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (14)
docs/docs/auto-docs/shared-components/CRUDModalTemplate/CRUDModalTemplate/variables/CRUDModalTemplate.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/shared-components/CRUDModalTemplate/CreateModal/variables/CreateModal.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/shared-components/CRUDModalTemplate/DeleteModal/variables/DeleteModal.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/shared-components/CRUDModalTemplate/EditModal/functions/EditModal.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/shared-components/CRUDModalTemplate/ViewModal/functions/ViewModal.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/shared-components/CRUDModalTemplate/interface/interfaces/InterfaceCRUDModalTemplateProps.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/shared-components/CRUDModalTemplate/interface/interfaces/InterfaceCreateModalProps.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/shared-components/CRUDModalTemplate/interface/interfaces/InterfaceCrudModalBaseProps.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/shared-components/CRUDModalTemplate/interface/interfaces/InterfaceDeleteModalProps.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/shared-components/CRUDModalTemplate/interface/interfaces/InterfaceEditModalProps.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/shared-components/CRUDModalTemplate/interface/interfaces/InterfaceModalFormState.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/shared-components/CRUDModalTemplate/interface/interfaces/InterfaceRecurringEventProps.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/shared-components/CRUDModalTemplate/interface/interfaces/InterfaceViewModalProps.mdis excluded by!**/docs/docs/**docs/docs/auto-docs/types/shared-components/CRUDModalTemplate/interface/type-aliases/ModalSize.mdis excluded by!**/docs/docs/**
📒 Files selected for processing (13)
public/locales/en/common.jsonpublic/locales/es/common.jsonpublic/locales/fr/common.jsonpublic/locales/hi/common.jsonpublic/locales/zh/common.jsonsrc/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsxsrc/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsxsrc/shared-components/CRUDModalTemplate/CreateModal.tsxsrc/shared-components/CRUDModalTemplate/DeleteModal.tsxsrc/shared-components/CRUDModalTemplate/EditModal.tsxsrc/shared-components/CRUDModalTemplate/ViewModal.tsxsrc/shared-components/CRUDModalTemplate/index.tssrc/types/shared-components/CRUDModalTemplate/interface.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
Zero TypeScript errors: Run pnpm run typecheck and must pass without errors or warnings
Files:
src/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsxsrc/shared-components/CRUDModalTemplate/ViewModal.tsxsrc/shared-components/CRUDModalTemplate/CreateModal.tsxsrc/shared-components/CRUDModalTemplate/EditModal.tsxsrc/shared-components/CRUDModalTemplate/DeleteModal.tsxsrc/types/shared-components/CRUDModalTemplate/interface.tssrc/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsxsrc/shared-components/CRUDModalTemplate/index.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/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsxpublic/locales/en/common.jsonpublic/locales/hi/common.jsonsrc/shared-components/CRUDModalTemplate/ViewModal.tsxsrc/shared-components/CRUDModalTemplate/CreateModal.tsxpublic/locales/fr/common.jsonsrc/shared-components/CRUDModalTemplate/EditModal.tsxpublic/locales/es/common.jsonsrc/shared-components/CRUDModalTemplate/DeleteModal.tsxpublic/locales/zh/common.jsonsrc/types/shared-components/CRUDModalTemplate/interface.tssrc/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsxsrc/shared-components/CRUDModalTemplate/index.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/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsx
🧠 Learnings (11)
📚 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/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsxsrc/shared-components/CRUDModalTemplate/ViewModal.tsxsrc/shared-components/CRUDModalTemplate/CreateModal.tsxsrc/shared-components/CRUDModalTemplate/EditModal.tsxsrc/shared-components/CRUDModalTemplate/DeleteModal.tsxsrc/shared-components/CRUDModalTemplate/CRUDModalTemplate.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/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsxsrc/shared-components/CRUDModalTemplate/ViewModal.tsxsrc/shared-components/CRUDModalTemplate/CreateModal.tsxsrc/shared-components/CRUDModalTemplate/EditModal.tsxsrc/shared-components/CRUDModalTemplate/DeleteModal.tsxsrc/shared-components/CRUDModalTemplate/CRUDModalTemplate.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/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsxsrc/shared-components/CRUDModalTemplate/ViewModal.tsxsrc/shared-components/CRUDModalTemplate/CreateModal.tsxsrc/shared-components/CRUDModalTemplate/EditModal.tsxsrc/shared-components/CRUDModalTemplate/DeleteModal.tsxsrc/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsx
📚 Learning: 2026-01-03T17:02:07.250Z
Learnt from: Jashan32
Repo: PalisadoesFoundation/talawa-admin PR: 6130
File: src/shared-components/pinnedPosts/pinnedPostCard.tsx:111-113
Timestamp: 2026-01-03T17:02:07.250Z
Learning: When reviewing PRs that replace react-toastify with NotificationToast, limit the scope to the API replacement and fixing tests that break. Do not add new test coverage for NotificationToast calls in the same PR unless tests already assert on toast behavior; any new test coverage should be handled in separate PRs. This guideline applies to TypeScript React component files under src/shared-components (e.g., src/shared-components/**/*.tsx) where the replacement occurs.
Applied to files:
src/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsxsrc/shared-components/CRUDModalTemplate/ViewModal.tsxsrc/shared-components/CRUDModalTemplate/CreateModal.tsxsrc/shared-components/CRUDModalTemplate/EditModal.tsxsrc/shared-components/CRUDModalTemplate/DeleteModal.tsxsrc/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsx
📚 Learning: 2025-12-30T09:43:02.090Z
Learnt from: palisadoes
Repo: PalisadoesFoundation/talawa-admin PR: 5868
File: src/shared-components/FormFieldGroup/types.ts:1-147
Timestamp: 2025-12-30T09:43:02.090Z
Learning: Enforce that all shared UI component types live in the dedicated interface.ts under src/types/shared-components/<Component>/interface.ts. For example, the FormFieldGroup component should define its types in src/types/shared-components/FormFieldGroup/interface.ts (not in src/shared-components/FormFieldGroup/types.ts). AdminPortal components should use src/types/AdminPortal/<Component>/interface.ts and UserPortal components should use src/types/UserPortal/<Component>/interface.ts. This aligns with the Reusable Components guide and makes types consistently discoverable across the codebase.
Applied to files:
src/types/shared-components/CRUDModalTemplate/interface.ts
📚 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/shared-components/CRUDModalTemplate/interface.tssrc/shared-components/CRUDModalTemplate/index.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/shared-components/CRUDModalTemplate/interface.tssrc/shared-components/CRUDModalTemplate/index.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/shared-components/CRUDModalTemplate/CRUDModalTemplate.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/shared-components/CRUDModalTemplate/CRUDModalTemplate.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/shared-components/CRUDModalTemplate/CRUDModalTemplate.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/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsx
🧬 Code graph analysis (6)
src/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsx (3)
src/shared-components/CRUDModalTemplate/index.ts (2)
CRUDModalTemplate(25-25)InterfaceCRUDModalTemplateProps(33-33)src/types/shared-components/CRUDModalTemplate/interface.ts (1)
InterfaceCRUDModalTemplateProps(92-135)src/shared-components/BaseModal/BaseModal.tsx (1)
BaseModal(28-100)
src/shared-components/CRUDModalTemplate/ViewModal.tsx (3)
src/shared-components/CRUDModalTemplate/index.ts (3)
ViewModal(29-29)InterfaceViewModalProps(37-37)CRUDModalTemplate(25-25)src/types/shared-components/CRUDModalTemplate/interface.ts (1)
InterfaceViewModalProps(248-272)src/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsx (1)
CRUDModalTemplate(42-148)
src/shared-components/CRUDModalTemplate/CreateModal.tsx (3)
src/shared-components/CRUDModalTemplate/index.ts (3)
CreateModal(26-26)InterfaceCreateModalProps(34-34)CRUDModalTemplate(25-25)src/types/shared-components/CRUDModalTemplate/interface.ts (1)
InterfaceCreateModalProps(142-160)src/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsx (1)
CRUDModalTemplate(42-148)
src/shared-components/CRUDModalTemplate/EditModal.tsx (2)
src/types/shared-components/CRUDModalTemplate/interface.ts (1)
InterfaceEditModalProps(170-202)src/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsx (1)
CRUDModalTemplate(42-148)
src/shared-components/CRUDModalTemplate/DeleteModal.tsx (3)
src/shared-components/CRUDModalTemplate/index.ts (3)
DeleteModal(28-28)InterfaceDeleteModalProps(36-36)CRUDModalTemplate(25-25)src/types/shared-components/CRUDModalTemplate/interface.ts (1)
InterfaceDeleteModalProps(209-239)src/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsx (1)
CRUDModalTemplate(42-148)
src/types/shared-components/CRUDModalTemplate/interface.ts (1)
src/shared-components/CRUDModalTemplate/index.ts (9)
ModalSize(40-40)InterfaceCrudModalBaseProps(32-32)InterfaceCRUDModalTemplateProps(33-33)InterfaceCreateModalProps(34-34)InterfaceEditModalProps(35-35)InterfaceDeleteModalProps(36-36)InterfaceViewModalProps(37-37)InterfaceModalFormState(38-38)InterfaceRecurringEventProps(39-39)
🔇 Additional comments (35)
public/locales/hi/common.json (1)
258-261: LGTM!The new Hindi translation keys (
required,deleteConfirmation,deleteEntityConfirmation,update) are correctly added and consistent with the CRUD modal template requirements. The{{entityName}}interpolation placeholder is properly preserved.public/locales/es/common.json (1)
257-260: LGTM!The Spanish translation keys are correctly added with appropriate translations. The
{{entityName}}interpolation is properly preserved.public/locales/zh/common.json (1)
258-261: LGTM!The Chinese translation keys are correctly added with appropriate translations. The
{{entityName}}interpolation placeholder is properly preserved with natural spacing in Chinese context.public/locales/en/common.json (1)
257-260: LGTM!The English source translation keys for the CRUD modal system are well-defined. The delete confirmation messages appropriately warn users about the irreversible nature of the action, and the
{{entityName}}interpolation allows for contextual entity-specific confirmations.public/locales/fr/common.json (1)
257-260: LGTM!The French translation keys are correctly added with grammatically appropriate translations. The
{{entityName}}interpolation is properly preserved, and French punctuation conventions (space before?) are correctly applied.src/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsx (4)
21-27: Well-structured test setup with proper isolation.Good use of
vi.fn()for mocks andafterEachwithvi.clearAllMocks()for cleanup. TherenderWithI18nhelper ensures consistent i18n context across tests.
277-306: Good keyboard navigation test coverage.Tests properly cover Escape key behavior including the edge case where Escape should not close the modal during loading state. This aligns with the accessibility requirements.
458-497: Good coverage of keyboard shortcuts (Ctrl/Cmd+Enter).Separate explicit test cases for Ctrl+Enter and Cmd+Enter (Mac) improve readability and make failures easier to diagnose when specific key handling regresses.
647-653: AddafterEachblock forEditModaldescribe block.Similar to the
CreateModalblock, theEditModaldescribe block should include anafterEachcleanup for consistent test isolation.Proposed fix
describe('EditModal', () => { const mockOnClose = vi.fn(); const mockOnSubmit = vi.fn(); beforeEach(() => { vi.clearAllMocks(); }); + + afterEach(() => { + vi.clearAllMocks(); + });Likely an incorrect or invalid review comment.
src/shared-components/CRUDModalTemplate/ViewModal.tsx (4)
8-57: Well-documented component with TSDoc.The TSDoc documentation is comprehensive with clear feature descriptions and two practical usage examples. This follows the project's documentation standards.
76-88: Footer button ordering may cause UX inconsistency.The
customActionsare rendered before the Close button, which places custom actions on the left. This differs from typical modal patterns where primary/custom actions are on the right. Verify this is the intended design, as it may differ fromCRUDModalTemplate's default button ordering.
90-105: Clean composition with CRUDModalTemplate.Good use of composition pattern, delegating modal behavior to
CRUDModalTemplatewhile providing a read-only specific configuration. ThecustomFooterapproach cleanly overrides default footer behavior.
58-71: No action needed. Thedataprop in the interface is correctly not destructured in the component signature. The ViewModal follows the intended pattern where the generic typeTanddataprop provide type safety for consumers, but the component itself delegates rendering tochildren. The JSDoc examples correctly show that consumers pass data and use it within the children content, not within ViewModal itself. This is the proper architectural approach for a read-only view container.src/shared-components/CRUDModalTemplate/EditModal.tsx (4)
65-76: Auto-focus effect implementation is correct.The effect properly guards against focusing during loading states and uses a selector that excludes hidden and disabled inputs. The 100ms setTimeout allows the modal animation to complete before focusing.
9-44: Good TSDoc documentation with practical examples.The documentation clearly describes the component's features and provides a realistic usage example. Follows the project's TSDoc guidelines.
90-111: Well-structured custom footer with proper i18n.The footer correctly uses
tCommon('cancel')andtCommon('update')for localized button labels. Button states properly reflect loading and disabled conditions.
83-88:requestSubmit()is adequately supported for modern browsers.
HTMLFormElement.requestSubmit()is supported in Chrome 76+, Edge 79+, Firefox 75+, and Safari 16+ (and correspondingly in mobile variants). Internet Explorer is not supported, but this is standard for modern SPAs using Material-UI v7.If explicit fallback support is required for older browsers, use:
form.requestSubmit ? form.requestSubmit() : form.submit().src/shared-components/CRUDModalTemplate/CreateModal.tsx (3)
9-43: Good TSDoc documentation.Clear feature descriptions and practical usage example following TSDoc guidelines.
44-57: Component signature and props look correct.Props are properly typed via external interface, with sensible defaults for
loading,centered, andsubmitDisabled.
110-133: Clean modal composition.Good delegation to
CRUDModalTemplatewith form wrapped inside. ThecustomFooterpattern maintains consistent button behavior.src/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsx (4)
9-41: Excellent TSDoc documentation.Comprehensive documentation covering features, accessibility attributes, and a practical usage example. Follows TSDoc guidelines properly.
64-78: Well-implemented keyboard handling with proper cleanup.The
handleEscapeKeycallback is properly memoized with correct dependencies, and the effect correctly adds/removes the event listener. The guard for!loadingprevents closing during operations.
122-147: Solid BaseModal integration with accessibility features.Good use of
backdrop='static'andkeyboard={!loading}to prevent accidental dismissal during operations. TheshowCloseButton={!loading}hides the close button appropriately. Error display with Alert component is well-placed.
86-120: Footer logic is clear and well-structured.The ternary chain
showFooter ? (customFooter ? customFooter : defaultFooter) : undefinedclearly expresses the rendering priority. Button variant styling withglobalStylesclasses is consistent with the project patterns.src/shared-components/CRUDModalTemplate/DeleteModal.tsx (2)
1-8: LGTM! Imports follow project architecture.The imports are well-organized, and the props interface is correctly imported from
types/shared-components/CRUDModalTemplate/interface.ts, adhering to the reusable component policy.
9-62: TSDoc documentation is comprehensive and well-structured.The documentation includes clear descriptions, feature lists, and multiple usage examples including the recurring event deletion pattern. This meets TSDoc requirements per coding guidelines.
src/shared-components/CRUDModalTemplate/index.ts (1)
1-41: Well-structured barrel file with proper TSDoc and exports.The index file correctly:
- Provides module-level TSDoc documentation with usage examples
- Exports all component implementations from their respective files
- Re-exports types from the centralized
interface.tsfile intypes/shared-components/This follows the reusable component architecture guidelines.
src/types/shared-components/CRUDModalTemplate/interface.ts (8)
16-85: Base props interface is well-documented and comprehensive.The
InterfaceCrudModalBasePropsinterface provides a solid foundation with:
- Clear TSDoc for each property
- Appropriate optional vs required property designations
- Standard modal configuration options (size, centered, className)
- Proper typing for callback functions
87-135: CRUDModalTemplateProps correctly extends base props.The interface properly extends
InterfaceCrudModalBasePropsand adds template-specific properties likecustomFooter,primaryVariant, andshowFooter. The documentation clearly explains the relationship betweencustomFooterand the default button props.
137-160: CreateModalProps interface is properly defined.The
childrenprop is correctly required (not optional) since a create modal needs form content. TheonSubmithandler acceptsFormEvent<HTMLFormElement>which is the correct type for form submission.
162-202: EditModalProps uses generics appropriately for type-safe data handling.The generic
<T = unknown>pattern allows consumers to specify the entity type while providing a safe default. Thedata?: T | nulltyping correctly handles loading states where data may be absent.
204-239: DeleteModalProps aligns with implementation.The interface correctly defines all props used by
DeleteModal.tsxincludingentityName,showWarning, andrecurringEventContent. TheonDeletereturn typevoid | Promise<void>properly supports both sync and async delete handlers.
241-272: ViewModalProps correctly requiresdataas non-optional.Unlike
EditModalPropswheredatais optional (for loading states),ViewModalPropsrequiresdata: Tsince a view modal must have data to display. This is an intentional and correct design decision.
274-321: Helper interfaces for form state and recurring events are well-designed.
InterfaceModalFormStateandInterfaceRecurringEventPropsprovide useful abstractions for common patterns. TheapplyTo: 'series' | 'instance'union type clearly documents the recurring event deletion options.
1-14: The ModalSize type definition is correct as-is.React-Bootstrap's Modal component supports only
'sm' | 'lg' | 'xl'sizes. Bootstrap modals do not have a'md'variant; the default (when no size is specified) provides medium-sized modals. TheModalSizetype accurately reflects the API contract, and adding'md'would introduce an unsupported size that BaseModal cannot accept.Likely an incorrect or invalid review comment.
| useEffect(() => { | ||
| if (isOpen && formRef.current) { | ||
| const firstInput = formRef.current.querySelector< | ||
| HTMLInputElement | HTMLTextAreaElement | ||
| >( | ||
| 'input:not([type="hidden"]):not([disabled]), textarea:not([disabled]), select:not([disabled])', | ||
| ); | ||
| if (firstInput) { | ||
| setTimeout(() => firstInput.focus(), 100); | ||
| } | ||
| } | ||
| }, [isOpen]); | ||
|
|
||
| const handleSubmit = (event: React.FormEvent<HTMLFormElement>) => { | ||
| event.preventDefault(); | ||
| onSubmit(event); | ||
| }; | ||
|
|
||
| const handleKeyDown = (event: React.KeyboardEvent<HTMLFormElement>) => { | ||
| if (event.key === 'Enter' && (event.ctrlKey || event.metaKey)) { | ||
| event.preventDefault(); | ||
| formRef.current?.requestSubmit(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Code duplication with EditModal for auto-focus and keyboard handling.
The auto-focus effect (lines 62-73), handleSubmit (lines 75-78), and handleKeyDown (lines 80-85) are nearly identical to EditModal.tsx. Consider extracting these into shared hooks:
useAutoFocusFirstInput(isOpen: boolean, isLoading?: boolean, formRef: RefObject<HTMLFormElement>)useFormKeyboardShortcuts(formRef: RefObject<HTMLFormElement>)
Example shared hook
// hooks/useAutoFocusFirstInput.ts
export const useAutoFocusFirstInput = (
isOpen: boolean,
isLoading: boolean,
formRef: React.RefObject<HTMLFormElement>
) => {
useEffect(() => {
if (isOpen && !isLoading && formRef.current) {
const firstInput = formRef.current.querySelector<
HTMLInputElement | HTMLTextAreaElement
>(
'input:not([type="hidden"]):not([disabled]), textarea:not([disabled]), select:not([disabled])',
);
if (firstInput) {
setTimeout(() => firstInput.focus(), 100);
}
}
}, [isOpen, isLoading, formRef]);
};| describe('CreateModal', () => { | ||
| const mockOnClose = vi.fn(); | ||
| const mockOnSubmit = vi.fn(); | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add afterEach block with vi.clearAllMocks() for test isolation.
The CreateModal describe block has beforeEach but is missing afterEach with vi.clearAllMocks(). Per project guidelines, each spec file should use explicit cleanup in afterEach to guarantee test isolation and prevent bleed-over between tests.
Proposed fix
describe('CreateModal', () => {
const mockOnClose = vi.fn();
const mockOnSubmit = vi.fn();
beforeEach(() => {
vi.clearAllMocks();
});
+
+ afterEach(() => {
+ vi.clearAllMocks();
+ });Based on learnings, ensure each spec file uses an explicit vi.clearAllMocks() in an afterEach block.
🤖 Prompt for AI Agents
In @src/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsx around
lines 358 - 364, Add an afterEach block to the CreateModal test suite that calls
vi.clearAllMocks() to ensure test isolation; place it alongside the existing
beforeEach in the describe('CreateModal') block so mocks like mockOnClose and
mockOnSubmit are cleared after each test (use afterEach(() =>
vi.clearAllMocks()) referencing the existing beforeEach and vi.clearAllMocks).
|
|
||
| rerender( | ||
| <I18nextProvider i18n={i18nForTest}> | ||
| <CreateModal | ||
| open={true} | ||
| title="Create Item" | ||
| onClose={mockOnClose} | ||
| onSubmit={mockOnSubmit} | ||
| > | ||
| <div>No inputs here</div> | ||
| </CreateModal> | ||
| </I18nextProvider>, | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use renderWithI18n helper consistently for rerender.
The rerender at line 585 uses the renderWithI18n helper, but the subsequent rerender at line 586 manually wraps with I18nextProvider. This inconsistency could cause confusion and maintenance issues.
Proposed fix
The first rerender should use the returned rerender function but still needs the I18nextProvider wrapper since rerender from RTL doesn't use the original wrapper. Consider extracting a helper or documenting this behavior.
Alternatively, you can create a custom render that returns a rerender with the provider:
const renderWithI18n = (component: React.ReactElement) => {
const Wrapper = ({ children }: { children: React.ReactNode }) => (
<I18nextProvider i18n={i18nForTest}>{children}</I18nextProvider>
);
return render(component, { wrapper: Wrapper });
};This would make rerender automatically use the wrapper.
🤖 Prompt for AI Agents
In @src/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsx around
lines 584 - 596, The test mixes using renderWithI18n and a manual
I18nextProvider wrap for rerender which is inconsistent; update the test so
rerender is obtained from renderWithI18n and used for subsequent renders (or
implement a custom renderWithI18n that returns RTL's rerender already wrapped
with I18nextProvider), ensuring all calls use the same i18nForTest provider
rather than manually wrapping CreateModal with I18nextProvider; reference the
renderWithI18n helper, the rerender function, I18nextProvider and i18nForTest
when making the change.
| describe('DeleteModal', () => { | ||
| const mockOnClose = vi.fn(); | ||
| const mockOnDelete = vi.fn(); | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add afterEach block for DeleteModal describe block.
Add explicit cleanup to match the pattern established in the parent CRUDModalTemplate describe block.
Proposed fix
describe('DeleteModal', () => {
const mockOnClose = vi.fn();
const mockOnDelete = vi.fn();
beforeEach(() => {
vi.clearAllMocks();
});
+
+ afterEach(() => {
+ vi.clearAllMocks();
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| describe('DeleteModal', () => { | |
| const mockOnClose = vi.fn(); | |
| const mockOnDelete = vi.fn(); | |
| beforeEach(() => { | |
| vi.clearAllMocks(); | |
| }); | |
| describe('DeleteModal', () => { | |
| const mockOnClose = vi.fn(); | |
| const mockOnDelete = vi.fn(); | |
| beforeEach(() => { | |
| vi.clearAllMocks(); | |
| }); | |
| afterEach(() => { | |
| vi.clearAllMocks(); | |
| }); |
🤖 Prompt for AI Agents
In @src/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsx around
lines 742 - 748, The DeleteModal describe block is missing an afterEach cleanup;
add an afterEach inside the 'DeleteModal' describe that calls vi.clearAllMocks()
and the testing-library cleanup (imported from '@testing-library/react' if not
already) to mirror the parent CRUDModalTemplate pattern and ensure mock/state
teardown for mockOnClose and mockOnDelete between tests.
| describe('ViewModal', () => { | ||
| const mockOnClose = vi.fn(); | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add afterEach block for ViewModal describe block.
Add explicit cleanup for consistency with the established test isolation pattern.
Proposed fix
describe('ViewModal', () => {
const mockOnClose = vi.fn();
beforeEach(() => {
vi.clearAllMocks();
});
+
+ afterEach(() => {
+ vi.clearAllMocks();
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| describe('ViewModal', () => { | |
| const mockOnClose = vi.fn(); | |
| beforeEach(() => { | |
| vi.clearAllMocks(); | |
| }); | |
| describe('ViewModal', () => { | |
| const mockOnClose = vi.fn(); | |
| beforeEach(() => { | |
| vi.clearAllMocks(); | |
| }); | |
| afterEach(() => { | |
| vi.clearAllMocks(); | |
| }); |
🤖 Prompt for AI Agents
In @src/shared-components/CRUDModalTemplate/CRUDModalTemplate.spec.tsx around
lines 879 - 884, The ViewModal test suite is missing an afterEach cleanup to
enforce test isolation; add an afterEach block inside the describe('ViewModal')
that calls testing-library's cleanup (ensure cleanup is imported from
@testing-library/react if not already) so DOM is unmounted between tests and
keep vi.clearAllMocks() either in beforeEach or also call it in afterEach for
explicit mock reset (refer to the describe('ViewModal') block and the
mockOnClose test helper).
| primaryText = 'Save', | ||
| secondaryText = 'Cancel', |
There was a problem hiding this comment.
Hardcoded English default values for button text.
primaryText = 'Save' and secondaryText = 'Cancel' are hardcoded English strings. While child components (CreateModal, EditModal, etc.) provide translated text via customFooter, any direct usage of CRUDModalTemplate with default footer would display untranslated text.
Consider using useTranslation to provide localized defaults:
Proposed fix
+import { useTranslation } from 'react-i18next';
+
export const CRUDModalTemplate: React.FC<InterfaceCRUDModalTemplateProps> = ({
open,
title,
onClose,
children,
onPrimary,
- primaryText = 'Save',
- secondaryText = 'Cancel',
+ primaryText,
+ secondaryText,
loading = false,
// ...
}) => {
+ const { t: tCommon } = useTranslation('common');
+ const resolvedPrimaryText = primaryText ?? tCommon('save');
+ const resolvedSecondaryText = secondaryText ?? tCommon('cancel');
// use resolvedPrimaryText and resolvedSecondaryText in the footer🤖 Prompt for AI Agents
In @src/shared-components/CRUDModalTemplate/CRUDModalTemplate.tsx around lines
48 - 49, The default props primaryText and secondaryText in CRUDModalTemplate
are hardcoded to English; update CRUDModalTemplate to use useTranslation from
react-i18next (e.g., const { t } = useTranslation()) and derive localized
defaults for primaryText and secondaryText (like t('common.save') and
t('common.cancel')) instead of literal 'Save'/'Cancel', making sure to keep
existing props override behavior (apply translation inside the component body if
props are undefined or set prop default values using the translated strings).
| export const DeleteModal: React.FC<InterfaceDeleteModalProps> = ({ | ||
| open, | ||
| title, | ||
| onClose, | ||
| children, | ||
| onDelete, | ||
| loading = false, | ||
| error, | ||
| size, | ||
| className, | ||
| centered = true, | ||
| 'data-testid': dataTestId, | ||
| entityName, | ||
| showWarning = true, | ||
| recurringEventContent, | ||
| }) => { | ||
| const { t: tCommon } = useTranslation('common'); | ||
| const isOpen = open ?? false; | ||
|
|
||
| const handleDelete = () => { | ||
| if (!loading) { | ||
| onDelete(); | ||
| } | ||
| }; | ||
|
|
||
| const customFooter = ( | ||
| <div className={styles.footerButtons}> | ||
| <Button | ||
| variant="secondary" | ||
| onClick={onClose} | ||
| disabled={loading} | ||
| data-testid="modal-cancel-btn" | ||
| > | ||
| {tCommon('cancel')} | ||
| </Button> | ||
| <Button | ||
| variant="danger" | ||
| onClick={handleDelete} | ||
| disabled={loading} | ||
| className={globalStyles.removeButton} | ||
| data-testid="modal-delete-btn" | ||
| > | ||
| {tCommon('delete')} | ||
| </Button> | ||
| </div> | ||
| ); | ||
|
|
||
| return ( | ||
| <CRUDModalTemplate | ||
| open={isOpen} | ||
| title={title} | ||
| onClose={onClose} | ||
| loading={loading} | ||
| error={error} | ||
| size={size} | ||
| className={className} | ||
| centered={centered} | ||
| data-testid={dataTestId} | ||
| customFooter={customFooter} | ||
| > | ||
| {showWarning && ( | ||
| <div className={styles.deleteWarningIcon}> | ||
| <i className="fa fa-exclamation-triangle"></i> | ||
| </div> | ||
| )} | ||
|
|
||
| {children ? ( | ||
| children | ||
| ) : ( | ||
| <p className={styles.deleteMessage}> | ||
| {entityName | ||
| ? tCommon('deleteEntityConfirmation', { entityName }) | ||
| : tCommon('deleteConfirmation')} | ||
| </p> | ||
| )} | ||
|
|
||
| {recurringEventContent && ( | ||
| <div className={styles.recurringOptions}>{recurringEventContent}</div> | ||
| )} | ||
| </CRUDModalTemplate> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify test file exists
echo "=== Checking for test file ==="
fd -e spec.tsx -e test.tsx "DeleteModal" src/shared-components/CRUDModalTemplate/
# Check i18n keys in common namespace across all locales
echo -e "\n=== Checking i18n keys in common.json files ==="
for locale in en es fr hi zh; do
echo "--- $locale/common.json ---"
if [ -f "public/locales/$locale/common.json" ]; then
jq '.cancel, .delete, .deleteConfirmation, .deleteEntityConfirmation' "public/locales/$locale/common.json" 2>/dev/null || echo "File exists but jq parsing failed"
else
echo "File not found"
fi
doneRepository: PalisadoesFoundation/talawa-admin
Length of output: 1093
Create DeleteModal.spec.tsx with ≥95% code coverage.
The component has no test file. i18n keys (cancel, delete, deleteConfirmation, deleteEntityConfirmation) are correctly configured across all five locales (en, es, fr, hi, zh).
Test file must be colocated at src/shared-components/CRUDModalTemplate/DeleteModal.spec.tsx and cover:
- Component rendering with required props
- Modal open/close behavior
- Delete button click handler (including disabled state during loading)
- Warning icon visibility based on
showWarningprop - Custom children vs default deletion message rendering
- Recurring event content conditional rendering
- Error state propagation from
CRUDModalTemplate - All text assertions using i18n translations
| const { t: tCommon } = useTranslation('common'); | ||
| const isOpen = open ?? false; | ||
|
|
||
| const handleDelete = () => { | ||
| if (!loading) { | ||
| onDelete(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider returning the promise from onDelete for proper async handling.
The handleDelete function calls onDelete() but doesn't return its result. If onDelete returns a promise, the caller may need to await it for error handling or chaining. Consider returning the result.
♻️ Suggested improvement
const handleDelete = () => {
if (!loading) {
- onDelete();
+ return onDelete();
}
};🤖 Prompt for AI Agents
In @src/shared-components/CRUDModalTemplate/DeleteModal.tsx around lines 79 -
86, handleDelete currently invokes onDelete() but ignores its return value;
change handleDelete to return the result of onDelete (i.e., return onDelete())
so callers can await or chain the promise—update the handleDelete implementation
in DeleteModal (and its type if needed) to forward the returned Promise from
onDelete, while preserving the loading check (e.g., if (!loading) return
onDelete()).
| {showWarning && ( | ||
| <div className={styles.deleteWarningIcon}> | ||
| <i className="fa fa-exclamation-triangle"></i> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Add accessibility attributes to the warning icon.
The Font Awesome icon lacks accessibility attributes. Screen readers may announce it incorrectly or provide no context. Since this is decorative, add aria-hidden="true" to hide it from assistive technologies.
🔧 Proposed fix
{showWarning && (
<div className={styles.deleteWarningIcon}>
- <i className="fa fa-exclamation-triangle"></i>
+ <i className="fa fa-exclamation-triangle" aria-hidden="true"></i>
</div>
)}🤖 Prompt for AI Agents
In @src/shared-components/CRUDModalTemplate/DeleteModal.tsx around lines 123 -
127, The warning icon rendered when showWarning is true is missing accessibility
attributes; since this Font Awesome <i> is decorative, update the <i
className="fa fa-exclamation-triangle"> element (inside the block gated by
showWarning and styled by deleteWarningIcon) to include aria-hidden="true" so
screen readers ignore it (alternatively use role="presentation" if preferred).
|
As per your comment, creating a test file for However, the issue description specifies that only |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 8512 🏁 Script executed: Length of output: 31752 🏁 Script executed: Length of output: 22238 @abhishekblue, you're absolutely right to question this. The consolidated approach is the correct one for this PR. Your current implementation with
My previous comment requesting separate test files was unnecessarily prescriptive and didn't account for the component architecture and project requirements. Please disregard that comment—your current testing strategy is sound. The consolidated test file appropriately validates the entire CRUD modal template system. Keep the existing |
1 participant
What kind of change does this PR introduce?
Feature
Issue Number:
Fixes #5292
Summary
CreateModal,EditModal,DeleteModal,ViewModalDoes this PR introduce a breaking change?
No
Checklist
CodeRabbit AI Review
Test Coverage
Have you read the contributing guide?
Yes
Summary by CodeRabbit
Release Notes
New Features
Tests
Localization
✏️ Tip: You can customize this high-level summary in your review settings.