[HOLD for payment 2023-07-12] [$1000] Web - Chat - Flagged attachment are collapsed when reacted with emojis

[kbecciv]

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. Login with user A and go to a chat with user B
2. Send an attachment ( picture/pdf) to user B
3. On user B, first flag the attachment as bullying. Next, ( this is an important step) click on the reveal message. Now react with any emojis
4. Notice the attachment is now hidden.

Expected Result:

Flagged attachment should not be collapsed when reacted with emojis

Actual Result:

Flagged attachments are collapsed when reacted with emojis

Workaround:

Unknown

Platforms:

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

• [ ] Android / native
• [ ] Android / Chrome
• [ ] iOS / native
• [ ] iOS / Safari
• [x] MacOS / Chrome / Safari
• [ ] MacOS / Desktop

Version Number: 1.3.27.6

Reproducible in staging?: yes

Reproducible in production?: yes

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

https://github.com/Expensify/App/assets/93399543/1e742059-34c7-4b9b-b07e-078c71cd06ce

https://github.com/Expensify/App/assets/93399543/83b083f8-5a9d-445f-939c-12f09f12c7f9

Expensify/Expensify Issue URL:

Issue reported by: @ashimsharma10

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1686217468053799

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0192768e5414216425
• Upwork Job ID: 1670794643847507968
• Last Price Increase: 2023-06-19

[melvin-bot[bot]]

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

