Skip to content

fix: prevent infinite timezone update loop for delegate sessions#84232

Closed
mayuegood wants to merge 2 commits intoExpensify:mainfrom
mayuegood:main
Closed

fix: prevent infinite timezone update loop for delegate sessions#84232
mayuegood wants to merge 2 commits intoExpensify:mainfrom
mayuegood:main

Conversation

@mayuegood
Copy link
Copy Markdown

@mayuegood mayuegood commented Mar 5, 2026

Context

Fixes issue #83765 - Infinite timezone update loop on multi-device setups with different timezones.

Changes

  • Skip auto-timezone updates for copilot/delegate sessions
  • Throttle timezone updates to once per hour to prevent infinite loop

Related Issues

$ #83765

@melvin-bot melvin-bot bot requested review from joekaufmanexpensify and removed request for a team March 5, 2026 03:47
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 5, 2026


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


马子夜 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

@melvin-bot melvin-bot bot requested review from grgia and removed request for a team March 5, 2026 03:48
@melvin-bot
Copy link
Copy Markdown

melvin-bot bot commented Mar 5, 2026

@grgia 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]

let lastUpdateTimestamp = 0;

const useAutoUpdateTimezone = () => {
const currentUserPersonalDetails = useCurrentUserPersonalDetails();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ CLEAN-REACT-PATTERNS-2 (docs)

currentUserPersonalDetails?.account accesses a property that does not exist on the PersonalDetails type (nor on CurrentUserPersonalDetails). This will always be undefined, so isActingAsDelegateSelector(account) will always return false, making the delegate check completely non-functional.

The isActingAsDelegateSelector expects OnyxEntry<Account> from the ONYXKEYS.ACCOUNT Onyx key, not a field from personal details. The established pattern used elsewhere in the codebase (e.g., ProductTrainingContext/index.tsx:62, AddNewCardPage.tsx:50) is:

import useOnyx from '@hooks/useOnyx';
import ONYXKEYS from '@src/ONYXKEYS';

const [isActingAsDelegate] = useOnyx(ONYXKEYS.ACCOUNT, {selector: isActingAsDelegateSelector});

Replace lines 12-13 with:

const [isActingAsDelegate] = useOnyx(ONYXKEYS.ACCOUNT, {selector: isActingAsDelegateSelector});

And update the references from isDelegate to isActingAsDelegate (or rename as preferred).


Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3eb6e3abd9

ℹ️ 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".

Comment on lines +14 to +15
const account = currentUserPersonalDetails?.account;
const isDelegate = isActingAsDelegateSelector(account);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Read delegate state from account Onyx key

In useAutoUpdateTimezone, isActingAsDelegateSelector is called with currentUserPersonalDetails?.account, but useCurrentUserPersonalDetails() provides personal-details data (timezone/accountID/etc.), not the ACCOUNT Onyx object that contains delegatedAccess. As a result account is undefined and isDelegate stays false, so delegate sessions still run updateAutomaticTimezone, leaving the delegate loop this change intended to stop effectively unfixed.

Useful? React with 👍 / 👎.


const THROTTLE_INTERVAL_MS = 60 * 60 * 1000; // 1 hour

let lastUpdateTimestamp = 0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Scope timezone throttle per active account

The throttle state is stored in a module-level lastUpdateTimestamp, so a timezone update for one signed-in context suppresses updates for every later context in the same app process for an hour. If a user switches accounts/delegators without a full reload, the next active account can keep a stale automatic timezone despite a mismatch, because this shared timestamp blocks its first sync.

Useful? React with 👍 / 👎.

- Use useOnyx to get Account data from ONYXKEYS.ACCOUNT (contains delegatedAccess)
- Use useRef to store per-account timestamps instead of global variable
- Each account now has its own throttle timer
@mayuegood
Copy link
Copy Markdown
Author

Updated Fix

I've updated the PR with corrections for the two issues identified:

Fix 1: Correct Account data location

  • Changed from using PersonalDetails to useOnyx(ONYXKEYS.ACCOUNT)
  • The delegatedAccess data is stored in Account, not PersonalDetails

Fix 2: Per-account throttle timer

  • Changed from global lastUpdateTimestamp variable to useRef<Record<number, number>>
  • Each account now has its own throttle timer
  • Switching accounts no longer affects other accounts' throttle timing

The fix has been pushed to the PR.

@mjasikowski
Copy link
Copy Markdown
Contributor

@mayuegood This PR is against the rules of our contributor program. You're required to submit a proposal in the parent issue first and submit a PR only when your proposal is accepted.

@mjasikowski mjasikowski closed this Mar 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants