Repository navigation
Conversation
04497b5 to
643c5a9
Compare
|
@MelvinBot review |
Code reviewLooks good to merge. I found no blocking issues. The refactor keeps behavior the same, adds no new import cycle, and every CI check passes except the expected What I checked
Nits (optional)
I didn't run the manual test steps. |
e443b16 to
7ff2fae
Compare
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@MelvinBot review |
Code review (updated approach)Looks good to merge. I found no blocking issues. The new version passes What I checked
Nits (optional)
My earlier review covered the |
|
Kept the old test and folded the new one into it: it now has Given/When/Then and a participant, so it fails if the anonymous check is removed |
|
@thelullabyy 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] |
…ule session Thread the anonymous-user flag through findLastAccessedReport, getMostRecentlyVisitedReport, canLeaveChat and navigateAfterOnboarding from their callers (via useIsAnonymousUser hook) instead of importing the anonymous-session check from actions/Session. Removes the now-unused isAnonymousUser helper from CurrentUserStore.
6c84dea to
657b6bd
Compare
…eportutils-anonymous-session-v1 # Conflicts: # src/libs/ReportUtils.ts
…eportutils-anonymous-session-v1 # Conflicts: # src/libs/navigateAfterOnboarding.ts # src/pages/OnboardingPersonalDetails/BaseOnboardingPersonalDetails.tsx # src/pages/OnboardingPrivateDomain/BaseOnboardingPrivateDomain.tsx # src/pages/OnboardingWorkspaces/BaseOnboardingWorkspaces.tsx # tests/unit/navigateAfterOnboardingTest.ts
…eportutils-anonymous-session-v1
…eportutils-anonymous-session-v1
|
@thelullabyy kind bump here |
…eportutils-anonymous-session-v1 # Conflicts: # src/libs/navigateAfterOnboarding.ts # src/pages/OnboardingPersonalDetails/BaseOnboardingPersonalDetails.tsx
…eportutils-anonymous-session-v1
…eportutils-anonymous-session-v1
Explanation of Change
findLastAccessedReport,getMostRecentlyVisitedReportandcanLeaveChatnow takeisAnonymousUser: booleanas a parameter instead of reading the anonymous-session flag from module state. This drops theReportUtilstoactions/Sessionimport edge (no-cycle part 17).useIsAnonymousUserhook (readssession.authTokenType); the non-React caller inactions/Link.tspassesisAnonymousUser()fromactions/Sessiondirectly. Same value as before, just supplied by the caller.navigateAfterOnboarding,navigateAfterOnboardingWithMicrotaskQueueandgetReportIDAfterOnboardingthread the new parameter through tofindLastAccessedReport.ReportDetailsMenuLeaveItem(the leave-room row, extracted into its own component on main): it feedscanLeaveChatand the post-leavefindLastAccessedReportlookup.This branch removes 14 import/no-cycle errors in src: 142 -> 128
Fixed Issues
$ #99650
PROPOSAL:
Tests
Test 1: Signed-in user in the same room (smoke)
My test public room:
iOS:
xcrun simctl openurl booted "new-expensify://r/6574590506765394"Android:
adb shell am start -W -a android.intent.action.VIEW -d "new-expensify://r/6574590506765394" com.expensify.chat.devTest 2: Anonymous visitor in a public room (smoke)
Offline tests
QA Steps
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
Android: Native
android.mov
Android: mWeb Chrome
android.web.mov
iOS: Native
ios.mov
iOS: mWeb Safari
ios.web.mov
MacOS: Chrome / Safari
web.mov