[HOLD for payment 2024-06-05] [$250] Android - Composer - Draft text reappears after clearing the compose box and navigating back

@mollfpr

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.69-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: N/A
Issue reported by: Applause - Internal Team

Action Performed:

Open Expensify app
Open a chat
Send a message
Type something inside the compose box but don't send it
Hold on the sent message on step 3 and tap on reply in thread
Tap on the three dot menu and tap on leave
Clear the draft text inside the compose box
Tap on the device back button

Expected Result:

The compose box doesn't contain any draft text

Actual Result:

The compose box contains the previously cleared draft text

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6468639_1714636140879.Screen_Recording_20240502_104349_New_Expensify.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01321f2f43059fc592
Upwork Job ID: 1787349096905330688
Last Price Increase: 2024-05-06
Automatic offers:

bernhardoj | Contributor | 0

Issue Owner

Current Issue Owner: @mollfpr

@kadiealexander

Triggered auto assignment to @kadiealexander (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.

@kadiealexander

@kadiealexander 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

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

Proposal

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

The draft text isn't cleared when we delete the message from another report screen.

What is the root cause of that problem?

When we leave a thread, it will navigate to the most recent report (except the current report).

App/src/libs/actions/Report.ts

Lines 2508 to 2515 in 7483d10

    
           if (lastAccessedReportID) { 
        
               const lastAccessedReportRoute = ROUTES.REPORT_WITH_ID.getRoute(lastAccessedReportID ?? ''); 
        
               // If it is not a chat thread we should call Navigation.goBack to pop the current route first before navigating to last accessed report. 
        
               if (!isChatThread) { 
        
                   // Fallback to the lastAccessedReportID route, if this is first route in the navigator 
        
                   Navigation.goBack(lastAccessedReportRoute); 
        
               } 
        
               Navigation.navigate(lastAccessedReportRoute, CONST.NAVIGATION.TYPE.FORCED_UP);

So, now there are 2 same reports in the navigation stack. If we clear the composer message from the 2nd report screen and go back, the 1st report screen will still show the composer message. It's because the onyx draft message and the composer message state aren't synchronized.

This doesn't happen on other platforms because we have an effect to sync it.

App/src/pages/home/report/ReportActionCompose/SilentCommentUpdater/index.tsx

Lines 25 to 37 in 7483d10

    
           useEffect(() => { 
        
               // Value state does not have the same value as comment props when the comment gets changed from another tab. 
        
               // In this case, we should synchronize the value between tabs. 
        
               const shouldSyncComment = prevCommentProp !== comment && value !== comment && !isCommentPendingSaved.current; 
        
               // As the report IDs change, make sure to update the composer comment as we need to make sure 
        
               // we do not show incorrect data in there (ie. draft of message from other report). 
        
               if (preferredLocale === prevPreferredLocale && reportID === prevReportId && !shouldSyncComment) { 
        
                   return; 
        
               } 
        
               updateComment(comment ?? ''); 
        
           }, [prevCommentProp, prevPreferredLocale, prevReportId, comment, preferredLocale, reportID, updateComment, value, commentRef, isCommentPendingSaved]);

But we don't have it for Android (it was removed in #32799)

App/src/pages/home/report/ReportActionCompose/SilentCommentUpdater/index.android.tsx

Lines 16 to 23 in 7483d10

    
           function SilentCommentUpdater({comment, updateComment}: SilentCommentUpdaterProps) { 
        
               useEffect(() => { 
        
                   updateComment(comment ?? ''); 
        
                   // eslint-disable-next-line react-hooks/exhaustive-deps -- We need to run this on mount 
        
               }, []); 
        
               return null; 
        
           }

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

We can update the navigate to the most recent report logic to just use goBack with the report screen as the fallback

App/src/libs/actions/Report.ts

Lines 2508 to 2515 in 7483d10

    
           if (lastAccessedReportID) { 
        
               const lastAccessedReportRoute = ROUTES.REPORT_WITH_ID.getRoute(lastAccessedReportID ?? ''); 
        
               // If it is not a chat thread we should call Navigation.goBack to pop the current route first before navigating to last accessed report. 
        
               if (!isChatThread) { 
        
                   // Fallback to the lastAccessedReportID route, if this is first route in the navigator 
        
                   Navigation.goBack(lastAccessedReportRoute); 
        
               } 
        
               Navigation.navigate(lastAccessedReportRoute, CONST.NAVIGATION.TYPE.FORCED_UP);

Navigation.goBack(lastAccessedReportRoute);

This will make sure we won't have 2 same report screens in the stack, so we don't need to sync the composer message state.

Job added to Upwork: https://www.upwork.com/jobs/~01321f2f43059fc592

@mollfpr

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

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

@mollfpr

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

Navigate back to the last accessed report when leaving a thread #42132

If no regressions arise, payment will be issued on 2024-06-05. 🎊

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

@mollfpr requires payment through NewDot Manual Requests
@bernhardoj requires payment automatic offer (Contributor)

@mollfpr

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:

[@mollfpr] The PR that introduced the bug has been identified. Link to the PR: [HOLD for payment 2024-06-05] [$250] Android - Composer - Draft text reappears after clearing the compose box and navigating back #41499 (comment)
[@mollfpr] 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: [HOLD for payment 2024-06-05] [$250] Android - Composer - Draft text reappears after clearing the compose box and navigating back #41499 (comment)
[@mollfpr] 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: [HOLD for payment 2024-06-05] [$250] Android - Composer - Draft text reappears after clearing the compose box and navigating back #41499 (comment)
[@mollfpr] Determine if we should create a regression test for this bug.
[@mollfpr] 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.
[@kadiealexander] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: https://github.com/Expensify/Expensify/issues/403795

@bernhardoj

Payouts due:

Contributor: $250 @bernhardoj
Contributor+: $250 @mollfpr

Upwork job is here.

@mollfpr

@mollfpr please don't forget about the checklist!

@mollfpr

@mollfpr bump!

@mollfpr

Sorry for the delay 🙏

[@mollfpr] The PR that introduced the bug has been identified. Link to the PR:
[@mollfpr] 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:

I couldn't trace the offending PR.

[@mollfpr] 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:

The regression step should be enough.

[@mollfpr] Determine if we should create a regression test for this bug.
[@mollfpr] 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.

Open any chat
Send a message
Type a message but don't send it
Right-click/long-press the sent message and choose Reply in the thread
Press the three-dot in the thread and choose Leave
Verify you are navigated to the chat from step 1
Only for mWeb/iOS/Android
Delete the composer message
Close the chat
Reopen the chat
Verify the composer is empty.
👍 or 👎

Payment summary here, please request payment via NewDot.

@mollfpr

$250 approved for @mollfpr

Navigation Menu

Repository navigation

[HOLD for payment 2024-06-05] [$250] Android - Composer - Draft text reappears after clearing the compose box and navigating back #41499

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Proposal

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

What is the root cause of that problem?

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

22 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	if (lastAccessedReportID) {
	const lastAccessedReportRoute = ROUTES.REPORT_WITH_ID.getRoute(lastAccessedReportID ?? '');
	// If it is not a chat thread we should call Navigation.goBack to pop the current route first before navigating to last accessed report.
	if (!isChatThread) {
	// Fallback to the lastAccessedReportID route, if this is first route in the navigator
	Navigation.goBack(lastAccessedReportRoute);
	}
	Navigation.navigate(lastAccessedReportRoute, CONST.NAVIGATION.TYPE.FORCED_UP);

	useEffect(() => {
	// Value state does not have the same value as comment props when the comment gets changed from another tab.
	// In this case, we should synchronize the value between tabs.
	const shouldSyncComment = prevCommentProp !== comment && value !== comment && !isCommentPendingSaved.current;

	// As the report IDs change, make sure to update the composer comment as we need to make sure
	// we do not show incorrect data in there (ie. draft of message from other report).
	if (preferredLocale === prevPreferredLocale && reportID === prevReportId && !shouldSyncComment) {
	return;
	}

	updateComment(comment ?? '');
	}, [prevCommentProp, prevPreferredLocale, prevReportId, comment, preferredLocale, reportID, updateComment, value, commentRef, isCommentPendingSaved]);

	function SilentCommentUpdater({comment, updateComment}: SilentCommentUpdaterProps) {
	useEffect(() => {
	updateComment(comment ?? '');
	// eslint-disable-next-line react-hooks/exhaustive-deps -- We need to run this on mount
	}, []);

	return null;
	}

[HOLD for payment 2024-06-05] [$250] Android - Composer - Draft text reappears after clearing the compose box and navigating back #41499

Description

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Activity

melvin-bot commented on May 2, 2024

lanitochka17 commented on May 2, 2024

lanitochka17 commented on May 2, 2024

bernhardoj commented on May 3, 2024

Proposal

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

What is the root cause of that problem?

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

melvin-bot commented on May 6, 2024

melvin-bot commented on May 6, 2024

22 remaining items

melvin-bot commented on May 29, 2024

melvin-bot commented on May 29, 2024

melvin-bot commented on May 29, 2024

kadiealexander commented on Jun 6, 2024

kadiealexander commented on Jun 6, 2024

kadiealexander commented on Jun 10, 2024

mollfpr commented on Jun 10, 2024

kadiealexander commented on Jun 11, 2024

JmillsExpensify commented on Jun 11, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions