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

kbecciv · 2023-06-15T12:15:48Z

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:

Login with user A and go to a chat with user B
Send an attachment ( picture/pdf) to user B
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
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?

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

Screen.Recording.2023-06-08.at.3.13.21.PM.1.mov

Recording.724.mp4

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 · 2023-06-15T12:15:54Z

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

melvin-bot · 2023-06-15T12:15:56Z

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 · 2023-06-15T13:00:48Z

Seems regression from #20737
That's exact case that I mentioned, is this expected? @aimane-chnaif

aimane-chnaif · 2023-06-15T13:12:56Z

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 · 2023-06-15T13:18:49Z

~~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 · 2023-06-15T13:37:27Z

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

aimane-chnaif · 2023-06-15T13:38:27Z

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

Screen.Recording.2023-06-15.at.2.34.17.PM.mov

muttmuure · 2023-06-19T13:40:53Z

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

muttmuure · 2023-06-19T13:45:57Z

Ah OK it was just taking a while

muttmuure · 2023-06-19T14:02:56Z

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 · 2023-06-19T14:04:22Z

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

melvin-bot · 2023-06-19T14:04:25Z

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

melvin-bot · 2023-06-19T14:04:32Z

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

mollfpr · 2023-06-20T13:55:19Z

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?

dummy-1111 · 2023-06-20T15:00:38Z

@mollfpr

This issue was fixed by this commit e44d211

mollfpr · 2023-06-21T13:01:28Z

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

Screen.Recording.2023-06-20.at.23.09.28.mov

dummy-1111 · 2023-06-21T13:04:42Z

@mollfpr

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

mollfpr · 2023-06-21T13:30:54Z

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

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

Screen.Recording.2023-06-21.at.21.24.07.mov

dummy-1111 · 2023-06-21T13:35:26Z

Let me check. I'll update you soon

dummy-1111 · 2023-06-21T16:11:51Z

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.

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

Lines 163 to 165 in 0b7455f

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

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 · 2023-06-23T13:48:19Z

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

🎀 👀 🎀 C+ reviewed!

melvin-bot · 2023-07-05T16:55:54Z

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

melvin-bot · 2023-07-05T16:55:55Z

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

