Draft
Conversation
Collaborator
- Add trailConditionReports table to DB schema with surface, condition, hazards, water crossings fields - Create DB migration SQL (0033_trail_conditions.sql) - Create API routes for trail conditions (list, create, list-mine, delete) - Register trail conditions routes in API index - Add enableTrailConditions feature flag to config.ts - Create trail-conditions feature module with types, store, hooks, components - Offline-first store using Legend State + SQLite persistence - SubmitConditionReportForm, TrailConditionReportCard, ConditionBadge components - Replace static mock data in trail-conditions screen with real API data - Add submit report modal to trail conditions screen - Add post-hike condition report prompt to trip detail screen - Add i18n translations for trail conditions Co-authored-by: andrew-bierman <94939237+andrew-bierman@users.noreply.github.com>
- Use c.req.valid('json') instead of c.req.json() in createReport handler
- Add PUT /api/trail-conditions/:reportId endpoint for updates
- Log errors in SubmitConditionReportForm catch block
- Use @ts-expect-error instead of @ts-ignore in useSubmitTrailConditionReport
- Validate date string in formatRelativeTime to handle Invalid Date
Co-authored-by: andrew-bierman <94939237+andrew-bierman@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Add trail condition reporting system for hikers
feat: Trail Condition Reporting System (MVP)
Mar 9, 2026
Collaborator
|
@coderabbitai Please review this PR for code quality, best practices, and potential issues. |
🧩 Analysis chain🏁 Script executed: Length of output: 1027 🏁 Script executed: Length of output: 18346 🏁 Script executed: Length of output: 5530 🏁 Script executed: Length of output: 22901 🏁 Script executed: Length of output: 7446
🔴 Critical Issues1.
|
| Query pattern | Missing index |
|---|---|
WHERE user_id = ? (sync endpoint) |
user_id |
WHERE deleted = false ORDER BY created_at (list endpoint) |
(deleted, created_at) |
WHERE trail_name = ? (trail filter) |
trail_name |
-- Add to migration
CREATE INDEX "trail_condition_reports_user_id_idx" ON "trail_condition_reports" ("user_id");
CREATE INDEX "trail_condition_reports_active_created_idx" ON "trail_condition_reports" ("deleted", "created_at" DESC);
CREATE INDEX "trail_condition_reports_trail_name_idx" ON "trail_condition_reports" ("trail_name");And in schema.ts:
export const trailConditionReports = pgTable(
'trail_condition_reports',
{ /* existing columns */ },
(table) => ({
userIdIdx: index('trail_condition_reports_user_id_idx').on(table.userId),
activeCreatedIdx: index('trail_condition_reports_active_created_idx').on(table.deleted, table.createdAt),
trailNameIdx: index('trail_condition_reports_trail_name_idx').on(table.trailName),
}),
);5. Hard-coded strings bypass i18n in trail-conditions.tsx
File: apps/expo/app/(app)/trail-conditions.tsx
Despite the i18n namespace being defined in en.json, the screen has numerous hard-coded English strings:
title="Trail Conditions" // should be t('trailConditions.title')
"+ Report" // no i18n key
"Report Trail Conditions" // t('trailConditions.reportConditionsTitle')
"Cancel" // t('common.cancel')
"Current trail conditions from recent hiker reports" // no i18n key
"No trail condition reports yet. Be the first to report!" // t('trailConditions.noReports') exists
"Submit a Report" // t('trailConditions.submitReport') existsformatRelativeTime in TrailConditionReportCard.tsx is also fully hard-coded English ("Today", "1 day ago", etc.) with no i18n support.
6. isSubmitting state is misleading — errors from store sync are silent
File: apps/expo/features/trail-conditions/components/SubmitConditionReportForm.tsx
submitReport() from useSubmitTrailConditionReport is synchronous — it writes to the Legend-State observable and returns immediately. The try/catch in handleSubmit will never catch a network error (those happen asynchronously in the sync layer). As a result:
- The success
Alertalways shows, even if the report will fail to sync. isSubmittingflips totrueand back tofalsewithin the same synchronous tick.
The submit UX should reflect the offline-first nature of the store — the report is saved locally and will sync in the background:
const handleSubmit = () => { // no async needed
if (!trailName.trim()) {
Alert.alert(t('common.error'), t('trailConditions.trailNameRequired'));
return;
}
submitReport({ ... });
// Report is queued for sync — just dismiss
onSuccess?.();
};🟡 Medium Priority
7. Double DB round-trip in updateReport
File: packages/api/src/routes/trailConditions/reports.ts
The handler calls UPDATE then a separate findFirst to get the updated row. Use .returning() instead:
// Replace:
await db.update(trailConditionReports).set(updateData).where(...);
const updated = await db.query.trailConditionReports.findFirst(...);
return c.json(updated!, 200);
// With:
const [updated] = await db
.update(trailConditionReports)
.set(updateData)
.where(and(eq(trailConditionReports.id, reportId), eq(trailConditionReports.userId, auth.userId)))
.returning();
if (!updated) return c.json({ error: 'Report not found or unauthorized' }, 403);
return c.json(updated, 200);This also removes the non-null assertion (updated!).
8. updateData typed as Record<string, unknown> bypasses type safety
File: packages/api/src/routes/trailConditions/reports.ts
const updateData: Record<string, unknown> = { updatedAt: new Date() };This should use the Drizzle inferred type:
import type { NewTrailConditionReport } from '@packrat/api/db/schema';
const updateData: Partial<NewTrailConditionReport> & { updatedAt: Date } = {
updatedAt: new Date(),
};9. Redundant id check in createReport
File: packages/api/src/routes/trailConditions/reports.ts
const data = c.req.valid('json'); // Zod already requires id
if (!data.id) return c.json({ error: 'Report ID is required' }, 400); // unreachableSince CreateReportRequestSchema has id: z.string() (non-optional), this check is unreachable and can be removed.
10. No upper bound on water crossings stepper
File: apps/expo/features/trail-conditions/components/SubmitConditionReportForm.tsx
The increment button has no cap:
onPress={() => setWaterCrossings(waterCrossings + 1)}A reasonable upper bound (e.g., 20) would prevent nonsensical data:
onPress={() => setWaterCrossings(Math.min(20, waterCrossings + 1))}11. No enum constraints at the DB level
File: packages/api/src/db/schema.ts
surface and overallCondition are plain text columns — invalid values can be written directly to the DB (bypassing the API). Consider using Postgres enums or a check constraint:
surface: text('surface')
.$type<TrailSurface>()
.notNull(),
// + add a check constraint in the migration12. Community reports not cached offline
File: apps/expo/features/trail-conditions/hooks/useTrailConditionReports.ts
useTrailConditionReports uses React Query with a 5-minute staleTime but has no persistence layer. Community trail condition data is unavailable when offline. The PR description mentions "Cached for offline access" as a requirement — consider persisting the React Query cache or reading from the local store as a fallback.
🔵 Minor / Nits
@ts-expect-errorinuseSubmitTrailConditionReport.ts: Consider usingtrailConditionReportsStore.set({ [id]: newReport })or the appropriate Legend-State typed API to avoid suppressing TypeScript errors.TrailConditionReportInStoreomitsuserIdbut the type comment doesn't explain why. A brief JSDoc would help future maintainers.- Photos field:
photosalways submits[]— the schema and API support it but the UI never allows photo capture. This should at minimum be tracked as a follow-up issue. - Accessibility:
Pressableelements inSubmitConditionReportFormandTrailConditionsScreenlackaccessibilityLabelandaccessibilityRoleprops. - Migration timestamp: The journal entry for
0033_trail_conditionshas"when": 1741514518951(March 2025), which is earlier than0032(1760175950793— October 2025). This may cause ordering issues in migration tooling.
✅ Actions performed
Review triggered.
Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.
📝 WalkthroughWalkthroughThis PR introduces a comprehensive trail condition reporting system to the PackRat application. It adds frontend components and hooks for submitting and viewing trail condition reports, a backend API with full CRUD operations, a reactive client-side store with SQLite persistence and sync, database schema and migrations, and localization support. The feature is gated behind a flag and integrates into existing screens. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client as Expo Client
participant Store as Observable Store
participant API as Backend API
participant DB as Database
rect rgba(100, 150, 200, 0.5)
Note over User,DB: Submitting a Trail Condition Report
User->>Client: Opens form and submits report
Client->>Store: Calls submitReport() via hook
Store->>Store: Generate ID with nanoid
Store->>Store: Create TrailConditionReportInStore
Store->>Store: Update observable store
Client->>Client: Show success alert
Note over Store: Sync triggered in background
Store->>API: POST /api/trail-conditions
API->>DB: Insert trail_condition_reports row
DB-->>API: Confirm insertion
API-->>Store: Return created report
Store->>Store: Update local store with server response
end
rect rgba(150, 100, 150, 0.5)
Note over User,DB: Fetching Trail Condition Reports
User->>Client: Opens trail conditions screen
Client->>Client: useTrailConditionReports hook triggers
Client->>API: GET /api/trail-conditions?trailName=...
API->>DB: Query trail_condition_reports (active reports)
DB-->>API: Return matching reports
API-->>Client: Return TrailConditionReport array
Client->>Client: Update React query state
Client->>Client: Render TrailConditionReportCard components
Client-->>User: Display reports with badges and details
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Collaborator
|
@copilot CodeRabbit has provided review feedback on this PR. Please address the comments and suggestions. Once you've made the necessary changes, I'll ask CodeRabbit to re-review. |
- Move /mine route before /{reportId} to prevent future routing hazard
- Fix /mine to filter soft-deleted reports and apply updatedAt incremental sync param
- Remove unreachable !data.id guard in createReport (Zod enforces id: string)
- Replace double DB round-trip in updateReport with single .returning() call
- Type updateData as Partial<NewTrailConditionReport> instead of Record<string, unknown>
- Replace two-query delete with atomic .returning() check in deleteReport
- Add DB indexes (userId, (deleted, createdAt), trailName) to schema and migration
- Fix migration journal timestamp for 0033_trail_conditions (was Mar 2025, now Mar 2026)
- Replace all hard-coded strings in trail-conditions.tsx with t() calls
- Add missing i18n keys: reportButton, subtitle, loadError, disclaimer
- Add accessibilityLabel/Role to interactive Pressable elements in TrailConditionsScreen
- Make SubmitConditionReportForm.handleSubmit synchronous (offline-first, store is sync)
- Remove misleading isSubmitting state and async try/catch from submit form
- Cap water crossings stepper at 20 to prevent nonsensical values
Co-authored-by: andrew-bierman <94939237+andrew-bierman@users.noreply.github.com>
Co-authored-by: andrew-bierman <94939237+andrew-bierman@users.noreply.github.com>
Contributor
Author
Addressed all actionable CodeRabbit feedback in commits Critical / High
Database
Mobile
|
Collaborator
|
@copilot CodeRabbit has reviewed this PR and provided feedback. Please address the review comments and push updates. Once you've made the changes, the monitor will ask CodeRabbit to re-review. |
Collaborator
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (4)
packages/api/src/db/schema.ts (1)
287-288: Consider adding DB-level constraints forsurfaceandoverallCondition.These columns accept free-form text but have a defined set of valid values (
paved | gravel | dirt | rocky | snow | mudandexcellent | good | fair | poor). While Zod validates at the API layer, DB-level constraints would provide defense-in-depth against invalid data from direct DB access or future code paths.♻️ Optional: Add pgEnum or CHECK constraints
+const trailSurfaceEnum = pgEnum('trail_surface', ['paved', 'gravel', 'dirt', 'rocky', 'snow', 'mud']); +const overallConditionEnum = pgEnum('overall_condition', ['excellent', 'good', 'fair', 'poor']); + export const trailConditionReports = pgTable( 'trail_condition_reports', { id: text('id').primaryKey(), trailName: text('trail_name').notNull(), trailRegion: text('trail_region'), - surface: text('surface').notNull(), // paved | gravel | dirt | rocky | snow | mud - overallCondition: text('overall_condition').notNull(), // excellent | good | fair | poor + surface: trailSurfaceEnum('surface').notNull(), + overallCondition: overallConditionEnum('overall_condition').notNull(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/db/schema.ts` around lines 287 - 288, Add DB-level constraints for the surface and overallCondition columns by replacing the free-form text columns with enum/check-backed columns: define a pgEnum (or SQL CHECK) for the surface values ("paved","gravel","dirt","rocky","snow","mud") and another for overallCondition ("excellent","good","fair","poor"), then use those enums in the table definition where surface and overallCondition are declared (referencing the surface and overallCondition column symbols) so the schema enforces valid values at the database level in addition to Zod validation.apps/expo/features/trail-conditions/components/SubmitConditionReportForm.tsx (1)
16-27: Separate stored values from localized labels.The form is still using English display text as domain values for hazards, and the other chips render capitalized raw enum strings. That makes localization brittle and will leak mixed-language values into stored reports once non-English labels are added. Use stable ids in state/API payloads and map them to
t(...)labels in the UI.Also applies to: 118-188, 219-237
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/expo/features/trail-conditions/components/SubmitConditionReportForm.tsx` around lines 16 - 27, The form currently uses localized display text as stored domain values (see SURFACE_OPTIONS, CONDITION_OPTIONS, HAZARD_OPTIONS, WATER_DIFFICULTY_OPTIONS in SubmitConditionReportForm), which will break localization and leak mixed-language values; replace these arrays with stable id/value pairs or enum keys (e.g., { id: 'paved', labelKey: 'trail.surface.paved' }) and update all UI chips/controls to display t(labelKey) while storing/sending only the id; update the code paths that read/write state and build the API payload (the handlers and submission logic referenced around the chips and selection code) to use the id values instead of raw strings so labels are purely presentational.apps/expo/features/trail-conditions/hooks/useSubmitTrailConditionReport.ts (1)
19-20: Replace the@ts-expect-errorstore write with a typed update path.This suppresses the type break on the main insert path, so any change to the store shape or Legend-State API will bypass compile-time protection here. Please switch to a typed root update instead of indexing through an ignored error.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/expo/features/trail-conditions/hooks/useSubmitTrailConditionReport.ts` around lines 19 - 20, Replace the unchecked index write that uses "// `@ts-expect-error`" with a typed root-update on the store: instead of directly doing "trailConditionReportsStore[id].set(newReport)", call the store's typed update/root mutation API (e.g., trailConditionReportsStore.updateRoot(root => { root[id] = newReport }) or the equivalent update(callback) method provided by Legend-State) and remove the ts-expect-error; this ensures the assignment to "id" is type-checked against the store's root shape and still sets the newReport value.apps/expo/features/trail-conditions/types.ts (1)
5-30: Keep the API report type strict and derive the looser store type separately.
packages/api/src/db/schema.tsexposes this entity as the DB select model, but this interface makes server-owned fields likeuserId,createdAt, andupdatedAtoptional. That weakens API consumers and is why downstream code has to guess between server and local timestamps. Consider a strict API DTO plus a separate looserTrailConditionReportInStore.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/expo/features/trail-conditions/types.ts` around lines 5 - 30, Change the exported types so the API DTO is strict and server-owned fields are required: create/rename the strict server-facing interface (e.g., TrailConditionReport) so userId, createdAt, updatedAt (and other server-owned fields) are non-optional/non-nullable, and keep localCreatedAt/localUpdatedAt and any client-only fields optional; then derive the looser store type (TrailConditionReportInStore) from that strict DTO (using Omit/Partial as needed) so downstream code can use the looser shape without weakening the API contract. Update any references to TrailConditionReport/TrailConditionReportInStore accordingly.
🤖 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/expo/app/`(app)/trail-conditions.tsx:
- Line 12: The screen uses useTrailConditionReports() (const { data: reports,
isLoading, error } = useTrailConditionReports()) which currently relies on a
non-persistent React Query cache, so reports vanish after restart or offline
launches; update the hook and/or app bootstrap to persist the React Query cache
(e.g., persistQueryClient using AsyncStorage/Expo SecureStore or the app's
storage layer) and ensure the hook reads from the persisted cache as a fallback
when network is unavailable so reports is populated from persisted data while
offline and after app restarts.
In
`@apps/expo/features/trail-conditions/components/SubmitConditionReportForm.tsx`:
- Around line 119-135: In SubmitConditionReportForm, the Pressable chips (the
one calling setOverallCondition) and the stepper +/- Pressables lack
accessibility metadata; update those Pressable elements (the overall condition
option Pressables and the stepper increment/decrement Pressables) to include
accessibilityRole="button", a computed accessibilityState={{ selected:
overallCondition === cond }} (or { disabled: true } when appropriate), and
explicit accessibilityLabel/accessibilityHint strings that describe the action
and current value (e.g., "Overall condition: Good, double tap to select" for
chips and "Increase hikers count, current 2" / "Decrease hikers count, current
2" for steppers). Ensure labels reflect dynamic state and that the handlers
(setOverallCondition and the stepper increment/decrement functions) are
unchanged except for using these props.
In `@apps/expo/features/trail-conditions/components/TrailConditionReportCard.tsx`:
- Around line 11-23: formatRelativeTime currently returns hard-coded English
strings and shows negative values for future dates; update the function to clamp
negative deltas to 0 (treat future timestamps as "Today") and replace hard-coded
text with a localized formatter or translation calls. Specifically, in
formatRelativeTime(dateStr?: string) ensure diffMs/diffDays are floored at 0,
then produce output via a localization mechanism (e.g. Intl.RelativeTimeFormat
or your i18n.t for keys like "today", "one_day_ago", "n_days_ago",
"one_week_ago", "n_weeks_ago") instead of literal strings so the UI is localized
and future dates render as Today.
- Around line 49-75: TrailConditionReportCard currently renders raw domain
values (report.surface, report.waterCrossingDifficulty, report.hazards) which
bypass localization; replace these direct values with translated labels by
mapping domain values to translation keys and calling t(...) before rendering
(e.g. create helper functions or mapping objects like getSurfaceLabel(surface),
getWaterCrossingDifficultyLabel(difficulty), and map hazards array via
getHazardLabel(h) then join), use the helpers inside the JSX where
report.surface, report.waterCrossingDifficulty and report.hazards are used, and
provide a sensible fallback (e.g. the raw value) if a translation key is
missing.
In `@apps/expo/features/trail-conditions/hooks/useTrailConditionReports.ts`:
- Around line 20-29: The hook useTrailConditionReports is only using in-memory
react-query cache and never hydrates from the local trail-condition store;
update useTrailConditionReports to read local cached reports first (e.g., call
your local store/read method) and pass them into useQuery as initialData (or
populate the cache via queryClient.setQueryData) so the UI is offline-readable
after restart, and also ensure onSuccess or a sync-complete callback merges
locally queued reports with server results (using fetchTrailConditionReports) so
queued reports show immediately even before sync completes.
In `@apps/expo/features/trail-conditions/store/trailConditionReports.ts`:
- Around line 31-33: The updateReport function currently accepts
Partial<TrailConditionReportInStore> which makes id optional and can produce a
PUT to /api/trail-conditions/undefined; change the parameter type so id is
required and only the other fields are optional (e.g. require an id property and
use Partial/Omit or an intersection like { id: <idType> } &
Partial<Omit<TrailConditionReportInStore, 'id'>>), update the function signature
for updateReport accordingly, and keep the existing body using the supplied id
to build the request path.
- Around line 11-18: The listMyReports callback currently ignores the syncedCrud
lastSync cursor, so incremental sync never runs; update the listMyReports
function to accept (_params, { lastSync }) and forward lastSync as an
updatedAtGte query param to the '/api/trail-conditions/mine' GET request (use
lastSync + 1 when lastSync is not null/undefined, otherwise omit the param),
preserving the existing error handling with handleApiError and the thrown Error
message.
In `@apps/expo/features/trips/screens/TripDetailScreen.tsx`:
- Around line 194-219: The trail condition prompt is currently shown for all
trips when featureFlags.enableTrailConditions is true; change the render guard
in TripDetailScreen to also require the trip be completed (e.g., check
trip.status === 'completed' or trip.isCompleted / trip.completedAt timestamp)
before rendering the Card and Button (which calls setShowConditionReport);
update the conditional around the JSX that uses
featureFlags.enableTrailConditions so it combines the feature flag and a
completed-trip check (or compute a helper like isTripCompleted(trip) and use
that) to ensure only post-hike trips see the prompt.
In `@packages/api/drizzle/0033_trail_conditions.sql`:
- Around line 7-11: The migration leaves columns nullable but the app requires
them non-null; update the trail table migration so "hazards" jsonb DEFAULT '[]'
NOT NULL, "photos" jsonb DEFAULT '[]' NOT NULL, and "water_crossings" integer
DEFAULT 0 NOT NULL (leave "water_crossing_difficulty" and "notes" nullable if
still optional). Ensure the ALTER/CREATE statement sets NOT NULL for those three
columns so inserted rows can never have NULL and match the app's required model
for hazards, photos, and waterCrossings.
In `@packages/api/src/routes/trailConditions/reports.ts`:
- Around line 38-52: The CreateReportRequestSchema currently allows any
non-negative integer for waterCrossings; update the waterCrossings field on
CreateReportRequestSchema (the z.number().int().min(0).optional().default(0)
entry) to also include .max(20) so the schema enforces the 20-crossing cap (the
partial/update schemas that derive from this will then inherit the restriction).
- Around line 144-163: The endpoint is failing permanently on retried offline
writes when a client-generated id already exists; wrap the insert of
trailConditionReports (the db.insert(...).values(...).returning() call) in a
try/catch that catches a unique constraint / duplicate key DB error (e.g.,
Postgres 23505) for the id constraint, and when caught query
trailConditionReports for the existing row by id and auth.userId (to ensure the
same user) and treat that as a successful replay by returning that existing
record instead of bubbling the error; if the duplicate id exists but belongs to
a different user, rethrow or return an appropriate error, and for any other DB
error rethrow as before. Ensure you reference the same symbols:
trailConditionReports, auth.userId, and the insertion flow in reports.ts.
---
Nitpick comments:
In
`@apps/expo/features/trail-conditions/components/SubmitConditionReportForm.tsx`:
- Around line 16-27: The form currently uses localized display text as stored
domain values (see SURFACE_OPTIONS, CONDITION_OPTIONS, HAZARD_OPTIONS,
WATER_DIFFICULTY_OPTIONS in SubmitConditionReportForm), which will break
localization and leak mixed-language values; replace these arrays with stable
id/value pairs or enum keys (e.g., { id: 'paved', labelKey:
'trail.surface.paved' }) and update all UI chips/controls to display t(labelKey)
while storing/sending only the id; update the code paths that read/write state
and build the API payload (the handlers and submission logic referenced around
the chips and selection code) to use the id values instead of raw strings so
labels are purely presentational.
In `@apps/expo/features/trail-conditions/hooks/useSubmitTrailConditionReport.ts`:
- Around line 19-20: Replace the unchecked index write that uses "//
`@ts-expect-error`" with a typed root-update on the store: instead of directly
doing "trailConditionReportsStore[id].set(newReport)", call the store's typed
update/root mutation API (e.g., trailConditionReportsStore.updateRoot(root => {
root[id] = newReport }) or the equivalent update(callback) method provided by
Legend-State) and remove the ts-expect-error; this ensures the assignment to
"id" is type-checked against the store's root shape and still sets the newReport
value.
In `@apps/expo/features/trail-conditions/types.ts`:
- Around line 5-30: Change the exported types so the API DTO is strict and
server-owned fields are required: create/rename the strict server-facing
interface (e.g., TrailConditionReport) so userId, createdAt, updatedAt (and
other server-owned fields) are non-optional/non-nullable, and keep
localCreatedAt/localUpdatedAt and any client-only fields optional; then derive
the looser store type (TrailConditionReportInStore) from that strict DTO (using
Omit/Partial as needed) so downstream code can use the looser shape without
weakening the API contract. Update any references to
TrailConditionReport/TrailConditionReportInStore accordingly.
In `@packages/api/src/db/schema.ts`:
- Around line 287-288: Add DB-level constraints for the surface and
overallCondition columns by replacing the free-form text columns with
enum/check-backed columns: define a pgEnum (or SQL CHECK) for the surface values
("paved","gravel","dirt","rocky","snow","mud") and another for overallCondition
("excellent","good","fair","poor"), then use those enums in the table definition
where surface and overallCondition are declared (referencing the surface and
overallCondition column symbols) so the schema enforces valid values at the
database level in addition to Zod validation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2b56afd1-6340-434a-ac47-5c827cf4f161
📒 Files selected for processing (20)
apps/expo/app/(app)/trail-conditions.tsxapps/expo/config.tsapps/expo/features/trail-conditions/components/ConditionBadge.tsxapps/expo/features/trail-conditions/components/SubmitConditionReportForm.tsxapps/expo/features/trail-conditions/components/TrailConditionReportCard.tsxapps/expo/features/trail-conditions/components/index.tsapps/expo/features/trail-conditions/hooks/index.tsapps/expo/features/trail-conditions/hooks/useSubmitTrailConditionReport.tsapps/expo/features/trail-conditions/hooks/useTrailConditionReports.tsapps/expo/features/trail-conditions/index.tsapps/expo/features/trail-conditions/store/trailConditionReports.tsapps/expo/features/trail-conditions/types.tsapps/expo/features/trips/screens/TripDetailScreen.tsxapps/expo/lib/i18n/locales/en.jsonpackages/api/drizzle/0033_trail_conditions.sqlpackages/api/drizzle/meta/_journal.jsonpackages/api/src/db/schema.tspackages/api/src/routes/index.tspackages/api/src/routes/trailConditions/index.tspackages/api/src/routes/trailConditions/reports.ts
| }; | ||
| export default function TrailConditionsScreen() { | ||
| const [showSubmitForm, setShowSubmitForm] = useState(false); | ||
| const { data: reports, isLoading, error } = useTrailConditionReports(); |
There was a problem hiding this comment.
This screen still isn't backed by offline-cached reports.
It renders entirely from useTrailConditionReports(), and the provided hook snippet uses a plain React Query cache with no persistence. After an app restart or offline launch, recent reports disappear, which misses the MVP requirement to cache recent conditions for offline viewing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/expo/app/`(app)/trail-conditions.tsx at line 12, The screen uses
useTrailConditionReports() (const { data: reports, isLoading, error } =
useTrailConditionReports()) which currently relies on a non-persistent React
Query cache, so reports vanish after restart or offline launches; update the
hook and/or app bootstrap to persist the React Query cache (e.g.,
persistQueryClient using AsyncStorage/Expo SecureStore or the app's storage
layer) and ensure the hook reads from the persisted cache as a fallback when
network is unavailable so reports is populated from persisted data while offline
and after app restarts.
apps/expo/features/trail-conditions/components/SubmitConditionReportForm.tsx
Show resolved
Hide resolved
apps/expo/features/trail-conditions/components/TrailConditionReportCard.tsx
Outdated
Show resolved
Hide resolved
apps/expo/features/trail-conditions/components/TrailConditionReportCard.tsx
Show resolved
Hide resolved
Comment on lines
+20
to
+29
| export function useTrailConditionReports(trailName?: string) { | ||
| const { isQueryEnabledWithAccessToken } = useAuthenticatedQueryToolkit(); | ||
|
|
||
| return useQuery({ | ||
| queryKey: ['trailConditionReports', trailName], | ||
| enabled: isQueryEnabledWithAccessToken, | ||
| queryFn: () => fetchTrailConditionReports(trailName), | ||
| staleTime: 1000 * 60 * 5, // 5 min | ||
| refetchOnWindowFocus: false, | ||
| }); |
There was a problem hiding this comment.
This hook still isn't offline-readable.
useQuery is only caching in memory here and never hydrates from the local trail-condition store, so reports disappear after an app restart offline and locally queued reports can't show up in the list until sync succeeds. That misses the MVP offline-viewing requirement.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/expo/features/trail-conditions/hooks/useTrailConditionReports.ts` around
lines 20 - 29, The hook useTrailConditionReports is only using in-memory
react-query cache and never hydrates from the local trail-condition store;
update useTrailConditionReports to read local cached reports first (e.g., call
your local store/read method) and pass them into useQuery as initialData (or
populate the cache via queryClient.setQueryData) so the UI is offline-readable
after restart, and also ensure onSuccess or a sync-complete callback merges
locally queued reports with server results (using fetchTrailConditionReports) so
queued reports show immediately even before sync completes.
apps/expo/features/trail-conditions/store/trailConditionReports.ts
Outdated
Show resolved
Hide resolved
Collaborator
|
@copilot Please address the CodeRabbit review feedback on this PR (11 inline comments). |
- Thread 1 & 5: useTrailConditionReports now seeds initialData from local Legend-State store so reports are visible offline and after restart - Thread 2: Added accessibilityRole, accessibilityState, accessibilityLabel to all chip and stepper Pressables in SubmitConditionReportForm - Thread 3: formatRelativeTime now clamps negative deltas to 0 and uses t() calls with i18n keys (today, oneDayAgo, nDaysAgo, oneWeekAgo, nWeeksAgo) - Thread 4: TrailConditionReportCard maps surface/difficulty/hazard domain values to translation keys before rendering - Thread 6: listMyReports in store forwards lastSync cursor as updatedAt ISO datetime to enable true incremental sync - Thread 7: updateReport parameter type now requires id (string) and makes only the other fields optional - Thread 8: Trail condition prompt in TripDetailScreen now gated to trips with endDate in the past (completed hikes) - Thread 9: hazards, photos, waterCrossings marked NOT NULL in SQL migration and .notNull() in schema.ts - Thread 10: waterCrossings Zod schema adds .max(20) validation - Thread 11: POST / createReport handles Postgres 23505 duplicate-key constraint as idempotent replay for the same user Co-authored-by: andrew-bierman <94939237+andrew-bierman@users.noreply.github.com>
Collaborator
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
- Improve 409 error message: 'Report ID already in use by another user' - Normalize hazard keys to lowercase for case-insensitive API value matching - Document intentional offline seed behavior in useTrailConditionReports - CodeQL: no new alerts Co-authored-by: andrew-bierman <94939237+andrew-bierman@users.noreply.github.com>
Collaborator
|
@copilot There are 11 CodeRabbit inline comments on this PR. Please address the review feedback. |
Collaborator
|
@copilot There are still 11 unresolved CodeRabbit inline comments. Please address the remaining review feedback. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
useTrailConditionReportsformatRelativeTimeand clamp future dateslastSynccursor to/mineAPI endpointidrequired (not optional) inupdateReporthazards,photos,waterCrossingsin SQL migration and schema.max(20)towaterCrossingsin API schemaPOST /idempotent — handle duplicate-key constraint as successful replaySecurity Summary: CodeQL found no alerts. No new vulnerabilities introduced.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
Summary by CodeRabbit