[HOLD for payment 2024-02-15] [HOLD for payment 2024-02-14] [$500] [Wave8] [Ideal Nav] Hide Subscriptions menu item on iOS and Android native

[trjExpensify]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: v1.4.36-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @AndrewGable
Slack conversation: https://expensify.slack.com/archives/C036QM0SLJK/p1707240281243759

Action Performed:

1. Go to new.expensify.com on iOS/Android native
2. Click the wrench tab
3. Observe the Subscriptions menu item in the LHN

Expected Result:

Due to restrictions with the App store, we should hide this menu item on iOS and Android native devices.

Actual Result:

We don't hide it, and that puts us at risk of being pulled from the app store.

Workaround:

Yes, they can go to mWeb or to web/desktop to see the subscription menu item.

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native (incl. iPadOS)
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

View all open jobs on GitHub

CC: @mountiny @hayata-suenaga

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e6bee6862397d920
• Upwork Job ID: 1754927211226951680
• Last Price Increase: 2024-02-06
• Automatic offers:
• aimane-chnaif | Reviewer | 28146833
• codinggeek2023 | Contributor | 28146834

[melvin-bot]

Current assignee @trjExpensify is eligible for the Bug assigner, not assigning anyone new.

[trjExpensify]

P.S when we raise the PR for this can we request a CP to staging, to minimise the delay in getting this onto production.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01e6bee6862397d920

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @aimane-chnaif (External)

[eucool]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Hide Subscriptions menu item on iOS and Android native

What is the root cause of that problem?

Currently we show the Subscriptions element in all 3 platforms

App/src/pages/home/sidebar/AllSettingsScreen.tsx

Lines 52 to 61 in 1da7032

	{
	translationKey: 'allSettingsScreen.subscriptions',
	icon: Expensicons.MoneyBag,
	action: () => {
	Link.openOldDotLink(CONST.OLDDOT_URLS.ADMIN_POLICIES_URL);
	},
	shouldShowRightIcon: true,
	iconRight: Expensicons.NewWindow,
	link: CONST.OLDDOT_URLS.ADMIN_POLICIES_URL,
	},

What changes do you think we should make in order to solve the problem?

We need to check if the device is native and if so, we should not dispaly the subscription option.

We need to get the platform and have just a single boolean check to check if the device is not native:

!isNative && { // Conditionally include "Subscriptions" item
            translationKey: 'allSettingsScreen.subscriptions',
            icon: Expensicons.MoneyBag,
            action: () => {
                Link.openOldDotLink(CONST.OLDDOT_URLS.ADMIN_POLICIES_URL);
            },
            shouldShowRightIcon: true,
            iconRight: Expensicons.NewWindow,
            link: CONST.OLDDOT_URLS.ADMIN_POLICIES_URL,
        },

What alternative solutions did you explore? (Optional)

Create a folder as AllSettingsScreen, in that have 3 platform specific files: index.tsx, index.ios.tsx, index.android,

in android.tsx and ios.tsx omit the part of subscriptions, remove the part where ;

translationKey: 'allSettingsScreen.subscriptions'

[abzokhattab]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Hide Subscriptions menu item on iOS and Android native

What is the root cause of that problem?

new feature

What changes do you think we should make in order to solve the problem?

we need to hide subsriptions for native ios or android

to do so we need to getplatform to get the current platform

        const platform = getPlatform();
        const isNative = platform === CONST.PLATFORM.IOS || platform === CONST.PLATFORM.ANDROID;

then show the subscriptions only if its not native here

App/src/pages/home/sidebar/AllSettingsScreen.tsx

Lines 52 to 61 in 26ccc06

	{
	translationKey: 'allSettingsScreen.subscriptions',
	icon: Expensicons.MoneyBag,
	action: () => {
	Link.openOldDotLink(CONST.OLDDOT_URLS.ADMIN_POLICIES_URL);
	},
	shouldShowRightIcon: true,
	iconRight: Expensicons.NewWindow,
	link: CONST.OLDDOT_URLS.ADMIN_POLICIES_URL,
	},

we can change the list assignemnt to:

