Conversation
…ent handling, and improved UI components - Added new API endpoints for service providers including create, update, delete, and fetch operations. - Implemented document upload and retrieval functionality for service providers. - Enhanced the AddEditServicePro component with country and state selection, image cropping, and improved form handling. - Updated ServicePros and ServiceProsDetail components to display loading states and error handling. - Refactored ServiceProCard and ServiceProProfileSection for better user experience and added document display features.
|
Caution Review failedThe pull request is closed. WalkthroughAdds a SERVICE_PROVIDER API group and a new ServiceProviderService (CRUD + documents). Updates ServicePros UI (list, detail, add/edit, card, profile section) to use live backend data, image cropping, country/state/city selectors, document uploads, and loading/error states. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as ServicePros UI (List / Detail / Form)
participant Svc as serviceProviderService (client)
participant API as Backend API
participant DB as Database
rect rgb(230,240,250)
Note over User,UI: List view flow
User->>UI: Navigate to Service Pros list
UI->>Svc: getAll(isActive=true)
Svc->>API: GET /api/service-provider
API->>DB: Query providers
DB-->>API: Records
API-->>Svc: JSON providers
Svc-->>UI: BackendServiceProvider[]
UI->>UI: Transform -> ServiceProCardData
UI-->>User: Render cards
end
rect rgb(240,250,240)
Note over User,UI: Create / Update flow with uploads
User->>UI: Submit Add/Edit form (+profile image/documents)
UI->>UI: Validate, optionally crop image
UI->>Svc: create/update(CreateServiceProviderDto)
Svc->>API: POST/PUT /api/service-provider
API->>DB: Insert/Update provider
DB-->>API: Provider record (id)
API-->>Svc: BackendServiceProvider
Svc-->>UI: Success (provider id)
UI->>Svc: upload documents (per file) using UPLOAD_DOCUMENT
loop per file
Svc->>API: POST /api/service-provider/{id}/documents
API->>DB: Store document metadata
DB-->>API: Acknowledged
API-->>Svc: Stored document
end
Svc-->>UI: All documents uploaded
UI-->>User: Navigate to list / show success
end
rect rgb(250,240,240)
Note over User,UI: Delete flow
User->>UI: Trigger delete on Detail
UI->>Svc: delete(id)
Svc->>API: DELETE /api/service-provider/{id}
API->>DB: Delete/archive
DB-->>API: Acknowledged
API-->>Svc: { message }
Svc-->>UI: Success
UI-->>User: Confirm and navigate away
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/pages/Dashboard/features/Tenants/Tenants.tsx (1)
136-141: Sort button appears empty — likely unintended change.The button retains its
onClickhandler and styling but renders no visible content. This makes the sort functionality inaccessible to users. Was this intentional, or should the label/icon be restored?src/pages/Dashboard/features/ServicePros/components/ServiceProCard.tsx (1)
44-48: Menu actions for "Send connection" and "Delete" are empty stubs.These menu items have
action: () => { }which does nothing when clicked. This could confuse users who expect the actions to work.Would you like me to help implement these handlers or add TODO comments to track this work?
src/pages/Dashboard/features/ServicePros/ServicePros.tsx (1)
182-187: Sort button is empty — same issue as Tenants.tsx.The button has
onClickbut no visible content, making the sort functionality inaccessible.
🧹 Nitpick comments (10)
src/pages/Dashboard/features/ServicePros/ServiceProsDetail.tsx (3)
9-40: Consider extracting shared helper functions to reduce duplication.These helper functions (
getInitials,formatPhoneNumber,formatCategory,formatAddress) are duplicated inServicePros.tsx. Consider extracting them to a shared utility module (e.g.,utils/service-provider.utils.ts) to maintain DRY principles.
82-88: Avoidanytypes for better type safety.Using
anyforserviceProanddocumentsloses the benefits of TypeScript. Consider defining explicit types:🔎 Suggested type improvements
+interface TransformedServicePro { + id: string; + initials: string; + name: string; + phone: string; + email: string; + outstanding: number; + deposits: number; + credits: number; + image?: string; + personalInfo: { /* ... fields */ }; + forwardingAddress: string; +} + -const [servicePro, setServicePro] = useState<any>(null); +const [servicePro, setServicePro] = useState<TransformedServicePro | null>(null); -const [documents, setDocuments] = useState<any[]>([]); +const [documents, setDocuments] = useState<BackendServiceProviderDocument[]>([]);
150-165: Archive handler updates local state without backend confirmation.After calling
serviceProviderService.update(), the code optimistically sets local state without verifying the response reflectsisActive: false. Consider using the returned data to update state:🔎 Suggested improvement
const handleArchive = async () => { if (!id || !servicePro) return; setIsArchiving(true); try { - await serviceProviderService.update(id, { isActive: false }); - setServicePro({ ...servicePro, isActive: false }); + const updated = await serviceProviderService.update(id, { isActive: false }); + // Re-transform the updated data to ensure consistency + setServicePro(transformServiceProvider(updated)); setIsArchiveModalOpen(false); } catch (err) {src/pages/Dashboard/features/ServicePros/components/ServiceProCard.tsx (1)
105-107: Status indicator is hardcoded to green.The green dot always renders regardless of the service provider's actual status. Consider passing an
isActiveprop and conditionally styling:🔎 Suggested approach
interface ServiceProCardProps { id: string | number; initials: string; name: string; phone: string; category: string; bgColor?: string; image?: string; + isActive?: boolean; }-<div className="absolute bottom-0 right-0 w-4 h-4 bg-green-500 border-2 border-white rounded-full"></div> +<div className={`absolute bottom-0 right-0 w-4 h-4 ${isActive !== false ? 'bg-green-500' : 'bg-gray-400'} border-2 border-white rounded-full`}></div>src/pages/Dashboard/features/ServicePros/components/ServiceProProfileSection.tsx (1)
4-7: Type inconsistency:servicePro.idisnumberbut documents usestringIDs.The
BackendServiceProviderinterface from the service layer usesid: string. Consider aligning theservicePro.idtype:servicePro: { - id: number; + id: string; name: string;src/pages/Dashboard/features/ServicePros/ServicePros.tsx (1)
26-47: Duplicate helper functions across files.
getInitials,formatPhoneNumber, andformatCategoryare duplicated inServiceProsDetail.tsx. Extract these to a shared utility module.src/services/service-provider.service.ts (3)
62-104: Consider extracting the error handling logic to reduce duplication.The error parsing pattern is repeated identically in every method. Extract to a helper:
🔎 Suggested refactor
private async handleResponse<T>(response: Response, defaultError: string): Promise<T> { if (!response.ok) { let errorMessage = defaultError; try { const errorData = await response.json(); if (Array.isArray(errorData.message)) { errorMessage = errorData.message.join('. '); } else if (errorData.message) { errorMessage = errorData.message; } else if (errorData.error) { errorMessage = errorData.error; } console.error(`${defaultError}:`, { status: response.status, statusText: response.statusText, errorData }); } catch (parseError) { errorMessage = `${defaultError}: ${response.statusText}`; console.error('Failed to parse error response:', parseError); } throw new Error(errorMessage); } return response.json(); }
114-120: GET requests don't requireContent-Type: application/jsonheader.The
Content-Typeheader is only meaningful for requests with a body. While harmless, removing it for GET requests aligns with HTTP semantics.
325-368: MissinguploadDocumentanddeleteDocumentmethods.The API endpoints for
UPLOAD_DOCUMENTandDELETE_DOCUMENTare defined inapi.config.ts, but corresponding service methods are not implemented. TheAddEditServicePro.tsxcomponent implementsuploadDocumentinline.Would you like me to generate the missing service methods to centralize document operations?
src/pages/Dashboard/features/ServicePros/AddEditServicePro.tsx (1)
127-156: UseEffects that reset child fields may cause unintended state resets.These effects reset
state/citywhenever dependencies change, including on initial mount or whenformDatais set during edit mode fetch. This could clear valid data loaded from the backend.Consider using a ref to track whether it's the initial load:
🔎 Suggested approach
const isInitialMount = useRef(true); useEffect(() => { if (formData.address.country) { setStates(State.getStatesOfCountry(formData.address.country)); // Only reset if not initial mount if (!isInitialMount.current) { setFormData(prev => ({ ...prev, address: { ...prev.address, state: '', city: '' } })); } } else { setStates([]); } }, [formData.address.country]); // Set isInitialMount.current = false after initial data load in fetchServiceProvider
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/config/api.config.tssrc/pages/Dashboard/features/ServicePros/AddEditServicePro.tsxsrc/pages/Dashboard/features/ServicePros/ServicePros.tsxsrc/pages/Dashboard/features/ServicePros/ServiceProsDetail.tsxsrc/pages/Dashboard/features/ServicePros/components/ServiceProCard.tsxsrc/pages/Dashboard/features/ServicePros/components/ServiceProProfileSection.tsxsrc/pages/Dashboard/features/Tenants/Tenants.tsxsrc/services/service-provider.service.ts
🧰 Additional context used
🧬 Code graph analysis (5)
src/pages/Dashboard/features/ServicePros/components/ServiceProCard.tsx (1)
src/pages/basewebsite/resources/sections/resourceIcons.tsx (1)
Phone(465-486)
src/pages/Dashboard/features/ServicePros/ServiceProsDetail.tsx (1)
src/services/service-provider.service.ts (2)
BackendServiceProvider(36-60)serviceProviderService(368-368)
src/pages/Dashboard/features/ServicePros/ServicePros.tsx (1)
src/services/service-provider.service.ts (2)
serviceProviderService(368-368)BackendServiceProvider(36-60)
src/services/service-provider.service.ts (1)
src/config/api.config.ts (1)
API_ENDPOINTS(4-107)
src/pages/Dashboard/features/ServicePros/AddEditServicePro.tsx (2)
src/services/service-provider.service.ts (2)
serviceProviderService(368-368)CreateServiceProviderDto(4-23)src/config/api.config.ts (1)
API_ENDPOINTS(4-107)
🔇 Additional comments (8)
src/config/api.config.ts (1)
96-106: LGTM!The new
SERVICE_PROVIDERendpoints follow the established pattern consistently with other entities (TENANT, PROPERTY, etc.). URL structure and parameterization are correct.src/pages/Dashboard/features/ServicePros/ServiceProsDetail.tsx (1)
91-127: Good implementation of data fetching with graceful document error handling.The approach of not failing the entire page when documents fail to load (lines 111-114) is a solid resilience pattern.
src/pages/Dashboard/features/ServicePros/components/ServiceProProfileSection.tsx (2)
114-121: Thedownloadattribute may not trigger downloads for cross-origin URLs.If
doc.fileUrlpoints to an external domain (e.g., S3), browsers may ignore thedownloadattribute due to CORS restrictions. Consider adding a backend proxy endpoint or informing users via tooltip.
76-133: Good implementation of conditional rendering for document states.The loading spinner, document grid, and empty state placeholder are well-structured. The document cards have clean layout with accessible actions.
src/pages/Dashboard/features/ServicePros/ServicePros.tsx (1)
49-78: Good data fetching implementation with proper error handling.The fetch logic properly handles loading states, transforms data, and provides meaningful error messages.
src/pages/Dashboard/features/ServicePros/AddEditServicePro.tsx (3)
490-506: Good form validation with clear required field checking.The validation covers all essential fields and scrolls to top on error for visibility.
1174-1185: Good implementation of image crop modal integration.The modal is conditionally rendered with proper props and handlers for cancel/complete flows.
240-306: No action needed—the dependency is correctly memoized.The
phoneCountryCodesvariable is wrapped inuseMemowith an empty dependency array[], which means it returns the same reference on every render. Including it in theuseEffectdependency array is the correct pattern and will not cause unnecessary re-fetches. The memoization effectively prevents the stale closure concern.Likely an incorrect or invalid review comment.
…ponent - Introduced rollback mechanism for new service providers if document upload fails. - Improved error messaging to inform users about the status of service provider creation and document upload. - Added logic to handle cleanup of service provider in case of upload failure, ensuring better user experience.
1 participant
…ent handling, and improved UI components
Summary by CodeRabbit
New Features
UI/UX Improvements
✏️ Tip: You can customize this high-level summary in your review settings.