Staging -> Main by cirex-web · Pull Request #247 · ScottyLabs/dining-api

cirex-web · 2026-02-21T23:25:48Z

No description provided.

Ratings avg

add report error endpoint

github-actions · 2026-02-21T23:26:56Z

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	82.86%	474 / 572
🔵	Statements	82.05%	494 / 602
🔵	Functions	80.37%	127 / 158
🔵	Branches	72.8%	257 / 353

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
src/env.ts	80%	100%	50%	80%	21
src/db/dbQueryUtils.ts	82.92%	93.75%	77.77%	84.61%	149-152, 169-172, 216-227
src/db/getLocations.ts	100%	100%	100%	100%
src/db/schema.ts	62.16%	100%	33.33%	62.16%	32, 56, 72, 90, 103, 120, 153, 177-183, 208-211, 238-246

Generated in workflow #467 for commit 35a9f85 by the Vitest Coverage Report Action

sentry · 2026-02-21T23:29:08Z

src/endpoints/misc.ts

-    return {};
+  async ({ cookie, body: { locationId, message } }) => {
+    const session = cookie["session_id"]!.value as string | undefined;
+    const userDetails = await fetchUserDetails(session);


Bug: The /report endpoint schema was updated to remove locationName, but Elysia.js will reject requests from older clients that still send this extra field, causing a validation error.
_{Severity: HIGH}

Suggested Fix

To maintain backward compatibility, configure the Elysia.js route to allow additional properties in the request body. This can be done by setting additionalProperties: true in the t.Object schema for the endpoint, which will instruct the validator to ignore extra fields like locationName.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: src/endpoints/misc.ts#L65 Potential issue: The change to the `/report` endpoint removes the `locationName` parameter from the request body schema. However, the Elysia.js framework, by default, strictly validates incoming requests and rejects any that contain fields not defined in the schema. Since older clients will continue to send the `locationName` field, their requests will fail with a 422 Unprocessable Entity validation error. This breaks backward compatibility for the endpoint, preventing older clients from successfully submitting reports.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

Copilot

Pull request overview

This PR merges staging changes into main, primarily adding persisted “reports” for locations and exposing rating aggregates (ratingsAvg/ratingsCount) in the locations API/schema.

Changes:

Add ratingsAvg + ratingsCount to location responses and update DB/query utilities + tests accordingly.
Add a new reports table/migration and update /report to persist reports (plus add a read endpoint for reports).
Update API schemas and README local setup instructions.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tests/reviews.test.ts	Adds a test for the new ratings average/count aggregation helper.
tests/database.test.ts	Updates expected location output to include `ratingsAvg`/`ratingsCount`.
src/env.ts	Changes env validation for session cookie signing secret (now defaulted).
src/endpoints/schemas.ts	Extends `LocationSchema` with `ratingsAvg`/`ratingsCount`.
src/endpoints/reviews.ts	Adds `GET /v2/locations/:locationId/reports` to return stored reports.
src/endpoints/misc.ts	Updates `POST /report` to look up location name, email/slack, and insert into `reports`.
src/db/schema.ts	Adds `reportsTable` definition.
src/db/getLocations.ts	Plumbs ratings aggregates into location objects returned by `getAllLocationsFromDB`.
src/db/dbQueryUtils.ts	Adds `getRatingsAvgsAndCounts()` query helper.
drizzle/meta/_journal.json	Registers the new migration in Drizzle’s journal.
drizzle/meta/0010_snapshot.json	Drizzle snapshot for the new migration state.
drizzle/0010_shallow_doctor_strange.sql	Migration creating the `reports` table + FKs.
README.md	Improves local DB startup instructions formatting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-21T23:30:04Z

src/db/schema.ts

+  }
+)


reports will likely be queried by location_id (and possibly by time). Adding an index on locationId (and optionally createdAt) would prevent full table scans as reports grow.

Suggested change

}

)

},

(t) => [

index("reports_location_created_idx").on(t.locationId, t.createdAt),

],

);

Copilot · 2026-02-21T23:30:04Z

src/db/dbQueryUtils.ts

+          // since all ratings are from 0 to 5, the whole part of the average cannot
+          // exceeds one. also we make the decimal digits longer for other consumers


The comment describing the avg rating cast is incorrect/unclear ("whole part ... cannot exceeds one"; the average can be up to 5) and has a grammar error. Update the comment to reflect the actual bounds and intent of the decimal(4,3) cast.

Suggested change

// since all ratings are from 0 to 5, the whole part of the average cannot

// exceeds one. also we make the decimal digits longer for other consumers

// Ratings are between 0 and 5, so their average is also in [0, 5] and fits

// in decimal(4,3) (one digit before the decimal, three after). We keep

// three decimal places for higher precision for downstream consumers.

Copilot · 2026-02-21T23:30:04Z

src/env.ts

    .default(false),
  ENV: z.enum(["dev", "staging", "prod"]),
-  SESSION_COOKIE_SIGNING_SECRET: z.string(),
+  SESSION_COOKIE_SIGNING_SECRET: z.string().default("lemon melon cookie"),


SESSION_COOKIE_SIGNING_SECRET is now given a hard-coded default. This weakens session signing if the env var is missing/misconfigured (especially in prod/staging), making cookies forgeable across deployments. Remove the default and require a secret to be explicitly set (or gate a test/dev-only default behind IN_TEST_MODE/ENV !== "prod" and validate minimum length/entropy).

Suggested change

SESSION_COOKIE_SIGNING_SECRET: z.string().default("lemon melon cookie"),

SESSION_COOKIE_SIGNING_SECRET: z

.string()

.min(32, "SESSION_COOKIE_SIGNING_SECRET must be at least 32 characters long"),

Copilot · 2026-02-21T23:30:04Z

src/endpoints/misc.ts

-    );
-    return {};
+  async ({ cookie, body: { locationId, message } }) => {
+    const session = cookie["session_id"]!.value as string | undefined;


cookie["session_id"]! will throw when the request has no session_id cookie, causing /report to 500 for anonymous users. Use optional access for the cookie and let fetchUserDetails handle undefined, or return a 401 only if you intend to require authentication for reporting.

Suggested change

const session = cookie["session_id"]!.value as string | undefined;

const session = cookie["session_id"]?.value as string | undefined;

Copilot · 2026-02-21T23:30:05Z

src/endpoints/misc.ts

+    const reports = await db.select().from(locationDataTable).where(eq(locationDataTable.id, locationId))
+    if (reports.length == 0) {
+      throw new Response(`Invalid location id ${locationId}`, {
+        status: 400,
+      });
+    }
+
+    if (reports.length > 1) {
+      throw new Response(`
+          Expected 1 restaurant corresponding to id=${locationId}. Somehow got 2.
+        `, { status: 500 }) // this should be unreachable
+    }
+
+    const locationName = reports[0]?.name ?? "Unnamed"


The variable reports is actually the location lookup result, and the reports.length > 1 branch should be unreachable because location_data.id is a primary key. Consider renaming the variable (e.g., locations/locationRows) and simplifying the lookup to a single-row query (limit(1)/findFirst) with a single not-found check.

Suggested change

const reports = await db.select().from(locationDataTable).where(eq(locationDataTable.id, locationId))

if (reports.length == 0) {

throw new Response(`Invalid location id ${locationId}`, {

status: 400,

});

}

if (reports.length > 1) {

throw new Response(`

Expected 1 restaurant corresponding to id=${locationId}. Somehow got 2.

`, { status: 500 }) // this should be unreachable

}

const locationName = reports[0]?.name ?? "Unnamed"

const [location] = await db

.select()

.from(locationDataTable)

.where(eq(locationDataTable.id, locationId))

.limit(1);

if (!location) {

throw new Response(`Invalid location id ${locationId}`, {

status: 400,

});

}

const locationName = location.name ?? "Unnamed";

Copilot · 2026-02-21T23:30:05Z

src/endpoints/reviews.ts

+  .get(
+    "/v2/locations/:locationId/reports",
+    async ({ params: { locationId } }) => {
+      const reports = await db.select().from(reportsTable).where(eq(reportsTable.locationId, locationId))
+
+      return reports
+    },
+    {
+      response: t.Array(
+        t.Object({
+          id: t.Number(),
+          userId: t.Nullable(t.Number()),
+          createdAt: t.Date(),
+          locationId: t.String(),
+          message: t.String()
+        })
+      )


GET /v2/locations/:locationId/reports exposes user-submitted report messages (and userId) without any authentication/authorization and is included in the public schema. If these reports are meant for internal moderation/admin use, add an auth check (and ideally an admin/role check) and/or hide the endpoint from OpenAPI to avoid leaking potentially sensitive data.

stanleymw and others added 24 commits January 17, 2026 18:04

add report error endpoint

de17752

feat: add ratingsAvg field for endpoint locations

11c2c7a

chore: fix typo

879287e

chore: fix test

d68120c

chore: explicite type cast

6a38c2c

Merge branch 'staging' into ratingsAvg

f5f40bb

feat: add ratingsCount field for endpoint locations

88089f9

chore: fix test

f946ec8

chore: reword email footer

9a903a4

chore: merge db queries

ead9e3c

chore: more decimal digits

c35a909

Merge branch 'staging' into report-incorrect-information

1159ddb

get report

4519564

test: test cases for ratingsAvg and ratingsCount

ae26237

fix: test dependency

85228b3

fix

e622d69

Merge pull request #243 from ScottyLabs/ratingsAvg

51febbe

Ratings avg

replace old endpoint, fix potential server crash

573544f

move insert into createreport

7a1d88f

move db insert out, so response is more accurate

4b848a8

fix backward compatibility with old frontend

2513ed5

return empty object fix

5f1f21f

Merge pull request #246 from ScottyLabs/report-incorrect-information

0e186df

add report error endpoint

db generate

35a9f85

Copilot AI review requested due to automatic review settings February 21, 2026 23:25

Copilot started reviewing on behalf of cirex-web February 21, 2026 23:26 View session

cirex-web merged commit 0db17c2 into main Feb 21, 2026
8 checks passed

sentry bot reviewed Feb 21, 2026

View reviewed changes

Copilot AI reviewed Feb 21, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Staging -> Main#247

Staging -> Main#247
cirex-web merged 24 commits intomainfrom
staging

cirex-web commented Feb 21, 2026

Uh oh!

Uh oh!

github-actions bot commented Feb 21, 2026

Uh oh!

sentry bot Feb 21, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 21, 2026

Uh oh!

Copilot AI Feb 21, 2026

Uh oh!

Copilot AI Feb 21, 2026

Uh oh!

Copilot AI Feb 21, 2026

Uh oh!

Copilot AI Feb 21, 2026

Uh oh!

Copilot AI Feb 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

-  }
-)
+  },
+  (t) => [
+    index("reports_location_created_idx").on(t.locationId, t.createdAt),
+  ],
+);

		// since all ratings are from 0 to 5, the whole part of the average cannot
		// exceeds one. also we make the decimal digits longer for other consumers

-          // since all ratings are from 0 to 5, the whole part of the average cannot
-          // exceeds one. also we make the decimal digits longer for other consumers
+          // Ratings are between 0 and 5, so their average is also in [0, 5] and fits
+          // in decimal(4,3) (one digit before the decimal, three after). We keep
+          // three decimal places for higher precision for downstream consumers.

	const session = cookie["session_id"]!.value as string \| undefined;
	const session = cookie["session_id"]?.value as string \| undefined;

Conversation

cirex-web commented Feb 21, 2026

Uh oh!

Uh oh!

github-actions bot commented Feb 21, 2026

Coverage Report

Uh oh!

sentry bot Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants