[HOLD for payment 2023-08-10] [$1000] Parent Message of thread chat on LHN doesn't updated when the parent message is updated

[kavimuru]

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

---

Action Performed:

1. Go to new chat
2. Send New message
3. Go to the thread chat of the above message
4. Send new message on the thread chat
5. Update the parent message
6. See title of thread chat on LHN

Expected Result:

Parent Message of thread chat on LHN should be updated

Actual Result:

Parent Message of thread chat on LHN doesn't updated

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.44-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
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2023-07-22.at.17.04.50.mov

Recording.1321.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dukenv0307
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1690020315164019

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016848c21d8e7b7eeb
• Upwork Job ID: 1683964849932365824
2023-07-25
• Automatic offers:
• dukenv0307 | Contributor | 25796886
• dukenv0307 | Reporter | 25796887

[melvin-bot]

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

[melvin-bot]

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

[samh-nl]

Proposal

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

When editing the parent message of a thread, the LHN item title does not update.

What is the root cause of that problem?

The LHN menu item uses an useMemo hook to determine when it should be updated.

App/src/components/LHNOptionsList/OptionRowLHNData.js

Lines 73 to 81 in cd851d0

	const optionItem = useMemo(() => {
	// Note: ideally we'd have this as a dependent selector in onyx!
	const item = SidebarUtils.getOptionData(fullReport, personalDetails, preferredLocale, policy);
	if (deepEqual(item, optionItemRef.current)) {
	return optionItemRef.current;
	}
	optionItemRef.current = item;
	return item;
	}, [fullReport, personalDetails, preferredLocale, policy]);

In the case of a thread, the title (report name) is derived using ReportUtils.getReportName, however this is not added as a dependency of useMemo.

Additionally, the component as a whole is also wrapped in React.memo, preventing a re-render:

App/src/components/LHNOptionsList/OptionRowLHNData.js

Line 134 in cd851d0

export default React.memo(

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

We should add the report name as a dependency of the useMemo hook, which can be determined using ReportUtils.getReportName. Additionally, we should update the React.memo to allow re-rendering in this circumstance.

What alternative solutions did you explore? (Optional)

Alternatively, we can add a different field to the report, such as the last modified date (e.g. parentLastModified) or the message of the parent, similar to how policy names are stored in the report This will ensure report changes, therefore causing a new render.

The advantage of this approach is that it will use the existing dependencies.

[dukenv0307]

Proposal

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

Parent Message of thread chat on LHN doesn't updated when the parent message is updated

What is the root cause of that problem?

When deleting the parent message of thread, we don't have any update on the report field of that thread report

App/src/components/LHNOptionsList/OptionRowLHNData.js

Line 73 in cd851d0

const optionItem = useMemo(() => {

So that optionItem in here is not rendered again

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

We can subscribe parentReportAction of the thread and we will get option item again if parentReportAction change
To do that

1. We need to get reportActions of report

withOnyx({
            parentReportActions: {
                key: (props) => ONYXKEYS.COLLECTION.REPORT_ACTIONS + props.fullReport.parentReportID,
                canEvict: false
        }})

2. Get ParentReportAction

 const parentReportAction = parentReportActions[fullReport.parentReportActionID]

3. Add parentReportAction to dependency of useEffect

    }, [fullReport, personalDetails, preferredLocale, policy, parentReportAction]);

What alternative solutions did you explore? (Optional)

We should introduce new field called updateReportInLHN. When updating the parent message of child thread we should add updateReportInLHN: true to the child report field. So the child report field is changed and the LHN is updated
We just do the same thing with policyName on report field on ONYX like we did in this PR

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~016848c21d8e7b7eeb

[melvin-bot]

Triggered auto assignment to @mallenexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot]

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

[tjferriss]

Overdue no more, Melvin. Adding External label.

[parasharrajat]

Ok, this is a tricky one. Both proposals are correct. But @dukenv0307's proposal is more elegant.

@dukenv0307 Is there a way to listen for only one action instead of the whole collection? May be using selector.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 @parasharrajat Please request via NewDot manual requests for the Reviewer role ($1000)

[melvin-bot]

📣 @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]

📣 @dukenv0307 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[dukenv0307]

@parasharrajat The PR is ready for review.

[melvin-bot]

🎯 ⚡️ Woah @parasharrajat / @dukenv0307, great job pushing this forwards! ⚡️

The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

• when @dukenv0307 got assigned: 2023-07-27 22:05:49 Z
• when the PR got merged: 2023-07-31 18:44:38 UTC

On to the next one 🚀

[tjferriss]

PR is merged but held on QA #23777 (comment)

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.49-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Parent Message of thread chat on LHN doesn't updated when the parent message is updated #23777

If no regressions arise, payment will be issued on 2023-08-10. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter @dukenv0307 $250
• Contributor that fixed the issue @dukenv0307 $1500
• Contributor+ that helped on the issue and/or PR @parasharrajat $1500

For reference, here are some details about the assignees on this issue:

• @parasharrajat does not require payment (Eligable for Manual Requests)
• @dukenv0307 requires payment offer (Contributor)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@parasharrajat] The PR that introduced the bug has been identified. Link to the PR:
• [@parasharrajat] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@parasharrajat] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@parasharrajat] Determine if we should create a regression test for this bug.
• [@parasharrajat] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@puneetlath / @tjferriss / @mallenexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[tjferriss]

Payment comment has been updated with contributors and amounts #23424 (comment)

@parasharrajat can you get started on the checklist when you have a chance?

[tjferriss]

All set for payment barring regression. @parasharrajat can you grab the checklist when you have a minute?

[parasharrajat]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@parasharrajat] The PR that introduced the bug has been identified. Link to the PR: **This is missing from the first implementation. I couldn't find the exact PR which added threads. But New Thread Report Empty State, Header, LHN, OpenReport, Buttons for DEV #18522 is a very close match. **
• [@parasharrajat] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: New Thread Report Empty State, Header, LHN, OpenReport, Buttons for DEV #18522 (comment)
• [@parasharrajat] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: https://expensify.slack.com/archives/C049HHMV9SM/p1691566374977849
• [@parasharrajat] Determine if we should create a regression test for this bug. Yes
• [@parasharrajat] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Steps

1. Go to the new chat.
2. Send New Message
3. Create a the thread chat for the above message
4. Send a new message on the thread chat.
5. Update the parent message on the thread.
6. Verify that the title of the thread chat on LHN is updated correctly

Do you agree 👍 or 👎 ?

[parasharrajat]

Payment requested.

[tjferriss]

@dukenv0307 has been paid via Upworks. Regression test has been submitted https://github.com/Expensify/Expensify/issues/307290

@parasharrajat did you request payment via NewDot?

[parasharrajat]

@tjferriss Yes.

[JmillsExpensify]

Reviewed the details for @parasharrajat. Payment summarized above by BZ member is approved for payment in NewDot.