const baseMenuItems = [
            {
                translationKey: 'common.workspaces',
                icon: Expensicons.Building,
                action: () => {
                    waitForNavigate(() => {
                        Navigation.navigate(ROUTES.SETTINGS_WORKSPACES);
                    })();
                },
                focused: !isSmallScreenWidth,
                brickRoadIndicator: hasGlobalWorkspaceSettingsRBR(policies, policyMembers) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined,
            },
            ...(!isNative
                ? [
                      {
                          translationKey: 'allSettingsScreen.subscriptions',
                          icon: Expensicons.MoneyBag,
                          action: () => {
                              Link.openOldDotLink(CONST.OLDDOT_URLS.ADMIN_POLICIES_URL);
                          },
                          shouldShowRightIcon: true,
                          iconRight: Expensicons.NewWindow,
                          link: CONST.OLDDOT_URLS.ADMIN_POLICIES_URL,
                      },
                  ]
                : []),
            {
                translationKey: 'allSettingsScreen.cardsAndDomains',
                icon: Expensicons.CardsAndDomains,
                action: () => {
                    Link.openOldDotLink(CONST.OLDDOT_URLS.ADMIN_DOMAINS_URL);
                },
                shouldShowRightIcon: true,
                iconRight: Expensicons.NewWindow,
                link: CONST.OLDDOT_URLS.ADMIN_DOMAINS_URL,
            },
        ];

alternatively

we can create a folder for the component where it should contain 3 files ios.index.ts, android.index.ts and index.ts

the index.ts is the same as the current AllSettingsScreen component but we can rename it to AllSettingsScreenBase and we need to add a new prop shouldShowSubscription then in the android/ios files we need to call the AllSettingsScreenBase with shouldShowSubscription set as true similar to what we currently do in the SelectionList

[mountiny]

We do not allow Platform.OS in our codebase, use platform specific files.

[eucool]

Updated Proposal

Following comments from @mountiny , updated proposal to have platform specific files

[sofi-a]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Hide Subscriptions menu item on native platforms.

What is the root cause of that problem?

The menu item was added intentionally.

What changes do you think we should make in order to solve the problem?

Create a module getShouldShowSubscriptionsMenu in src/libs that returns a platform-specific value for showing/hiding the subscription menu item.

src/libs/getShouldShowSubscriptionsMenu/index.ts

/**
 * Indicates whether the subscription menu should show in the all settings screen
 */
const getShouldShowSubscriptionsMenu = (): boolean => true;

export default getShouldShowSubscriptionsMenu;

src/libs/getShouldShowSubscriptionsMenu/index.native.ts

/**
 * Indicates whether the subscription menu should show in the all settings screen
 */
const getShouldShowSubscriptionsMenu = (): boolean => false;

export default getShouldShowSubscriptionsMenu;

Import and use the module to conditionally insert the Subscriptions menu item in src/pages/home/sidebar/AllSettingsScreen.tsx.

import getShouldShowSubscriptionsMenu from '@libs/getShouldShowSubscriptionsMenu';
...
            },
            ...(getShouldShowSubscriptionsMenu() ? [{
                translationKey: 'allSettingsScreen.subscriptions',
                icon: Expensicons.MoneyBag,
                action: () => {
                    Link.openOldDotLink(CONST.OLDDOT_URLS.ADMIN_POLICIES_URL);
                },
                shouldShowRightIcon: true,
                iconRight: Expensicons.NewWindow,
                link: CONST.OLDDOT_URLS.ADMIN_POLICIES_URL,
            }] : []),
            {
...

What alternative solutions did you explore? (Optional)

N/A

[allgandalf]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Hide Subscriptions menu item on iOS and Android native

What is the root cause of that problem?

Currently we show the Subscriptions element on all platforms

App/src/pages/home/sidebar/AllSettingsScreen.tsx

Lines 52 to 61 in 1da7032

	{
	translationKey: 'allSettingsScreen.subscriptions',
	icon: Expensicons.MoneyBag,
	action: () => {
	Link.openOldDotLink(CONST.OLDDOT_URLS.ADMIN_POLICIES_URL);
	},
	shouldShowRightIcon: true,
	iconRight: Expensicons.NewWindow,
	link: CONST.OLDDOT_URLS.ADMIN_POLICIES_URL,
	},

What changes do you think we should make in order to solve the problem?

Add a simple check to see if the platform is native, and if it is native, then don't show ti subscription option.

const platform = getPlatform();
const isNotNative = platform !== CONST.PLATFORM.IOS && platform !== CONST.PLATFORM.ANDROID;
.
.
.
.
isNotNative && { // Conditionally include "Subscriptions" item
            translationKey: 'allSettingsScreen.subscriptions',
            icon: Expensicons.MoneyBag,
            action: () => {
                Link.openOldDotLink(CONST.OLDDOT_URLS.ADMIN_POLICIES_URL);
            },
            shouldShowRightIcon: true,
            iconRight: Expensicons.NewWindow,
            link: CONST.OLDDOT_URLS.ADMIN_POLICIES_URL,
        },

                

Important

We should not have the check for isNative, because this will cause a unexpected regression when the platform is Desktop

What alternative solutions did you explore? (Optional)

N/A

[hayata-suenaga]

asking for a quick proposal review.

[aimane-chnaif]

This is straightforward issue and I'd like to recommend contributor who provided the most accurate solution.
@sofi-a are you available to raise PR right now? This is quite urgent. Otherwise, I will recommend other.

[allgandalf]

I am urgently available to work on this issue :)

[abzokhattab]

Why dont we make it the same way we implement other components like in the selectionlist above?

also available now .. so should be ready in 30 mins

[eucool]

I am also available to work on the issue right away :)

[aimane-chnaif]

Why dont we make it the same way we implement other components like in the selectionlist above?

We don't need to create separate files for entire AllSettingsScreen code for this minor change.

[abzokhattab]

I see but its just a wapper component so a minor code will be introduced with the shouldshowsubscription as true .. just like here https://github.com/Expensify/App/blob/a79485c2d1c14f81623058ce9ce4a84c9c82e501/src/components/SelectionList/index.ios.tsx

[allgandalf]

@aimane-chnaif , what would be ETA to wait for @sofi-a 's response 🤔

[aimane-chnaif]

10 min more. The next candidate is @codinggeek2023 since they proposed first

[eucool]

Alright, will keep the Code ready on my end :) and pushed onto github

[aimane-chnaif]

Seems like @sofi-a is off for the day.
Let's assign @codinggeek2023
Please create platform files in /libs similar to willBlurTextInputOnTapOutside
🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @luacmartins, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[eucool]

okay PR will be ready in less than 30 minutes

[eucool]

PR up for review :) @aimane-chnaif

[melvin-bot]

📣 @aimane-chnaif 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @codinggeek2023 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.37-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Hide Subscriptions menu item on iOS and Android native #35953

If no regressions arise, payment will be issued on 2024-02-14. 🎊

For reference, here are some details about the assignees on this issue:

• @aimane-chnaif requires payment automatic offer (Reviewer)
• @codinggeek2023 requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@aimane-chnaif] The PR that introduced the bug has been identified. Link to the PR:
• [@aimane-chnaif] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@aimane-chnaif] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@aimane-chnaif] Determine if we should create a regression test for this bug.
• [@aimane-chnaif] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@trjExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.38-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Hide Subscriptions menu item on iOS and Android native #35953

If no regressions arise, payment will be issued on 2024-02-15. 🎊

For reference, here are some details about the assignees on this issue:

• @aimane-chnaif requires payment automatic offer (Reviewer)
• @codinggeek2023 requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@aimane-chnaif] The PR that introduced the bug has been identified. Link to the PR:
• [@aimane-chnaif] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@aimane-chnaif] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@aimane-chnaif] Determine if we should create a regression test for this bug.
• [@aimane-chnaif] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@trjExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[trjExpensify]

Alright, going to pay this one out. No regression, something we missed in implementation design.

[trjExpensify]

Paid! Summary as follows:

• $500 to @codinggeek2023 for the fix
• $500 to @aimane-chnaif for the review.