[melvin-bot[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

[hungvu193]

Seems regression from https://github.com/Expensify/App/pull/20737 That's exact case that I mentioned, is this expected? @aimane-chnaif

[aimane-chnaif]

It's not a regression. We agreed to restore hidden state back when refresh page. And this already happens in production, while that PR is on staging. Though I am not able to reproduce this on latest main. Instead another weird bug happens which is not related to that PR: Flagged attachment completely disappearing. And flagged text message isn't hidden at all.

[hungvu193]

~Seems this was logged before that pr hit stagging, I couldn't reproduce on latest main as well~ I'm able to reproduce it after try to delete cache.

[ashimsharma10]

@hungvu193 are you sure the same issue mentioned above is reproduced?

[aimane-chnaif]

I think there were another recent changes related to flag behavior. Maybe backend updates. After some time, flag cancels.

https://github.com/Expensify/App/assets/96077027/a8a1de54-2f56-4ad5-ad0a-a1796b0f246f

[muttmuure]

I've been trying to reproduce this but I'm seeing that the message isn't being hidden at all

[muttmuure]

Ah OK it was just taking a while

[muttmuure]

OK yep this is reproducible - note it only happens with attachments, when you flag a comment using an option that removes the comment from view, and when you reveal the comment when it is hidden and add an emoji

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~0192768e5414216425

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[mollfpr]

Thanks for the proposal @s-alves10

Could you elaborate on what backend should be fixed and the changes to solve this to remove the pendingHide and this.props.isSmallScreenWidth for the check?

Why this.props.isSmallScreenWidth fix this issue, while the issue is not only on the small screen?

[s-alves10]

@mollfpr

This issue was fixed by this commit https://github.com/Expensify/App/commit/e44d21194de4077cc161762d5cd173263d7fba90

[mollfpr]

@s-alves10 I am still able to reproduce it in staging.

https://github.com/Expensify/App/assets/25520267/1604d81e-6c15-491b-8f76-1b32f0be92e4

[s-alves10]

@mollfpr

Please check it on main branch. Maybe it was not deployed yet

[mollfpr]

@s-alves10 I can reproduce in the latest main (v1.3.30-0) after some time.

1. Send an attachment to User B, e.g., a PDF
2. On User B sent reaction to the PDF and flag the attachment as bullying
3. On User A quickly add another reaction to the attachment and click on reveal message

https://github.com/Expensify/App/assets/25520267/d5fd6c7c-645a-4b04-a628-aeeaa0875e2f

[s-alves10]

Let me check. I'll update you soon

[s-alves10]

Proposal

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

Flagged attachment is hidden when reacting with emojis

What is the root cause of that problem?

We're now setting isHidden flag when latest moderation decision is pendingHide or hidden. This is correct. The problem is when this effect is called. https://github.com/Expensify/App/blob/0b7455fa832b53eccb99ca23c70e131b3464fb1d/src/pages/home/report/ReportActionItem.js#L163-L165

As you can see above, we now call the effect when actionName or moderationDecisions changes, but moderationDecisions is an array(reference) and can change anytime. So this effect is called even when no moderationDecisions is changed. This is the root cause

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

We're interested in the value of moderationDecisions[0].decision. We should change the dependency of the effect. Change the useEffect code block as follows

    const latestDecision = _.get(props, ['action', 'message', 0, 'moderationDecisions', 0, 'decision'], '');
    useEffect(() => {
        if (!props.action.actionName === CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT || _.isEmpty(latestDecision)) {
            return;
        }

        if (latestDecision === CONST.MODERATION.MODERATOR_DECISION_PENDING_HIDE || latestDecision === CONST.MODERATION.MODERATOR_DECISION_HIDDEN) {
            setIsHidden(true);
        }
        setModerationDecision(latestDecision);

    }, [latestDecision, props.action.actionName]);

This works as expected

What alternative solutions did you explore? (Optional)

[mollfpr]

Thanks @s-alves10 for the proposal. @muttmuure The proposal looks good to me 👍

🎀 👀 🎀 C+ reviewed!

[melvin-bot[bot]]

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

[muttmuure]

just need @tylerkaraszewski to assign @s-alves10

[melvin-bot[bot]]

📣 @mollfpr You have been assigned to this job! Please apply to this job in Upwork here and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing 📖

[tylerkaraszewski]

Assigned.

[melvin-bot[bot]]

📣 @s-alves10 🎉 An offer has been automatically sent to your Upwork account 🎉

Contributor - [$1000] Web - Chat - Flagged attachment are collapsed when reacted with emojis 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[bot]]

📣 @ashimsharma10 You have been assigned to this job! Please apply to this job in Upwork here and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing 📖

[s-alves10]

@mollfpr

PR is ready for review

[tylerkaraszewski]

PR is merged.

[tylerkaraszewski]

@muttmuure - I am leaving on sabbatical today. I don't expect this issue to need any further engineering input, but if it does, please unassign me and re-apply the Engineering label to get a new engineer to finish this issue, as I'll be out until the end of August.

[muttmuure]

Waiting for PR to push to prod

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

• https://github.com/Expensify/App/pull/21836

If no regressions arise, payment will be issued on 2023-07-12. :confetti_ball:

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

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

• [ ] [@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:
• [ ] [@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:
• [ ] [@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.
• [x] [@muttmuure] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: https://github.com/Expensify/Expensify/issues/302154

[ashimsharma10]

Updates on this?

[mollfpr]

[@mollfpr] The PR that introduced the bug has been identified. Link to the PR:

https://github.com/Expensify/App/pull/19476

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

https://github.com/Expensify/App/pull/19476/files#r1261372039

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

I think the regression step should be enough. The reviewer will watch a useEffect dependency where it's already documented that we should reference the correct value for its change.

[@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.

1. Login with users A and B on 2 devices
2. A: Send an attachment(picture or pdf) to user B
3. B: Flag the attachment as bullying
4. B: Click on the revealed message to see the attachment
5. B: React with any emojis
6. Verify that the attachment is not hidden
7. 👍 or 👎

[mollfpr]

I notice that the bonus calculation is not posted here.

The assignment is on Jun 29 PR merged Jun 30

[mollfpr]

Friendly bump @muttmuure

[muttmuure]

@mollfpr invited

@ashimsharma10 can you link your upwork profile, we have two Ashim S's in Upwork

[mollfpr]

@muttmuure Applied, thank you!

[muttmuure]

Offer sent

[ashimsharma10]

@muttmuure This is my upwork https://www.upwork.com/freelancers/~018a92cf13e1e88eed

[s-alves10]

@muttmuure

I'm contacting you to know when I would get paid for this issue

cc @mollfpr

[mollfpr]

Friendly bump @muttmuure

[muttmuure]

@ashimsharma10 invited

[muttmuure]

@mollfpr paid

[muttmuure]

@s-alves10 paid

[ashimsharma10]

@muttmuure Accepted invitation. waiting for payment

[muttmuure]

Offer sent

[muttmuure]

@ashimsharma10 paid, test rail proposal created.