[HOLD for payment 2024-04-05] [$500] Incorrect title for "Edited money request" thread

[m-natarajan]

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.44-11
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
Expensify/Expensify Issue URL:
Issue reported by: @paultsimura
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1709138721803769

Action Performed:

1. Request money
2. Open the created money request
3. Modify a field (e.g. merchant)
4. Find the system message that says "changed the merchant to ..."
5. Click "Reply in thread"
6. Check the title in the header

Expected Result:

It should reflect the text of the system

Actual Result:

It shows "You" (or the other actor's name)

Workaround:

unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Recording.2804.mp4

Screen.Recording.2024-02-28.at.17.44.31-compressed.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0195989b41b3f25ded
• Upwork Job ID: 1762897098637885440
• Last Price Increase: 2024-02-28

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0195989b41b3f25ded

[melvin-bot]

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

[melvin-bot]

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

[FitseTLT]

Proposal

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

Incorrect title for "Edited money request" thread

What is the root cause of that problem?

We are using message?.[0]?.text of parent report action

App/src/libs/ReportUtils.ts

Lines 2512 to 2513 in fe6d902

	const parentReportActionMessage = (parentReportAction?.message?.[0]?.text ?? '').replace(/(\r\n|\n|\r)/gm, ' ');
	if (isAttachment && parentReportActionMessage) {

This case assumes that message array has one item and all the data that should be displayed is found in it but it falls short in most case as in this one where the reportaction is MODIFIEDEXPENSE or another example is when it is SUBMITTED or APPROVED report action on when the submission or approval was done on behalf of someone (like admin submitting the employee report or auto approval of concierge) which have multiple text to concatenate and display in reportAction.message

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

What we should do is the same as what we use in report action display where we join the message fragment and display it

App/src/pages/home/report/ReportActionItemMessage.tsx

Lines 37 to 38 in fe6d902

	const fragments = (action.previousMessage ?? action.message ?? []).filter((item) => !!item);
	const isIOUReport = ReportActionsUtils.isMoneyRequestAction(action);

App/src/pages/home/report/ReportActionItemMessage.tsx

Lines 75 to 79 in fe6d902

	const reportActionItemFragments = fragments.map((fragment, index) => (
	<ReportActionItemFragment
	/* eslint-disable-next-line react/no-array-index-key */
	key={`actionFragment-${action.reportActionID}-${index}`}
	fragment={fragment}

Here are some of the report actions we display it in this manner

App/src/pages/home/report/ReportActionItem.js

Lines 471 to 479 in fe6d902

	} else {
	const hasBeenFlagged =
	!_.contains([CONST.MODERATION.MODERATOR_DECISION_APPROVED, CONST.MODERATION.MODERATOR_DECISION_PENDING], moderationDecision) &&
	!ReportActionsUtils.isPendingRemove(props.action);
	children = (
	<ShowContextMenuContext.Provider value={contextValue}>
	{_.isUndefined(props.draftMessage) ? (
	<View style={props.displayAsGroup && hasBeenFlagged ? styles.blockquote : {}}>
	<ReportActionItemMessage

So what we should do is join the fragment of the message instead of using only message.[0].text for all report action types so that when it has only one fragment it will be same as before and additionally We will also be able to solve this current issue with others that can arise when there are multiple fragments for eg cases I have mentioned above.
We can use this method here by making it as a util

App/src/pages/home/report/ContextMenu/ContextMenuActions.tsx

Lines 39 to 42 in fe6d902

	/** Gets the text version of the message in an action */
	function getActionText(reportAction: OnyxEntry<ReportAction>): string {
	return reportAction?.message?.reduce((acc, curr) => `${acc}${curr.text}`, '') ?? '';
	}

What alternative solutions did you explore? (Optional)

Alternatively we can apply this logic for only selected report action types such as MODIFIEDEXPENSE APPROVED SUBMITTED etc

[dukenv0307]

Proposal

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

• Incorrect title for "Edited money request" thread

What is the root cause of that problem?

• In case of MODIFIEDEXPENSE, the text "You" is from the below logic:
  

  App/src/libs/ReportUtils.ts

  Line 2512 in c6dd8ca

  const parentReportActionMessage = (parentReportAction?.message?.[0]?.text ?? '').replace(/(\r\n|\n|\r)/gm, ' ');

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

• We can check if the action is modified expense message, and use ModifiedExpenseMessage.getForReportAction(report?.reportID, parentReportAction) instead
• For example, we can update this to:

         if (ReportActionsUtils.isModifiedExpenseAction(parentReportAction)) {
            return ModifiedExpenseMessage.getForReportAction(report?.reportID, parentReportAction);
        }
        return parentReportActionMessage;

What alternative solutions did you explore? (Optional)

• NA

Result

Screencast.from.29-02-2024.02.06.07.webm

[mkhutornyi]

@dukenv0307 your proposal doesn't fix other types of report actions.
i.e. The thread title of You submitted this report to xxx is still "You"

[mkhutornyi]

@FitseTLT's proposal looks good to me.
Thread title should be exactly same as the text user sees in parent action. So sharing same logic between them makes sense.
🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[dukenv0307]

@mkhutornyi Yes. When creating the proposal, I just plan to fix the action type in this issue, which is MODIFIEDEXPENSE. So we need to confirm if we need to scale the scope of this issue

[FitseTLT]

@mkhutornyi Yes. When creating the proposal, I just plan to fix the action type in this issue, which is MODIFIEDEXPENSE. So we need to confirm if we need to scale the scope of this issue

@dukenv0307 your proposal doesn't deal with the problem of this issue which is to align what is displayed in the report action with the thread report header text. And in terms of scope, my proposal is not based on dealing with other report actions type but correcting the logic of using only the first fragment of the message to looping through all message fragments which is what we use to display the report actions and hence it will solve the problem from the root cause and of course, as a result it also fixes other similar issues with other types of report actions as well 👍

[dukenv0307]

which is to align what is displayed in the report action with the thread report header text

• Yes. This is what my proposal mentioned. Here is how we display the MODIFIEDEXPENSE action:
  

  App/src/pages/home/report/ReportActionItem.js

  Lines 465 to 466 in fe6d902

  	} else if (props.action.actionName === CONST.REPORT.ACTIONS.TYPE.MODIFIEDEXPENSE) {
  	children = <ReportActionItemBasicMessage message={ModifiedExpenseMessage.getForReportAction(props.report.reportID, props.action)} />;

  
  and I suggest we should use the same in case of report header text

• Let C+ and internal team confirm this one.

[dukenv0307]

@mkhutornyi Also, I think that the selected solution does not align what is displayed in the report action with the thread report header text because in case of MODIFIEDEXPENSE. Because we are using ModifiedExpenseMessage.getForReportAction to get the displayed report action but the selected solution will join all the message fragment

Please correct me if I misunderstand something

[mkhutornyi]

join all the message fragment

@dukenv0307 would you mind sharing any example case where this is wrong?

[dukenv0307]

@mkhutornyi

• In case you change any field such as amount, merchant, ... then change the language, because getActionText do not have translation logic.
• Or, with the action that has a message is (You can create by modifying the request `s amount):

[
    {
        "type": "TEXT",
        "style": "strong",
        "text": "You"
    },
    {
        "type": "TEXT",
        "style": "normal",
        "text": " modified expense ''. "
    },
    {
        "type": "TEXT",
        "style": "normal",
        "text": "New amount is '₫1,212,123' (previously '₫1,212'). "
    }
]

we are displaying the report action as "changed the amount to ₫1,212,123 (previously ₫1,212).". But if we just join the fragments, the similar cannot be created

[mkhutornyi]

I didn't mean just join the fragments. I approved the solution of appying same logic used to display parent action message.
@FitseTLT can you confirm?

[mkhutornyi]

ok please share your testing branches. @FitseTLT @dukenv0307
I planned to fix any regressions in PR but as @dukenv0307 persists, I will reject the solution which causes any minor regressions.

[dukenv0307]

@mkhutornyi Here is my testing branch

[dukenv0307]

@mkhutornyi Is this your final decision?

[mkhutornyi]

Been waiting for @FitseTLT to share his branch.
If no update from them today, will recommend @dukenv0307 assuming that @FitseTLT's solution caused regression which is not fixable.

[melvin-bot]

@cristipaval @lschurr @mkhutornyi @dukenv0307 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!

[dukenv0307]

@mkhutornyi PR #38267 is ready to review

[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.4.57-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix/37448: Incorrect title thread #38267

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

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

• @mkhutornyi requires payment (Needs manual offer from BZ)
• @dukenv0307 requires payment (Needs manual offer from BZ)
• @paultsimura requires payment (Needs manual offer from BZ)

[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:

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

[lschurr]

@dukenv0307 can you work through the checklist for this one?

[lschurr]

Payment summary:

• Contributor: @dukenv0307 $500 (Offer sent in Upwork)
• Contributor+: @mkhutornyi $500 (Offer sent in Upwork)

[melvin-bot]

Payment Summary

Upwork Job

• ROLE: @mkhutornyi paid $(AMOUNT) via Upwork (LINK)
• ROLE: @dukenv0307 paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@lschurr)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1762897098637885440/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[lschurr]

Still waiting for @dukenv0307 and @mkhutornyi to accept the offers. I will move to Weekly in the meantime.

[lschurr]

Both offers have expired. I'm going to close this GH for now since I haven't heard from @dukenv0307 or @mkhutornyi - Please reopen or reach out via Slack when you are able to accept a new offer in Upwork.

[lschurr]

New offer sent to @dukenv0307 - https://www.upwork.com/nx/wm/offer/102258589

[dukenv0307]

@lschurr I accepted the offer!

[lschurr]

Great! Payment issued in Upwork to you @dukenv0307

[lschurr]

New offer sent to @mkhutornyi - https://www.upwork.com/nx/wm/offer/102626655