Fix: Android error page on second trip edit attempt

[Copilot]

After a successful trip edit, navigating back to edit the same trip again consistently showed Expo Router's error page. The issue was a throwing assertDefined guard in EditTripScreen combined with a stale global location store leaking between sessions.

Changes

• EditTripScreen — replaced assertDefined(trip) (which throws) with a graceful loading state. A 10-second timeout is added: if the store does not resolve (e.g. stale/deleted trip ID), the spinner is replaced with an error message and a "Go back" button so the user is never stuck indefinitely.

• TripForm — several improvements:

  • Location store lifecycle: A useEffect resets tripLocationStore to null on both mount (clearing stale state from a previous session) and unmount (cleaning up for the next form), preventing location data from one edit leaking into the next.
  • Form default location: defaultValues.location now uses trip?.location instead of the potentially-stale global store value.
  • Clear button for existing locations: The "Clear" button now also appears when the trip has a pre-existing saved location (not only when a new location has just been picked). Clearing resets both the store and the form field.
  • Removed dead assertions: Removed two assertDefined(dateStr) calls on the result of split()[0], which can never be undefined.
  • Date cross-validation: Added a Zod .refine() that ensures end date is not before start date, using Date object comparison.

• location-search.tsx — two improvements:

  • Moved GOOGLE_MAPS_API_KEY derivation to module scope (was re-evaluated on every render).
  • Removed console.warn/console.error statements that leaked internal data in production builds (user feedback is already handled by Alert).

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Android - Error page appears on second edit attempt (Trips tab)</issue_title>
<issue_description>Description

• An error page appears after using the Edit function for a trip in the Trips tab. The first edit attempt works successfully, but the error consistently occurs on the second edit attempt for the same trip.

Steps to Reproduce

• Go to the Trips tab
• Select an existing trip
• Click Edit
• Modify any field
• Save → ✅ Works as expected
• Click Edit again on the same trip
• ❌ Redirects to error page

Screenshots / Screen Recordings
https://github.com/user-attachments/assets/542a5a20-fb28-4807-9ec6-5c7321cebcb6

Expected Behavior
The trip should update successfully every time and return to the trip details or trips list view.

Actual Behavior
The second edit attempt results in an error page.

Environment

• Platform: Android
• Environment: (dev/staging/prod)

Notes

• First edit works normally
• Issue occurs consistently on second attempt</issue_description>

Comments on the Issue (you are @copilot in this section)

@andrew-bierman @copilot Assigned for POC development. Please investigate and create a proof of concept implementation.

• Fixes Android - Error page appears on second edit attempt (Trips tab) #1852

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[andrew-bierman]

@coderabbitai Please review this PR for code quality, best practices, and potential issues.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find the relevant files
fd -e tsx -e ts "EditTripScreen|TripForm" --full-path

Length of output: 100

---

🏁 Script executed:

#!/bin/bash
# Find tripLocationStore usage
rg -n "tripLocationStore" --type=ts --type=tsx -C3
rg -n "tripLocationStore" -C3

Length of output: 3399

---

🏁 Script executed:

#!/bin/bash
cat apps/expo/features/trips/screens/EditTripScreen.tsx
echo "---FILE_SEPARATOR---"
cat apps/expo/features/trips/components/TripForm.tsx
echo "---FILE_SEPARATOR---"
cat apps/expo/features/trips/store/tripLocationStore.ts
echo "---FILE_SEPARATOR---"
cat apps/expo/app/\(app\)/trip/location-search.tsx

Length of output: 18404

---

@andrew-bierman: Here's a thorough review of the PR covering code quality, best practices, and potential issues.

---

Overall Assessment

The core fix is well-reasoned — replacing the throwing assertDefined(trip) with a graceful ActivityIndicator fallback, and using a mount/unmount useEffect to fence off the global location store, correctly addresses both root causes. Below are the issues worth addressing before merge.

---

🔴 High — Infinite Loading Spinner (EditTripScreen.tsx)

If a trip ID is stale (e.g., the trip was deleted server-side), useTripDetailsFromStore will never resolve, and the ActivityIndicator spins forever with no user feedback or escape hatch.

// Current — spins indefinitely
if (!trip) {
  return (
    <SafeAreaView className="flex-1 bg-background items-center justify-center">
      <ActivityIndicator />
    </SafeAreaView>
  );
}

Consider adding a timeout-based error state or, at minimum, a retry/back option:

const [timedOut, setTimedOut] = useState(false);

useEffect(() => {
  const timer = setTimeout(() => setTimedOut(true), 10_000);
  return () => clearTimeout(timer);
}, []);

if (!trip) {
  return (
    <SafeAreaView className="flex-1 bg-background items-center justify-center">
      {timedOut ? (
        <>
          <Text>{t('errors.tripNotFound')}</Text>
          <Pressable onPress={() => router.back()}>
            <Text>{t('common.goBack')}</Text>
          </Pressable>
        </>
      ) : (
        <ActivityIndicator />
      )}
    </SafeAreaView>
  );
}

