[$250] Group chat - On deleting parent message,text "Deleted message" is not shown

[lanitochka17]

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: 9.0.21 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Issue reported by: Applause - Internal Team

Action Performed:

1. Launch app
2. Tap on a group chat
3. Send a message
4. Long press the sent message and select reply in thread
5. Send a message
6. Tap header and note header title is shown under avatar
7. Navigate back and delete the parent message
8. Tap header and note under avatar parent message is shown and not "deleted message " text as in header title

Expected Result:

In group chat, on deleting parent message, the text " Deleted message " must be shown under avatar

Actual Result:

In group chat, on deleting parent message, the text " Deleted message " is not shown under avatar

Workaround:

Unknown

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/c966a403-c557-40f0-be97-ac1f03bf8ea5

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0129be1229dfa60701
• Upwork Job ID: 1825843284584468948
• Last Price Increase: 2024-08-20
• Automatic offers:
• ahmedGaber93 | Reviewer | 103689325

Issue Owner

Current Issue Owner: @ahmedGaber93

[melvin-bot[bot]]

Triggered auto assignment to @bfitzexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[lanitochka17]

We think that this bug might be related to #vip-vsp

[lanitochka17]

@bfitzexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[FitseTLT]

Edited by proposal-police: This proposal was edited at 2024-08-16 12:36:08 UTC.

Proposal

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

On deleting parent message,text "Deleted message" is not shown

What is the root cause of that problem?

We are using getGroupChatName to get the report name here https://github.com/Expensify/App/blob/d4d5a2586910ff46147219ee3e98bb3e936f8037/src/pages/ReportDetailsPage.tsx#L244-L245 Because isGroupChat will be true for chat threads opened inside group chats

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

isGroupChat is not enough and we also need to check that it is not chat thread

    const reportName = ReportUtils.isDeprecatedGroupDM(report) || (isGroupChat && !isChatThread) ? ReportUtils.getGroupChatName(undefined, false, report) : ReportUtils.getReportName(report);

or

    const reportName = ReportUtils.isDeprecatedGroupDM(report) || (isGroupChat && !isThread) ? ReportUtils.getGroupChatName(undefined, false, report) : ReportUtils.getReportName(report);

same fix should be applied here too if we are not going to update isGroupChat as suggested in my alternative approach (which fix the problem from the root cause) https://github.com/Expensify/App/blob/578006fe4d2b7ba13155e795bcef541019aad8ea/src/components/LHNOptionsList/OptionRowLHN.tsx#L134

What alternative solutions did you explore? (Optional)

As chat threads with group chat as their parent report also have chatType as group; everywhere we are using isGroupChat in the app, we are wrongly including these chat threads. so the current code to determine isGroupChat is not correct and the one we should fix it from the root cause and avoid similar problems in the whole code base

https://github.com/Expensify/App/blob/d4d5a2586910ff46147219ee3e98bb3e936f8037/src/libs/ReportUtils.ts#L1140-L1141 so should be changed to

    return getChatType(report) === CONST.REPORT.CHAT_TYPE.GROUP && !isThread(report);

[bernhardoj]

Proposal

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

If a thread is created on a group chat and the thread is deleted with a message, the report detail page shows the thread name instead of [Deleted message]

What is the root cause of that problem?

If the report is a group chat, then we use getGroupChatName to get the name. A thread always inherits its parent chat type, so a thread in a group chat is a group chat. https://github.com/Expensify/App/blob/578006fe4d2b7ba13155e795bcef541019aad8ea/src/pages/ReportDetailsPage.tsx#L244

getGroupChatName itself returns the reportName of a report if found and a thread reportName is the thread parent message. https://github.com/Expensify/App/blob/578006fe4d2b7ba13155e795bcef541019aad8ea/src/libs/ReportUtils.ts#L2140-L2144

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

Remove the group chat check and just use ReportUtils.getReportName. ReportUtils.getReportName already handles the group chat case. https://github.com/Expensify/App/blob/578006fe4d2b7ba13155e795bcef541019aad8ea/src/libs/ReportUtils.ts#L3736-L3738

We need to fix in OptionRowLHN too by removing the group chat check. https://github.com/Expensify/App/blob/578006fe4d2b7ba13155e795bcef541019aad8ea/src/components/LHNOptionsList/OptionRowLHN.tsx#L134

[FitseTLT]

Updated to add a better alternative approach

[melvin-bot[bot]]

@bfitzexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~0129be1229dfa60701

[melvin-bot[bot]]

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

[ahmedGaber93]

@bernhardoj In details page getGroupChatName use shouldApplyLimit with false value but getReportName use it with true value https://github.com/Expensify/App/blob/d4d5a2586910ff46147219ee3e98bb3e936f8037/src/pages/ReportDetailsPage.tsx#L244-L245

https://github.com/Expensify/App/blob/578006fe4d2b7ba13155e795bcef541019aad8ea/src/libs/ReportUtils.ts#L3736-L3738

I know this is may achieve the desired consistency in the other issue, but can you please add more context about this change affect? FYI, they have been added in this PR https://github.com/Expensify/App/pull/40134

[bernhardoj]

Beside the limit, the change will simply make it consistent with other parts of the app where we prioritize other naming case before the group chat, such as the thread.

https://github.com/Expensify/App/blob/ad7c1d5e394c8a9ee1ce7427233aa085350bb500/src/libs/ReportUtils.ts#L3667-L3733

Btw, if we look at the PR changes for ReportDetailsPage, we can see that previously we passed shouldApplyLimit as true.

Then, there is a discussion in the PR whether it's safe to pass shouldApplyLimit as true in ReportSettingsPage where we previously pass it as false. Then, this commit comes as the result of the discussion which changes shouldApplyLimit as false in ReportDetailsPage instead of ReportSettingsPage.

So, I think it's an overlook when changing the code.

[nkdengineer]

Proposal

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

In group chat, on deleting parent message, the text " Deleted message " is not shown under avatar

What is the root cause of that problem?

The thread of a group chat also has a chat type group then in this case we get the wrong report name of the thread by calling getGroupChatName

https://github.com/Expensify/App/blob/50de692ff915d7998bff3fb0d024446b59912b52/src/pages/ReportDetailsPage.tsx#L250

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

We should only get the report name by calling getGroupChatName if the chat has a chat type group and not is a thread. We already have isRootGroupChat function that covers this case

const reportName = ReportUtils.isRootGroupChat(report) ? ReportUtils.getGroupChatName(undefined, false, report) : ReportUtils.getReportName(report); 

https://github.com/Expensify/App/blob/50de692ff915d7998bff3fb0d024446b59912b52/src/pages/ReportDetailsPage.tsx#L250

We also need to fix in LHN here

const isGroupChat = ReportUtils.isRootGroupChat(report);

https://github.com/Expensify/App/blob/50de692ff915d7998bff3fb0d024446b59912b52/src/components/LHNOptionsList/OptionRowLHN.tsx#L132

What alternative solutions did you explore? (Optional)

[ahmedGaber93]

@bernhardoj's proposal LGTM!

getReprtName has the two cases isChatThread and isGroupChat that needed to fix this issue, also getReprtName used on the header here so using it on details page can fix the consistency issue https://github.com/Expensify/App/issues/47803

🎀 👀 🎀 C+ reviewed

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[ahmedGaber93]

Not overdue, the issue has just been assigned and we are waiting PR.

[bernhardoj]

PR is ready

cc: @ahmedGaber93

[bfitzexpensify]

I am heading out of office until September 21st, so assigning a buddy to watch over this in my absence.

Current status: PR in review

[melvin-bot[bot]]

Triggered auto assignment to @miljakljajic (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[miljakljajic]

@bfitzexpensify my leave starts tomorrow so I will leave this with you

[ahmedGaber93]

Regression Test Proposal

1. Open a group chat
2. Send a message and reply in a thread
3. Delete the thread parent message
4. Open the details page
5. Verify the chat name is [Deleted message]
6. Verify the thread title in LHN is [Deleted message]

Do we agree 👍 or 👎

[ahmedGaber93]

@bfitzexpensify Bump for payment, The production deploy automation failed, but based on this deploy checklist https://github.com/Expensify/App/issues/48954 This is issue was due for payment 2024-09-18.

[ahmedGaber93]

@bfitzexpensify Bump ^

[bfitzexpensify]

Payment summary:

@ahmedGaber93 due $250 for C+ work - offer sent via Upwork @bernhardoj due $250 for contributor work via manual request

[bernhardoj]

Requested in ND.

[JmillsExpensify]

$250 approved for @bernhardoj

[ahmedGaber93]

@bfitzexpensify Offer accepted.

[bfitzexpensify]

Payment complete.