feat:Implemented the escrow status system #150
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update introduces an escrow feature within the web application. A new page now conditionally displays either an escrow table for development mode or a "Feature Not Available" message in production. The escrow table component manages record display and state using Supabase for data operations, while a custom hook handles real-time escrow status updates. Additionally, configuration changes allow toggling the escrow feature based on environment variables, and exports have been streamlined for easier consumption. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TestPage
participant FeatureNotAvailable
participant EscrowTable
User->>TestPage: Access escrow feature page
TestPage->>TestPage: Check NODE_ENV
alt NODE_ENV == "production"
TestPage->>FeatureNotAvailable: Render "feature not available" message
else NODE_ENV != "production"
TestPage->>EscrowTable: Render escrow table component
end
sequenceDiagram
participant Component
participant useEscrow
participant Supabase
Component->>useEscrow: Invoke hook with escrowId
useEscrow->>Supabase: Fetch escrow record
Supabase-->>useEscrow: Return record data
useEscrow->>Component: Provide escrow data and update functions
Suggested labels
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
coderabbitai
bot
added
documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (9)
apps/web/hooks/escrow/useEscrow.ts (1)
39-41: Consider clearly communicating 'CANCELLED' status instead of returning null.
We could enhance this by returning an explicit state for 'CANCELLED', which would keep the status object consistent and avoid potential null-pointer issues elsewhere in the code.apps/web/app/test/escrow/page.tsx (2)
3-10: Nice fallback component for production environments.
We could enhance this by adding a link or guidance to let users know whom to contact or what to do if they attempt to access this feature in production.
12-22: Consider using the config-based feature toggle.
We could enhance this by usingappConfig.features.enableEscrowFeatureinstead of checkingprocess.env.NODE_ENVdirectly, unifying all feature toggles in one place.apps/web/components/EscrowTable/EscrowTable.tsx (6)
8-10: Consider deriving EscrowStatusType from database types.The
EscrowStatusTypecould be derived from the database types to ensure type consistency and reduce maintenance overhead.-type EscrowStatusType = 'NEW' | 'FUNDED' | 'ACTIVE' | 'COMPLETED' | 'DISPUTED' | 'CANCELLED' +type EscrowStatusType = Tables['escrow_status']['Row']['status']
12-19: Enhance accessibility with ARIA attributes.The component could benefit from improved accessibility for screen readers.
export const FeatureNotAvailable = () => ( - <div className="container mx-auto p-4"> + <div className="container mx-auto p-4" role="alert" aria-label="Feature availability notice"> <div className="bg-white rounded-lg shadow-lg p-6"> - <h1 className="text-2xl font-bold mb-4">Feature Not Available</h1> + <h1 className="text-2xl font-bold mb-4" tabIndex={0}>Feature Not Available</h1> <p>This feature is only available in development mode.</p> </div> </div>
31-38: Consider moving status colors to a constants file.The status colors mapping could be moved to a shared constants file for better maintainability and reusability across components.
Consider creating a new file
constants/escrow.ts:export const ESCROW_STATUS_COLORS: Record<EscrowStatusType, string> = { NEW: 'bg-gray-100', FUNDED: 'bg-blue-100', ACTIVE: 'bg-green-100', COMPLETED: 'bg-purple-100', DISPUTED: 'bg-red-100', CANCELLED: 'bg-yellow-100', } as const;
79-84: Use template literals for error messages.Consider using template literals for better readability and maintainability.
- alert( - 'Error updating status: ' + - (err instanceof Error ? err.message : 'Unknown error'), - ) + alert(`Error updating status: ${err instanceof Error ? err.message : 'Unknown error'}`)🧰 Tools
🪛 Biome (1.9.4)
[error] 81-82: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
173-184: Enhance table accessibility.Consider adding ARIA attributes and semantic table elements for better screen reader support.
-<table className="min-w-full bg-white border"> +<table + className="min-w-full bg-white border" + role="grid" + aria-label="Escrow records" +> <thead> - <tr> + <tr role="row"> - <th className="border p-2">ID</th> + <th className="border p-2" role="columnheader" scope="col">ID</th> // Apply similar changes to other th elements </tr> </thead>
217-226: Use semantic HTML for status legend.Consider using a more semantic structure for the status legend section.
-<div className="mt-4"> +<section className="mt-4" aria-labelledby="status-legend"> - <h3 className="font-medium mb-2">Status Legend:</h3> + <h3 id="status-legend" className="font-medium mb-2">Status Legend:</h3> <div className="grid grid-cols-3 gap-2"> {Object.entries(statusColors).map(([status, color]) => ( <div key={status} + role="status" className={`${color} py-2 rounded text-[10px] text-center md:text-[20px]`} > {status} </div> ))} </div> -</div> +</section>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/app/test/escrow/page.tsx(1 hunks)apps/web/components/EscrowTable/EscrowTable.tsx(1 hunks)apps/web/components/EscrowTable/index.ts(1 hunks)apps/web/hooks/escrow/useEscrow.ts(1 hunks)apps/web/lib/config/appConfig.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/components/EscrowTable/index.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.ts`: Review TypeScript files ensuring type safety with...
**/*.ts: Review TypeScript files ensuring type safety with no 'any' usage unless justified. Verify named exports, proper error handling with typed errors, and pure functions. Check module encapsulation and consistent naming using camelCase for functions and PascalCase for types. Validate unit test coverage for utilities if required.
apps/web/lib/config/appConfig.tsapps/web/hooks/escrow/useEscrow.ts
🪛 Biome (1.9.4)
apps/web/hooks/escrow/useEscrow.ts
[error] 91-91: This hook does not specify all of its dependencies: fetchEscrowStatus
This dependency is not specified in the hook dependency list.
(lint/correctness/useExhaustiveDependencies)
[error] 91-91: This hook does not specify all of its dependencies: supabase
.channel
This dependency is not specified in the hook dependency list.
(lint/correctness/useExhaustiveDependencies)
[error] 91-91: This hook does not specify all of its dependencies: transformRecord
This dependency is not specified in the hook dependency list.
(lint/correctness/useExhaustiveDependencies)
apps/web/components/EscrowTable/EscrowTable.tsx
[error] 81-82: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 95-95: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 115-116: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 154-157: 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] 161-164: 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)
🔇 Additional comments (1)
apps/web/lib/config/appConfig.ts (1)
1-5: Configuration approach looks good.
We could enhance this by normalizing environment variables (for example, ensuring type consistency or adding logs) to confirm the correct feature toggle logic is applied.
apps/web/hooks/escrow/useEscrow.ts
Outdated
| useEffect(() => { | ||
| fetchEscrowStatus() | ||
|
|
||
| const channel = supabase | ||
| .channel(`escrow_status:${escrowId}`) | ||
| .on( | ||
| 'postgres_changes', | ||
| { | ||
| event: '*', | ||
| schema: 'public', | ||
| table: 'escrow_status', | ||
| filter: `escrow_id=eq.${escrowId}`, | ||
| }, | ||
| (payload) => { | ||
| if (payload.new) { | ||
| const newRecord = payload.new as EscrowRecord | ||
| setRawRecord(newRecord) | ||
| const transformed = transformRecord(newRecord) | ||
| if (transformed) setStatus(transformed) | ||
| } | ||
| }, | ||
| ) | ||
| .subscribe() | ||
|
|
||
| return () => { | ||
| channel.unsubscribe() | ||
| } | ||
| }, [escrowId]) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure the effect’s dependencies are comprehensive for stable subscriptions.
We could enhance this by adding necessary references (e.g., fetchEscrowStatus, transformRecord, supabase) to the dependency array or by memoizing these references. This prevents unintentional re-subscriptions or missed updates.
Suggested fix:
useEffect(() => {
fetchEscrowStatus()
const channel = supabase
.channel(`escrow_status:${escrowId}`)
...
return () => {
channel.unsubscribe()
}
- }, [escrowId])
+ }, [escrowId, fetchEscrowStatus, supabase, transformRecord])📝 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.
| useEffect(() => { | |
| fetchEscrowStatus() | |
| const channel = supabase | |
| .channel(`escrow_status:${escrowId}`) | |
| .on( | |
| 'postgres_changes', | |
| { | |
| event: '*', | |
| schema: 'public', | |
| table: 'escrow_status', | |
| filter: `escrow_id=eq.${escrowId}`, | |
| }, | |
| (payload) => { | |
| if (payload.new) { | |
| const newRecord = payload.new as EscrowRecord | |
| setRawRecord(newRecord) | |
| const transformed = transformRecord(newRecord) | |
| if (transformed) setStatus(transformed) | |
| } | |
| }, | |
| ) | |
| .subscribe() | |
| return () => { | |
| channel.unsubscribe() | |
| } | |
| }, [escrowId]) | |
| useEffect(() => { | |
| fetchEscrowStatus() | |
| const channel = supabase | |
| .channel(`escrow_status:${escrowId}`) | |
| .on( | |
| 'postgres_changes', | |
| { | |
| event: '*', | |
| schema: 'public', | |
| table: 'escrow_status', | |
| filter: `escrow_id=eq.${escrowId}`, | |
| }, | |
| (payload) => { | |
| if (payload.new) { | |
| const newRecord = payload.new as EscrowRecord | |
| setRawRecord(newRecord) | |
| const transformed = transformRecord(newRecord) | |
| if (transformed) setStatus(transformed) | |
| } | |
| }, | |
| ) | |
| .subscribe() | |
| return () => { | |
| channel.unsubscribe() | |
| } | |
| }, [escrowId, fetchEscrowStatus, supabase, transformRecord]) |
🧰 Tools
🪛 Biome (1.9.4)
[error] 91-91: This hook does not specify all of its dependencies: fetchEscrowStatus
This dependency is not specified in the hook dependency list.
(lint/correctness/useExhaustiveDependencies)
[error] 91-91: This hook does not specify all of its dependencies: supabase
.channel
This dependency is not specified in the hook dependency list.
(lint/correctness/useExhaustiveDependencies)
[error] 91-91: This hook does not specify all of its dependencies: transformRecord
This dependency is not specified in the hook dependency list.
(lint/correctness/useExhaustiveDependencies)
| const isFeatureEnabled = useMemo(() => | ||
| appConfig.features.enableEscrowFeature, | ||
| [] | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding appConfig to useMemo dependencies.
The useMemo hook for isFeatureEnabled should include appConfig in its dependency array to ensure proper reactivity.
const isFeatureEnabled = useMemo(() =>
appConfig.features.enableEscrowFeature,
- []
+ [appConfig.features.enableEscrowFeature]
);📝 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.
| const isFeatureEnabled = useMemo(() => | |
| appConfig.features.enableEscrowFeature, | |
| [] | |
| ); | |
| const isFeatureEnabled = useMemo(() => | |
| appConfig.features.enableEscrowFeature, | |
| [appConfig.features.enableEscrowFeature] | |
| ); |
| <button | ||
| onClick={insertTestData} | ||
| className="bg-blue-500 text-white px-4 py-2 rounded hover:bg-blue-600" | ||
| > | ||
| Insert Test Data | ||
| </button> | ||
|
|
||
| <button | ||
| onClick={fetchRecords} | ||
| className="bg-green-500 text-white px-4 py-2 rounded hover:bg-green-600" | ||
| > | ||
| Refresh Records | ||
| </button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add type attributes to buttons.
Buttons should have explicit type attributes to prevent unintended form submissions.
<button
+ type="button"
onClick={insertTestData}
className="bg-blue-500 text-white px-4 py-2 rounded hover:bg-blue-600"
>
Insert Test Data
</button>
<button
+ type="button"
onClick={fetchRecords}
className="bg-green-500 text-white px-4 py-2 rounded hover:bg-green-600"
>
Refresh Records
</button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| onClick={insertTestData} | |
| className="bg-blue-500 text-white px-4 py-2 rounded hover:bg-blue-600" | |
| > | |
| Insert Test Data | |
| </button> | |
| <button | |
| onClick={fetchRecords} | |
| className="bg-green-500 text-white px-4 py-2 rounded hover:bg-green-600" | |
| > | |
| Refresh Records | |
| </button> | |
| <button | |
| type="button" | |
| onClick={insertTestData} | |
| className="bg-blue-500 text-white px-4 py-2 rounded hover:bg-blue-600" | |
| > | |
| Insert Test Data | |
| </button> | |
| <button | |
| type="button" | |
| onClick={fetchRecords} | |
| className="bg-green-500 text-white px-4 py-2 rounded hover:bg-green-600" | |
| > | |
| Refresh Records | |
| </button> |
🧰 Tools
🪛 Biome (1.9.4)
[error] 154-157: 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] 161-164: 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)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web/hooks/escrow/useEscrow.ts (1)
39-62: Consider using type guards for safer type casting.We could enhance the type safety of metadata handling by using type guards instead of type assertion.
- const metadata = typeof record.metadata === 'object' && record.metadata - ? record.metadata as { milestoneStatus?: { total: number; completed: number } } - : { milestoneStatus: { total: 0, completed: 0 } } + const isValidMetadata = (data: unknown): data is { milestoneStatus: { total: number; completed: number } } => { + return typeof data === 'object' && data !== null && + 'milestoneStatus' in data && + typeof (data as any).milestoneStatus?.total === 'number' && + typeof (data as any).milestoneStatus?.completed === 'number'; + } + const metadata = isValidMetadata(record.metadata) + ? record.metadata + : { milestoneStatus: { total: 0, completed: 0 } }apps/web/components/EscrowTable/EscrowTable.tsx (1)
49-126: Consider using template literals for string concatenation.We could enhance string readability by using template literals instead of concatenation.
- "Error updating status: " + - (err instanceof Error ? err.message : "Unknown error") + `Error updating status: ${err instanceof Error ? err.message : "Unknown error"}` - escrow_id: "test-" + Date.now(), + escrow_id: `test-${Date.now()}`, - "Error inserting test data: " + - (err instanceof Error ? err.message : "Unknown error") + `Error inserting test data: ${err instanceof Error ? err.message : "Unknown error"}`🧰 Tools
🪛 Biome (1.9.4)
[error] 90-91: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 102-102: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 122-123: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/components/EscrowTable/EscrowTable.tsx(1 hunks)apps/web/hooks/escrow/useEscrow.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.ts`: Review TypeScript files ensuring type safety with...
**/*.ts: Review TypeScript files ensuring type safety with no 'any' usage unless justified. Verify named exports, proper error handling with typed errors, and pure functions. Check module encapsulation and consistent naming using camelCase for functions and PascalCase for types. Validate unit test coverage for utilities if required.
apps/web/hooks/escrow/useEscrow.ts
🪛 Biome (1.9.4)
apps/web/hooks/escrow/useEscrow.ts
[error] 118-118: fetchEscrowStatus changes on every re-render and should not be used as a hook dependency.
To fix this, wrap the definition of fetchEscrowStatus in its own useCallback() hook.
(lint/correctness/useExhaustiveDependencies)
[error] 118-118: transformRecord changes on every re-render and should not be used as a hook dependency.
To fix this, wrap the definition of transformRecord in its own useCallback() hook.
(lint/correctness/useExhaustiveDependencies)
apps/web/components/EscrowTable/EscrowTable.tsx
[error] 90-91: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 102-102: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 122-123: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 32-32: This hook specifies more dependencies than necessary: appConfig.features.enableEscrowFeature
This dependency can be removed from the list.
(lint/correctness/useExhaustiveDependencies)
🔇 Additional comments (5)
apps/web/hooks/escrow/useEscrow.ts (2)
1-38: Well-structured type definitions and state management!The type definitions are comprehensive and the state management follows React best practices. The use of TypeScript's strict typing enhances code reliability.
120-200: Excellent implementation of update functions!The update functions are well-implemented with:
- Comprehensive error handling
- Consistent patterns across functions
- Proper validation checks
apps/web/components/EscrowTable/EscrowTable.tsx (3)
1-48: Great implementation of feature toggle and status colors!The component setup demonstrates good practices with:
- Proper feature toggle implementation
- Efficient memoization of static values
🧰 Tools
🪛 Biome (1.9.4)
[error] 32-32: This hook specifies more dependencies than necessary: appConfig.features.enableEscrowFeature
This dependency can be removed from the list.
(lint/correctness/useExhaustiveDependencies)
179-228: Well-structured and accessible table implementation!The table implementation follows best practices with:
- Semantic HTML structure
- Proper handling of status updates
- Clear visual feedback through status colors
229-242: Excellent status legend implementation!The status legend enhances user experience with:
- Clear visual mapping of statuses
- Responsive design for different screen sizes
| useEffect(() => { | ||
| fetchEscrowStatus() | ||
|
|
||
| const channel = supabase | ||
| .channel(`escrow_status:${escrowId}`) | ||
| .on( | ||
| 'postgres_changes', | ||
| { | ||
| event: '*', | ||
| schema: 'public', | ||
| table: 'escrow_status', | ||
| filter: `escrow_id=eq.${escrowId}`, | ||
| }, | ||
| (payload) => { | ||
| if (payload.new) { | ||
| const newRecord = payload.new as EscrowRecord | ||
| setRawRecord(newRecord) | ||
| const transformed = transformRecord(newRecord) | ||
| if (transformed) setStatus(transformed) | ||
| } | ||
| }, | ||
| ) | ||
| .subscribe() | ||
|
|
||
| return () => { | ||
| channel.unsubscribe() | ||
| } | ||
| }, [escrowId, fetchEscrowStatus, supabase, transformRecord]) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Memoize dependencies to prevent unnecessary re-renders.
The dependencies fetchEscrowStatus and transformRecord should be memoized to prevent unnecessary re-renders and re-subscriptions.
+ const transformRecord = useCallback((record: EscrowRecord): EscrowStatusState | null => {
if (record.status === 'CANCELLED') return null
// ... rest of the function
- }
+ }, [])
+ const fetchEscrowStatus = useCallback(async () => {
try {
// ... rest of the function
} catch (err) {
// ... error handling
}
- }
+ }, [supabase, transformRecord])🧰 Tools
🪛 Biome (1.9.4)
[error] 118-118: fetchEscrowStatus changes on every re-render and should not be used as a hook dependency.
To fix this, wrap the definition of fetchEscrowStatus in its own useCallback() hook.
(lint/correctness/useExhaustiveDependencies)
[error] 118-118: transformRecord changes on every re-render and should not be used as a hook dependency.
To fix this, wrap the definition of transformRecord in its own useCallback() hook.
(lint/correctness/useExhaustiveDependencies)
AndlerRL
added
supabase
|
Hey @davedumto I see that on your last commit f678c2b you have characters that are not part of the Biome.js configuration. Please let us know if you are encountering any issues with the development tools! cc - @Bran18 |
Add Escrow Status Monitoring System
Description
This PR implements a development-only escrow status monitoring system that allows us to track and manage escrow states in real-time. The system includes a tabular interface for viewing and managing escrow records with support for status updates and test data insertion.
Features
Technical Implementation
escrow_statuswith proper indexingDatabase Changes