[$500] LHN - First line and second line of the message are visible on LHN

kbecciv commented 8 months ago

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.35-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: https://expensify.testrail.io/index.php?/tests/view/4262578&group_by=cases:section_id&group_order=asc&group_id=296775 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:

Log in with the following account - applausetester+multiline@applause.expensifail.com
Once logged in navigate to each of the conversations available (multiline+1 to multiline+9) and check LHN
Verify each of the multiline preview messages are displayed correctly in the LHN, only the first line of the message should be visible

Expected Result:

Only the first line of the message should be visible on LHN

Actual Result:

First line and second line of the message are visible on LHN

Workaround:

Unknown

Platforms:

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

[x] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01be20407fd253f3da
Upwork Job ID: 1753175258047799296
Last Price Increase: 2024-02-22
Automatic offers:

akinwale | Reviewer | 0
dukenv0307 | Contributor | 0

melvin-bot[bot] commented 8 months ago

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

melvin-bot[bot] commented 8 months ago

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

melvin-bot[bot] commented 8 months ago

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

dukenv0307 commented 8 months ago

Proposal

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

LHN - First line and second line of the message are visible on LHN
What is the root cause of that problem?
With ADDCOMMENT action, the message to display in LHN is from lastMessageText, that was removed all the line break character. See the logic
What changes do you think we should make in order to solve the problem?
With the ADDCOMMENT action, we can get the message to display from lastMessageHtml, then use parser.htmlToText

For example, we can update this to:

 return lastMessageTextFromReport || (lastActionName === 'ADDCOMMENT' ? parser.htmlToText(report?.lastMessageText).split("\n")[0] : report?.lastMessageHtml ?? '');

What alternative solutions did you explore? (Optional)

FitseTLT commented 8 months ago

This is intentional behavior that aligns with other social media apps; when the last message has text with line breaks it will be joined with space and displayed on the last message preview.

puneetlath commented 8 months ago

Hmm, I agree. I think this is the intentional behavior.

@kbecciv is the expected behavior from a regression test?

kbecciv commented 8 months ago

@puneetlath The issue is raised when executing steps in TR https://expensify.testrail.io/index.php?/tests/view/4262578&group_by=cases:section_id&group_order=asc&group_id=296775

puneetlath commented 7 months ago

Ok, seems like a legit bug then. @kbecciv would you mind confirming if you're still seeing the issue?

kbecciv commented 7 months ago

@puneetlath Checking

kbecciv commented 7 months ago

Issue is still reproducible image (7)

melvin-bot[bot] commented 7 months ago

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

ghost commented 7 months ago

Can't login using this account - applausetester+multiline@applause.expensifail.com, because I can't enter magic code

puneetlath commented 7 months ago

You don't need to be able to log in. You can see in the screenshot what the message is like.

ghost commented 7 months ago

okay, thanks for updating

melvin-bot[bot] commented 7 months ago

@puneetlath @ntdiary this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

ntdiary commented 7 months ago

Issue is still reproducible

Hi, @puneetlath, it seems that we still want to address this issue? If so, could you please reassign another c+? I'm still unavailable until Feb 18th. :)

melvin-bot[bot] commented 7 months ago

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

melvin-bot[bot] commented 7 months ago

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

akinwale commented 7 months ago

We can move forward with @dukenv0307's proposal here.

@dukenv0307 Please compare the performance difference between splitting the string and using a substring from the start index to the first newline and implement whichever approach is faster.

🎀👀🎀 C+ reviewed.

melvin-bot[bot] commented 7 months ago

Current assignee @puneetlath is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

puneetlath commented 7 months ago

Hmm, so the proposal makes sense, but why did this start happening all of a sudden? Do we know why this was working before and now isn't?

dukenv0307 commented 7 months ago

Do we know why this was working before and now isn't?

@puneetlath Maybe because of the PR

melvin-bot[bot] commented 7 months ago

@puneetlath @akinwale this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

melvin-bot[bot] commented 7 months ago

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

melvin-bot[bot] commented 7 months ago

@puneetlath, @akinwale Eep! 4 days overdue now. Issues have feelings too...

dukenv0307 commented 7 months ago

@puneetlath Do you have any feedback about my comment here?

puneetlath commented 7 months ago

Sorry for the delay. Let's go ahead.

melvin-bot[bot] commented 7 months ago

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

melvin-bot[bot] commented 7 months ago

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

melvin-bot[bot] commented 7 months ago

@puneetlath @akinwale @dukenv0307 this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:

Decide whether any proposals currently meet our guidelines and can be approved as-is today
If no proposals meet that standard, please take this issue internal and treat it as one of your highest priorities
If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

melvin-bot[bot] commented 7 months ago

Current assignee @akinwale is eligible for the Internal assigner, not assigning anyone new.

puneetlath commented 7 months ago

@dukenv0307 is working on it. @dukenv0307 do you have an ETA?

dukenv0307 commented 7 months ago

@akinwale @puneetlath PR https://github.com/Expensify/App/pull/37581 is ready to review

melvin-bot[bot] commented 6 months ago

This issue has not been updated in over 15 days. @puneetlath, @akinwale, @dukenv0307 eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

melvin-bot[bot] commented 4 months ago

@puneetlath, @akinwale, @dukenv0307, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

melvin-bot[bot] commented 4 months ago

@puneetlath, @akinwale, @dukenv0307, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

puneetlath commented 3 months ago

Still on the list. Have been working on higher priority #newdot-quality and #fast-apis stuff.

puneetlath commented 2 months ago

Asking for a re-test to see if this is still occurring.

m-natarajan commented 2 months ago

Still reproducible image (2)

puneetlath commented 3 weeks ago

Will try to get to it at some point, but it's really not high priority.

puneetlath commented 2 days ago

I'm going to close this as I don't think this is a priority right now with #vip-vsb being closed. Comment or reopen if you disagree!

akinwale commented 8 hours ago

@puneetlath This would be eligible for compensation since a PR was created and we went through the review process.

Expensify / App