Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$500] Thread - Unable to send a reply in thread after leaving the thread in offline mode #34424

Closed
1 of 6 tasks
kavimuru opened this issue Jan 12, 2024 · 55 comments
Closed
1 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review

Comments

@kavimuru
Copy link

kavimuru commented Jan 12, 2024

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


Version Number:
Reproducible in staging?:
Reproducible in production?:
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:
Slack conversation:

Action Performed:

  1. Go to workspace chat
  2. Go offline.
  3. Send a message and reply in thread.
  4. Leave the thread.
  5. Open the thread and send a reply again.

Expected Result:

User is able to send a reply in thread after leaving the thread in offline mode.

Actual Result:

User is unable to send a reply in thread after leaving the thread in offline mode.

Workaround:

unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6339832_1705045271304.20240111_230544.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0190e8891c6eb5ad7c
  • Upwork Job ID: 1745748146336444416
  • Last Price Increase: 2024-01-26
  • Automatic offers:
    • mollfpr | Reviewer | 28143176
    • tienifr | Contributor | 28143177
@kavimuru kavimuru added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jan 12, 2024
@melvin-bot melvin-bot bot changed the title Thread - Unable to send a reply in thread after leaving the thread in offline mode [$500] Thread - Unable to send a reply in thread after leaving the thread in offline mode Jan 12, 2024
Copy link

melvin-bot bot commented Jan 12, 2024

Job added to Upwork: https://www.upwork.com/jobs/~0190e8891c6eb5ad7c

Copy link

melvin-bot bot commented Jan 12, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jan 12, 2024
Copy link

melvin-bot bot commented Jan 12, 2024

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

@tienifr
Copy link
Contributor

tienifr commented Jan 12, 2024

Proposal

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

User is unable to send a reply in thread after leaving the thread in offline mode.

What is the root cause of that problem?

After leaving thread, we will delete the report along with report actions, that why if users want to visit again, we have to load it from BE -> In offline mode we just can see the loading page

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

We should not delete the thread report/ report action after leaving thread, we just need to update the notification preference to not receive the notification.

In leaveRoom function:

    const optimisticData: OnyxUpdate[] = [
        {
            onyxMethod: Onyx.METHOD.MERGE,
            key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
            value: {
                      notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN,
                  }
        },
    ];

we need to update successfulData as well.

