LHN- Previously grayed chats in LHN have their accounts grayed in attendees list

[kbecciv]

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. Go to https://staging.new.expensify.com/
2. Go offline
3. Create Split Bill with several accounts you didn't have messages with
4. Grayed chats are created and visible in LHN
5. While still offline click FAB > Split Bill
6. Input amount and proceed to attendees page

Expected Result:

No any accounts should be grayed out on attendees page

Actual Result:

Previously grayed chats in LHN have their accounts grayed in attendees list

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.2.59.1

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Notes/Photos/Videos: Any additional supporting documentation

video_36.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a657b2f21b30feb1
• Upwork Job ID: 1620494456348491776
• Last Price Increase: 2023-01-31

[NicMendonca]

@kbecciv just to confirm - is this only occurring on MacOS / Chrome / Safari?

[kbecciv]

@NicMendonca issue is occurring in all environments.

[NicMendonca]

Thanks! Don't forget to ✔️ the Platforms in the GH!

[NicMendonca]

@kbecciv I can't seem to reproduce this:

[NicMendonca]

@kbecciv bump!

[kbecciv]

@NicMendonca Checking, update you shortly

[kbecciv]

@NicMendonca QA team is able to reproduce this issue with latest build 1.2.62.1

video_52.mp4

[NicMendonca]

@kbecciv thank you!

Looks like you can still split with the "greyed" attendees, but agree its a bug that they are grey

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

Current assignee @NicMendonca is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[melvin-bot]

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

[tienifr]

Proposal

Problem

Because we create new chats offline, these reports will be marked pendingAction === add. And then OfflineWithFeedback Component will check if an option has pendingAction !== null, this option will be grayed

Solution

I suggest that we should add a new prop (shouldRemovePendingAction) for getSection function in Expensify/App/src/libs/OptionsListUtils.js

And then we will pass shouldRemovePendingAction === true when we need to remove pendingAction and don't want to gray out report that has pendingAction

diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js
index d88d1292a..b51d5a179 100644
--- a/src/libs/OptionsListUtils.js
+++ b/src/libs/OptionsListUtils.js
@@ -446,6 +446,7 @@ function getOptions(reports, personalDetails, {
     showChatPreviewLine = false,
     sortPersonalDetailsByAlphaAsc = true,
     forcePolicyNamePreview = false,
+    shouldRemovePendingAction = false,
 }) {
     let recentReportOptions = [];
     let personalDetailsOptions = [];
@@ -614,6 +615,24 @@ function getOptions(reports, personalDetails, {
         }], ['asc']);
     }
 
+    if (shouldRemovePendingAction) {
+        if (personalDetailsOptions && personalDetailsOptions.length > 0) {
+            personalDetailsOptions = _.map(personalDetailsOptions, option => ({
+                ...option, pendingAction: null,
+            }));
+        }
+        if (recentReportOptions && recentReportOptions.length > 0) {
+            recentReportOptions = _.map(recentReportOptions, option => ({
+                ...option, pendingAction: null,
+            }));
+        }
+        if (userToInvite && userToInvite.length > 0) {
+            userToInvite = _.map(userToInvite, option => ({
+                ...option, pendingAction: null,
+            }));
+        }
+    }
+
     return {
         personalDetails: personalDetailsOptions,
         recentReports: recentReportOptions,
@@ -709,6 +728,8 @@ function getNewChatOptions(
         includePersonalDetails: true,
         maxRecentReportsToShow: 5,
         excludeLogins,
+        shouldRemovePendingAction: true,
+
     });
 }
 

Result

Screen.Recording.2023-01-28.at.21.48.07.mp4

[bernhardoj]

Proposal

I think it's expected #12150. If we want to change the behavior, then we can just remove the pending action for createOption.

diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js
index d88d1292a..ebe2cfdb0 100644
--- a/src/libs/OptionsListUtils.js
+++ b/src/libs/OptionsListUtils.js
@@ -309,7 +309,7 @@ function createOption(logins, personalDetails, report, reportActions = {}, {
         result.shouldShowSubscript = result.isPolicyExpenseChat && !report.isOwnPolicyExpenseChat && !result.isArchivedRoom;
         result.allReportErrors = getAllReportErrors(report, reportActions);
         result.brickRoadIndicator = !_.isEmpty(result.allReportErrors) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : '';
-        result.pendingAction = report.pendingFields ? (report.pendingFields.addWorkspaceRoom || report.pendingFields.createChat) : null;
+        result.pendingAction = report.pendingFields ? report.pendingFields.addWorkspaceRoom : null;
         result.ownerEmail = report.ownerEmail;
         result.reportID = report.reportID;
         result.isUnread = ReportUtils.isUnread(report);

or remove it completely.

[tienifr]

Proposal

Solution 2

We add a field shouldRemovePendingAction in OptionRow component to decide whether we should pass pendingAction in OfflineWithFeedback component

diff --git a/src/components/OptionRow.js b/src/components/OptionRow.js
index e25b0b807..166ebc964 100644
--- a/src/components/OptionRow.js
+++ b/src/components/OptionRow.js
@@ -59,6 +59,7 @@ const propTypes = {
 
     style: PropTypes.oneOfType([PropTypes.arrayOf(PropTypes.object), PropTypes.object]),
 
+    shouldRemovePendingAction: PropTypes.bool,
     ...withLocalizePropTypes,
 };
 
@@ -73,6 +74,8 @@ const defaultProps = {
     optionIsFocused: false,
     style: null,
     shouldHaveOptionSeparator: false,
+    shouldRemovePendingAction: false,
+
 };
 
 const OptionRow = (props) => {
@@ -104,7 +107,7 @@ const OptionRow = (props) => {
 
     return (
         <OfflineWithFeedback
-            pendingAction={props.option.pendingAction}
+            pendingAction={props.shouldRemovePendingAction ? null : props.option.pendingAction}
             errors={props.option.allReportErrors}
             shouldShowErrorMessages={false}
         >

and we will pass shouldRemovePendingAction === true in IOUParticipantsRequest.js and IOUParticipantsSplit.js

diff --git a/src/pages/iou/steps/IOUParticipantsPage/IOUParticipantsRequest.js b/src/pages/iou/steps/IOUParticipantsPage/IOUParticipantsRequest.js
index 0b29c1df1..5b43261c5 100755
--- a/src/pages/iou/steps/IOUParticipantsPage/IOUParticipantsRequest.js
+++ b/src/pages/iou/steps/IOUParticipantsPage/IOUParticipantsRequest.js
@@ -134,6 +134,7 @@ class IOUParticipantsRequest extends Component {
         );
         return (
             <OptionsSelector
+                shouldRemovePendingAction
                 sections={this.getSections()}
                 value={this.state.searchTerm}
                 onSelectRow={this.addSingleParticipant}
diff --git a/src/pages/iou/steps/IOUParticipantsPage/IOUParticipantsSplit.js b/src/pages/iou/steps/IOUParticipantsPage/IOUParticipantsSplit.js
index 7da28e971..698580d02 100755
--- a/src/pages/iou/steps/IOUParticipantsPage/IOUParticipantsSplit.js
+++ b/src/pages/iou/steps/IOUParticipantsPage/IOUParticipantsSplit.js
@@ -217,6 +217,7 @@ class IOUParticipantsSplit extends Component {
                             {this.props.translate('common.to')}
                         </Text>
                         <OptionsSelector
+                            shouldRemovePendingAction
                             canSelectMultipleOptions
                             sections={sections}
                             selectedOptions={this.props.participants}

[Beamanator]

@rushatgabhane are you available to review the proposals here? :D

[s77rt]

Proposal

diff --git a/src/components/LHNOptionsList/OptionRowLHN.js b/src/components/LHNOptionsList/OptionRowLHN.js
index c57d51d29e..bda0ef7ef1 100644
--- a/src/components/LHNOptionsList/OptionRowLHN.js
+++ b/src/components/LHNOptionsList/OptionRowLHN.js
@@ -102,6 +102,7 @@ const OptionRowLHN = (props) => {
             pendingAction={optionItem.pendingAction}
             errors={optionItem.allReportErrors}
             shouldShowErrorMessages={false}
+            shouldApplyPendingOpacityWhenOffline={false}
         >
             <Hoverable>
                 {hovered => (
diff --git a/src/components/OfflineWithFeedback.js b/src/components/OfflineWithFeedback.js
index e80618ce0a..9613affdd9 100644
--- a/src/components/OfflineWithFeedback.js
+++ b/src/components/OfflineWithFeedback.js
@@ -31,6 +31,9 @@ const propTypes = {
     /** Whether we should show the error messages */
     shouldShowErrorMessages: PropTypes.bool,
 
+    /** Whether we should apply pending opacity when offline */
+    shouldApplyPendingOpacityWhenOffline: PropTypes.bool,
+
     /** A function to run when the X button next to the error is clicked */
     onClose: PropTypes.func,
 
@@ -56,6 +59,7 @@ const defaultProps = {
     pendingAction: null,
     errors: null,
     shouldShowErrorMessages: true,
+    shouldApplyPendingOpacityWhenOffline: true,
     onClose: () => {},
     style: [],
     contentContainerStyle: [],
@@ -85,7 +89,7 @@ const OfflineWithFeedback = (props) => {
     const isOfflinePendingAction = props.network.isOffline && props.pendingAction;
     const isUpdateOrDeleteError = hasErrors && (props.pendingAction === 'delete' || props.pendingAction === 'update');
     const isAddError = hasErrors && props.pendingAction === 'add';
-    const needsOpacity = (isOfflinePendingAction && !isUpdateOrDeleteError) || isAddError;
+    const needsOpacity = (isOfflinePendingAction && props.shouldApplyPendingOpacityWhenOffline && !isUpdateOrDeleteError) || isAddError;
     const needsStrikeThrough = props.network.isOffline && props.pendingAction === 'delete';
     const hideChildren = !props.network.isOffline && props.pendingAction === 'delete' && !hasErrors;
     let children = props.children;
diff --git a/src/components/OptionRow.js b/src/components/OptionRow.js
index f31def0582..31372682f9 100644
--- a/src/components/OptionRow.js
+++ b/src/components/OptionRow.js
@@ -143,6 +143,7 @@ class OptionRow extends Component {
                 pendingAction={this.props.option.pendingAction}
                 errors={this.props.option.allReportErrors}
                 shouldShowErrorMessages={false}
+                shouldApplyPendingOpacityWhenOffline={false}
             >
                 <Hoverable
                     containerStyles={[

RCA

When we create a new chat it's created with pendingAction: add.
In options list we use <OfflineWithFeedback /> where we have a condition needsOpacity that applies opacity if we have a pending action and we are offline. That's the case here.

App/src/components/OfflineWithFeedback.js

Line 88 in 6a787ca

const needsOpacity = (isOfflinePendingAction && !isUpdateOrDeleteError) || isAddError;

The fix is to add a new prop to OfflineWithFeedback where we can opt out of this feature. In my proposal the new added prop is shouldApplyPendingOpacityWhenOffline which is passed with false for LHN Options and Report Options (the ones used on IOU split/request/etc.)

[rushatgabhane]

@Beamanator @NicMendonca we greyed out the participants when creating a chat/requesting money offline in #12150, https://github.com/Expensify/Expensify/issues/232287

Should we still fix this bug?
cc: @neil-marcellini (PR author)

[rushatgabhane]

I think we should do nothing

[Beamanator]

Ooh great find @rushatgabhane - I bet this is a case of not updating TestRail.

@kbecciv Can you please let us know which TestRail cases were violated which led you to create this issue?

Also @neil-marcellini I know your PR was merged a while back so sorry to drag you back into this, but:

Looks like you can still split with the "greyed" attendees, but agree its a bug that they are grey

Is it expected that users can split bills with accounts that haven't "officially" been created yet? I assume yes, which means we just need to add a few new testrail cases for Offline UI?

[neil-marcellini]

Is it expected that users can split bills with accounts that haven't "officially" been created yet? I assume yes, which means we just need to add a few new testrail cases for Offline UI?

Yep. If you are offline and go to split a bill the first time with two new accounts, then they show in the attendee list with full opacity. After creating that initial split the chats with those users are grayed out in the LHN because that is a pending action while offline. When you go to create another split with those same users, I think it makes sense that they are grayed out because the act of creating a chat with those users is currently pending. Does that sounds good?

[Beamanator]

Def makes sense @neil-marcellini 👍 Now I see that the regression steps were supposed to be created via the final point here: #13586 (comment)

So I think we can just purely close this out 👍 Or do you think we should close once the test cases have been updated?

[neil-marcellini]

Let's close this out. #13586 is the issue that needs to be completed.