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

Hold #28997 [$500] Mweb/Chrome - Thread - Tapping leave thread, displays "U" for a second instead of Room name in header #31220

Closed
1 of 6 tasks
lanitochka17 opened this issue Nov 10, 2023 · 27 comments
Assignees
Labels
External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Nov 10, 2023

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: 1.3.98-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: Applause - Internal Team
Slack conversation:

Action Performed:

  1. Go to https://staging.new.expensify.com/
  2. Tap on a room with a thread reply message
  3. Tap on reply
  4. Tap 3 dots on top
  5. Select Leave thread

Expected Result:

Tapping leave thread, must not display unavailable "U" for a second instead of Room name in header

Actual Result:

Tapping leave thread, displays unavailable "U" for a second instead of Room name in header.

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

Bug6271878_1699653563009.u.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~014350424b71769cdf
  • Upwork Job ID: 1723110772857241600
  • Last Price Increase: 2024-01-12
@lanitochka17 lanitochka17 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 Nov 10, 2023
Copy link

melvin-bot bot commented Nov 10, 2023

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

@melvin-bot melvin-bot bot changed the title Mweb/Chrome - Thread - Tapping leave thread, displays "U" for a second instead of Room name in header [$500] Mweb/Chrome - Thread - Tapping leave thread, displays "U" for a second instead of Room name in header Nov 10, 2023
Copy link

melvin-bot bot commented Nov 10, 2023

Job added to Upwork: https://www.upwork.com/jobs/~014350424b71769cdf

Copy link

melvin-bot bot commented Nov 10, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 10, 2023
Copy link

melvin-bot bot commented Nov 10, 2023

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

@DylanDylann
Copy link
Contributor

DylanDylann commented Nov 11, 2023

Proposal

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

  • Mweb/Chrome - Thread - Tapping leave thread, displays "U" for a second instead of Room name in header

What is the root cause of that problem?

  • After clicking on "Leave thread", we set the optimistic data for this thread as below:

    App/src/libs/actions/Report.ts

    Lines 2103 to 2118 in 52adf77

    const optimisticData: OnyxUpdate[] = [
    {
    onyxMethod: Onyx.METHOD.MERGE,
    key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
    value: isWorkspaceMemberLeavingWorkspaceRoom
    ? {
    notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN,
    }
    : {
    reportID: null,
    stateNum: CONST.REPORT.STATE_NUM.APPROVED,
    statusNum: CONST.REPORT.STATUS_NUM.CLOSED,
    notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN,
    },
    },
    ];
  • Then we use this useEffect to handle case onyxReportID: undefined. It will navigate the user to the previous report.
  • When the above useEffect executing, it will display the above optimistic report for a while.

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

  • We need to navigate the user to the parent report after API call, the same as what we did in case we left the room. Can do it by updating

    App/src/libs/actions/Report.js

    Lines 2087 to 2091 in f285c9c

    if (isWorkspaceMemberLeavingWorkspaceRoom) {
    const participantAccountIDs = PersonalDetailsUtils.getAccountIDsByLogins([CONST.EMAIL.CONCIERGE]);
    const chat = ReportUtils.getChatByParticipants(participantAccountIDs);
    Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(chat.reportID));
    }

    to
  if (isWorkspaceMemberLeavingWorkspaceRoom) {
        const participantAccountIDs = PersonalDetailsUtils.getAccountIDsByLogins([CONST.EMAIL.CONCIERGE]);
        const chat = ReportUtils.getChatByParticipants(participantAccountIDs);
        Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(chat.reportID));
+     } else {
+        Navigation.goBack();
+    }

What alternative solutions did you explore? (Optional)

  • Also we can hide the Join button (and the HeaderView) once the report.reportID is empty as we did in other screens.
  • To hide the Join button, we can update:
    const canJoin = canJoinOrLeave && !isWhisperAction && props.report.notificationPreference === CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN;

    to
   const canJoin = props.report.reportID && canJoinOrLeave && !isWhisperAction && props.report.notificationPreference === CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN;
    const isLoading = !props.report.reportID || !props.report || !title;

