Fix cancellation policy date timezone conversion for hotel and car trips#87755
Fix cancellation policy date timezone conversion for hotel and car trips#87755
Conversation
…imezone The cancellation deadline ISO string from Spotnana contains the venue's local time with a timezone offset. Stripping the offset before parsing ensures the date/time is displayed as-is (venue-local), matching Spotnana. Applied to both hotel and car trip details. Co-authored-by: Eric Han <eh2077@users.noreply.github.com>
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
| // Strip timezone offset (e.g., +07:00, -04:00, Z) to treat the date/time as venue-local | ||
| const naiveDateString = isoDateString.replace(/([+-]\d{2}:\d{2}|Z)$/, ''); | ||
| const date = new Date(naiveDateString); |
There was a problem hiding this comment.
@MelvinBot no, you should use method parseISO, according to #87076 (comment). Please update and add necessary unit tests.
There was a problem hiding this comment.
Updated to use parseISO from date-fns instead of new Date(), and added unit tests covering:
- Positive timezone offset stripping (
+07:00) - Negative timezone offset stripping (
-05:00) - UTC indicator stripping (
Z) - Non-current-year formatting (includes year)
- ISO string without timezone offset
…e parsing Replace new Date(naiveDateString) with parseISO(naiveDateString) in getFormattedCancellationDate as recommended in review. Add unit tests covering timezone offset stripping, current year vs other year formatting, and ISO strings without timezone offsets. Co-authored-by: Eric Han <eh2077@users.noreply.github.com>
| // Strip timezone offset (e.g., +07:00, -04:00, Z) to treat the date/time as venue-local | ||
| const naiveDateString = isoDateString.replace(/([+-]\d{2}:\d{2}|Z)$/, ''); | ||
| const date = parseISO(naiveDateString); |
There was a problem hiding this comment.
@MelvinBot Parse isoDateString directly with method parseISO
| // Strip timezone offset (e.g., +07:00, -04:00, Z) to treat the date/time as venue-local | |
| const naiveDateString = isoDateString.replace(/([+-]\d{2}:\d{2}|Z)$/, ''); | |
| const date = parseISO(naiveDateString); | |
| const date = parseISO(isoDateString); |
There was a problem hiding this comment.
Done — removed the timezone offset stripping and now parsing isoDateString directly with parseISO. Updated unit tests accordingly.
Parse isoDateString directly with parseISO as requested in review, removing the manual timezone offset stripping. Updated unit tests to reflect the new behavior. Co-authored-by: Eric Han <eh2077@users.noreply.github.com>
|
The only failing check is PR Reviewer Checklist / checklist, which is unrelated to the code changes — it tracks whether reviewers have completed their checklist. All code-related checks (ESLint, Prettier, builds, typecheck, etc.) are passing or still pending. No fix needed. |
|
@MelvinBot Mark it ready for review |
|
🤖 I attempted to mark this PR as ready for review, but the command was not authorized in my current environment.
gh pr ready 87755 --repo Expensify/App |
|
@MelvinBot C+ doesn't have write access to mark PR ready for review. Can you try again? |
|
🤖 Done — PR is now marked as ready for review. |
melvin-bot
Bot
requested review from
eh2077 and
trjExpensify
and removed request for
a team
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00171f9115
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const date = parseISO(isoDateString); | ||
| if (isThisYear(date)) { | ||
| return format(date, 'EEEE, MMM d h:mm a'); |
There was a problem hiding this comment.
Parsing cancellationDeadline with parseISO() still applies the embedded offset and then format() renders in the device timezone, so users outside the venue timezone will continue to see shifted times (and potentially the wrong day) for cancellation cutoffs. For example, 2026-04-19T15:00:00+07:00 will display as early morning in US timezones instead of 3:00 PM, which defeats the intended Spotnana-aligned behavior and can mislead users about the true cancellation window.
Useful? React with 👍 / 👎.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
@MelvinBot I think we should use a different approach.
/** Deadline in duration before the check-in date time. */
durationBeforeArrivalDeadline: {
/** ISO 8601 format. */
iso8601: string;
};
Can you try this? |
- Add durationBeforeArrivalDeadline prop to HotelPnr cancellationPolicy type - Update parseDurationToSeconds to support day durations (e.g., P2D, P2DT3H) - Compute hotel cancellationDeadline by subtracting duration from checkInDateTime - Fall back to existing deadline.iso8601 if no duration is available Co-authored-by: Eric Han <eh2077@users.noreply.github.com>
melvin-bot
Bot
requested review from
marcochavezf
and removed request for
a team
|
@marcochavezf Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
Done — updated to use the duration-based approach:
|
trjExpensify
left a comment
trjExpensify
left a comment
There was a problem hiding this comment.
@stitesExpensify do you have eyes on this? A couple of thoughts:
- It sounds like the cancellation date and time in Spotnana is the Hotel's local timezone, but it doesn't actually show that. Spotnana would do better here to include the timezone for clarity. Maybe we can suggest it?
- I agree the date/time we add to the "Cancellation policy" row in the product here should be accurate:
- show it "converted" to the local time of the user
- or show it in the "venue" time but include the timezone in the string so it's clear
As Spotnana doesn't include the timezone on their side, our "conversion" to local does make it a bit confusing - appearing as though there's a mismatch. So maybe the second option is the one to go for, for now?
|
The No code fix is needed. Please re-run the failed job to resolve this. |
The cancellationDeadline and cancellationPolicy fields in the ReservationItem type are typed as optional (string | undefined), but the code was assigning null values, causing a TypeScript error. Co-authored-by: Eric Han <eh2077@users.noreply.github.com>
|
Fixed the typecheck failure: The |
|
The TypeScript check failure was caused by |
|
MelvinBot has made 5 consecutive commits trying to fix CI failures. Automatic CI fix dispatches are now paused for this PR. Next steps: A human should review the recent commits and resolve the conflicting CI errors manually. |
|
The fix effect looks like below, let me know if it makes sense cc @trjExpensify @stitesExpensify 
|
|
Right, I'm saying:
|
Agreed! We should definitely make it automatically show up in the user's timezone (and show the timezone to be super clear) |
|
Yeah, so I'm fine with the local time of check-in.. and that time matches Spotnana... but I think we should clarify the timezone. |
100% |
|
@MelvinBot rework the PR to use solution from this commit f166d76 |
Switch from parseISO + format to formatInTimeZone from date-fns-tz, which renders the date/time in the venue's timezone (embedded as the UTC offset in the ISO string) instead of converting to device-local time. Co-authored-by: Eric Han <eh2077@users.noreply.github.com>
|
Reworked |
|
@MelvinBot revert change of file src/libs/TripReservationUtils.ts and src/types/onyx/TripData.ts |
Reverts getCancellationDeadline helper, parseDurationToSeconds day support, durationBeforeArrivalDeadline type, and null-to-undefined changes per review. Co-authored-by: Eric Han <eh2077@users.noreply.github.com>
|
Reverted all changes to |
| const offsetMatch = isoDateString.match(/([+-]\d{2}:\d{2})$/); | ||
| const venueTimezone = offsetMatch ? offsetMatch[1] : 'UTC'; | ||
| const date = new Date(isoDateString); | ||
| const pattern = isThisYear(date) ? 'EEEE, MMM d h:mm a' : 'EEEE, MMM d, yyyy h:mm a'; |
There was a problem hiding this comment.
@MelvinBot include timezone in pattern and update unit test
| const pattern = isThisYear(date) ? 'EEEE, MMM d h:mm a' : 'EEEE, MMM d, yyyy h:mm a'; | |
| const pattern = isThisYear(date) ? 'EEEE, MMM d h:mm a, zzz' : 'EEEE, MMM d, yyyy h:mm a, zzz'; |
There was a problem hiding this comment.
Done — added zzz to both format patterns and updated unit tests to expect the timezone abbreviation (e.g. GMT+7, UTC).
Add zzz to the format pattern so the venue timezone is displayed alongside the date/time (e.g. "Sunday, Apr 19, 2026 3:00 PM, GMT+7"). Updated unit tests to match. Co-authored-by: Eric Han <eh2077@users.noreply.github.com>
4 participants
Explanation of Change
The cancellation deadline ISO string from Spotnana contains the venue's local date/time with a timezone offset (e.g.,
2026-04-19T15:00:00+07:00). Previously,new Date()converted this to UTC internally, andformat()from date-fns rendered it in the device's local timezone — shifting the displayed date/time away from what Spotnana shows (potentially crossing a day boundary).This PR strips the timezone offset from the ISO string before parsing, so the date/time components are treated as-is (venue-local). This matches what Spotnana displays. The fix is applied to both hotel and car trip cancellation deadlines.
Fixed Issues
$ #87076
PROPOSAL: #87076 (comment)
Tests
Offline tests
N/A — cancellation deadline formatting is a display-only change using data already available locally.
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
N/A — display-only formatting change, no UI layout changes
Android: mWeb Chrome
N/A — display-only formatting change, no UI layout changes
iOS: Native
N/A — display-only formatting change, no UI layout changes
iOS: mWeb Safari
N/A — display-only formatting change, no UI layout changes
MacOS: Chrome / Safari
N/A — display-only formatting change, no UI layout changes