Ensure that disabled feature page cannot be accessed using deep-link #37902
Conversation
…hub.com/rezkiy37/Expensify into feature/37785-restrict-access-WorkspaceMoreFeaturesPage
…ture/37785-restrict-access-WorkspaceMoreFeaturesPage
…ture/37785-restrict-access-WorkspaceMoreFeaturesPage
…ture/37785-restrict-access-WorkspaceMoreFeaturesPage
|
@rezkiy37 is this ready for a review? Can you please resolve conflicts |
|
updated with fullscreen! |
src/pages/ErrorPage/NotFoundPage.tsx
Outdated
| }; | ||
|
|
||
| // eslint-disable-next-line rulesdir/no-negated-variables | ||
| function NotFoundPage({onBackButtonPress}: NotFoundPageProps) { | ||
| function NotFoundPage({onBackButtonPress, shouldForceFullScreen = false}: NotFoundPageProps) { |
There was a problem hiding this comment.
Lets use FullPageNotFoundView
…rict-access-WorkspaceMoreFeaturesPage
|
Resolved conflicts and comment |
|
Clicking back from the not found page takes you to the all workspaces list. I think this is fine, I guess it just depends on how you have navigated to that page too |
|
@mountiny yes, it was a suggestion from @luacmartins |
|
@waterim Similar bug to #37902 (comment) doesn't seem to be related to me, could you check? Screen.Recording.2024-03-20.at.3.34.39.in.the.afternoon.mov |
| </View> | ||
| </Section> | ||
| </View> | ||
| <FeatureEnabledAccessOrNotFoundWrapper |
There was a problem hiding this comment.
I think we should move this to wrap WorkspacePageWithSections to avoid this:
Screen.Recording.2024-03-20.at.3.42.00.in.the.afternoon.mov
|
|
|
Updated |
|
@getusha added :) |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 1.4.56-0 🚀
|
|
🚀 Deployed to production by https://github.com/yuwenmemon in version: 1.4.56-8 🚀
|
|
|
||
| function FeatureEnabledAccessOrNotFoundComponent(props: FeatureEnabledAccessOrNotFoundComponentProps) { | ||
| const isPolicyIDInRoute = !!props.policyID?.length; | ||
| const shouldShowFullScreenLoadingIndicator = props.isLoadingReportData !== false && (!Object.entries(props.policy ?? {}).length || !props.policy?.id); |
There was a problem hiding this comment.
If we created a new Workspace and then enabled a new feature right after that (ie: Workflows), we called create workspace and enable feature consecutively, this condition won't be enough to address that case so It will introduce the bug: #39443.
More detail in this comment: #39443 (comment)
Details
The PR restricts access to feature pages. It allows to open a feature page only when it is enabled, otherwise it redirects the user to the "More features" page.
Fixed Issues
$ #37785
PROPOSAL: N/A
Tests
Offline tests
Same as "Tests".
QA Steps
Same as "Tests".
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel so 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.mp4
Android: mWeb Chrome
Android.Chrome.mp4
iOS: Native
IOS.mp4
iOS: mWeb Safari
IOS.Safari.mp4
MacOS: Chrome / Safari
Chrome.mp4
MacOS: Desktop
Desktop.mp4