Skip to content

chore: upgrade to arktype 2#116

Merged
jhechtf merged 1 commit intomainfrom
chore/upgrade-to-arktype-2
Apr 20, 2025
Merged

chore: upgrade to arktype 2#116
jhechtf merged 1 commit intomainfrom
chore/upgrade-to-arktype-2

Conversation

@jhechtf
Copy link
Copy Markdown
Contributor

@jhechtf jhechtf commented Feb 8, 2025

TL;DR

Updated arktype to v2.0.4 and improved form validation patterns across authentication flows.

What changed?

  • Upgraded arktype from v1.0.27-alpha to v2.0.4
  • Added @jhecht/arktype-utils dependency
  • Updated email validation pattern from 'email' to 'string.email'
  • Enhanced currency validation to enforce uppercase 3-letter codes
  • Removed redundant session validation check in bills deletion
  • Cleaned up console.log statements in forgot password flow

How to test?

  1. Test form submissions in registration, login, and forgot password flows
  2. Verify currency inputs only accept 3-letter uppercase codes (e.g., USD, EUR)
  3. Confirm email validation works correctly across all forms
  4. Test bill deletion still functions properly
  5. Verify password requirements remain intact (minimum 8 characters)

Why make this change?

The upgrade to arktype v2.0.4 provides more robust type validation and better pattern matching capabilities. The standardization of email validation patterns and currency format requirements improves data consistency and reduces potential validation errors.

Summary by CodeRabbit

  • New Features

    • Added a reusable set of input validation schemas for IDs, due dates, and currency codes.
    • Introduced a new function to retrieve users for a household (currently a placeholder).
  • Refactor

    • Unified and strengthened form data validation by switching to an external validation utility throughout the app.
    • Improved validation schemas for bills, payments, and user-related forms, including more explicit email and currency formats.
    • Enhanced error handling and reporting for form submissions.
  • Bug Fixes

    • Strengthened input validation for emails and currency codes to prevent invalid data.
  • Chores

    • Updated dependencies to use the latest validation libraries.
    • Removed local form data utility files in favor of external packages.
  • Style

    • Updated household member display to hide user identification information in the UI.
  • Tests

    • Removed obsolete unit tests for deprecated form data utilities.

@vercel
Copy link
Copy Markdown

vercel Bot commented Feb 8, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
bill-tracker-mono ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 20, 2025 10:56pm

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Feb 8, 2025

Walkthrough

This update refactors form data validation and parsing across the application by replacing a custom local utility (formData.ts and its tests) with the external package @jhecht/arktype-utils. All imports and usages of the local form data utility are updated accordingly. Validation schemas are revised for stricter typing, especially for email and currency fields, and new common validation modules are introduced. Some internal logic for bill and payment validators is refactored for consistency and maintainability. Additionally, a placeholder function for fetching users by household is added, and minor UI adjustments are made to the household members list.

Changes

File(s) Change Summary
apps/website/package.json Added @jhecht/arktype-utils@^1.0.2; upgraded arktype from 1.0.27-alpha to 2.0.4.
apps/website/src/lib/util/formData.ts, apps/website/src/lib/util/formData.test.ts Deleted local form data parsing and validation utilities and their tests.
apps/website/src/lib/components/validators/common.ts Added new base module with common validators for ULID, due date, and currency.
apps/website/src/lib/components/validators/bills.ts Refactored bill validators to use new base validators, added baseBillValidator, improved currency validation.
apps/website/src/lib/server/actions/invites.action.ts Updated email validation schemas from 'email' to 'string.email'.
apps/website/src/lib/server/actions/payments.actions.ts Switched to external formDataToObject, refactored validation logic, replaced instanceOf(File) with 'File'.
apps/website/src/lib/server/actions/users.actions.ts Added placeholder usersForHousehold(householdId: string) function.
apps/website/src/routes/dashboard/+page.server.ts Switched form data validation to external package; added debug log.
apps/website/src/routes/dashboard/bills/+page.server.ts, .../delete/+page.server.ts Switched to external form data validation; improved error handling and currency validation; used validateUserSession.
apps/website/src/routes/dashboard/household/+page.server.ts, .../[id=ulid]/+page.server.ts Switched to external form data validation; refined email validation types.
apps/website/src/routes/dashboard/household/+page.svelte Removed display of member names/emails from UI.
apps/website/src/routes/dashboard/payments/+page.server.ts Switched to external form data validation; updated file validation type.
apps/website/src/routes/login/+page.server.ts, .../forgot/+page.server.ts Switched to external form data validation; updated email schema; removed debug logs.
apps/website/src/routes/profile/+page.server.ts Switched to external form data validation; updated email schema.
apps/website/src/routes/register/+page.server.ts Switched to external form data validation; updated email schema.
apps/website/src/routes/signup/+page.server.ts Switched to external form data validation; updated email schema; minor variable refactor.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Server
    participant ArkTypeUtils

    User->>Server: Submit form (e.g., add bill, register)
    Server->>ArkTypeUtils: validateFormData(formData, schema)
    ArkTypeUtils-->>Server: Validated & parsed data or errors
    alt Data valid
        Server->>Server: Proceed with business logic
    else Validation errors
        Server->>User: Return error/failure with issues
    end
Loading

Possibly related PRs

  • sungmanito/mono#115: Refactors household page layout and sidebar invites, overlapping with changes to household-related UI and invite handling in this PR.

Suggested labels

enhancement

Poem

In the warren, code hops anew,
Old forms replaced with something true.
Validators crisp and neat,
Parsing errors in retreat!
Currency, emails—checked with care,
Rabbits dance, no bugs to snare.
🐇✨ The code is clean, the future bright—
Validation’s never felt so light!

Tip

⚡💬 Agentic Chat (Pro Plan, General Availability)
  • We're introducing multi-step agentic chat in review comments and issue comments, within and outside of PR's. This feature enhances review and issue discussions with the CodeRabbit agentic chat by enabling advanced interactions, including the ability to create pull requests directly from comments and add commits to existing pull requests.
✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@jhechtf jhechtf force-pushed the refactor/redoing-household-page branch from 3026df8 to 5198569 Compare February 17, 2025 08:07
@jhechtf jhechtf force-pushed the chore/upgrade-to-arktype-2 branch from 1a57851 to adab3dd Compare February 17, 2025 08:07
@jhechtf jhechtf force-pushed the refactor/redoing-household-page branch from 5198569 to 9a6dcd9 Compare February 19, 2025 05:30
@jhechtf jhechtf force-pushed the chore/upgrade-to-arktype-2 branch from adab3dd to ffefa70 Compare February 19, 2025 05:30
@jhechtf jhechtf force-pushed the refactor/redoing-household-page branch from 9a6dcd9 to 470cbed Compare February 19, 2025 23:03
@jhechtf jhechtf force-pushed the chore/upgrade-to-arktype-2 branch from ffefa70 to 7fcede5 Compare February 19, 2025 23:03
@jhechtf jhechtf force-pushed the chore/upgrade-to-arktype-2 branch from 7fcede5 to 72750c0 Compare February 20, 2025 05:16
@jhechtf jhechtf force-pushed the chore/upgrade-to-arktype-2 branch from 72750c0 to c8b97e3 Compare February 22, 2025 20:37
@jhechtf jhechtf force-pushed the refactor/redoing-household-page branch from 470cbed to 810eb05 Compare April 20, 2025 19:42
@jhechtf jhechtf force-pushed the chore/upgrade-to-arktype-2 branch from c8b97e3 to e6f0c67 Compare April 20, 2025 19:42
Copy link
Copy Markdown
Contributor Author

jhechtf commented Apr 20, 2025

Merge activity

  • Apr 20, 6:53 PM EDT: A user started a stack merge that includes this pull request via Graphite.
  • Apr 20, 6:55 PM EDT: Graphite rebased this pull request as part of a merge.
  • Apr 20, 6:57 PM EDT: A user merged this pull request with Graphite.

@jhechtf jhechtf changed the base branch from refactor/redoing-household-page to graphite-base/116 April 20, 2025 22:53
@jhechtf jhechtf changed the base branch from graphite-base/116 to main April 20, 2025 22:54
@jhechtf jhechtf force-pushed the chore/upgrade-to-arktype-2 branch from 5d40bee to 7f1fbf6 Compare April 20, 2025 22:55
@jhechtf jhechtf merged commit f614673 into main Apr 20, 2025
4 of 7 checks passed
@jhechtf jhechtf deleted the chore/upgrade-to-arktype-2 branch April 20, 2025 22:57
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🔭 Outside diff range comments (1)
apps/website/src/routes/dashboard/household/+page.server.ts (1)

134-136: ⚠️ Potential issue

Inconsistency in success response

The addHousehold function seems to always return success: false even when the operation succeeds (as indicated by returning invites). This might lead to confusing behavior for API consumers.

     return {
-      success: false,
+      success: true,
       invites: responses,
     };
🧹 Nitpick comments (2)
apps/website/src/routes/dashboard/+page.server.ts (1)

3-3: Consider removing debug console.log statement.

While the validation changes look good, the added console.info(data) statement on line 129 appears to be for debugging purposes. Consider removing this before merging to production.

 const data = await request.formData();
 const today = new Date();
