feat: Fiqh compliance alignment, asset amount history, and sync/auth bug fixes#295
feat: Fiqh compliance alignment, asset amount history, and sync/auth bug fixes#295
Conversation
… database error on re-login - The closeDb() function was fetching the DB but never calling destroy() - This left the DB instance in RxDB's internal registry, causing DB8 errors on re-login - Now properly destroys the instance with safety checks before nulling the promise - Preserves the 200ms delay needed for RxDB registry cleanup - Fixes logout/re-login flow in authentication
…idance, and knowledge hub FAQs
There was a problem hiding this comment.
Pull request overview
This PR aims to (1) align Zakat/Hawl behavior with updated fiqh guidance, (2) introduce an asset amount history ledger (events + snapshots + API + UI), and (3) fix several sync/auth-related bugs across client/server.
Changes:
- Added deductible-liability support (schema + form + wealth calculator) and adjusted Hawl interruption behavior.
- Introduced asset amount event/snapshot services, API routes, client UI components, and unit tests for the new history feature.
- Fixed migration URL double-prefixing, improved RxDB shutdown to prevent DB8 errors, and added a CouchDB “system DB init” guard.
Reviewed changes
Copilot reviewed 29 out of 32 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/src/types/assetAmountEvent.ts | Adds shared TypeScript types for asset amount history APIs. |
| server/src/services/AssetAmountEventService.ts | Implements event creation/querying/backport + audit integration + snapshot regeneration trigger. |
| server/src/services/AssetAmountSnapshotService.ts | Implements snapshot generation, querying, and gap-filling. |
| server/src/routes/asset-amount-events.ts | Adds protected endpoints for history queries, amount-at-date, backport, manual event creation, and reversal. |
| server/src/services/AssetService.ts | Emits CREATED/UPDATED amount events on asset create/update. |
| server/src/services/hawlTrackingService.ts | Adjusts Hawl interruption logic to only break at “absolute zero”. |
| server/src/services/SyncService.ts | Adds ensureCouchDbInitialized() to avoid fresh-install CouchDB system DB errors. |
| server/prisma/migrations/add_asset_amount_history.sql | Adds raw SQL migration for event/snapshot tables + audit column. |
| server/prisma/schema.prisma | Adds deductibleAmount to Liability. |
| client/src/components/AssetAmountHistory.tsx | Displays per-asset event history (with bearer auth header). |
| client/src/components/BackportData.tsx | Adds CSV backport UI for historical entries. |
| client/src/components/liabilities/LiabilityForm.tsx | Adds deductible amount input and wiring. |
| client/src/core/calculations/wealthCalculator.ts | Uses deductibleAmount override when present. |
| client/src/db/schema/liability.schema.ts | Updates RxDB liability schema (currently problematic per comments). |
| client/src/hooks/useMigration.ts | Fixes /api/api/... double-prefix bug. |
| server/tests/unit/services/*.test.ts | Adds unit tests for the new event/snapshot services. |
| server/prisma/test/test.db-shm | Adds a generated DB artifact (should not be committed). |
Files not reviewed (1)
- server/package-lock.json: Language not supported
| const response = await fetch(`${apiBaseUrl}/assets/${assetId}/backport`, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json' | ||
| }, |
There was a problem hiding this comment.
This fetch() call does not include an Authorization header, but the corresponding server route is protected by authenticate. As-is, imports will 401 for logged-in users. Add the bearer token header (same pattern used in AssetAmountHistory.tsx / apiService) or use the existing apiService abstraction.
| const response = await fetch(`${apiBaseUrl}/assets/${assetId}/backport`, { | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'application/json' | |
| }, | |
| const token = | |
| typeof window !== 'undefined' | |
| ? window.localStorage.getItem('accessToken') || window.localStorage.getItem('token') | |
| : null; | |
| const headers: HeadersInit = { | |
| 'Content-Type': 'application/json' | |
| }; | |
| if (token) { | |
| headers['Authorization'] = `Bearer ${token}`; | |
| } | |
| const response = await fetch(`${apiBaseUrl}/assets/${assetId}/backport`, { | |
| method: 'POST', | |
| headers, |
| ...(options?.startDate && { effectiveDate: { gte: options.startDate } }), | ||
| ...(options?.endDate && { effectiveDate: { lte: options.endDate } }), |
There was a problem hiding this comment.
When both startDate and endDate are provided, the where clause spreads two separate effectiveDate objects, so the latter overrides the former and one bound is dropped. Combine them into a single effectiveDate: { gte, lte } object to ensure both filters apply.
| ...(options?.startDate && { effectiveDate: { gte: options.startDate } }), | |
| ...(options?.endDate && { effectiveDate: { lte: options.endDate } }), | |
| ...(options?.startDate || options?.endDate | |
| ? { | |
| effectiveDate: { | |
| ...(options?.startDate && { gte: options.startDate }), | |
| ...(options?.endDate && { lte: options.endDate }) | |
| } | |
| } | |
| : {}), |
| // NEW: Create initial asset amount event | ||
| try { | ||
| const eventService = new AssetAmountEventService(); | ||
| await eventService.createEvent(userId, { | ||
| assetId: asset.id, | ||
| eventType: 'CREATED', | ||
| amount: assetData.value, | ||
| effectiveDate: assetData.acquisitionDate, | ||
| description: 'Asset created', | ||
| source: 'manual' | ||
| }); | ||
| } catch (err) { | ||
| this.logger.error('Failed to create initial asset amount event', err); | ||
| // Don't throw - asset creation should succeed even if event creation fails | ||
| } |
There was a problem hiding this comment.
Asset creation and initial amount-event creation are not atomic: the asset is created first, then the event is attempted and any failure is swallowed. This can permanently leave assets without a corresponding CREATED event (and snapshots/audit trail incomplete). Consider creating the asset + initial event in a single transaction, or at least persisting a retry/reconciliation mechanism instead of silently continuing.
| if (!eventType || !amount || !effectiveDate) { | ||
| throw new Error('eventType, amount, and effectiveDate are required'); | ||
| } |
There was a problem hiding this comment.
POST /:assetId/events checks if (!eventType || !amount || !effectiveDate), which rejects valid amount: 0. Use amount === undefined/amount === null (and Number.isFinite) rather than a falsy check.
| return prisma.assetAmountEvent.findMany({ | ||
| where: { | ||
| assetId: options?.assetId || { in: assetIds }, | ||
| isReversed: false, | ||
| ...(options?.startDate && { effectiveDate: { gte: options.startDate } }), | ||
| ...(options?.endDate && { effectiveDate: { lte: options.endDate } }) |
There was a problem hiding this comment.
Same date-filter bug as getAssetHistory: spreading { effectiveDate: { gte } } and { effectiveDate: { lte } } means one will overwrite the other if both options are set. Build a single effectiveDate filter containing both bounds.
| return prisma.assetAmountEvent.findMany({ | |
| where: { | |
| assetId: options?.assetId || { in: assetIds }, | |
| isReversed: false, | |
| ...(options?.startDate && { effectiveDate: { gte: options.startDate } }), | |
| ...(options?.endDate && { effectiveDate: { lte: options.endDate } }) | |
| const effectiveDateFilter: { gte?: Date; lte?: Date } = {}; | |
| if (options?.startDate) { | |
| effectiveDateFilter.gte = options.startDate; | |
| } | |
| if (options?.endDate) { | |
| effectiveDateFilter.lte = options.endDate; | |
| } | |
| return prisma.assetAmountEvent.findMany({ | |
| where: { | |
| assetId: options?.assetId || { in: assetIds }, | |
| isReversed: false, | |
| ...(Object.keys(effectiveDateFilter).length && { effectiveDate: effectiveDateFilter }) |
| try { | ||
| const eventService = new AssetAmountEventService(); | ||
| await eventService.createEvent(userId, { | ||
| assetId: assetId, | ||
| eventType: 'UPDATED', | ||
| amount: updateData.value, | ||
| effectiveDate: new Date(), | ||
| description: 'Asset value updated', | ||
| source: 'manual' | ||
| }); | ||
| } catch (err) { | ||
| this.logger.error('Failed to create asset amount event', err); | ||
| // Don't throw - asset update should succeed even if event creation fails | ||
| } |
There was a problem hiding this comment.
Asset updates write Asset.value first and then attempt to append an amount event, swallowing failures. This can desync current state vs. history (and snapshot regeneration) with no way to detect/fix it later. Prefer updating the asset and creating the UPDATED event in the same DB transaction, or record a durable “event creation failed” signal for later reconciliation.
| try { | |
| const eventService = new AssetAmountEventService(); | |
| await eventService.createEvent(userId, { | |
| assetId: assetId, | |
| eventType: 'UPDATED', | |
| amount: updateData.value, | |
| effectiveDate: new Date(), | |
| description: 'Asset value updated', | |
| source: 'manual' | |
| }); | |
| } catch (err) { | |
| this.logger.error('Failed to create asset amount event', err); | |
| // Don't throw - asset update should succeed even if event creation fails | |
| } | |
| const eventService = new AssetAmountEventService(); | |
| await eventService.createEvent(userId, { | |
| assetId: assetId, | |
| eventType: 'UPDATED', | |
| amount: updateData.value, | |
| effectiveDate: new Date(), | |
| description: 'Asset value updated', | |
| source: 'manual' | |
| }); |
| const eventService = new AssetAmountEventService(); | ||
| const events = await eventService.backportHistoricalData(userId, assetId, entries); | ||
|
|
There was a problem hiding this comment.
backportHistoricalData() in the service expects effectiveDate: Date, but the route forwards entries from the request body without parsing effectiveDate. Convert/validate each effectiveDate to a Date (and reject invalid dates) before passing to the service to avoid storing invalid values or relying on implicit coercion.
| CREATE TABLE IF NOT EXISTS "asset_amount_events" ( | ||
| "id" TEXT NOT NULL PRIMARY KEY DEFAULT (uuid()), | ||
| "assetId" TEXT NOT NULL, | ||
| "eventType" TEXT NOT NULL, | ||
| "amount" REAL NOT NULL, | ||
| "currency" TEXT NOT NULL DEFAULT 'USD', | ||
| "effectiveDate" TEXT NOT NULL, | ||
| "recordedAt" TEXT NOT NULL DEFAULT (datetime('now')), |
There was a problem hiding this comment.
This migration targets SQLite (per Prisma datasource), but uses DEFAULT (uuid()) for primary keys. SQLite doesn't provide a built-in uuid() function, so these CREATE TABLE statements will fail on a fresh install. Use Prisma-generated migrations / application-side UUID generation, or switch to a SQLite-compatible default (or remove the default and always supply IDs).
| // Get all events in the date range | ||
| const events = await prisma.assetAmountEvent.findMany({ | ||
| where: { | ||
| assetId, | ||
| effectiveDate: { gte: range.startDate, lte: range.endDate }, | ||
| isReversed: false | ||
| }, | ||
| orderBy: { effectiveDate: 'asc' } | ||
| }); | ||
|
|
||
| if (events.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| // Group events by date (start of day) | ||
| const eventsByDate = new Map<string, typeof events>(); | ||
| for (const event of events) { | ||
| const dateKey = this.getStartOfDay(event.effectiveDate).toISOString(); | ||
| if (!eventsByDate.has(dateKey)) { | ||
| eventsByDate.set(dateKey, []); | ||
| } | ||
| eventsByDate.get(dateKey)!.push(event); | ||
| } | ||
|
|
||
| // For each date, create/update snapshot with latest event | ||
| for (const [dateStr, dayEvents] of eventsByDate) { | ||
| // Get the latest event for this date (by recordedAt) | ||
| const latestEvent = dayEvents.sort( | ||
| (a, b) => new Date(b.recordedAt).getTime() - new Date(a.recordedAt).getTime() | ||
| )[0]; | ||
|
|
||
| const date = new Date(dateStr); | ||
| const eventCount = dayEvents.length; | ||
|
|
||
| // Upsert snapshot | ||
| await prisma.assetAmountSnapshot.upsert({ | ||
| where: { | ||
| assetId_date: { | ||
| assetId, | ||
| date | ||
| } | ||
| }, | ||
| update: { | ||
| amount: latestEvent.amount, | ||
| eventCount | ||
| }, | ||
| create: { | ||
| assetId, | ||
| date, | ||
| amount: latestEvent.amount, | ||
| eventCount | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| this.logger.info(`Regenerated ${eventsByDate.size} snapshots for asset ${assetId}`); |
There was a problem hiding this comment.
This service calls prisma.assetAmountEvent / prisma.assetAmountSnapshot, but the Prisma schema in this repo does not define those models. Without adding them to schema.prisma (and regenerating Prisma client), this code will fail to compile and the endpoints will not work.
| // Get all events in the date range | |
| const events = await prisma.assetAmountEvent.findMany({ | |
| where: { | |
| assetId, | |
| effectiveDate: { gte: range.startDate, lte: range.endDate }, | |
| isReversed: false | |
| }, | |
| orderBy: { effectiveDate: 'asc' } | |
| }); | |
| if (events.length === 0) { | |
| return; | |
| } | |
| // Group events by date (start of day) | |
| const eventsByDate = new Map<string, typeof events>(); | |
| for (const event of events) { | |
| const dateKey = this.getStartOfDay(event.effectiveDate).toISOString(); | |
| if (!eventsByDate.has(dateKey)) { | |
| eventsByDate.set(dateKey, []); | |
| } | |
| eventsByDate.get(dateKey)!.push(event); | |
| } | |
| // For each date, create/update snapshot with latest event | |
| for (const [dateStr, dayEvents] of eventsByDate) { | |
| // Get the latest event for this date (by recordedAt) | |
| const latestEvent = dayEvents.sort( | |
| (a, b) => new Date(b.recordedAt).getTime() - new Date(a.recordedAt).getTime() | |
| )[0]; | |
| const date = new Date(dateStr); | |
| const eventCount = dayEvents.length; | |
| // Upsert snapshot | |
| await prisma.assetAmountSnapshot.upsert({ | |
| where: { | |
| assetId_date: { | |
| assetId, | |
| date | |
| } | |
| }, | |
| update: { | |
| amount: latestEvent.amount, | |
| eventCount | |
| }, | |
| create: { | |
| assetId, | |
| date, | |
| amount: latestEvent.amount, | |
| eventCount | |
| } | |
| }); | |
| } | |
| this.logger.info(`Regenerated ${eventsByDate.size} snapshots for asset ${assetId}`); | |
| // This service requires Prisma models `assetAmountEvent` and `assetAmountSnapshot`, | |
| // which are not defined in the current Prisma schema. Until those models are added | |
| // and the Prisma client is regenerated, snapshot regeneration cannot be performed. | |
| const message = | |
| 'AssetAmountSnapshotService.regenerateForDateRange is not available: ' + | |
| 'missing Prisma models `assetAmountEvent` and/or `assetAmountSnapshot` in schema.prisma.'; | |
| this.logger.error(message, { assetId, range }); | |
| throw new Error(message); |
| // Iterate through each day in the range | ||
| const currentDate = new Date(range.startDate); | ||
| while (currentDate <= range.endDate) { | ||
| const dateKey = currentDate.toISOString(); | ||
|
|
||
| // If we don't have a snapshot for this date | ||
| if (!existingDates.has(dateKey)) { | ||
| // Check if we have events for this date | ||
| const dayEvents = eventsByDate.get(dateKey); | ||
|
|
There was a problem hiding this comment.
fillMissingSnapshots() groups events by getStartOfDay(...).toISOString(), but later looks them up with currentDate.toISOString() (which includes the time component from range.startDate). This makes eventsByDate.get(dateKey) miss events and existingDates.has(dateKey) miss snapshots unless the range starts exactly at midnight UTC. Normalize currentDate to start-of-day (and use that consistently for keys) before lookups/creates.
Summary
This PR delivers two major bodies of work:
1. Fiqh Compliance — Aligns with Shaykh Husain Abdul Sattar's scholarship
liabilitiesfield to schema,LiabilityFormcomponent, and wire-up inwealthCalculator.tsso deductible debts reduce nisab-threshold calculation per Hanafi fiqhhawlTrackingService.tsnow correctly handles interruptions to the lunar year tracking cycleJewelryGuidance.tsxnow displays a scholarly warning that precious stones (diamonds, rubies, etc.) have no Zakat, only the gold/silver weight itself2. Asset Amount History Feature
AssetAmountHistorycomponent for displaying per-asset change historyAssetAmountEventServiceandAssetAmountSnapshotServicefor server-side event sourcingGET /api/assets/:assetId/history(protected)add_asset_amount_history.sqlshared/src/types/assetAmountEvent.ts3. Sync / Auth Bug Fixes
/api/api/user/encryption-status404) — FixeduseMigration.ts_users404 on fresh install) —SyncService.ensureCouchDbInitialized()addedAssetAmountHistory.tsxnow sendsAuthorization: BearerheadercloseDb()now properly destroys the RxDB instanceVerification
.env.devNOT committedPre-existing Issues (not introduced here)
prisma/schema.prismais missing theassetAmountEventmodel — the SQL migration exists but was never integrated into Prisma's schema