feat: ui: adding tenant meta management#147
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive frontend tenant metadata management system, enhancing church administrative information handling with verification badges, tooltips, and extended form fields. The implementation follows established patterns with React Hook Form, Zod validation, and comprehensive UI testing.
- Enhanced TenantCard with church info tooltip and verification badge display
- Extended TenantForm with metadata fields (tax_id, contact_email, address, website, phone_number) and validation
- Added service layer functions for tenant metadata management
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/components/Tenants/TenantCard.tsx |
Added verification badge and church info dialog integration |
src/components/Tenants/TenantForm.tsx |
Complete rewrite with React Hook Form, Zod validation, and metadata fields |
src/components/Tenants/TenantUpdateDialog.tsx |
Extended to handle metadata fields with proper service integration |
src/components/Tenants/TenantCreateDialog.tsx |
Enhanced to create metadata alongside tenant creation |
src/components/Tenants/ChurchInfoDialog.tsx |
New dialog component for displaying church metadata |
src/lib/tenant-service.ts |
Added functions for metadata management |
src/lib/types.ts |
Added TenantWithUsageAndMeta type definition |
| Translation files | Added comprehensive metadata-related translations |
| Test files | Extended UI tests for metadata display functionality |
Comments suppressed due to low confidence (2)
tests/rls/tenant-meta.rls.test.ts:122
- The variable '_error' suggests the error is intentionally unused, but the test doesn't verify the error state. Consider either removing the variable assignment or adding assertions about the expected error behavior.
const { data, error: _error } = await member.client
src/components/Tenants/TenantUpdateDialog.tsx:102
- [nitpick] The submitText uses t('updated') which translates to 'Tenant updated', but this should be an action verb like 'Update' or 'Save Changes' for a submit button.
submitText={t("updated")}
| /** | ||
| * Creates tenant metadata for new tenant | ||
| */ | ||
| export async function createTenantMetadata( |
There was a problem hiding this comment.
The function name is 'createTenantMetadata' but it uses 'update' operation instead of 'insert'. This will fail if no metadata record exists for the tenant. Either rename to 'updateTenantMetadata' or change to use 'upsert' operation.
| const infoButtons = screen.getAllByRole("button"); | ||
| const churchInfoButton = infoButtons.find( | ||
| (button) => | ||
| button.classList.contains("text-muted-foreground") && button.querySelector("svg"), | ||
| ); | ||
|
|
||
| expect(churchInfoButton).toBeInTheDocument(); | ||
|
|
||
| if (churchInfoButton) { | ||
| await user.click(churchInfoButton); | ||
|
|
||
| // After clicking, the dialog should be opened | ||
| // We can test this by checking if the dialog content appears | ||
| // Since the ChurchInfoDialog component will render its content | ||
| } |
There was a problem hiding this comment.
Using getAllByRole('button') and then filtering by CSS classes is fragile. Consider using more specific test selectors like data-testid or getByLabelText for better test reliability.
| const infoButtons = screen.getAllByRole("button"); | |
| const churchInfoButton = infoButtons.find( | |
| (button) => | |
| button.classList.contains("text-muted-foreground") && button.querySelector("svg"), | |
| ); | |
| expect(churchInfoButton).toBeInTheDocument(); | |
| if (churchInfoButton) { | |
| await user.click(churchInfoButton); | |
| // After clicking, the dialog should be opened | |
| // We can test this by checking if the dialog content appears | |
| // Since the ChurchInfoDialog component will render its content | |
| } | |
| const churchInfoButton = screen.getByTestId("church-info-button"); | |
| expect(churchInfoButton).toBeInTheDocument(); | |
| await user.click(churchInfoButton); | |
| // After clicking, the dialog should be opened | |
| // We can test this by checking if the dialog content appears | |
| // Since the ChurchInfoDialog component will render its content |
| </FormControl> | ||
| <FormMessage /> | ||
| <p className="text-xs text-muted-foreground"> | ||
| {t("name")}: /tenant/{field.value || "example"} |
There was a problem hiding this comment.
The help text shows '{t("name")}:' but should show 'URL:' or similar to indicate this is showing the URL format, not just repeating the field name.
| {t("name")}: /tenant/{field.value || "example"} | |
| URL: /tenant/{field.value || "example"} |
| const watchName = form.watch("name"); | ||
|
|
||
| // Auto-generate slug from name if enabled | ||
| if (autoGenerateSlug) { | ||
| const slugValue = watchName | ||
| .trim() | ||
| .toLowerCase() | ||
| .replace(/\s+/g, "-") | ||
| .replace(/[^a-z0-9-]/g, ""); | ||
|
|
||
| if (slugValue !== form.getValues("slug")) { | ||
| form.setValue("slug", slugValue); | ||
| } | ||
| } |
There was a problem hiding this comment.
Using form.watch() inside the component body will cause re-renders on every keystroke. Consider moving the auto-slug generation logic inside useEffect or using a more efficient pattern.
Purpose
Frontend implementation of tenant metadata UI components with comprehensive form handling, tooltip displays, verification badges, and testing for the multi-tenant chabod church management system.
Core Principles
Goal
Add tenant metadata UI components to display and edit church administrative information with proper tooltips, verification badges, form validation, and comprehensive testing.
Why
What
Frontend implementation of tenant metadata UI with: