feat: order creation #95

davide · 2026-07-15T13:41:13Z

davide commented

2026-07-15 13:41:13 +00:00

No description provided.

davide added 1 commit

2026-07-15 13:41:13 +00:00

feat: order creation

check-commitizen.yaml / Check the PR title (pull_request) Successful in 2s

Details

check-formatter.yaml / Checks with formatters (pull_request) Failing after 12s

Details

check-linter.yaml / Checks with linters (pull_request) Failing after 22s

Details

check-build.yaml / Check that project builds (pull_request) Successful in 1m4s

Details

check-syntax.yaml / Checks with static tools (pull_request) Successful in 26s

Details

check-yamllint.yaml / Lint yaml files (pull_request) Has been cancelled

Details

forgejo-agent / LLM Bot response generation (pull_request) Successful in 2s

Details

7882c72b1c

davide requested review from llm-bot

2026-07-15 13:41:23 +00:00

davide added 1 commit

2026-07-15 13:42:22 +00:00

chore: fix lints

check-formatter.yaml / Checks with formatters (pull_request) Successful in 15s

Details

check-linter.yaml / Checks with linters (pull_request) Successful in 33s

Details

check-build.yaml / Check that project builds (pull_request) Successful in 1m5s

Details

check-yamllint.yaml / Lint yaml files (pull_request) Successful in 2s

Details

check-syntax.yaml / Checks with static tools (pull_request) Successful in 27s

Details

a27cfcc3b8

llm-bot reviewed

2026-07-15 14:22:08 +00:00

llm-bot left a comment

PR Review: feat: order creation

Good work on implementing the order creation flow. Here are my findings:

✅ What's Good

Clean entity design — The Order class in src/lib/entities/cart/order/index.ts is well-structured with clear create and resetCounter methods.
Transaction safety — The counter increment and order insertion are properly wrapped in a single transaction.
Cart cleanup logic — getUserLatestOrCreate correctly excludes already-ordered carts using notExists.
New countItems() method — Useful guard before creating an order.
Price type fix — Changing price from int() to real() on the product table and discount from numeric() to real() on the order table is correct (prices need decimal precision).
Schema separation — Splitting priceSchema and pricePositiveSchema is a good pattern.

🔍 Issues to Address

1. Unsafe type cast on counter value (Medium)

File: src/lib/entities/cart/order/index.ts, line 21

const counterValue = (counterRow.value as number) + 1;

The ordersState.value column is text in the database. Casting directly to number is unsafe — if the value is corrupted or not a valid number, this silently produces NaN. Consider:

const counterValue = (parseInt(counterRow.value, 10) ?? 0) + 1;

2. Inconsistent price validation (Low)

File: src/routes/_schemas.ts vs src/routes/admin/products/[categoryId]/_schemas.ts

The order creation schema uses priceSchema (allows negative values), while admin product forms use pricePositiveSchema (requires ≥ 0). The "discount" field on the home page label says "Modifica prezzo" (Modify price) which is ambiguous — is it a discount or a final price? Consider:

Renaming the field to be clearer (e.g., finalPrice or discountAmount)
Using pricePositiveSchema if the value must be ≥ 0

3. Migration file missing trailing newline (Cosmetic)

File: migrations/0010_curvy_phalanx.sql

The file doesn't end with a newline character. This is a minor style issue that the formatter should catch.

4. Index creation in migration (Cosmetic)

The migration creates product_categoryId index. Verify this isn't already being created by Drizzle's table definition — if it is, the migration will have a duplicate index. Looking at the table definition in product.ts, the index IS defined via .index(), so this could cause a conflict.

ℹ️ Notes (Not blocking)

Receipt printing TODO: The comment // Here we should also print receipts. in Order.create() is fine as a placeholder.
resetCounter unused: The resetCounter method is exported but not called anywhere. This is fine as a utility for future admin use.
Orders page: The orders/+page.svelte has a TODO placeholder — expected for this PR.
No tests: The project has no test files yet. Consider adding tests for the Order entity.

✅ CI Status

format:check — ✅ Passes
lint:check — ⚠️ Biome crashes on the cups workspace (pre-existing, unrelated to this PR)
check (svelte-check) — ✅ Passes (0 errors, only pre-existing warnings)

Verdict: Approved with suggestions

The core logic is sound and well-structured. The main concern is the unsafe type cast on the counter value (issue #1). The other items are lower priority.

## PR Review: feat: order creation Good work on implementing the order creation flow. Here are my findings: ### ✅ What's Good 1. **Clean entity design** — The `Order` class in `src/lib/entities/cart/order/index.ts` is well-structured with clear `create` and `resetCounter` methods. 2. **Transaction safety** — The counter increment and order insertion are properly wrapped in a single transaction. 3. **Cart cleanup logic** — `getUserLatestOrCreate` correctly excludes already-ordered carts using `notExists`. 4. **New `countItems()` method** — Useful guard before creating an order. 5. **Price type fix** — Changing `price` from `int()` to `real()` on the product table and `discount` from `numeric()` to `real()` on the order table is correct (prices need decimal precision). 6. **Schema separation** — Splitting `priceSchema` and `pricePositiveSchema` is a good pattern. ### 🔍 Issues to Address #### 1. Unsafe type cast on counter value (Medium) **File:** `src/lib/entities/cart/order/index.ts`, line 21 ```ts const counterValue = (counterRow.value as number) + 1; ``` The `ordersState.value` column is `text` in the database. Casting directly to `number` is unsafe — if the value is corrupted or not a valid number, this silently produces `NaN`. Consider: ```ts const counterValue = (parseInt(counterRow.value, 10) ?? 0) + 1; ``` #### 2. Inconsistent price validation (Low) **File:** `src/routes/_schemas.ts` vs `src/routes/admin/products/[categoryId]/_schemas.ts` The order creation schema uses `priceSchema` (allows negative values), while admin product forms use `pricePositiveSchema` (requires ≥ 0). The "discount" field on the home page label says "Modifica prezzo" (Modify price) which is ambiguous — is it a discount or a final price? Consider: - Renaming the field to be clearer (e.g., `finalPrice` or `discountAmount`) - Using `pricePositiveSchema` if the value must be ≥ 0 #### 3. Migration file missing trailing newline (Cosmetic) **File:** `migrations/0010_curvy_phalanx.sql` The file doesn't end with a newline character. This is a minor style issue that the formatter should catch. #### 4. Index creation in migration (Cosmetic) The migration creates `product_categoryId` index. Verify this isn't already being created by Drizzle's table definition — if it is, the migration will have a duplicate index. Looking at the table definition in `product.ts`, the index IS defined via `.index()`, so this could cause a conflict. ### ℹ️ Notes (Not blocking) - **Receipt printing TODO**: The comment `// Here we should also print receipts.` in `Order.create()` is fine as a placeholder. - **`resetCounter` unused**: The `resetCounter` method is exported but not called anywhere. This is fine as a utility for future admin use. - **Orders page**: The `orders/+page.svelte` has a TODO placeholder — expected for this PR. - **No tests**: The project has no test files yet. Consider adding tests for the `Order` entity. ### ✅ CI Status - `format:check` — ✅ Passes - `lint:check` — ⚠️ Biome crashes on the `cups` workspace (pre-existing, unrelated to this PR) - `check` (svelte-check) — ✅ Passes (0 errors, only pre-existing warnings) ### Verdict: **Approved with suggestions** The core logic is sound and well-structured. The main concern is the unsafe type cast on the counter value (issue #1). The other items are lower priority.

src/lib/entities/cart/order/index.ts

					
				@ -0,0 +18,4 @@

				      const counterRow = await tx

				        .select({ value: s.ordersState.value })

				        .from(s.ordersState)

				        .where(eq(s.ordersState.key, COUNTER_DB_KEY))

Unsafe type cast: counterRow.value is text from DB but cast directly to number. Use parseInt with fallback.

Unsafe type cast: `counterRow.value` is `text` from DB but cast directly to `number`. Use `parseInt` with fallback.

src/routes/+page.svelte

					
				@ -91,0 +93,4 @@

				      <Form form={orderForm} class="mt-auto">

				        <NumericInput field={orderForm.fields.discount} value={0} label="Modifica prezzo" />

Field labeled 'Modifica prezzo' but named 'discount' — consider renaming for clarity. Also consider using pricePositiveSchema if negative values aren't intended.

Field labeled 'Modifica prezzo' but named 'discount' — consider renaming for clarity. Also consider using `pricePositiveSchema` if negative values aren't intended.

src/routes/_schemas.ts

					
				@ -26,3 +27,7 @@ export const addProductToOrderSchema = v.lazyAsync(async (input) => {

				    ),

				  });

				});

This field name is confusing. 'discount' suggests a reduction, but the UI label says 'Modify price'. Clarify the intent.

davide merged commit 75d03a0952 into main

2026-07-15 14:41:29 +00:00

davide deleted branch feat/order-creation

2026-07-15 14:41:29 +00:00

davide referenced this pull request from a commit

2026-07-15 14:41:30 +00:00

feat: order creation (#95)

Sign in to join this conversation.

No reviewers

No labels

No milestone

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

projects/events-cash-register!95

Rows
Columns