@bernhardoj
Copy link
Contributor

Should be fixed in #28997

@laurenreidexpensify
Copy link
Contributor

@eVoloshchak do you agree this will be covered in #28997 and we can close?

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Nov 14, 2023
Copy link

melvin-bot bot commented Nov 17, 2023

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@laurenreidexpensify
Copy link
Contributor

@eVoloshchak bump

@melvin-bot melvin-bot bot removed the Overdue label Nov 20, 2023
@DylanDylann
Copy link
Contributor

DylanDylann commented Nov 20, 2023

@eVoloshchak I think the RCA of this issue (31220) is that we do not navigate user to the parent thread after leaving thread. The 28997 is about the behavior when we re-join the leaved thread. Both of them are not the same. Also, we are still discussing about the expected solution for 28997

@eVoloshchak
Copy link
Contributor

#28997 and this issue do have different causes, but depending on which approach will be taken to fix #28997 this one might be resolved too (#28997 (comment))

@laurenreidexpensify, let's put this on HOLD for #28997

@laurenreidexpensify laurenreidexpensify changed the title [$500] Mweb/Chrome - Thread - Tapping leave thread, displays "U" for a second instead of Room name in header h[$500] Mweb/Chrome - Thread - Tapping leave thread, displays "U" for a second instead of Room name in header Nov 22, 2023
@laurenreidexpensify laurenreidexpensify changed the title h[$500] Mweb/Chrome - Thread - Tapping leave thread, displays "U" for a second instead of Room name in header Hold ##28997 [$500] Mweb/Chrome - Thread - Tapping leave thread, displays "U" for a second instead of Room name in header Nov 22, 2023
@laurenreidexpensify laurenreidexpensify changed the title Hold ##28997 [$500] Mweb/Chrome - Thread - Tapping leave thread, displays "U" for a second instead of Room name in header Hold #28997 [$500] Mweb/Chrome - Thread - Tapping leave thread, displays "U" for a second instead of Room name in header Nov 22, 2023
@laurenreidexpensify laurenreidexpensify added Weekly KSv2 and removed Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Nov 22, 2023
Copy link

melvin-bot bot commented Nov 24, 2023

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@laurenreidexpensify
Copy link
Contributor

Still on hold on #28997

Copy link

melvin-bot bot commented Dec 1, 2023

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@laurenreidexpensify
Copy link
Contributor

Still on hold on #28997

Copy link

melvin-bot bot commented Dec 8, 2023

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Dec 14, 2023
@laurenreidexpensify
Copy link
Contributor

Still on hold on #28997 but that has payment due on 15 Dec - so should be off hold v soon

@melvin-bot melvin-bot bot removed the Overdue label Dec 14, 2023
Copy link

melvin-bot bot commented Dec 15, 2023

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@laurenreidexpensify
Copy link
Contributor

@eVoloshchak #28997 has been on prod for a solid 12 days now, can we lift the hold?

@eVoloshchak
Copy link
Contributor

@laurenreidexpensify, according to my tests, this was resolved by a fix for #28997.
Would like for someone from QA to double check that

Copy link

melvin-bot bot commented Dec 22, 2023

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@laurenreidexpensify
Copy link
Contributor

@Expensify/applauseleads please confirm if this is still reproducible in KI tests

Copy link

melvin-bot bot commented Dec 29, 2023

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@laurenreidexpensify
Copy link
Contributor

@applausebot please confirm if this is still reproducible in KI tests

Copy link

melvin-bot bot commented Jan 5, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

1 similar comment
Copy link

melvin-bot bot commented Jan 12, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Jan 12, 2024
@laurenreidexpensify
Copy link
Contributor

I am closing based on @eVoloshchak's comment that this is resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2
Projects
None yet
Development

No branches or pull requests

5 participants