[HOLD for payment 2022-12-20] Split Bill - Attendees can be selected/deselected on Final Review page of Split Bill

[kbecciv]

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

---

Action Performed:

1. Go to https://staging.new.expensify.com/
   and login with HT Expensifail account
2. Click FAB
3. Click Split Bill
4. Input any amount and Click Next
5. Select several Attendees and click Next
6. Final Review page is displayed

Expected Result:

User should not be able to select/deselect the attendees

Actual Result:

Attendees can be selected/deselected on Final Review page of Split Bill

Workaround:

Unknown

Platform:

Where is this issue occurring?

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

Version Number: 1.2.34.1

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers):

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Bug5842879_video_29.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @stephanieelliott (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[kbecciv]

QA team is Failing on Final Review page section, Step 5 says "Verify that you cannot select/deselect the attendees".
https://expensify.testrail.io/index.php?/tests/view/2971939

[marcaaron]

Linked video appears to be unrelated to the reproduction steps.

[s77rt]

Proposal

diff --git a/src/components/IOUConfirmationList.js b/src/components/IOUConfirmationList.js
index 4f4eb415ed..1aa94e34f4 100755
--- a/src/components/IOUConfirmationList.js
+++ b/src/components/IOUConfirmationList.js
@@ -275,27 +275,24 @@ class IOUConfirmationList extends Component {
         const shouldShowSettlementButton = this.props.iouType === CONST.IOU.IOU_TYPE.SEND;
         const shouldDisableButton = selectedParticipants.length === 0;
         const recipient = this.state.participants[0];
-        const canModifyParticipants = !this.props.isIOUAttachedToExistingChatReport && this.props.hasMultipleParticipants;
 
         return (
             <OptionsSelector
                 sections={this.getSections()}
                 value={this.props.comment}
-                onSelectRow={canModifyParticipants ? this.toggleOption : undefined}
                 onConfirmSelection={this.confirm}
                 onChangeText={this.props.onUpdateComment}
                 textInputLabel={this.props.translate('iOUConfirmationList.whatsItFor')}
                 placeholderText={this.props.translate('common.optional')}
                 selectedOptions={this.getSelectedOptions()}
-                canSelectMultipleOptions={canModifyParticipants}
-                disableArrowKeysActions={!canModifyParticipants}
-                isDisabled={!canModifyParticipants}
+                disableArrowKeysActions
+                isDisabled
                 hideAdditionalOptionStates
                 forceTextUnreadStyle
                 autoFocus
                 shouldDelayFocus
                 shouldTextInputAppearBelowOptions
-                optionHoveredStyle={canModifyParticipants ? styles.hoveredComponentBG : {}}
+                optionHoveredStyle={{}}
                 footerContent={shouldShowSettlementButton
                     ? (
                         <SettlementButton

Details

canModifyParticipants is redundant and not desired as we can go back and modify the participants. Also since it's the confirmation page it's more appropriate to be read only

[marcaaron]

@s77rt Please wait until issues are triaged.

[trjExpensify]

Happened across this issue. Agree the video reproduction doesn't match, but also, we should confirm this is only a bug with the "global create > Split bill" flow. It's expected in the "Group DM > + > Split bill" flow (where all the chat participants of an existing group chat are preselected and can be deselected).

Related PR from a while back for reference: #5895

[kavimuru]

@trjExpensify @marcaaron Correct video attached.

[kavimuru]

@trjExpensify In the PR #5895

If bill split flow initiated via FAB, we will Not Allow attendee deselection on bill split confirmation page.
If bill split flow initiated from Group Chat we will Allow attendee deselection on bill split confirmation page.

But it is contradictory, Able to select / deselect attendee if initiated from FAB and not able to if initiated from Group DM

[trjExpensify]

Thanks for adding the video!

But it is contradictory, Able to select / deselect attendee if initiated from FAB and not able to if initiated from Group DM

It actually says the opposite to that, which is correct, so it's not contradictory to the below?

we should confirm this is only a bug with the "global create > Split bill" flow. It's expected in the "Group DM > + > Split bill" flow (where all the chat participants of an existing group chat are preselected and can be deselected).

[tgolen]

I'll go ahead and swap the logic for this.

[melvin-bot]

@tgolen, @stephanieelliott Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

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

• Correct the logic for when bill split particpants can be selected #13279

If no regressions arise, payment will be issued on 2022-12-20. 🎊

After the hold period, please check if any of the following need payment for this issue, and if so check them off after paying:

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[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:

• [@tgolen] The PR that introduced the bug has been identified. Link to the PR:
• [@tgolen] 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:
• [@tgolen] 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:
• [@stephanieelliott] A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here:

[sobitneupane]

@stephanieelliott I am C+ in the PR. Can you please help with the payment.

[stephanieelliott]

Hey @sobitneupane, sorry for the wait on this! I've created the job in Upwork to make the C+ payment, can you please claim it when you get a chance? https://www.upwork.com/jobs/~0114c5212c5918f200

[sobitneupane]

@stephanieelliott Accepted the offer

[sobitneupane]

@stephanieelliott
Waiting for the payment.

[stephanieelliott]

Thanks for the bump @sobitneupane and sorry for the wait, all paid up!