Skip to content
New issue

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

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

Already on GitHub? # to your account

[$500] Chat - In offline mode, starting a new chat results in the header being greyed out and tapping on it doesn't trigger any action #33678

Closed
3 of 6 tasks
lanitochka17 opened this issue Dec 28, 2023 · 19 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 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

Comments

@lanitochka17
Copy link

lanitochka17 commented Dec 28, 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.4.18-2
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 profile icon---Preferences
  3. Toggle on forced offline
  4. Navigate to LHN
  5. Tap on fab---Start chat
  6. Enter a new user email id eg: tail@gmail.com
    and select it
  7. Tap on the header

Expected Result:

Tapping on header must direct user to profile page

Actual Result:

In offline, header greyed out and tapping on it doesn't trigger any action

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

Bug6327114_1703714413547.tail.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0159feb175c7125e53
  • Upwork Job ID: 1740170117380186112
  • Last Price Increase: 2024-01-04
@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 Dec 28, 2023
@melvin-bot melvin-bot bot changed the title Chat - In offline, header greyed out and tapping on it doesn't trigger any action. [$500] Chat - In offline, header greyed out and tapping on it doesn't trigger any action. Dec 28, 2023
Copy link

melvin-bot bot commented Dec 28, 2023

Job added to Upwork: https://www.upwork.com/jobs/~0159feb175c7125e53

Copy link

melvin-bot bot commented Dec 28, 2023