---

🟡 Medium — Cannot Clear a Pre-Existing Trip Location (TripForm.tsx)

The "Clear" button only appears when location (the reactive store) is non-null. When editing a trip that already has a saved trip?.location, the store starts at null (reset on mount), so the clear button is never shown. The user has no way to remove the existing location without first picking a new one.

// "Clear" only visible if the store has a newly-picked location
{location && (
  <Pressable onPress={() => setLocation(null)}>
    <Text className="text-red-500 font-semibold px-2">{t('common.clear')}</Text>
  </Pressable>
)}

A fix would be to show the clear button whenever either a new location has been picked OR the trip has a saved location:

{(location || trip?.location) && (
  <Pressable
    onPress={() => {
      setLocation(null);
      form.setFieldValue('location', undefined);
    }}
  >
    <Text className="text-red-500 font-semibold px-2">{t('common.clear')}</Text>
  </Pressable>
)}

---

🟡 Medium — Dead assertDefined on dateStr (TripForm.tsx)

Array.prototype.split always returns an array with at least one element, so [0] is never undefined. The assertDefined call cannot fire and gives a false sense of safety.

// Lines in both date pickers
const dateStr = date.toISOString().split('T')[0];
assertDefined(dateStr); // ← dead code; split()[0] is always a string
field.handleChange(dateStr);

Simply remove the assertion:

const dateStr = date.toISOString().split('T')[0];
field.handleChange(dateStr);

---

🟡 Medium — No Start/End Date Cross-Validation (TripForm.tsx)

The Zod schema only checks that each date is non-empty. A user can submit an end date that precedes the start date with no error.

const tripFormSchema = z.object({
  startDate: z.string().min(1, 'Start date is required'),
  endDate: z.string().min(1, 'End date is required'),
  // ...
});

Add a cross-field refinement:

const tripFormSchema = z.object({
  // ...
  startDate: z.string().min(1, 'Start date is required'),
  endDate: z.string().min(1, 'End date is required'),
}).refine(
  ({ startDate, endDate }) => !startDate || !endDate || endDate >= startDate,
  { message: 'End date must be after start date', path: ['endDate'] },
);

---

🟢 Low — GOOGLE_MAPS_API_KEY Re-evaluated on Every Render (location-search.tsx)

The API key is derived inside the component body, so it is re-computed on every render. Move it to module level (or a stable ref/memo):

// location-search.tsx — move outside the component
const GOOGLE_MAPS_API_KEY =
  Constants.expoConfig?.extra?.googleMapsApiKey ||
  process.env.EXPO_PUBLIC_GOOGLE_MAPS_API_KEY;

export default function LocationSearchScreen() { ... }

---

🟢 Low — console.warn / console.error in Production (location-search.tsx)

console.warn('Google Maps response:', data);   // line ~57
console.error('Error searching location:', error); // line ~66

These leak internal data in production builds. Replace with a proper logging abstraction or strip them.

---

✅ Nit — Unused assertDefined Import in EditTripScreen.tsx

assertDefined is imported and correctly used for effectiveId — this is fine. No action needed.

---

Summary Table

Severity	File	Issue
🔴 High	EditTripScreen.tsx	Infinite spinner with no error/timeout recovery
🟡 Medium	TripForm.tsx	Cannot clear pre-existing trip location
🟡 Medium	TripForm.tsx	Dead assertDefined(dateStr) assertion
🟡 Medium	TripForm.tsx	No start/end date cross-validation
🟢 Low	location-search.tsx	API key re-derived on every render
🟢 Low	location-search.tsx	console.warn/console.error in production

---

🧠 Learnings used

Learnt from: CR
Repo: PackRat-AI/PackRat PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-01T10:08:42.658Z
Learning: Applies to apps/expo/app/**/*.{ts,tsx} : When modifying the mobile app, update apps/expo/app/ for screen changes using Expo Router

[andrew-bierman]

@copilot CodeRabbit has provided review feedback on this PR. Please review and address the comments.

[andrew-bierman]

@coderabbitai review

[coderabbitai]

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

[coderabbitai]

Warning

Rate limit exceeded

@andrew-bierman has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 9 minutes and 43 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 73015e4a-037d-4d23-9b34-528b85dd608c

📥 Commits

Reviewing files that changed from the base of the PR and between 3f94763 and 9b29b68.

📒 Files selected for processing (3)

• apps/expo/app/(app)/trip/location-search.tsx
• apps/expo/features/trips/components/TripForm.tsx
• apps/expo/features/trips/screens/EditTripScreen.tsx

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch copilot/fix-trips-tab-edit-error

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[andrew-bierman]

@coderabbitai review

[coderabbitai]

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