[HOLD for payment 2023-04-27] [HOLD for payment 2023-04-26] [$1000] The edited message doesn't update on LHN in offline mode

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Go to web chrome
2. Turn off your wifi
3. Go to any chat and send a message
4. Now edit the message and notice that it doesn't update on LHN

Expected Result:

The edited message should be updated on LHN on offline mode

Actual Result:

The edited message doesn't update on LHN on offline mode

Workaround:

unknown

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.2.97-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
Notes/Photos/Videos: Any additional supporting documentation

error-2023-04-09_14.23.14.mp4

Recording.182.mp4

Expensify/Expensify Issue URL:
Issue reported by: @priya-zha
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1681029679583489

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01953a5f80d1aa943c
• Upwork Job ID: 1645857226176536576
• Last Price Increase: 2023-04-11

[MelvinBot]

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

[MelvinBot]

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

[sonialiap]

Reproducible

[MelvinBot]

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

[sonialiap]

Same behavior on the Android native app. I wonder if this is intentional

[francoisl]

Editing a message should be an optimistic action, so I don't think it's supposed to be intentional.

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~01953a5f80d1aa943c

[MelvinBot]

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

[MelvinBot]

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

[dukenv0307]

Proposal

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

The edited message doesn't update on LHN in offline mode

What is the root cause of that problem?

App/src/libs/actions/Report.js

Line 826 in a177ba3

function editReportComment(reportID, originalReportAction, textForNewComment) {

In UpdateComment API, we are setting optimisticData for ReportActions field but not for the Report field. So that the lastMessageText in the Report field in ONYX is not updated when editing offline

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

If we updated the last report action, we should add optimistcData for the Report field that includes: lastActionCreated, lastActorEmail, lastMessageHtml, lastMessageText.

EDITED: should ignore this suggestion because I see that we also don't add pendingAction into optimisticReport in deleteComment API and addComment API
Optional, for consistency, we can consider adding pendingAction for the optimisticReport like optimisticReportActions.

What alternative solutions did you explore? (Optional)

NA

Result

Screen.Recording.2023-04-11.at.15.07.23.mov

[MelvinBot]

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

[fedirjh]

@dukenv0307 Thank you for the proposal. Could you please consider reformatting your proposal to improve its readability ? Specifically this part :

Optional, we can consider adding pendingAction for the Report field in the optimistic Data. If do that we also need to reset pedingAction of Report is null in failureData and successData

---

The proposal looks good to me. Looking at the codebase addComment and deleteComment both set optimisticReport , while editReportComment doesn’t. Adding optimisticReport seems like a straightforward solution. Just one suggestion , we should add optimisticReport only when the edited message is the last visible message.

All yours @francoisl

C+ reviewed 🎀👀🎀

[dukenv0307]

Could you please consider reformatting your proposal to improve its readability ? Specifically this part :

@fedirjh Updated proposal

[kaushiktd]

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

The edited message doesn't update on LHN in offline mode

What is the root cause of that problem?

When the user edited an existing message, editReportComment() is called from below mentioned file:

App/src/libs/actions/Report.js

Line 826 in a177ba3

function editReportComment(reportID, originalReportAction, textForNewComment) {

This function is setting up optimisticData only for ReportActions, not for Reports fields. So the last message is not updating in the Report field.

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

We need to add optimisticData for the Report field so it can be updated for Report Fields. For this please add the below code in mentioned file:

App/src/libs/actions/Report.js

Line 871 in a177ba3

const currentTime = DateUtils.getDBTime();
const optimisticReport = {
    lastVisibleActionCreated: currentTime,
    lastMessageText: ReportUtils.formatReportLastMessageText(htmlForNewComment),
    lastActorEmail: currentUserEmail,
    lastReadTime: currentTime,
};

and update optimisticData value like below:

const optimisticData = [
       {
           onyxMethod: CONST.ONYX.METHOD.MERGE,
           key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
           value: optimisticReport,
       },
       {
           onyxMethod: CONST.ONYX.METHOD.MERGE,
           key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`,
           value: optimisticReportActions,
       },
   ];

Video File:

https://drive.google.com/file/d/1k54k7EluDa8sBIU1C78QmJtUabV6-dht/view?usp=share_link

[fedirjh]

@kaushiktd Your proposal is a duplicate of @dukenv0307's proposal , Please make sure to follow contributing rules when proposing new proposal.

Note: Before submitting a proposal on an issue, be sure to read any other existing proposals. ALL NEW PROPOSALS MUST BE DIFFERENT FROM EXISTING PROPOSALS. The difference should be important, meaningful or considerable.

[francoisl]

👍 @dukenv0307 your proposal sounds good, let's go with it.

[MelvinBot]

📣 @dukenv0307 You have been assigned to this job by @francoisl!
Please apply to this job in Upwork 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 📖

[MelvinBot]

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

[MelvinBot]

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

• Fix: Not update edited message on LHN in offline mode #17324
• Fix/17212: Also update html message and get last message in LHN when edit offline #17586

If no regressions arise, payment will be issued on 2023-04-26. 🎊

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
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

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

[MelvinBot]

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:

• [@fedirjh] The PR that introduced the bug has been identified. Link to the PR:
• [@fedirjh] 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:
• [@fedirjh] 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:
• [@sonialiap] Determine if we should create a regression test for this bug.
• [@fedirjh] 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.
• [@sonialiap] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[MelvinBot]

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

• Fix/17212: Also update html message and get last message in LHN when edit offline #17586

If no regressions arise, payment will be issued on 2023-04-27. 🎊

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
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

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

[MelvinBot]

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:

• [@fedirjh] The PR that introduced the bug has been identified. Link to the PR:
• [@fedirjh] 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:
• [@fedirjh] 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:
• [@sonialiap] Determine if we should create a regression test for this bug.
• [@fedirjh] 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.
• [@sonialiap] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[fedirjh]

1. The PR that introduced the bug has been identified. Link to the PR: Edit Comments #2320 This PR implemented the 'edit comments' feature, but it misses the case where a user edits their last sent message while offline
2. The offending PR has been commented on : Not sure if this apply here.
3. A discussion in #expensify-bugs has been started : There is an item in the checklist related to Offline testing
4. Regression test : [HOLD for payment 2023-04-27] [HOLD for payment 2023-04-26] [$1000] The edited message doesn't update on LHN in offline mode #17212 (comment)

[MelvinBot]

@francoisl, @sonialiap, @fedirjh, @dukenv0307 Eep! 4 days overdue now. Issues have feelings too...

[fedirjh]

It's awaiting payment.

[sonialiap]

@dukenv0307 offer sent for fixing the issue (+ bonus for merging under 3 days)
@fedirjh offer sent for reviewing (+ bonus for merging under 3 days, - regression = base price for issue)

[sonialiap]

@priya-zha is this your Upwork profile? Confirming so that I make sure I send the offer to the correct person :)

[priya-zha]

@sonialiap The profile that you mentioned is not mine. My profile is : https://www.upwork.com/freelancers/~011e32317dbbd489e0

[melvin-bot]

@francoisl, @sonialiap, @fedirjh, @dukenv0307 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[sonialiap]

@dukenv0307 paid
@fedirjh paid

@priya-zha thanks for the link. Offer sent for reporting issue

[priya-zha]

Thanks accepted.

[sonialiap]

@priya-zha paid! :)