Triggered auto assignment to @jliexpensify (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 Dec 28, 2023
Copy link

melvin-bot bot commented Dec 28, 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

Copy link

melvin-bot bot commented Dec 28, 2023

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

@unidev727
Copy link
Contributor

unidev727 commented Dec 28, 2023

Proposal

from: @unicorndev-727

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

In offline, header greyed out and tapping on it doesn't trigger any action.

What is the root cause of that problem?

The root cause is that shouldDisableDetailPage is true here becuase the chat report only has one participant and that participant is the optimistic personal detail.
This issue happens if we don't have personal detail of a participant when creating a chat in offline state.

const shouldDisableDetailPage = ReportUtils.shouldDisableDetailPage(props.report);

App/src/libs/ReportUtils.ts

Lines 776 to 778 in 595bf40

if (participantAccountIDs.length === 1) {
return isOptimisticPersonalDetail(participantAccountIDs[0]);
}

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

Solution 1: We can return false if the report is chat report

if (isChatRoom(report) || isPolicyExpenseChat(report) || isChatThread(report) || isTaskReport(report) || isChatReport(report)) {
        return false;
    }

Solution 2: We can delete this condition in shouldDisableDetailPage function.

App/src/libs/ReportUtils.ts

Lines 776 to 778 in 595bf40

if (participantAccountIDs.length === 1) {
return isOptimisticPersonalDetail(participantAccountIDs[0]);
}

screen-capture.1.webm

What alternative solutions did you explore? (Optional)

App/src/libs/ReportUtils.ts

Lines 770 to 780 in 595bf40

function shouldDisableDetailPage(report: OnyxEntry<Report>): boolean {
const participantAccountIDs = report?.participantAccountIDs ?? [];
if (isChatRoom(report) || isPolicyExpenseChat(report) || isChatThread(report) || isTaskReport(report)) {
return false;
}
if (participantAccountIDs.length === 1) {
return isOptimisticPersonalDetail(participantAccountIDs[0]);
}
return false;
}

if we exclude this code snippet here

if (participantAccountIDs.length === 1) {
        return isOptimisticPersonalDetail(participantAccountIDs[0]);
    }

shouldDisableDetailPage always returns false.
So we don't need shouldDisableDetailPage function.
cc: @jliexpensify @c3024

if we have to return false always, we can delete disable attribute here.

disabled={shouldDisableDetailPage}

@Tony-MK
Copy link
Contributor

Tony-MK commented Dec 28, 2023

I am assuming, but this is expected. To be sure, could we confirm? Thank you 😄

App/src/libs/ReportUtils.ts

Lines 760 to 765 in 376bb95

/**
* Check if the report is a single chat report that isn't a thread
* and personal detail of participant is optimistic data
*/
function shouldDisableDetailPage(report: OnyxEntry<Report>): boolean {

@jliexpensify
Copy link
Contributor

I can reproduce on 1.4.18-3:

image

@jliexpensify jliexpensify changed the title [$500] Chat - In offline, header greyed out and tapping on it doesn't trigger any action. [$500] Chat - In offline mode, header is greyed out and tapping on it doesn't trigger any action. Dec 28, 2023
@jliexpensify jliexpensify changed the title [$500] Chat - In offline mode, header is greyed out and tapping on it doesn't trigger any action. [$500] Chat - In offline mode, starting a new chat results in the header being greyed out and tapping on it doesn't trigger any action Dec 28, 2023
@jliexpensify
Copy link
Contributor

jliexpensify commented Dec 28, 2023

I also did some additional testing, and this seems to only occur with new chats, so changed the title.

Posted here - https://expensify.slack.com/archives/C066HJM2CAZ/p1703734643674599

@tienifr
Copy link
Contributor

tienifr commented Dec 28, 2023

Proposal

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

In offline, header greyed out and tapping on it doesn't trigger any action

What is the root cause of that problem?

Since this PR, we disabled going to profile page if the personal detail of the user is optimistic.

This is because of the follow reasoning

When we choose a contact which we have not interacted before, we create a temporary option with a random accountID that is generated by generateAccountID function.
But it is an invalid accountID and not present in personalDetails. After openPublicProfilePage API is called, the data returns with accountID is -1.
That makes details empty and not found page displays

But now it's different, there will always be personal details returned from the back-end so we don't need that logic any more.

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

  1. Remove this function entirely, and its usage, because now there's no need to disable this action.
  2. We can consider removing some (or all) of the restriction (based on the same optimistic personal detail check) that was added via this PR
  3. After this, when we go to the profile page of this optimistic user and then comes online, we'll see the data being replaced by another user's data (the real data from back-end). If we think this is an issue, we can use the pattern where we'll show loading indicator on the profile page until the user data is fully loaded, so the user will see infinite loading if they're offline and try to access a profile page with optimistic personal detail, instead of seeing optimistic data then replaced by real data.

What alternative solutions did you explore? (Optional)

NA

@c3024
Copy link
Contributor

c3024 commented Dec 31, 2023

@jliexpensify

Posted here - https://expensify.slack.com/archives/C066HJM2CAZ/p1703734643674599

Could you please tell what the conclusion from the discussion was? Is this expected behaviour or does this need to be fixed?

@jliexpensify
Copy link
Contributor

The Project leader is OOO until the first week of January, so I'm not sure we'll get an answer before then. Will keep this issue updated though!

@melvin-bot melvin-bot bot added the Overdue label Jan 3, 2024
Copy link

melvin-bot bot commented Jan 4, 2024

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

Copy link

melvin-bot bot commented Jan 4, 2024

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

@jliexpensify
Copy link
Contributor

jliexpensify commented Jan 5, 2024

@melvin-bot melvin-bot bot removed the Overdue label Jan 5, 2024
@jliexpensify
Copy link
Contributor

@c3024 - bump on proposal reviews please!

@c3024
Copy link
Contributor

c3024 commented Jan 5, 2024

This appears to be intended behaviour to me. That is why I asked for the conclusion of the internal discussion. Suggested solutions in proposals are fine but I think it is better if we proceed after confirming if we want to fix this or not.

@jliexpensify
Copy link
Contributor

Cool, that makes sense - thanks for sharing your thoughts here!

@tienifr
Copy link
Contributor

tienifr commented Jan 5, 2024

@jliexpensify @c3024 we accepted that before due to a back-end limitation, now the back-end is working fine, there's no need to disable it, which will make the UX better.

More details in the proposal.

We can check if we want to improve it because it definitely is possible now (it was not possible to improve before)

@jliexpensify
Copy link
Contributor

Quick update here: I've spoken to Gabi (the project leader of #vip-split) and we've made the decision to close off this GH.

For a little more context, as of 2023-12-07 we’ve been prioritizing fixing issues that affect waves and VIP projects, so we really want to focus on issues that we can specifically tie into these. This one is a bit of an odd one - it's an annoyance, but we think it's low priority, and since it also doesn't fit in with any of the waves or projects we're working on, that's why this decision was made. Thanks!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 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
Projects
None yet
Development

No branches or pull requests

6 participants