[HR Import] ConnectToHRFlow popup handling and authentication on native#92075
[HR Import] ConnectToHRFlow popup handling and authentication on native#92075JakubKorytko wants to merge 7 commits into
Conversation
|
@codex review |
1 similar comment
|
@codex review |
![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: 1db3b33212
ℹ️ 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".
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.
|
There was a problem hiding this comment.
Pull request overview
Refactors ConnectToHRFlow on native to authenticate via a short-lived auth token appended to the setup URL (rather than injecting Cookie: authToken=... headers), and splits iOS into its own expo-web-browser/openAuthSessionAsync implementation. On Android, the WebView now handles onOpenWindow popups with a back-dismissible overlay and pre-clears cookies via an incognito about:blank load.
Changes:
- Add
getShortLivedAuthTokenURLhelper inLink.tsthat hitsOPEN_OLD_DOT_LINKand appends&authToken=...to a setup link. - Rewrite
ConnectToHRFlow/index.native.tsxto drop session cookie injection, fetch an auth token URL, pre-clear cookies in a hidden incognito WebView, and overlay popups opened viaonOpenWindow. - Introduce
ConnectToHRFlow/index.ios.tsxthat opens the authenticated URL viaopenAuthSessionAsyncwithpreferEphemeralSession: trueand closes the modal when the session ends.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/libs/actions/Link.ts | New exported getShortLivedAuthTokenURL helper. |
| src/components/ConnectToHRFlow/index.native.tsx | Switches to token-in-URL auth; adds popup overlay handling and incognito cookie pre-clear. |
| src/components/ConnectToHRFlow/index.ios.tsx | New iOS implementation using expo-web-browser openAuthSessionAsync. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff25d79e37
ℹ️ 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".
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3dbccc674
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c284f34907
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c284f34907
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cadd73b61
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cadd73b61
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
melvin-bot
Bot
requested review from
ChavdaSachin and
heyjennahay
and removed request for
a team
|
@ChavdaSachin 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] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| const [cookiesCleared, setCookiesCleared] = useState(false); | ||
| const hasFetched = useRef(false); | ||
|
|
||
| const fetchAuthUrl = useCallback(() => { |
There was a problem hiding this comment.
❌ CLEAN-REACT-PATTERNS-0 (docs)
fetchAuthUrl is wrapped in useCallback. React Compiler is enabled in this codebase and automatically memoizes closures based on their captured variables, making manual useCallback redundant.
Remove the useCallback wrapper and use a plain function:
const fetchAuthUrl = () => {
if (hasFetched.current) {
return;
}
hasFetched.current = true;
getShortLivedAuthTokenURL(setupLink).then(setAuthenticatedUrl);
};Note: This assumes the file compiles with React Compiler. If it does not, this flag can be disregarded.
Reviewed at: c43a4ea | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| }; | ||
|
|
||
| const openSession = () => { | ||
| if (hasOpened.current) { |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
The auth URL fetching logic (ref guard, useNetwork with onReconnect, useEffect with offline check) is duplicated between index.ios.tsx and index.native.tsx. Both files implement the same pattern:
- A ref guard (
hasOpened/hasFetched) to prevent double execution useNetwork({onReconnect: ...})for reconnection handlinguseEffectwith offline check and eslint-disable- Calling
getShortLivedAuthTokenURL(setupLink)to obtain an authenticated URL
Extract the shared logic into a common hook (e.g., useAuthenticatedSetupLink) that both platform files can consume:
function useAuthenticatedSetupLink(setupLink: string) {
const [authenticatedUrl, setAuthenticatedUrl] = useState<string | null>(null);
const hasFetched = useRef(false);
const fetchAuthUrl = () => {
if (hasFetched.current) return;
hasFetched.current = true;
getShortLivedAuthTokenURL(setupLink).then(setAuthenticatedUrl);
};
const {isOffline} = useNetwork({onReconnect: fetchAuthUrl});
useEffect(() => {
if (isOffline) return;
fetchAuthUrl();
}, []);
return {authenticatedUrl, isOffline};
}Reviewed at: c43a4ea | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| // available from the incognito session teardown. | ||
| setTimeout(() => setCookiesCleared(true), CONST.MERGE_HR.COOKIE_CLEAR_DELAY_MS); | ||
| }} | ||
| style={styles.opacity0} |
There was a problem hiding this comment.
❌ PERF-12 (docs)
The setTimeout inside the WebView onLoadEnd handler is not cleaned up. If the component unmounts (e.g., user dismisses the modal) before the 500ms timeout fires, setCookiesCleared(true) will be called on an unmounted component.
Consider storing the timeout ID in a ref and clearing it on unmount:
const cookieTimerRef = useRef<ReturnType<typeof setTimeout>>();
// In onLoadEnd:
onLoadEnd={() => {
cookieTimerRef.current = setTimeout(() => setCookiesCleared(true), CONST.MERGE_HR.COOKIE_CLEAR_DELAY_MS);
}}
// Add cleanup effect:
useEffect(() => {
return () => {
if (cookieTimerRef.current) {
clearTimeout(cookieTimerRef.current);
}
};
}, []);Reviewed at: c43a4ea | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
cristipaval
requested review from
bernhardoj
and removed request for
ChavdaSachin
Explanation of Change
ConnectToHRFlowto authenticate via a short-lived auth token URL instead of injecting session cookies into WebView headers.getShortLivedAuthTokenURLhelper inLink.tsthat fetches a token and appends it to the setup link.expo-web-browser'sopenAuthSessionAsync.onOpenWindow, clears cookies with an incognito blank load, and supports a back-navigable popup overlay.Fixed Issues
$ #91246
PROPOSAL: N/A
Tests
Offline tests
FullPageOfflineBlockingViewis displayed, preventing interaction until connectivity is restoredQA Steps
Same as tests
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
Android.mov
Android: mWeb Chrome
iOS: Native
iOS.mov
iOS: mWeb Safari
MacOS: Chrome / Safari