Repository navigation
Conversation
|
@Eskalifer1 @DylanDylann One of you needs to 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] |
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.
|
![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: b406cace47
ℹ️ 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 [primaryLogin] = useOnyx(ONYXKEYS.ACCOUNT, {selector: primaryLoginSelector}); | ||
| const [sessionEmail] = useOnyx(ONYXKEYS.SESSION, {selector: emailSelector}); | ||
| const isBlockedFromSpotnanaTravel = isBetaEnabled(CONST.BETAS.PREVENT_SPOTNANA_TRAVEL); | ||
| const isBlockedFromSpotnanaTravel = Permissions.isBetaEnabled(CONST.BETAS.PREVENT_SPOTNANA_TRAVEL, allBetas); |
There was a problem hiding this comment.
In non-production, when a tester force-enables preventSpotnanaTravel from Troubleshoot while the account's server betas do not include it, this two-argument call still returns false because Permissions.isBetaEnabled only reads overrides from its fourth argument, whereas the removed usePermissions call supplied them. The FAB item consequently calls openTravelDotLink instead of exercising the blocked flow; the same regression exists in QuickCreationActionsBar and useCreateNavigationSuggestions. Preserve the override input while intentionally bypassing betaConfiguration.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Its exactly how it used to work before this PR
| const shouldShowBookTravel = !!travelEnabledPolicy; | ||
|
|
||
| const isBlockedFromSpotnanaTravel = isBetaEnabled(CONST.BETAS.PREVENT_SPOTNANA_TRAVEL); | ||
| const isBlockedFromSpotnanaTravel = Permissions.isBetaEnabled(CONST.BETAS.PREVENT_SPOTNANA_TRAVEL, allBetas); |
There was a problem hiding this comment.
Please use isBetaEnabled from usePermissions to avoid redundant subscriptions. We only need to add one flag to determine whether betaConfiguration needs to be passed or not.
|
closing this PR |
2 participants
Explanation of Change
Reverts the
preventSpotnanaTravelpart of #99444.That PR moved three travel call sites (
TravelMenuItem,QuickCreationActionsBar,useCreateNavigationSuggestions) fromPermissions.isBetaEnabled(beta, allBetas)tousePermissions().isBetaEnabled(beta).usePermissionsappliesbetaConfiguration, andpreventSpotnanaTravelis an exclusion beta, so on accounts that only have theallbeta it now resolves tofalseand those entry points stopped blocking travel.This restores the previous two argument resolution at those three sites, plus the test mock that covers it. The other four call sites (
BookTravelButton,BookOrManageYourTrip,DynamicTripDetailsPage,TermsStep) already resolved throughusePermissionsbefore #99444 and are untouched.Note this puts the beta back to resolving two different ways in the app. The consistent behaviour is the correct end state, so the follow up is on the testing side: to exercise travel blocking, the account needs
preventSpotnanaTravelset explicitly rather than relying onall.Fixed Issues
$ #100851
PROPOSAL:
Tests
Precondition: a paid group workspace provisioned for travel with travel terms accepted, set as the active workspace.
allbeta and does not havepreventSpotnanaTravelset explicitlyallnorpreventSpotnanaTravel, repeat steps 2 and 3, verify both open TravelDot in a new tabOffline tests
Same as tests. Beta resolution reads Onyx only, there is no network behaviour involved.
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari