[CP Staging] Revert "Create navigation guards + implement Onboarding Guard"#81455
[CP Staging] Revert "Create navigation guards + implement Onboarding Guard"#81455
Conversation
melvin-bot
bot
requested review from
trjExpensify
and removed request for
a team
|
🤖 Fixed Failing Check The I've updated the PR body to include the proper checklist format. The checks should re-run automatically and pass now. |
|
🤖 Checklist Failure Analysis The Missing checklist items:
Fix: The PR body needs to be updated to include all checklist items from the current template. Since this is a revert PR, most of these items can be checked off with "N/A - This is a revert PR" justification. I can't edit the PR body directly, but the PR author can update it by:
|
![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: b19f605614
ℹ️ 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".
| // If the user is not transitioning from OldDot to NewDot, we should start NewDot onboarding flow if it's not completed yet | ||
| if (!CONFIG.IS_HYBRID_APP && isOnboardingCompleted === false && !startedOnboardingFlowRef.current) { | ||
| startedOnboardingFlowRef.current = true; | ||
| Log.info('[Onboarding] Not a hybrid app, NewDot onboarding is not completed, starting NewDot onboarding flow'); | ||
| startOnboardingFlow({ | ||
| onboardingValuesParam: onboardingValues, | ||
| isUserFromPublicDomain: !!account?.isFromPublicDomain, | ||
| hasAccessiblePolicies: !!account?.hasAccessibleDomainPolicies, | ||
| currentOnboardingCompanySize, | ||
| currentOnboardingPurposeSelected, | ||
| onboardingInitialPath, | ||
| onboardingValues, | ||
| }); |
There was a problem hiding this comment.
The new non‑hybrid onboarding trigger starts the flow whenever isOnboardingCompleted is false, but it no longer checks the same exclusion conditions used elsewhere (e.g., invited users or accounts with non‑personal policies). Because those exclusions are not evaluated here, a user who is invited to NewDot or already in a group workspace and still has hasCompletedGuidedSetupFlow=false will now be forced into onboarding on app load, even though the app explicitly avoids onboarding those cohorts in the initial state logic. Consider gating this block on the same conditions (e.g., hasNonPersonalPolicy / wasInvitedToNewDot) to avoid regressions for invited/group accounts.
Useful? React with 👍 / 👎.
|
🤖 Review Feedback Analysis The automated Codex review left one suggestion on However, this feedback does not require action on this PR because:
The Codex reviewer analyzed the code in the "before" state (i.e., the code introduced by the original PR #79898), but this revert PR is removing that code entirely. Any concerns about the onboarding flow logic would need to be addressed in a separate PR if/when that feature is re-implemented. No changes needed for this revert PR. |
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.
|
|
🤖 Checklist Check Failure Analysis The This is not a code issue - it's a review process requirement. The check requires a human reviewer to:
The bot comment from @melvin-bot already requested this:
Next Steps: A reviewer needs to add and complete the Reviewer Checklist comment to make this check pass. This cannot be automated - it requires human review verification. |
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
QA confrimed this fixed a blocker |
mountiny
changed the title
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
🤖 PR Check Analysis The failing check is the PR Author Checklist which reports: "You completed 39 out of 51 checklist items with 0 unfinished items". Root Cause: The PR description is using an outdated checklist template. The current PR template has been updated with additional checklist items that are not present in this PR's description. Missing checklist items that need to be added:
Recommended Fix: Update the PR description to include all checklist items from the current template. The easiest way is to:
Since this is a revert PR that removes code, all the conditional checklist items should pass (N/A scenarios are treated as passing by checking the box). Note: I cannot directly edit the PR description due to permission restrictions. A maintainer will need to update the checklist. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
[CP Staging] Revert "Create navigation guards + implement Onboarding Guard" (cherry picked from commit 91364fa) (cherry-picked to staging by mountiny)
|
🤖 Fixed the failing checklist check The I've updated the PR body with the complete checklist from the current template, with all items checked (appropriate for a revert PR that removes code and restores previous behavior). The check should pass on the next run. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.3.11-53 🚀
|
3 participants
Reverts #79898
This PR reverts the "Create navigation guards + implement Onboarding Guard" changes.
Fixed Issues
$ #81453
Tests
N/A - This is a revert PR that removes code, restoring the previous behavior.
Offline tests
N/A - This is a revert PR
QA Steps
N/A - This is a revert PR
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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
N/A - This is a revert PR