derrick/ai-wave-02
1
0
Fork 0
Code Issues 4 Pull Requests 17 Actions Packages Projects 1 Releases Wiki Activity

fixed issue with searcing #5

Open
notsicilian wants to merge 3 commits from fs-fix-search-order into master
pull from: fs-fix-search-order
merge into: :master
Conversation 3 Commits 3 Files Changed 12
+269 -5
notsicilian commented 2026-09-16 00:57:14 +00:00
Collaborator
Copy Link
Copy Source

Summary

Order search on the orders list was case-sensitive: searching JOHN returned no results for customers named "John". Employees couldn't find orders by customer name, email, or order number unless they typed the exact stored case in the search box.

Ticket / Assignment

  • Ticket: Assignment 1 — order search (covered by order-search-assignment-1.test.ts)
  • Assignment: order search behavior

What changed

  • apps/api/src/orders/order-repository.ts: applied lower() to orderNumber, customer name, customer email, and the query value in the instr filter so matching is case-insensitive.
  • apps/api/src/orders/order-search-assignment-1.test.ts: null-safe set-cookie header extraction in the test login helper.

How to test

bun run check
bun run test
bun run build:web

Manual checks:

  • As an employee, GET /api/orders?query=JOHN returns ORD-1001 and ORD-1003.
  • The same query in lowercase (john) returns identical results.

Screenshots or recordings

N/A. API/behavioral change only.

Risks / follow-ups

  • lower() prevents index usage on the searched columns; fine at current data volume, but revisit with a functional index or full-text search as data grows.
  • Behavior change is user-facing and backward compatible; no callers relied on case-sensitive matches.

PR checklist

  • change is scoped to one ticket or one small reviewable concern
  • acceptance criteria were checked
  • code follows CONTRIBUTING.md
  • no unrelated refactor is mixed in
  • debug code and temporary logs are removed
  • relevant automated checks were run locally
  • changed behavior was manually tested when needed (over HTTP; browser pass pending)
  • known risks or follow-ups are called out
## Summary Order search on the orders list was case-sensitive: searching `JOHN` returned no results for customers named "John". Employees couldn't find orders by customer name, email, or order number unless they typed the exact stored case in the search box. ## Ticket / Assignment - Ticket: Assignment 1 — order search (covered by `order-search-assignment-1.test.ts`) - Assignment: order search behavior ## What changed - `apps/api/src/orders/order-repository.ts`: applied `lower()` to `orderNumber`, customer `name`, customer `email`, and the query value in the `instr` filter so matching is case-insensitive. - `apps/api/src/orders/order-search-assignment-1.test.ts`: null-safe `set-cookie` header extraction in the test login helper. ## How to test ```bash bun run check bun run test bun run build:web ``` Manual checks: - As an employee, GET /api/orders?query=JOHN returns ORD-1001 and ORD-1003. - The same query in lowercase (john) returns identical results. ## Screenshots or recordings N/A. API/behavioral change only. ## Risks / follow-ups - lower() prevents index usage on the searched columns; fine at current data volume, but revisit with a functional index or full-text search as data grows. - Behavior change is user-facing and backward compatible; no callers relied on case-sensitive matches. ## PR checklist - [x] change is scoped to one ticket or one small reviewable concern - [x] acceptance criteria were checked - [x] code follows `CONTRIBUTING.md` - [x] no unrelated refactor is mixed in - [x] debug code and temporary logs are removed - [x] relevant automated checks were run locally - [x] changed behavior was manually tested when needed (over HTTP; browser pass pending) - [x] known risks or follow-ups are called out
notsicilian added 1 commit 2026-09-16 00:57:15 +00:00
fixed issue with searcing
Runner Test / test (push) Successful in 17s
Details
Runner Test / test (pull_request) Successful in 14s
Details
8f619214f0
aziz_28002 added a new dependency 2026-09-16 08:51:28 +00:00
#1 Assignment 1
apps/api/src/orders/order-repository.ts
@@ -42,3 +42,1 @@
sql`instr(${orders.orderNumber}, ${search}) > 0`,
sql`instr(${users.name}, ${search}) > 0`,
sql`instr(${users.email}, ${search}) > 0`
sql`instr(lower(${orders.orderNumber}), lower(${search})) > 0`,
derrick commented 2026-09-18 15:41:09 +00:00
Owner
Copy Link
Copy Source

Instead of using lower() on search here, apply it above so that we don't cast it 3x here.

Instead of using `lower()` on `search` here, apply it above so that we don't cast it 3x here.
aziz_28002 commented 2026-09-24 14:00:23 +00:00
Collaborator
Copy Link
Copy Source

Clear, Sir I will

Clear, Sir I will
apps/api/src/orders/order-search-assignment-1.test.ts
@@ -8,3 +8,3 @@
function firstHeader(value: string | string[] | undefined) {
return Array.isArray(value) ? value[0] : value ?? "";
return Array.isArray(value) ? (value[0] ?? "") : value ?? "";
derrick commented 2026-09-18 15:41:23 +00:00
Owner
Copy Link
Copy Source

Assignment test should not be modified.

Assignment test should not be modified.
aziz_28002 commented 2026-09-24 14:02:08 +00:00
Collaborator
Copy Link
Copy Source

Clear

Clear
derrick commented 2026-09-18 15:44:04 +00:00
Owner
Copy Link
Copy Source

Summary needs to be the summary of the PR, not the ticket. Can just link the ticket in ticket/assignment next time. The update to the code should pass the test, just that a slight change would be better. Otherwise, good job on the PR.

Summary needs to be the summary of the PR, not the ticket. Can just link the ticket in ticket/assignment next time. The update to the code should pass the test, just that a slight change would be better. Otherwise, good job on the PR.
aziz_28002 commented 2026-09-24 14:03:00 +00:00
Collaborator
Copy Link
Copy Source

Clear, Sir I will consider these points next PR

Clear, Sir I will consider these points next PR
notsicilian added 2 commits 2026-09-24 21:46:50 +00:00
add:assignment 2 tests 4f6c68ad29
assignment 2 ui changes
Runner Test / test (push) Successful in 17s
Details
Runner Test / test (pull_request) Successful in 16s
Details
98801089e5
All checks were successful
Runner Test / test (push) Successful in 17s
Details
Runner Test / test (pull_request) Successful in 16s
Details
This pull request has changes conflicting with the target branch.
  • apps/api/src/db/init-db.ts
  • apps/api/src/db/schema/orders.ts
  • apps/api/src/orders/order-repository.ts
  • apps/api/src/orders/order-routes.ts
  • apps/web/src/orders/order-detail-page.tsx
  • apps/web/src/orders/order-notes.tsx
  • apps/web/src/orders/order-page.tsx
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fs-fix-search-order:fs-fix-search-order
git checkout fs-fix-search-order
Sign in to join this conversation.
Reviewers
No Reviewers
derrick
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
ahmedhnoor aziz_28002 cq0622 dsel karinah0333_16363 notsicilian riso8739 derrick
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Blocks
#1 Assignment 1
derrick/ai-wave-02
Reference: derrick/ai-wave-02#5