fix: Payments page with history navigation, month filter, and progress bar#140
fix: Payments page with history navigation, month filter, and progress bar#140
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 25 minutes and 39 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
apps/website/src/lib/remotes/payments.remote.ts (1)
62-75: Prefer a month range overextract()onforMonthD.Wrapping the column in
extract()makes this new hot path much harder to satisfy from an index onforMonthD. A[monthStart, nextMonthStart)predicate will scale better here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/src/lib/remotes/payments.remote.ts` around lines 62 - 75, Replace the extract() predicates on schema.payments.forMonthD with a half-open range on the date column: compute a monthStart (first day of isoMonth/isoYear) and nextMonthStart (first day of the following month) from isoMonth and isoYear, then replace the two eq(sql`extract(...)`) clauses with comparisons gte(schema.payments.forMonthD, monthStart) and lt(schema.payments.forMonthD, nextMonthStart); keep the inArray filter on userHouseholds.map((h) => h.id) unchanged. Ensure monthStart/nextMonthStart are proper Date or ISO date strings matching the column type so the DB can use an index on forMonthD.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/website/src/lib/remotes/payments.remote.ts`:
- Around line 13-17: The optionalISODateValidator currently only checks format
and allows impossible dates (e.g., 2026-13-40) which later cause new
Date(...).toISOString() to throw; update the validator
(optionalISODateValidator) to add a runtime refinement that, for non-undefined
and non-Date string values, constructs a Date and verifies it's valid (e.g., new
Date(value) produces a date with !isNaN(date.getTime())) before accepting, and
similarly ensure any branch that accepts Date objects checks date.getTime() is
not NaN; apply the same refinement to the other validator in this file that uses
the same regex so invalid calendar dates are rejected at validation time rather
than causing a RangeError later.
- Around line 19-40: Mutations togglePayment, uploadImage, markPayment, and
unmarkPayment only invalidate getCurrentPayments() and getPayment(), leaving
getCurrentPaymentsByHousehold(date) cached per-argument and stale; update each
mutation to also refresh the grouped query by either accepting the date argument
and calling the remote refresh for getCurrentPaymentsByHousehold(date) after
success, or (if you can't pass date) call your cache/remote invalidation that
targets all keys for getCurrentPaymentsByHousehold so every date-argument entry
is invalidated; locate these changes in the mutation handlers for togglePayment,
uploadImage, markPayment, and unmarkPayment and ensure they reference
getCurrentPaymentsByHousehold when triggering refreshes.
In `@apps/website/src/routes/dashboard/payments/`+page.svelte:
- Around line 105-108: The displayed month/day is using
selectedMonth.toLocaleDateString without specifying timeZone which causes US
timezones to render the previous day/month; update the toLocaleDateString calls
that render selectedMonth (and the similar call at the other occurrence) to pass
timeZone: 'UTC' in the options (e.g.,
selectedMonth.toLocaleDateString(undefined, { month: 'long', year: 'numeric',
timeZone: 'UTC' })) so the UI matches the UTC-normalized selectedMonth and the
date-only forMonthD values.
- Around line 141-143: The comparison uses payment.forMonthD < new Date() which
compares a date-only value to a timestamp and makes bills due today appear
overdue; update the checks that set 'variant-outline-error' (the expressions
using payment.forMonthD < new Date() at the shown condition and the similar
occurrences at lines 153-154) to compare only the date portions (e.g., normalize
both sides to start-of-day or compare formatted yyyy-mm-dd / toDateString()) so
that payments due today are not treated as past due—apply the same normalization
in every place that currently does payment.forMonthD < new Date().
- Around line 30-35: paymentHistoryMonths is used without guarding for an empty
array causing paymentHistoryMonths[0].month and selectedMonth.toISOString() to
throw; modify the initialization so selectedMonth is derived defensively from
paymentHistoryMonths (e.g., selectedMonth = $derived(paymentHistoryMonths, p =>
p.length ? p[0].month : null) or default to new Date()), and update
paymentsByHousehold to only call
getCurrentPaymentsByHousehold(selectedMonth.toISOString()) when selectedMonth is
non-null (or pass a safe default), referencing the symbols paymentHistoryMonths,
selectedMonth, paymentsByHousehold, getPaymentHistoryMonths, and
getCurrentPaymentsByHousehold.
- Around line 117-120: The Progress bar is being sized against the current month
(today/new Date()) instead of the filtered month; update the Progress props to
use the selectedMonth range so pins scale correctly — pass today as the
selectedMonth (or new Date(selectedMonth) if it's a string) and set end to
getLastDayOfMonth(selectedMonth) (keep pins={paymentPins} and the Progress
component name the same).
---
Nitpick comments:
In `@apps/website/src/lib/remotes/payments.remote.ts`:
- Around line 62-75: Replace the extract() predicates on
schema.payments.forMonthD with a half-open range on the date column: compute a
monthStart (first day of isoMonth/isoYear) and nextMonthStart (first day of the
following month) from isoMonth and isoYear, then replace the two
eq(sql`extract(...)`) clauses with comparisons gte(schema.payments.forMonthD,
monthStart) and lt(schema.payments.forMonthD, nextMonthStart); keep the inArray
filter on userHouseholds.map((h) => h.id) unchanged. Ensure
monthStart/nextMonthStart are proper Date or ISO date strings matching the
column type so the DB can use an index on forMonthD.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1626a4f2-7823-4ff1-ad4b-0855d8fd8007
📒 Files selected for processing (2)
apps/website/src/lib/remotes/payments.remote.tsapps/website/src/routes/dashboard/payments/+page.svelte
| const optionalISODateValidator = type( | ||
| /^\d{4}-\d{2}-\d{2}((?:T|\s)\d{2}:\d{2}:\d{2}(\.\d+)?(Z|[+-]\d{2}:\d{2})?)?$/, | ||
| ) | ||
| .or('undefined') | ||
| .or('Date'); |
There was a problem hiding this comment.
Reject impossible dates before calling toISOString().
The regex accepts structurally valid but impossible values like 2026-13-40. new Date(date).toISOString() then throws a RangeError, so bad input becomes a 500 instead of a validation failure.
🛡️ Proposed fix
export const getCurrentPayments = query(
optionalISODateValidator,
async (date) => {
const userHouseholds = await getUserHouseholds();
- const isoDate = (date ? new Date(date) : new Date()).toISOString();
+ const parsedDate = date ? new Date(date) : new Date();
+ if (Number.isNaN(parsedDate.getTime())) {
+ throw error(400, 'Invalid date');
+ }
+ const isoDate = parsedDate.toISOString();
const [isoYear, isoMonth] = isoDate.split('T')[0].split('-').map(Number);Also applies to: 47-48
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/website/src/lib/remotes/payments.remote.ts` around lines 13 - 17, The
optionalISODateValidator currently only checks format and allows impossible
dates (e.g., 2026-13-40) which later cause new Date(...).toISOString() to throw;
update the validator (optionalISODateValidator) to add a runtime refinement
that, for non-undefined and non-Date string values, constructs a Date and
verifies it's valid (e.g., new Date(value) produces a date with
!isNaN(date.getTime())) before accepting, and similarly ensure any branch that
accepts Date objects checks date.getTime() is not NaN; apply the same refinement
to the other validator in this file that uses the same regex so invalid calendar
dates are rejected at validation time rather than causing a RangeError later.
| export const getCurrentPaymentsByHousehold = query( | ||
| optionalISODateValidator, | ||
| async (date) => { | ||
| const payments = await getCurrentPayments(date); | ||
| const grouped: Record<string, { name: string; payments: typeof payments }> = | ||
| payments.reduce( | ||
| (acc, payment) => { | ||
| if (!acc[payment.householdId]) { | ||
| acc[payment.householdId] = { | ||
| name: payment.household.name, | ||
| payments: [], | ||
| }; | ||
| } | ||
|
|
||
| acc[payment.householdId].payments.push(payment); | ||
|
|
||
| return acc; | ||
| }, | ||
| {} as Record<string, { name: string; payments: typeof payments }>, | ||
| ); | ||
| return grouped; | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In the Svelte remote query API used by $app/server, if query A (getCurrentPaymentsByHousehold(date)) awaits query B (getCurrentPayments(date)), does calling getCurrentPayments().refresh() invalidate active subscriptions to query A, or must query A and its specific arguments be refreshed explicitly?
💡 Result:
Calling getCurrentPayments(date).refresh does not invalidate active subscriptions to query A (getCurrentPaymentsByHousehold(date)). Query A must be refreshed explicitly with its specific arguments, e.g. getCurrentPaymentsByHousehold(householdId, date).refresh. Remote queries in SvelteKit are cached and identified by a combination of their function name (or ID) and the stringified/encoded arguments passed to them. Different arguments result in different cache keys, even if the functions call shared downstream queries internally. Refreshing a query only invalidates and refetches that specific instance (with its arguments); it does not propagate to other queries with different arguments or names. Official docs confirm: "Any query can be re-fetched via its refresh method" with examples like getPosts.refresh, implying per-instance refresh. GitHub issues highlight the need for explicit argument specification to refresh parameterized queries, as the server cannot automatically invalidate unknown client-side instances (e.g., #14367 discusses needing keys or parameters to refresh dependents without knowing exact args). No automatic dependency invalidation exists; this is a known limitation driving feature requests for keyed invalidation like TanStack Query.
Citations:
- 1: Remote Functions: Keyed Refresh sveltejs/kit#14367
- 2: Remote Functions: allow opt out of refresh. sveltejs/kit#14079
- 3: https://svelte.dev/docs/kit/remote-functions
- 4: https://svelte-changelog.dev/pull/sveltejs/kit/14563
- 5: feat: improve remote function caching by sorting object keys sveltejs/kit#15570
- 6: Ensure cache key generated during ssr matches what is generated durin… sveltejs/kit#14563
- 7: fix: create separate cache entries for non-exported queries sveltejs/kit#14499
🏁 Script executed:
cat -n apps/website/src/lib/remotes/payments.remote.tsRepository: sungmanito/mono
Length of output: 12178
🏁 Script executed:
rg "getCurrentPaymentsByHousehold" --type ts --type jsRepository: sungmanito/mono
Length of output: 158
🏁 Script executed:
rg "getCurrentPaymentsByHousehold" --type svelteRepository: sungmanito/mono
Length of output: 89
🏁 Script executed:
rg "getCurrentPaymentsByHousehold" -g "*.svelte" -g "*.ts" -g "*.js"Repository: sungmanito/mono
Length of output: 375
🏁 Script executed:
cat -n apps/website/src/routes/dashboard/payments/+page.svelte | head -100Repository: sungmanito/mono
Length of output: 3616
🏁 Script executed:
cat -n apps/website/src/routes/dashboard/payments/+page.svelte | tail -100Repository: sungmanito/mono
Length of output: 4396
Add getCurrentPaymentsByHousehold refresh to all payment mutations.
Currently, mutations (togglePayment, uploadImage, markPayment, unmarkPayment) only refresh getCurrentPayments() and getPayment(). Since getCurrentPaymentsByHousehold(date) is cached separately by argument in SvelteKit's remote query system, the grouped payment list remains stale after these mutations. The progress bar and payment list on the payments page will show outdated data until navigation.
Update each mutation to also refresh the grouped query. Note: mutations must either accept the date as a parameter or find another way to invalidate all instances of getCurrentPaymentsByHousehold() since the cache key includes the specific date argument.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/website/src/lib/remotes/payments.remote.ts` around lines 19 - 40,
Mutations togglePayment, uploadImage, markPayment, and unmarkPayment only
invalidate getCurrentPayments() and getPayment(), leaving
getCurrentPaymentsByHousehold(date) cached per-argument and stale; update each
mutation to also refresh the grouped query by either accepting the date argument
and calling the remote refresh for getCurrentPaymentsByHousehold(date) after
success, or (if you can't pass date) call your cache/remote invalidation that
targets all keys for getCurrentPaymentsByHousehold so every date-argument entry
is invalidated; locate these changes in the mutation handlers for togglePayment,
uploadImage, markPayment, and unmarkPayment and ensure they reference
getCurrentPaymentsByHousehold when triggering refreshes.
| const paymentHistoryMonths = $derived(await getPaymentHistoryMonths()); | ||
| let selectedMonth = $derived(paymentHistoryMonths[0].month); | ||
|
|
||
| const paymentsByHousehold = $derived( | ||
| await getCurrentPaymentsByHousehold(selectedMonth.toISOString()), | ||
| ); |
There was a problem hiding this comment.
Guard the no-history path before dereferencing the first month.
If getPaymentHistoryMonths() returns [], Line 31 throws on paymentHistoryMonths[0].month and Line 34 follows with .toISOString(). Users with no payments never reach an empty state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/website/src/routes/dashboard/payments/`+page.svelte around lines 30 -
35, paymentHistoryMonths is used without guarding for an empty array causing
paymentHistoryMonths[0].month and selectedMonth.toISOString() to throw; modify
the initialization so selectedMonth is derived defensively from
paymentHistoryMonths (e.g., selectedMonth = $derived(paymentHistoryMonths, p =>
p.length ? p[0].month : null) or default to new Date()), and update
paymentsByHousehold to only call
getCurrentPaymentsByHousehold(selectedMonth.toISOString()) when selectedMonth is
non-null (or pass a safe default), referencing the symbols paymentHistoryMonths,
selectedMonth, paymentsByHousehold, getPaymentHistoryMonths, and
getCurrentPaymentsByHousehold.
| Showing all payments for {selectedMonth.toLocaleDateString(undefined, { | ||
| month: 'long', | ||
| year: 'numeric', | ||
| })} |
There was a problem hiding this comment.
Render these calendar dates in UTC.
selectedMonth is UTC-normalized in apps/website/src/lib/remotes/payments.remote.ts:335-338, and forMonthD is a date-only field in packages/db/src/tables/payments.table.ts:33. Without timeZone: 'UTC', US timezones can show the previous month/day here.
🗓️ Proposed fix
<p class="font-xl font-semibold text-zinc-400">
Showing all payments for {selectedMonth.toLocaleDateString(undefined, {
month: 'long',
year: 'numeric',
+ timeZone: 'UTC',
})}
</p>
@@
<em>
Due by {payment.forMonthD.toLocaleString(undefined, {
month: 'long',
day: 'numeric',
+ timeZone: 'UTC',
})}
</em>Also applies to: 206-209
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/website/src/routes/dashboard/payments/`+page.svelte around lines 105 -
108, The displayed month/day is using selectedMonth.toLocaleDateString without
specifying timeZone which causes US timezones to render the previous day/month;
update the toLocaleDateString calls that render selectedMonth (and the similar
call at the other occurrence) to pass timeZone: 'UTC' in the options (e.g.,
selectedMonth.toLocaleDateString(undefined, { month: 'long', year: 'numeric',
timeZone: 'UTC' })) so the UI matches the UTC-normalized selectedMonth and the
date-only forMonthD values.
| <Progress | ||
| today={new Date()} | ||
| end={getLastDayOfMonth(new Date())} | ||
| pins={paymentPins} |
There was a problem hiding this comment.
Use the filtered month to size the progress bar.
The page is filtering by selectedMonth, but Line 119 still uses the current month. February and 31-day months will render pins on the wrong scale.
📊 Proposed fix
<Progress
today={new Date()}
- end={getLastDayOfMonth(new Date())}
+ end={getLastDayOfMonth(selectedMonth)}
pins={paymentPins}
/>📝 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.
| <Progress | |
| today={new Date()} | |
| end={getLastDayOfMonth(new Date())} | |
| pins={paymentPins} | |
| <Progress | |
| today={new Date()} | |
| end={getLastDayOfMonth(selectedMonth)} | |
| pins={paymentPins} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/website/src/routes/dashboard/payments/`+page.svelte around lines 117 -
120, The Progress bar is being sized against the current month (today/new
Date()) instead of the filtered month; update the Progress props to use the
selectedMonth range so pins scale correctly — pass today as the selectedMonth
(or new Date(selectedMonth) if it's a string) and set end to
getLastDayOfMonth(selectedMonth) (keep pins={paymentPins} and the Progress
component name the same).
| 'variant-outline-success': payment.paidAt !== null, | ||
| 'variant-outline-error': | ||
| payment.paidAt === null && payment.forMonthD < new Date(), |
There was a problem hiding this comment.
Compare due dates as dates, not timestamps.
payment.forMonthD is a date-only value, so payment.forMonthD < new Date() flips to overdue as soon as the due day starts. Bills due today will already render the error state and alert icon.
⏰ Proposed fix
{`#each` payments as payment}
{`@const` paymentForm = togglePayment.for(payment.id)}
+ {`@const` isPastDue =
+ payment.paidAt === null &&
+ payment.forMonthD.toISOString().slice(0, 10) <
+ new Date().toISOString().slice(0, 10)}
<form {...paymentForm}>
<div
class={[
'card',
{
'variant-outline-success': payment.paidAt !== null,
- 'variant-outline-error':
- payment.paidAt === null && payment.forMonthD < new Date(),
+ 'variant-outline-error': isPastDue,
},
]}
role="listitem"
>
@@
- {:else if payment.paidAt === null && payment.forMonthD < new Date()}
+ {:else if isPastDue}
<ClockAlertIcon class="text-error-500" size="1em" />
{/if}Also applies to: 153-154
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/website/src/routes/dashboard/payments/`+page.svelte around lines 141 -
143, The comparison uses payment.forMonthD < new Date() which compares a
date-only value to a timestamp and makes bills due today appear overdue; update
the checks that set 'variant-outline-error' (the expressions using
payment.forMonthD < new Date() at the shown condition and the similar
occurrences at lines 153-154) to compare only the date portions (e.g., normalize
both sides to start-of-day or compare formatted yyyy-mm-dd / toDateString()) so
that payments due today are not treated as past due—apply the same normalization
in every place that currently does payment.forMonthD < new Date().
jhechtf
force-pushed
the
fix/payments-page
branch
from
d09d07e to
e2bd9f1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/website/src/lib/remotes/payments.remote.ts (1)
335-338:⚠️ Potential issue | 🟡 MinorType annotation
sql<string>contradicts.mapWith()behavior.The
sql<string>declares the return as a string, but.mapWith(schema.payments.forMonthD)uses the column's decoder, which converts toDate(sinceforMonthDhasmode: 'date'). The actual runtime type isDate, notstring.This mismatch can cause TypeScript confusion downstream.
🔧 Proposed fix
month: - sql<string>`date_trunc('month', ${schema.payments.forMonthD})::timestamp at time zone 'UTC'`.mapWith( + sql<Date>`date_trunc('month', ${schema.payments.forMonthD})::timestamp at time zone 'UTC'`.mapWith( schema.payments.forMonthD, ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/src/lib/remotes/payments.remote.ts` around lines 335 - 338, The sql<> generic for the "month" field is declared as sql<string> but .mapWith(schema.payments.forMonthD) decodes to a Date; update the SQL type annotation to match the decoder (e.g., use sql<Date> or remove the generic so the mapper's Date type is used) for the month property to ensure the declared return type aligns with .mapWith(schema.payments.forMonthD) and avoid downstream TypeScript type mismatches.
🧹 Nitpick comments (3)
apps/website/src/routes/dashboard/payments/+page.svelte (2)
113-115: Unused progress placeholder markup.These empty
<div class="progress">and<div class="progress-track">elements appear to be leftover code, as the actual<Progress>component is rendered separately below (lines 117-121).🧹 Proposed fix
<p class="text-zinc-400 mt-4">Monthly Progress</p> - <div class="progress"> - <div class="progress-track"></div> - </div> - <Progress🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/src/routes/dashboard/payments/`+page.svelte around lines 113 - 115, Remove the leftover empty placeholder markup: delete the two divs with class="progress" and class="progress-track" since the real component <Progress> is rendered separately (see the existing <Progress> usage) and these empty elements are unused; ensure no other logic references the "progress" or "progress-track" elements before removing them.
90-99: Duplicate fetch of payment history months.
getPaymentHistoryMonths()is already called and stored inpaymentHistoryMonths(line 30), but line 90 calls it again for the<select>options. Use the existing derived value instead.♻️ Proposed fix
<select name="currentMonth" class="select" bind:value={selectedMonth}> - <svelte:boundary> - {`#snippet` pending()} - <option disabled>Loading</option> - {/snippet} - - {`#each` await getPaymentHistoryMonths() as month} + {`#each` paymentHistoryMonths as month} {`@const` dateObj = new Date(month.month)} <option value={month.month}> {dateObj.toLocaleDateString(undefined, { month: 'long', year: 'numeric', timeZone: 'UTC', })} </option> {/each} - </svelte:boundary> </select>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/src/routes/dashboard/payments/`+page.svelte around lines 90 - 99, The select is re-fetching months by calling getPaymentHistoryMonths() again; instead iterate over the already-fetched value paymentHistoryMonths in the {`#each`} block (replace "{`#each` await getPaymentHistoryMonths() as month}" with iterating over paymentHistoryMonths and keep the same option rendering, ensuring you await or subscribe only if paymentHistoryMonths is a Promise/store), and remove the duplicate getPaymentHistoryMonths() call.apps/website/src/lib/remotes/payments.remote.ts (1)
68-75: Inconsistentextractfield casing.
'month'(line 69) is lowercase with quotes whileYEAR(line 73) is uppercase without quotes. Both work in PostgreSQL, but consistent style improves readability.✨ Proposed fix
eq( - sql`extract('month' from ${schema.payments.forMonthD})`, + sql`extract(month from ${schema.payments.forMonthD})`, isoMonth, ), eq( - sql`extract(YEAR from ${schema.payments.forMonthD})`, + sql`extract(year from ${schema.payments.forMonthD})`, isoYear, ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/src/lib/remotes/payments.remote.ts` around lines 68 - 75, Summary: inconsistent casing/quoting in the SQL EXTRACT calls. Fix: make both EXTRACT units use the same style — for example change the second call to use a lowercase quoted unit like the first, i.e. use sql`extract('year' from ${schema.payments.forMonthD})` so both EXTRACTs use quoted lowercase; locate the two occurrences around the eq/sql calls that reference schema.payments.forMonthD and isoMonth/isoYear and update the uppercase YEAR to 'year' to match the 'month' style.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/website/src/lib/remotes/dashboard.remote.ts`:
- Around line 36-42: The month/year filters are mixing server-local JS time
(today) with DB now(), causing inconsistent timezone boundaries; change to use
JS-derived isoMonth/isoYear consistently (as in payments.remote.ts) instead of
sql`extract('year' from now())`: compute isoMonth and isoYear from today and
replace the year-extract clause that references now() with an equality against
the JS isoYear, keeping the month clause using today.getMonth()+1 (or using
isoMonth) and the payment column reference schema.payments.forMonthD so both
comparisons use the same time source.
---
Duplicate comments:
In `@apps/website/src/lib/remotes/payments.remote.ts`:
- Around line 335-338: The sql<> generic for the "month" field is declared as
sql<string> but .mapWith(schema.payments.forMonthD) decodes to a Date; update
the SQL type annotation to match the decoder (e.g., use sql<Date> or remove the
generic so the mapper's Date type is used) for the month property to ensure the
declared return type aligns with .mapWith(schema.payments.forMonthD) and avoid
downstream TypeScript type mismatches.
---
Nitpick comments:
In `@apps/website/src/lib/remotes/payments.remote.ts`:
- Around line 68-75: Summary: inconsistent casing/quoting in the SQL EXTRACT
calls. Fix: make both EXTRACT units use the same style — for example change the
second call to use a lowercase quoted unit like the first, i.e. use
sql`extract('year' from ${schema.payments.forMonthD})` so both EXTRACTs use
quoted lowercase; locate the two occurrences around the eq/sql calls that
reference schema.payments.forMonthD and isoMonth/isoYear and update the
uppercase YEAR to 'year' to match the 'month' style.
In `@apps/website/src/routes/dashboard/payments/`+page.svelte:
- Around line 113-115: Remove the leftover empty placeholder markup: delete the
two divs with class="progress" and class="progress-track" since the real
component <Progress> is rendered separately (see the existing <Progress> usage)
and these empty elements are unused; ensure no other logic references the
"progress" or "progress-track" elements before removing them.
- Around line 90-99: The select is re-fetching months by calling
getPaymentHistoryMonths() again; instead iterate over the already-fetched value
paymentHistoryMonths in the {`#each`} block (replace "{`#each` await
getPaymentHistoryMonths() as month}" with iterating over paymentHistoryMonths
and keep the same option rendering, ensuring you await or subscribe only if
paymentHistoryMonths is a Promise/store), and remove the duplicate
getPaymentHistoryMonths() call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2f9363d4-8eaa-4a7a-9e61-451ef3632092
📒 Files selected for processing (3)
apps/website/src/lib/remotes/dashboard.remote.tsapps/website/src/lib/remotes/payments.remote.tsapps/website/src/routes/dashboard/payments/+page.svelte
| sql`extract('month' from ${schema.payments.forMonthD})`, | ||
| today.getMonth() + 1, | ||
| ), | ||
| eq( | ||
| sql`extract('year' from ${schema.payments.forMonthD})`, | ||
| sql`extract('year' from now())`, | ||
| ), |
There was a problem hiding this comment.
Month/year constraints use inconsistent time sources.
The month comparison (line 37) uses the JavaScript today object (server-local timezone), while the year comparison (lines 40-41) uses the database's now() (session timezone). If these timezones diverge—especially around midnight—month and year could reference different calendar days, causing payments to be incorrectly included or excluded.
For consistency, either use the JS-derived values for both (as done in payments.remote.ts with isoYear/isoMonth) or use now() for both.
🛠️ Proposed fix using consistent JS-derived values
.leftJoin(
schema.payments,
and(
eq(
sql`extract('month' from ${schema.payments.forMonthD})`,
today.getMonth() + 1,
),
eq(
sql`extract('year' from ${schema.payments.forMonthD})`,
- sql`extract('year' from now())`,
+ today.getFullYear(),
),
eq(schema.payments.billId, schema.bills.id),
),
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/website/src/lib/remotes/dashboard.remote.ts` around lines 36 - 42, The
month/year filters are mixing server-local JS time (today) with DB now(),
causing inconsistent timezone boundaries; change to use JS-derived
isoMonth/isoYear consistently (as in payments.remote.ts) instead of
sql`extract('year' from now())`: compute isoMonth and isoYear from today and
replace the year-extract clause that references now() with an equality against
the JS isoYear, keeping the month clause using today.getMonth()+1 (or using
isoMonth) and the payment column reference schema.payments.forMonthD so both
comparisons use the same time source.
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 3 file(s) based on 6 unresolved review comments. A stacked PR containing fixes has been created.
Time taken: |
jhechtf
force-pushed
the
fix/payments-page
branch
from
e2bd9f1 to
0912161
Compare
jhechtf
force-pushed
the
fix/payments-page
branch
from
0912161 to
d97bf99
Compare
1 participant
Summary by CodeRabbit
New Features
Bug Fixes