Then navigate to the previous chat.

    if (isWorkspaceMemberLeavingWorkspaceRoom) {
...
else{
        Navigation.goBack(ROUTES.HOME);
}

What alternative solutions did you explore? (Optional)

NA

Result

Screen.Recording.2024-01-12.at.17.29.59.mov

@melvin-bot melvin-bot bot added the Overdue label Jan 14, 2024
@Tony-MK
Copy link
Contributor

Tony-MK commented Jan 15, 2024

Proposal

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

Thread - Unable to send a reply inside of a thread after leaving the thread in offline mode

What is the root cause of that problem?

The canLeaveRoom function is used in the HeaderView component to display the leave room menu option.

const isWorkspaceMemberLeavingWorkspaceRoom = lodashGet(props.report, 'visibility', '') === CONST.REPORT.VISIBILITY.RESTRICTED && isPolicyMember;
threeDotMenuItems.push({
icon: Expensicons.ChatBubbles,
text: translate('common.leave'),
onSelected: Session.checkIfActionIsAllowed(() => Report.leaveRoom(props.report.reportID, isWorkspaceMemberLeavingWorkspaceRoom)),
});

I believe solely using isWorkspaceMemberLeavingWorkspaceRoom to create a condition in the leaveRoom function for this PR #33053 was not a good idea.

App/src/libs/actions/Report.ts

Lines 2092 to 2097 in f268c39

value: isWorkspaceMemberLeavingWorkspaceRoom
? {
notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN,
}
: {
reportID: null,

This is because PR removes the report.ReportID from the onyx if isWorkspaceMemberLeavingWorkspaceRoom is false.
Hence, threads are also affected.

const isTransitioning = report && report.reportID !== reportIDFromPath;
return reportIDFromPath !== '' && report.reportID && !isTransitioning;

Therefore, isReportReadyForDisplay will always be false, and the ReportScreen will load infinitely.

{isReportReadyForDisplay && !isLoadingInitialReportActions && !isLoading && (
<ReportActionsView

It is just not displaying the ReportActionsView component.

Finally, the user can not reply because of the ReportFooter component over here.

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

Part 1

We will use the isChatThread function with the isWorkspaceMemberLeavingWorkspaceRoom variable to create a better condition.

value: isWorkspaceMemberLeavingWorkspaceRoom

Modify the above condition to something similar to the one below:

value: isWorkspaceMemberLeavingWorkspaceRoom || isChatThread(report)

App/src/libs/actions/Report.ts

Lines 2152 to 2154 in f268c39

API.write('LeaveRoom', parameters, {optimisticData, successData, failureData});
if (isWorkspaceMemberLeavingWorkspaceRoom) {

Part 2

Finally, we create a new condition after API.write shown above.
It will navigate the user back to the parent report.

if (isChatThread(report)) {
    Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(report.parentReportID));
}

Question
Is the navigating back part not supposed to happen for threads? 🤔
I added part 2 in case it was needed.

Result (macOS - Chrome)

296661827-ba321858-6fd0-49de-8e87-2ecb0eda39ae.mov

@joekaufmanexpensify
Copy link
Contributor

Pending proposal reviews from @thesahindia

@melvin-bot melvin-bot bot removed the Overdue label Jan 15, 2024
@joekaufmanexpensify
Copy link
Contributor

Same

@thesahindia
Copy link
Member

Sorry won't be able to take this. Please reassign!

@thesahindia thesahindia removed their assignment Jan 16, 2024
@joekaufmanexpensify joekaufmanexpensify added External Added to denote the issue can be worked on by a contributor and removed External Added to denote the issue can be worked on by a contributor labels Jan 16, 2024
Copy link

melvin-bot bot commented Jan 16, 2024

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

@aldo-expensify aldo-expensify removed the External Added to denote the issue can be worked on by a contributor label Jan 16, 2024
@aldo-expensify aldo-expensify added the External Added to denote the issue can be worked on by a contributor label Jan 16, 2024
Copy link

melvin-bot bot commented Jan 16, 2024

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

@aldo-expensify
Copy link
Contributor

Reassigned C+ since @eVoloshchak doesn't look active

@joekaufmanexpensify
Copy link
Contributor

TY!

@mollfpr
Copy link
Contributor

mollfpr commented Jan 17, 2024

I'm sure this was expected behavior; I'll try to find the conversation and link here.

@melvin-bot melvin-bot bot added the Overdue label Feb 5, 2024
@joekaufmanexpensify
Copy link
Contributor

DM'ed Georgia about proposal

@melvin-bot melvin-bot bot removed the Overdue label Feb 5, 2024
@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 5, 2024
Copy link

melvin-bot bot commented Feb 5, 2024

📣 @mollfpr 🎉 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

Copy link

melvin-bot bot commented Feb 5, 2024

📣 @tienifr 🎉 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 📖

@grgia
Copy link
Contributor

grgia commented Feb 5, 2024

All yours @tienifr

@joekaufmanexpensify
Copy link
Contributor

@grgia mind confirming this question? Seems like the PR review is blocked on it.

@joekaufmanexpensify
Copy link
Contributor

DM'ed Georgia about question on PR

@mollfpr
Copy link
Contributor

mollfpr commented Mar 1, 2024

We have another PR that fixed this issue, and the solution is kinda similar. I have checked the PR #37355, and it fixes this issue.

I think we can close the PR, but I don't know what the next process.

cc @joekaufmanexpensify

@tienifr

This comment was marked as outdated.

@joekaufmanexpensify
Copy link
Contributor

Got it. I'll take a look.

@joekaufmanexpensify joekaufmanexpensify added Daily KSv2 and removed Weekly KSv2 labels Mar 4, 2024
@joekaufmanexpensify
Copy link
Contributor

Discussing

@joekaufmanexpensify
Copy link
Contributor

joekaufmanexpensify commented Mar 6, 2024

We discussed and landed on the following:

  • @tienifr - $500 for contributor role, since you were hired for this job, wrote the PR, and the delay was on us internally. Paid via Upwork.
  • @mollfpr - $250 for C+, since you reviewed proposals, but did not do a full review. Paid via NewDot.

@joekaufmanexpensify
Copy link
Contributor

@tienifr $500 sent and contract ended!

@joekaufmanexpensify
Copy link
Contributor

@mollfpr please accept our offer so we can pay you.

@joekaufmanexpensify
Copy link
Contributor

I'm OOO the next two days, so we'll issue payment to @mollfpr on Monday (once you accept the offer).

@mollfpr
Copy link
Contributor

mollfpr commented Mar 7, 2024

Sorry for the late reply @joekaufmanexpensify I did manual request in NewDot.

@joekaufmanexpensify
Copy link
Contributor

Ah, all good. I canceled the offer in upwork, and updated my post above to clarify that you're being paid via NewDot.

@joekaufmanexpensify
Copy link
Contributor

Payment summary is here.

@joekaufmanexpensify
Copy link
Contributor

All set!

@JmillsExpensify
Copy link

$250 approved for @mollfpr based on this summary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

10 participants