-console.info(data);
 const formData = validateFormData(

Also applies to: 129-130

apps/website/src/routes/login/forgot/+page.server.ts (1)

19-21: Consider adding more specific error messages

Both success and error cases return a generic fail(400) response without a message, which might make it difficult for users to understand what went wrong. Consider adding specific error messages to improve user experience.

-    if (err) return fail(400);
+    if (err) return fail(400, { message: err.message || 'Failed to process password reset request' });
 
-    return fail(400);
+    return fail(400, { message: 'Password reset email sent. Please check your inbox.' });
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c8dbcf1 and 7f1fbf6.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (20)
  • apps/website/package.json (2 hunks)
  • apps/website/src/lib/components/validators/bills.ts (1 hunks)
  • apps/website/src/lib/components/validators/common.ts (1 hunks)
  • apps/website/src/lib/server/actions/invites.action.ts (1 hunks)
  • apps/website/src/lib/server/actions/payments.actions.ts (3 hunks)
  • apps/website/src/lib/server/actions/users.actions.ts (1 hunks)
  • apps/website/src/lib/util/formData.test.ts (0 hunks)
  • apps/website/src/lib/util/formData.ts (0 hunks)
  • apps/website/src/routes/dashboard/+page.server.ts (2 hunks)
  • apps/website/src/routes/dashboard/bills/+page.server.ts (4 hunks)
  • apps/website/src/routes/dashboard/bills/[id=ulid]/delete/+page.server.ts (1 hunks)
  • apps/website/src/routes/dashboard/household/+page.server.ts (5 hunks)
  • apps/website/src/routes/dashboard/household/+page.svelte (1 hunks)
  • apps/website/src/routes/dashboard/household/[id=ulid]/+page.server.ts (4 hunks)
  • apps/website/src/routes/dashboard/payments/+page.server.ts (2 hunks)
  • apps/website/src/routes/login/+page.server.ts (1 hunks)
  • apps/website/src/routes/login/forgot/+page.server.ts (2 hunks)
  • apps/website/src/routes/profile/+page.server.ts (2 hunks)
  • apps/website/src/routes/register/+page.server.ts (1 hunks)
  • apps/website/src/routes/signup/+page.server.ts (2 hunks)
💤 Files with no reviewable changes (2)
  • apps/website/src/lib/util/formData.test.ts
  • apps/website/src/lib/util/formData.ts
🧰 Additional context used
🧬 Code Graph Analysis (3)
apps/website/src/lib/server/actions/users.actions.ts (1)
apps/website/src/lib/server/db/client.ts (1)
  • db (12-12)
apps/website/src/routes/register/+page.server.ts (3)
apps/website/src/routes/login/+page.server.ts (1)
  • actions (29-79)
apps/website/src/routes/login/forgot/+page.server.ts (1)
  • actions (5-27)
apps/website/src/routes/signup/+page.server.ts (1)
  • actions (16-70)
apps/website/src/lib/components/validators/bills.ts (1)
apps/website/src/lib/components/validators/common.ts (1)
  • base (3-7)
🪛 GitHub Check: Linter
apps/website/src/lib/server/actions/users.actions.ts

[failure] 17-17:
'householdId' is defined but never used

🔇 Additional comments (40)
apps/website/src/lib/components/validators/common.ts (1)

1-7: Good implementation of reusable validation schemas.

Creating a centralized module for common validation patterns is an excellent approach. The validators follow the arktype v2 syntax correctly and align with the PR objective of enforcing stricter validation, especially for currency codes.

apps/website/src/routes/login/+page.server.ts (1)

2-2: Import successfully migrated to the external package.

The change aligns with the PR objective of migrating from local validation utilities to the standardized @jhecht/arktype-utils package.

apps/website/src/routes/dashboard/bills/[id=ulid]/delete/+page.server.ts (2)

2-2: Import successfully migrated to the external package.

The function import has been correctly updated to use the new dependency.


33-36:

Details

❌ Incorrect review comment

Consider removing redundant session validation.

According to the PR objectives, one goal was "removing a redundant session validation check in the bills deletion process." There's still a session validation check at line 35 in addition to the check in the load function at line 12.


🏁 Script executed:

#!/bin/bash
# Check if session validation is performed elsewhere in the request lifecycle
grep -A 5 -B 5 "validateUserSession" apps/website/src/routes/dashboard/bills/[id=ulid]/delete/+page.server.ts

Length of output: 1068


Retain session validation in the deleteBill action

The validateUserSession(session) call in your load function only protects page‐load (GET) requests. SvelteKit does not invoke load before running form actions (POST), so an unauthenticated client could bypass the load check and call your deleteBill action directly. The session check inside deleteBill is therefore necessary to secure that endpoint and should be kept.

Likely an incorrect or invalid review comment.

apps/website/package.json (2)

71-71: Successfully added new dependency.

The addition of @jhecht/arktype-utils aligns with the PR objective of improving form validation patterns.


80-80: Successfully upgraded arktype to v2.

Upgrading from v1.0.27-alpha to v2.0.4 provides access to more robust type validation capabilities as mentioned in the PR objectives.

apps/website/src/routes/dashboard/household/+page.svelte (1)

155-155: Removed display of member identification information.

The change removes user identification information from the household members list by emptying the div elements. Is this intentional or should there be replacement content?

While this change might improve privacy, it could affect usability if users can no longer identify household members. Consider adding non-identifying labels or indicators if complete removal wasn't intended.

apps/website/src/routes/register/+page.server.ts (1)

2-2: The arktype validation upgrade looks good.

The change from a local utility to validateFormData from @jhecht/arktype-utils and the updated email validation pattern to string.email aligns with the PR's goal of upgrading to arktype 2.0.4. These changes improve validation patterns across authentication flows.

Also applies to: 10-10, 17-17

apps/website/src/routes/dashboard/payments/+page.server.ts (1)

14-14: Validation type changes look good, but remove instanceOf import.

The changes properly implement the new validation from @jhecht/arktype-utils and update the File validation from likely using instanceOf(File) to the string literal 'File'. This is consistent with the arktype 2.0.4 upgrade.

Also applies to: 18-18, 73-73

apps/website/src/routes/profile/+page.server.ts (1)

1-1: Profile validation updates correctly implemented.

The migration to validateFormData from @jhecht/arktype-utils and the transition from email to string.email validation pattern improves the email validation in the profile update form.

Also applies to: 12-12, 17-17

apps/website/src/routes/signup/+page.server.ts (3)

4-4: Library migration from local utility to external package

The code now uses the standardized form validation utility from @jhecht/arktype-utils instead of a local implementation. This aligns with the PR objective to leverage arktype v2.0.4's improved validation capabilities.


22-24: Improved form data handling pattern

Storing form data in a variable before validation improves code readability and makes debugging easier by providing a clear reference to the form data object.


26-26: Enhanced email validation with more explicit type

The email validation has been updated from 'email' to 'string.email', providing more precise type information. This matches the PR objective to enhance email validation patterns across authentication flows.

apps/website/src/lib/server/actions/invites.action.ts (2)

8-8: Updated email validator to use more explicit type

The email validator now uses 'string.email' instead of just 'email', providing more precise type information and aligning with the updated validation patterns across the application.


10-10: Updated email array validator to use more explicit type

The email array validator now uses 'string.email[]' instead of 'email[]', providing more precise type information and aligning with the updated validation patterns across the application.

apps/website/src/routes/login/forgot/+page.server.ts (2)

1-1: Library migration from local utility to external package

The code now imports the standardized form validation utility from @jhecht/arktype-utils instead of a local implementation. This aligns with the PR objective to leverage arktype v2.0.4's improved validation capabilities.


10-10: Enhanced email validation with more explicit type

The email validation has been updated from 'email' to 'string.email', providing more precise type information. This matches the PR objective to enhance email validation patterns across authentication flows.

apps/website/src/routes/dashboard/household/+page.server.ts (5)

4-4: Library migration from local utility to external package

The code now imports the standardized form validation utility from @jhecht/arktype-utils instead of a local implementation. This aligns with the PR objective to leverage arktype v2.0.4's improved validation capabilities.


45-49: Updated form validation implementation

The code has been updated to use the standardized validateFormData function from the external package, maintaining the same validation logic but leveraging the improved capabilities of arktype v2.0.4.


102-108: Enhanced validation with more explicit types

The validation has been updated to use the standardized validateFormData function, and the email array validation now uses the more explicit 'string.email[]' type. This provides more precise type information and aligns with the PR objective to enhance validation patterns.


143-148: Updated form validation implementation

The code has been updated to use the standardized validateFormData function from the external package, maintaining the same validation logic but leveraging the improved capabilities of arktype v2.0.4.


184-190: Updated form validation implementation

The code has been updated to use the standardized validateFormData function from the external package, maintaining the same validation logic but leveraging the improved capabilities of arktype v2.0.4.

apps/website/src/lib/server/actions/payments.actions.ts (5)

3-3: Import updated to use the new external package.

The code now imports formDataToObject from @jhecht/arktype-utils instead of using a local utility, which aligns with the PR objective to upgrade to arktype 2.0.4.


7-7: Simplified imports from arktype.

The import from arktype has been simplified to only include type, removing instanceOf which is no longer needed with the new validation approach.


153-156: Simplified ID validation.

The refactored validation approach now directly assigns the validation result to ids, making the code cleaner and more aligned with arktype 2.0.4 patterns.


160-160: Improved error type checking.

Changed error check to use instanceof type.errors instead of checking a separate errs variable, providing more type safety and better error identification.


19-19:

Details

❓ Verification inconclusive

Validator type change for file uploads.

Changed from (likely) instanceOf(File) to string literal 'File' for proof validation, which matches the new arktype 2.0.4 approach to type validation.


🏁 Script executed:

#!/bin/bash
# Check how 'File' type is handled in arktype 2.x
rg -A 3 "File['\"]|['\"]File" node_modules/@jhecht/arktype-utils

Length of output: 145


Validator type change for file uploads – please verify string-based File validator usage

The input schema now specifies the optional proof field with the string literal 'File' instead of instanceOf(File). This should align with arktype v2.0.4’s new validation API, but I couldn’t locate the @jhecht/arktype-utils (or equivalent) module in node_modules to confirm that 'File' is supported as a type specifier:

• File: apps/website/src/lib/server/actions/payments.actions.ts
• Line 19:

  'proof?': 'File',

Please verify that arktype v2.0.4 accepts 'File' for file uploads in its validator definitions.

apps/website/src/routes/dashboard/household/[id=ulid]/+page.server.ts (5)

2-2: Updated import to use the external package.

The code now uses the standardized validateFormData function from @jhecht/arktype-utils instead of a local utility, aligning with the project-wide upgrade to arktype 2.0.4.


10-10: Enhanced email validation.

Changed email validation from 'email | string' to 'string.email | string' to leverage the more robust type validation in arktype 2.0.4, which provides better validation for email addresses.


70-73: Standardized form data validation.

Replaced local utility with validateFormData from the external package, improving consistency across the codebase while maintaining the same validation schema.


103-108: Standardized form data validation.

Replaced local utility with validateFormData from the external package, improving consistency across the codebase while maintaining the same validation schema.


236-239: Standardized form data validation.

Replaced local utility with validateFormData from the external package, improving consistency across the codebase while maintaining the same validation schema.

apps/website/src/lib/components/validators/bills.ts (3)

2-2: Imported shared validators.

Added import of base validators from the common module, enabling standardized validation patterns across the application.


4-12: Created comprehensive base bill validator.

The new baseBillValidator provides a complete and standardized schema for bill validation with improved type safety:

  • Uses consistent ULID validation for IDs
  • Standardizes date validation
  • Adds optional notes field
  • Makes currency required with a default of 'USD'

This change enhances data consistency and reduces validation errors as mentioned in the PR objectives.


14-14: Simplified new bill validator.

The newBillValidator is now derived from baseBillValidator by omitting the 'id' field, providing better code reuse and ensuring consistency between validation schemas.

apps/website/src/routes/dashboard/bills/+page.server.ts (5)

7-7: Updated import to use the external package.

The code now uses the standardized validateFormData function from @jhecht/arktype-utils instead of a local utility, aligning with the project-wide upgrade to arktype 2.0.4.


42-55: Enhanced form validation with proper error handling.

The form validation has been significantly improved:

  1. Changed schema construction from .compile() to .type('formData')
  2. Added structured try-catch error handling
  3. Provides detailed 400 errors for validation issues
  4. Properly logs unexpected errors as 500 failures

These changes make the code more robust and error messages more helpful to users.


101-111: Standardized form data validation.

Replaced local utility with validateFormData from the external package, improving consistency across the codebase while maintaining the same validation schema.


109-109: Strengthened currency validation.

Updated currency validation to enforce uppercase 3-letter codes (e.g., USD, EUR) as mentioned in the PR objectives, ensuring data consistency and reducing validation errors.


148-153: Standardized form data validation.

Replaced local utility with validateFormData from the external package, improving consistency across the codebase while maintaining the same validation schema.

Comment on lines +17 to +19
export function usersForHousehold(householdId: string) {
return db.select({});
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Incomplete implementation of usersForHousehold function

The function appears to be a placeholder as it:

  1. Takes a householdId parameter but doesn't use it in the query
  2. Returns a database select with an empty object ({}) without specifying any table or columns
  3. Lacks documentation explaining its intended purpose

This implementation will likely result in a runtime error or unexpected behavior when called.

Consider implementing the function properly or marking it as a TODO if it's for future use:

+ /**
+  * Retrieves users that belong to a specific household
+  * @param householdId - The ID of the household to filter users by
+  * @returns Users belonging to the specified household
+  * @todo Implement proper filtering logic
+  */
export function usersForHousehold(householdId: string) {
-  return db.select({});
+  return db
+    .select({
+      id: schema.users.id,
+      email: schema.users.email,
+      // Add other relevant user fields
+    })
+    .from(schema.users)
+    .innerJoin(
+      schema.householdUsers,
+      eq(schema.users.id, schema.householdUsers.userId)
+    )
+    .where(eq(schema.householdUsers.householdId, householdId));
}

🏁 Script executed:

#!/bin/bash
# Check if there's a household users relationship table or similar structure

echo "Checking for household user relationship tables..."
fd -e ts "schema" --exec grep -l "household.*user" {} \;

echo "Checking how users are associated with households in the schema..."
fd -e ts "schema" --exec grep -A 10 -B 10 "household" {} \;

Length of output: 374


🏁 Script executed:

#!/bin/bash
# Inspect imports and existing code in users.actions.ts to determine available schema & db usage
sed -n '1,200p' apps/website/src/lib/server/actions/users.actions.ts

Length of output: 604


🏁 Script executed:

#!/bin/bash
# List project root structure to locate potential local DB package
echo "Project root files and directories:"
ls -1

# Inspect tsconfig.json for path mappings to '@sungmanito/db'
echo -e "\nTSConfig path mappings (compilerOptions.paths):"
sed -n '1,200p' tsconfig.json | sed -n '/"paths"/,/\]/p'

Length of output: 415


🏁 Script executed:

#!/bin/bash
set -e

echo "Listing exported constants in the db schema package:"
grep -R "export const " -n packages/db/src

echo -e "\nSearching for household-related definitions in the db schema package:"
grep -R -i "household" -n packages/db/src

Length of output: 7936


#!/bin/bash
set -e

echo "== packages/db/src/index.ts =="
sed -n '1,200p' packages/db/src/index.ts

echo -e "\n== packages/db/src/tables/usersToHouseholds.table.ts =="
sed -n '1,200p' packages/db/src/tables/usersToHouseholds.table.ts

Incomplete usersForHousehold implementation – needs proper filtering and async handling

The current function is just a placeholder and will fail at runtime. You need to:

  • Mark the function async and await the query
  • Use the usersToHouseholds join table (not a non‑existent householdUsers)
  • Select from schema.users and inner‐join on schema.usersToHouseholds
  • Filter by householdId and return the desired user fields

Suggested diff in apps/website/src/lib/server/actions/users.actions.ts:

-export function usersForHousehold(householdId: string) {
-  return db.select({});
-}
+/**
+ * Retrieves all users belonging to a given household.
+ * @param householdId – ID of the household to filter users by
+ * @returns Array of user records for the specified household
+ */
+export async function usersForHousehold(householdId: string) {
+  return await db
+    .select({
+      id: schema.users.id,
+      email: schema.users.email,
+      // add any other user fields you need
+    })
+    .from(schema.users)
+    .innerJoin(
+      schema.usersToHouseholds,
+      eq(schema.users.id, schema.usersToHouseholds.userId)
+    )
+    .where(eq(schema.usersToHouseholds.householdId, householdId));
+}
📝 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.

Suggested change
export function usersForHousehold(householdId: string) {
return db.select({});
}
/**
* Retrieves all users belonging to a given household.
* @param householdId ID of the household to filter users by
* @returns Array of user records for the specified household
*/
export async function usersForHousehold(householdId: string) {
return await db
.select({
id: schema.users.id,
email: schema.users.email,
// add any other user fields you need
})
.from(schema.users)
.innerJoin(
schema.usersToHouseholds,
eq(schema.users.id, schema.usersToHouseholds.userId)
)
.where(eq(schema.usersToHouseholds.householdId, householdId));
}
🧰 Tools
🪛 GitHub Check: Linter

[failure] 17-17:
'householdId' is defined but never used

const all = await Promise.allSettled(
ids.id.map(async (id) => {
const validatedData = validate(rawData[id], updatePaymentValidator);
const validatedData = updatePaymentValidator.assert(rawData[id]);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Direct assertion of validated data.

Replaced previous validation approach with direct assertion using updatePaymentValidator.assert(), which simplifies the code but may throw exceptions rather than returning errors.


🏁 Script executed:

#!/bin/bash
# Check how error handling differs between the previous approach and .assert()
rg -A 5 "\.assert\(" node_modules/arktype

Length of output: 104


🏁 Script executed:

#!/bin/bash
set -e

echo "🔍 Searching for updatePaymentValidator occurrences in the repository"
rg -n "updatePaymentValidator" -C 3 .

echo
echo "📋 Showing import section of payments.actions.ts"
sed -n '1,30p' apps/website/src/lib/server/actions/payments.actions.ts

Length of output: 2698


Handle validation errors explicitly instead of using .assert()

The call to

const validatedData = updatePaymentValidator.assert(rawData[id]);

will throw on validation failure, causing an uncaught exception rather than returning structured errors. To maintain robust error handling, replace it with a non‑throwing approach or wrap the assertion in a try/catch. For example:

// Using parse() to capture errors
const result = updatePaymentValidator.parse(rawData[id]);
if (!result.success) {
  // return or throw a Kit error response with result.error.message
  throw error(400, { message: result.error.message });
}
const validatedData = result.data;

—or—

// Wrapping assert() in try/catch
let validatedData;
try {
  validatedData = updatePaymentValidator.assert(rawData[id]);
} catch (validationError) {
  throw error(400, { message: validationError.message });
}

• File: apps/website/src/lib/server/actions/payments.actions.ts
• Line: 166

'notes?': 'string',
'amount?': 'number>0',
'currency?': 'string>=3',
currency: base.currency.default('USD'),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Added default currency value.

The currency field now has a default value of 'USD', which ensures data consistency even when the currency isn't explicitly provided.

Verify that all existing code that relies on bill validation handles the new required currency field with default:


🏁 Script executed:

#!/bin/bash
# Check for places in the codebase that might not specify currency
rg "currency.*undefined" --type=ts

Length of output: 173


🏁 Script executed:

#!/bin/bash
# Inspect the currency schema definition in the bills validator
rg "currency" -n apps/website/src/lib/components/validators/bills.ts

Length of output: 113


#!/bin/bash
# Display the bill validator schema
echo "=== apps/website/src/lib/components/validators/bills.ts ==="
sed -n '1,200p' apps/website/src/lib/components/validators/bills.ts

# Show how +page.server.ts provides currency to validation
echo -e "\n=== apps/website/src/routes/dashboard/bills/+page.server.ts (context) ==="
rg -n -C3 "currency:" apps/website/src/routes/dashboard/bills/+page.server.ts

🏁 Script executed:

#!/bin/bash
# Inspect the bills validator exports
rg -n "export.*" apps/website/src/lib/components/validators/bills.ts

# Display the full validator schema to confirm signature and usage
sed -n '1,200p' apps/website/src/lib/components/validators/bills.ts

# Find all places where the bills validator is used (parse, safeParse, etc.)
rg -nE "(bills?Validator|\.parse|\.safeParse)" -n apps/website

Length of output: 733


🏁 Script executed:

#!/bin/bash
# Display +page.server.ts content for bill routes
sed -n '1,200p' apps/website/src/routes/dashboard/bills/+page.server.ts

# Find all usages of newBillValidator across the codebase
rg -n "newBillValidator" -n apps/website

Length of output: 4722


Ensure the default currency is applied in updateBill

We’ve added a default of 'USD' to the base bill validator, but the updateBill action in
apps/website/src/routes/dashboard/bills/+page.server.ts still sets currency to undefined when none is supplied. This bypasses the schema default. Please update it to fall back to 'USD', for example:

 apps/website/src/routes/dashboard/bills/+page.server.ts
@@ updateBill: async ({ request, locals }) => {
-       currency: data['currency'] ? data['currency'].toUpperCase() : undefined,
+       currency: data['currency']
+         ? data['currency'].toUpperCase()
+         : 'USD',

Additionally, we didn’t find any usages of baseBillValidator or newBillValidator in our route handlers—consider using the shared validator for both create and update to automatically apply defaults and keep validation logic DRY.

📝 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.

Suggested change
currency: base.currency.default('USD'),
--- a/apps/website/src/routes/dashboard/bills/+page.server.ts
+++ b/apps/website/src/routes/dashboard/bills/+page.server.ts
@@ updateBill: async ({ request, locals }) => {
- currency: data['currency'] ? data['currency'].toUpperCase() : undefined,
+ currency: data['currency']
+ ? data['currency'].toUpperCase()
+ : 'USD',

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant