[HOLD for payment 2022-05-20] [$500] Remove Send Money when no members in room - reported by @thesahindia

[mvtglobally]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Create a workspace
2. Visit the Announce or Admins room of that policy ( it shouldn't have any other member except you )
3. Tap on ➕ (on left side, near the composer)
4. Select send money/request money

Expected Result:

It shouldn't show option to send money/ request money

Actual Result:

It's showing the option and user is able to go through the flow and get error in the end as the user is trying to request/send money to himself

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.1.46-3
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2022-03-17.at.1.21.26.AM.mov

Expensify/Expensify Issue URL:
Issue reported by: @thesahindia
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1647460332448239

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @tjferriss (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[tjferriss]

I agree that we shouldn't error out when this happens, but perhaps instead of completely removing the option we keep it and direct people to invite someone to the room instead.

[melvin-bot]

Triggered auto assignment to @danieldoglas (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[danieldoglas]

Agreed with @tjferriss solution above.

[melvin-bot]

Triggered auto assignment to @Christinadobrzyn (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[Christinadobrzyn]

Posted in Upwork!

Internal Job - https://www.upwork.com/ab/applicants/1509813441766039552/job-details
External job - https://www.upwork.com/jobs/~0176081f4f95677487

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (Exported)

[melvin-bot]

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

[allroundexperts]

@iwiznia What about my proposal then?

[sobitneupane]

@iwiznia What about my proposal then?

If I am allowed to comment on your proposal, it looks like you are proposing to show all the options if there are multiple participants which I guess is not the intended behavior.

[Santhosh-Sellavel]

@sobitneupane

I think we are intentionally showing only the split bill options in all the rooms. I am not sure, though.

No, It was accidentally showing because the report participant's value is wrong, which introduces this issue as well, fix should be the same for both.

cc: @iwiznia

[Santhosh-Sellavel]

@iwiznia

Still looking for a better proposal.
As I said here proposals are similar & partial

[allroundexperts]

@Santhosh-Sellavel The instructions are not clear. Can you specify what's missing in the above proposals?

[Santhosh-Sellavel]

@allroundexperts

This is the expected outcome,

Show Send Money and Request Money Options if there is a participant other than the logged in User and Show Split Bill Option if there are more than one participants other than the logged in User. In this case, even rooms with a participant other than the logged-in user will have Send Money and Request Money options.

@sobitneupane Already commented what's wrong with your proposal here

[allroundexperts]

@Santhosh-Sellavel Here's the updated proposal.

Proposal

First of all, we need to make sure that the hasMultipleParticipants variable has an accurate value. The reportParticipants variable contains the logged in user for admins and announce channels but ignores them for single or group channels.

The hasMultipleParticipants variable is thus removed and replaced with a new variable called reportParticipantsOtherThanLoggedInUser .

Suggested Change 1

App/src/pages/home/report/ReportActionCompose.js

Line 395 in 8665582

const hasMultipleParticipants = reportParticipants.length > 1;

const reportParticipantsOtherThanLoggedInUser = _.filter(reportParticipants, email => this.props.myPersonalDetails.login !== email);

The condition which checks for the menu items to display is also changed such that if reportParticipantsOtherThanLoggedInUser.length === 1, we show the send and request money options. If reportParticipantsOtherThanLoggedInUser.length > 1, we show the splitBill option.

Suggested Change 2

App/src/pages/home/report/ReportActionCompose.js

Lines 454 to 492 in 8665582

	menuItems={[
	...(!hasExcludedIOUEmails
	&& Permissions.canUseIOU(this.props.betas) ? [
	hasMultipleParticipants
	? {
	icon: Expensicons.Receipt,
	text: this.props.translate('iou.splitBill'),
	onSelected: () => {
	Navigation.navigate(
	ROUTES.getIouSplitRoute(
	this.props.reportID,
	),
	);
	},
	} : {
	icon: Expensicons.MoneyCircle,
	text: this.props.translate('iou.requestMoney'),
	onSelected: () => {
	Navigation.navigate(
	ROUTES.getIouRequestRoute(
	this.props.reportID,
	),
	);
	},
	},
	] : []),
	...(!hasExcludedIOUEmails && Permissions.canUseIOUSend(this.props.betas) && !hasMultipleParticipants ? [
	{
	icon: Expensicons.Send,
	text: this.props.translate('iou.sendMoney'),
	onSelected: () => {
	Navigation.navigate(
	ROUTES.getIOUSendRoute(
	this.props.reportID,
	),
	);
	},
	},
	] : []),

menuItems={[
                                                    ...(!hasExcludedIOUEmails
                                                        && Permissions.canUseIOU(this.props.betas)
                                                        && reportParticipantsOtherThanLoggedInUser.length === 1 ? [
                                                            {
                                                                icon: Expensicons.MoneyCircle,
                                                                text: this.props.translate('iou.requestMoney'),
                                                                onSelected: () => {
                                                                    Navigation.navigate(
                                                                        ROUTES.getIouRequestRoute(
                                                                            this.props.reportID,
                                                                        ),
                                                                    );
                                                                },
                                                            },
                                                            {
                                                                icon: Expensicons.Send,
                                                                text: this.props.translate('iou.sendMoney'),
                                                                onSelected: () => {
                                                                    Navigation.navigate(
                                                                        ROUTES.getIOUSendRoute(
                                                                            this.props.reportID,
                                                                        ),
                                                                    );
                                                                },
                                                            },
                                                        ] : []),
                                                    ...(!hasExcludedIOUEmails
                                                        && Permissions.canUseIOU(this.props.betas)
                                                        && reportParticipantsOtherThanLoggedInUser.length > 1 ? [
                                                            {
                                                                icon: Expensicons.Receipt,
                                                                text: this.props.translate('iou.splitBill'),
                                                                onSelected: () => {
                                                                    Navigation.navigate(
                                                                        ROUTES.getIouSplitRoute(
                                                                            this.props.reportID,
                                                                        ),
                                                                    );
                                                                },
                                                            },
                                                        ] : []),

Result

Screen.Recording.2022-04-25.at.11.02.38.PM.mov

[Santhosh-Sellavel]

@allroundexperts
Please try to include the exact code changes you are proposing instead of linking. Example like this #8357 (comment)

[Santhosh-Sellavel]

@allroundexperts
Kindly update your proposal accordingly so it would be easy to follow with explanations. Let me know after updating thanks!

[allroundexperts]

@Santhosh-Sellavel Updated.

[Santhosh-Sellavel]

@iwiznia
Proposals are similar, so let's go with @sobitneupane here.
@sobitneupane Option 1 proposal here looks good to me.
One suggestion, let's move the IOU options preparation logic to a new method. Since the code is not DRY & too clumsy in Render Block.

🎀👀🎀 C+ reviewed.
Thanks!

[iwiznia]

Proposal looks good to me and I agree about the new method. @kadiealexander can you hire @sobitneupane please?

[Santhosh-Sellavel]

@sobitneupane Any update?

[Puneet-here]

Hi the proposals above aren't complete if we only make the changes for actions we still be seeing our own contact whenever we try to send/reques/split bill as you can see in the screenshot.

As all the proposals are incomplete here's my proposal.

Proposal

We need to filter out our own login from the participant using _.filter

App/src/pages/iou/IOUModal.js

Line 110 in 66f0643

const participants = lodashGet(props, 'report.participants', []);

We will use two variables and based on them we will be showing different options, if showSplitBillOption is true will be showing the split bills and when showSendRequestMoney is true we will show the options of send and request money
( it is already proposed in the proposals above )

    const showSplitBillOption = _.filter(reportParticipants, email => this.props.myPersonalDetails.login !== email).length > 1;
    const showSendRequestMoney = _.filter(reportParticipants, email => this.props.myPersonalDetails.login !== email).length === 1;

We can create a new method showActions which will return an array of actions based on the conditions for different actions and we can just call it here

App/src/pages/home/report/ReportActionCompose.js

Line 451 in 66f0643

menuItems={[

cc: @iwiznia @Santhosh-Sellavel

[Santhosh-Sellavel]

@Puneet-here Your proposal has nothing new here, what are you trying to say.

Even your proposal is incomplete here.

if we only make the changes for actions we still be seeing our own contact whenever we try to send/reques/split bill as you can see in the screenshot.

I believe this would be handled in this PR, I see the accepted proposal has logic to solve the same!

[melvin-bot]

📣 @sobitneupane You have been assigned to this job by @kadiealexander!
Please apply to this job in Upwork 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 📖

[kadiealexander]

@sobitneupane please let me know once you've applied on Upwork!

[sobitneupane]

@sobitneupane please let me know once you've applied on Upwork!

Applied.

[kadiealexander]

Sent offers in Upwork to:
Hired contributor: @sobitneupane
C+: @Santhosh-Sellavel
Reporting bonus: @thesahindia

[kadiealexander]

@sobitneupane could you please share an update here? Thanks!

[Santhosh-Sellavel]

@kadiealexander PR is merged & it's on production now!

[kadiealexander]

Thanks @sobitneupane!!!

[kadiealexander]

Everyone is paid! Thanks for your work, team.