Fix/issue 20810 #21836

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

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 · 2023-07-05T16:55:57Z

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.
[@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 · 2023-07-12T10:26:01Z

Updates on this?

mollfpr · 2023-07-12T15:50:06Z

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

#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.

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

mollfpr · 2023-07-12T15:52:04Z

I notice that the bonus calculation is not posted here.

The assignment is on Jun 29
PR merged Jun 30

mollfpr · 2023-07-14T04:29:07Z

Friendly bump @muttmuure

muttmuure · 2023-07-17T13:52:41Z

@mollfpr invited

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

mollfpr · 2023-07-17T13:54:58Z

@muttmuure Applied, thank you!

muttmuure · 2023-07-17T14:11:26Z

Offer sent

ashimsharma10 · 2023-07-17T16:46:36Z

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

dummy-1111 · 2023-07-18T23:04:20Z

@muttmuure

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

cc @mollfpr

mollfpr · 2023-07-20T13:37:36Z

Friendly bump @muttmuure

muttmuure · 2023-07-20T17:16:11Z

@ashimsharma10 invited

muttmuure · 2023-07-20T17:21:08Z

@mollfpr paid

muttmuure · 2023-07-20T17:28:33Z

@s-alves10 paid

ashimsharma10 · 2023-07-21T08:23:28Z

@muttmuure Accepted invitation. waiting for payment

muttmuure · 2023-07-21T10:51:23Z

Offer sent

muttmuure · 2023-07-23T22:43:31Z

@ashimsharma10 paid, test rail proposal created.

kbecciv added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 15, 2023

melvin-bot bot assigned muttmuure Jun 15, 2023

melvin-bot bot added the Overdue label Jun 19, 2023

melvin-bot bot removed the Overdue label Jun 19, 2023

muttmuure added the External Added to denote the issue can be worked on by a contributor label Jun 19, 2023

melvin-bot bot changed the title ~~Web - Chat - Flagged attachment are collapsed when reacted with emojis~~ [$1000] Web - Chat - Flagged attachment are collapsed when reacted with emojis Jun 19, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 19, 2023

melvin-bot bot assigned mollfpr Jun 19, 2023

melvin-bot bot added the Awaiting Payment Auto-added when associated PR is deployed to production label Jul 5, 2023

melvin-bot bot changed the title ~~[$1000] Web - Chat - Flagged attachment are collapsed when reacted with emojis~~ [HOLD for payment 2023-07-12] [$1000] Web - Chat - Flagged attachment are collapsed when reacted with emojis Jul 5, 2023

melvin-bot bot removed the Reviewing Has a PR in review label Jul 5, 2023

melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jul 11, 2023

mollfpr mentioned this issue Jul 12, 2023

[No QA] Handle Moderated Messages in UI #19476

Merged

57 tasks

melvin-bot bot added the Overdue label Jul 17, 2023

melvin-bot bot removed the Overdue label Jul 17, 2023

melvin-bot bot added the Overdue label Jul 23, 2023

muttmuure closed this as completed Jul 23, 2023

melvin-bot bot removed the Overdue label Jul 23, 2023

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

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

Comments

kbecciv commented Jun 15, 2023 • edited Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

melvin-bot bot commented Jun 15, 2023

melvin-bot bot commented Jun 15, 2023

Bug0 Triage Checklist (Main S/O)

hungvu193 commented Jun 15, 2023

aimane-chnaif commented Jun 15, 2023 • edited Loading

hungvu193 commented Jun 15, 2023 • edited Loading

ashimsharma10 commented Jun 15, 2023

aimane-chnaif commented Jun 15, 2023

muttmuure commented Jun 19, 2023

muttmuure commented Jun 19, 2023

muttmuure commented Jun 19, 2023 • edited Loading

melvin-bot bot commented Jun 19, 2023

melvin-bot bot commented Jun 19, 2023

melvin-bot bot commented Jun 19, 2023

mollfpr commented Jun 20, 2023

dummy-1111 commented Jun 20, 2023

mollfpr commented Jun 21, 2023

dummy-1111 commented Jun 21, 2023

mollfpr commented Jun 21, 2023 • edited Loading

dummy-1111 commented Jun 21, 2023

dummy-1111 commented Jun 21, 2023

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?

What alternative solutions did you explore? (Optional)

mollfpr commented Jun 23, 2023

melvin-bot bot commented Jul 5, 2023

melvin-bot bot commented Jul 5, 2023

melvin-bot bot commented Jul 5, 2023 • edited by muttmuure Loading

ashimsharma10 commented Jul 12, 2023

mollfpr commented Jul 12, 2023

mollfpr commented Jul 12, 2023

mollfpr commented Jul 14, 2023

muttmuure commented Jul 17, 2023

mollfpr commented Jul 17, 2023

muttmuure commented Jul 17, 2023

ashimsharma10 commented Jul 17, 2023

dummy-1111 commented Jul 18, 2023 • edited Loading

mollfpr commented Jul 20, 2023

muttmuure commented Jul 20, 2023

muttmuure commented Jul 20, 2023

muttmuure commented Jul 20, 2023

ashimsharma10 commented Jul 21, 2023

muttmuure commented Jul 21, 2023

muttmuure commented Jul 23, 2023

kbecciv commented Jun 15, 2023 •

edited

Loading

aimane-chnaif commented Jun 15, 2023 •

edited

Loading

hungvu193 commented Jun 15, 2023 •

edited

Loading

muttmuure commented Jun 19, 2023 •

edited

Loading

mollfpr commented Jun 21, 2023 •

edited

Loading

melvin-bot bot commented Jul 5, 2023 •

edited by muttmuure

Loading

dummy-1111 commented Jul 18, 2023 •

edited

Loading