[HOLD for payment 2023-07-10] [$1000] Users are able to flag their own comment #21212

kbecciv · 2023-06-21T14:30:12Z

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 2 account (A and B)
Create a group chat between them, send some messages from account A.
From account B, right click and choose Flag as offensive, copy the link and paste to the browser that account A is logging in.
The Flag As Offensive page opened and you can flag your own comment.

Expected Result:

Users shouldn't be able to flag their own message

Actual Result:

Users are able to flag their own comment.

Workaround:

Unknown

Platforms:

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

Version Number: 1.3.27-6

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

Screen.Recording.2023-06-14.at.18.22.57.3.mov

Screen.Recording.2023-06-14.at.18.22.57.2.mov

Expensify/Expensify Issue URL:

Issue reported by: @hungvu193

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

Recording.802.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~012f9f19f3eff4c785
Upwork Job ID: 1671569938552557568
Last Price Increase: 2023-06-21

melvin-bot · 2023-06-21T14:30:17Z

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

melvin-bot · 2023-06-21T14:30:19Z

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

kbecciv · 2023-06-21T14:31:08Z

Proposal

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

Users are able to flag their own comment.

What is the root cause of that problem?

We are not checking if current reportAction is your own comment inside FlagCommentPage.

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

We should check if current reportAction is your own reportAction then we can decide the next step (it's depend on expected behavior). We can show not found view or set severityMenuItems to empty and leave a placeholder or dismiss the modal.

    const [shouldShowBlockingView, setShowBlockingView] = React.useState(false)
    React.useEffect(() => {
        let reportToCheck;
        // handle for thread
        if (reportAction === undefined) {
            reportToCheck = ReportActionsUtils.getParentReportAction(props.report);
        } else {
            reportToCheck = reportAction
        }

        if (props.session.email === reportToCheck.actorEmail) {
            setShowBlockingView(true); // or we can dismiss this modal
        }
    }, [reportAction, props.report]);

    if (shouldShowBlockingView) {
        return <NotFoundPage />
    }

What alternative solutions did you explore? (Optional)

N/A

dukenv0307 · 2023-06-21T15:13:39Z

Proposal

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

Users are able to flag their own comment

What is the root cause of that problem?

We don't prevent the user access to the page to flag their comment by URL flag/:reportID/:reportActionID

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

In FlagCommentPage, We should use FullPageNotFoundView with conditions like here

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

Lines 317 to 323 in 5f5dd02

    
           shouldShow: (type, reportAction, isArchivedRoom, betas, menuTarget, isChronosReport, reportID) => 
        
               type === CONTEXT_MENU_TYPES.REPORT_ACTION && 
        
               ReportUtils.canFlagReportAction(reportAction, reportID) && 
        
               !isArchivedRoom && 
        
               !isChronosReport && 
        
               !ReportUtils.isConciergeChatReport(reportID) && 
        
               reportAction.actorEmail !== CONST.EMAIL.CONCIERGE,

Also should use isLoadingReportData props from ONYX to check if the app is loading

<ScreenWrapper includeSafeAreaPaddingBottom={false}>
            {({safeAreaPaddingBottomStyle}) => (
                <FullPageNotFoundView shouldShow={
                       !isLoadingReportData && 
                       ReportUtils.canFlagReportAction(reportAction, reportID) &&
                       !isArchivedRoom &&
                       !isChronosReport &&
                       !ReportUtils.isConciergeChatReport(reportID) &&
                       reportAction.actorEmail !== CONST.EMAIL.CONCIERGE
               }>
                    <HeaderWithBackButton title={props.translate('reportActionContextMenu.flagAsOffensive')} />
                    <ScrollView
                     ....
                </FullPageNotFoundView>
            )}
        </ScreenWrapper>

We also should move this code to outer of FlagComment Function to handle thread case

App/src/pages/FlagCommentPage.js

Lines 119 to 122 in 0b7455f

    
           if (reportAction === undefined) { 
        
               reportID = ReportUtils.getParentReport(props.report).reportID; 
        
               reportAction = ReportActionsUtils.getParentReportAction(props.report); 
        
           }

As this comment, In case, one account has 2 contact method (A and B), If we decide that A can't flag the comment of B we can update canEditReportAction function (because as I suggest above, we also use this function in FlagCommentPage)

App/src/libs/ReportUtils.js

Lines 201 to 210 in 5f5dd02

    
           function canEditReportAction(reportAction) { 
        
               return ( 
        
                   reportAction.actorEmail === sessionEmail && 
        
                   reportAction.actionName === CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT && 
        
                   !isReportMessageAttachment(lodashGet(reportAction, ['message', 0], {})) && 
        
                   !ReportActionsUtils.isDeletedAction(reportAction) && 
        
                   !ReportActionsUtils.isCreatedTaskReportAction(reportAction) && 
        
                   reportAction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE 
        
               ); 
        
           }

We can use props.loginList instead of only sessionEmail to check

Result

melvin-bot · 2023-06-21T17:25:06Z

Job added to Upwork: https://www.upwork.com/jobs/~012f9f19f3eff4c785

melvin-bot · 2023-06-21T17:25:10Z

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

melvin-bot · 2023-06-21T17:25:16Z

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

ygshbht · 2023-06-21T20:43:58Z

Proposal

Hello Expensify Team,

I'm Yogesh, a seasoned web developer with a robust track record on Upwork. My profile, which can be viewed here: https://www.upwork.com/freelancers/~01ca41cc1d47038084, showcases my 100% job success rate and a slew of positive reviews from satisfied clients. For a comprehensive look at the diverse projects I've contributed to, feel free to visit my portfolio at yogeshbhatt.com.

On to the matter at hand: users being able to flag their own comments. My initial approach to resolve this would be to implement a backend check that verifies whether the commenter and the flagger are the same person. The outcome of this check could then be relayed to the frontend, where an appropriate error message could be displayed if necessary.

However, it's important to note that the exact solution can only be ascertained after a thorough examination of your existing codebase and its architecture. As such, while the approach I've outlined seems plausible based on the information currently at hand, I would need to delve deeper into your code to confirm its viability and ensure its seamless integration with your current setup.

As an experienced contributor, I've familiarized myself thoroughly with your Contributor Guidelines and README documentation. If granted the opportunity, I'm ready to uphold the highest standards of coding and conduct as part of your team.

I look forward to the possibility of working together and helping to further enhance the user experience on your platform.

Best regards,
Yogesh

abekkala · 2023-06-22T13:54:56Z

@ygshbht can you review the contributor guidelines here and ensure your proposal is in the correct format.

Thanks!

ygshbht · 2023-06-22T14:43:14Z

@abekkala Thank you for pointing that out. As I'm relatively new here, your guidance is much appreciated

Proposal

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

The problem is that users are able to flag their own comments, which could lead to misuse and an inconsistent user experience.

What is the root cause of that problem?

The issue arises due to a lack of validation to prevent users from flagging their own comments.

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

A potential fix involves introducing a check in the front-end, if data is available, to validate whether the flagger and the commenter are the same user. If they match, flagging should be prevented. However, once I start working, alternative solutions might emerge, depending on where the root cause lies

abekkala · 2023-06-26T14:56:06Z

@mananjadhav have you had time to review the proposals?

mananjadhav · 2023-06-27T03:24:40Z

@hungvu193 @dukenv0307 The RCA is correct for both the proposals, but could you explain how will you check if the comment is by the logged in user? Also consider the secondary login aspect.

@ygshbht Some bits of technical details on what will be changed, where, how will you put the validation would be helpful to review the proposal.

hungvu193 · 2023-06-27T03:28:17Z

props.session.email === reportToCheck.actorEmail

@mananjadhav We can always get the actorEmail, I think we already had something similar here:

function canFlagReportAction(reportAction) {
    return (
        reportAction.actorEmail !== sessionEmail &&
        reportAction.actionName === CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT &&
        !ReportActionsUtils.isDeletedAction(reportAction) &&
        !ReportActionsUtils.isCreatedTaskReportAction(reportAction)
    );
}

And you're right, we should update the condition to compare it the loginList instead of only current logged email. When I changed the contact methods, it will also change the actorEmail in reportAction.

Should be something like this:

if (_.keys(props.loginList).includes(reportAction.actorEmail) {
  // then decide to do something here
}

We can also update the canFlagReportAction function.

dukenv0307 · 2023-06-27T04:29:11Z

@mananjadhav Thanks for your reminder, updated proposal

mananjadhav · 2023-06-27T10:23:22Z

While both the proposals identified the correct root cause, I think @dukenv0307's proposal is good, because we're using FullPageNotFoundView.

While I don't think it's a good idea to use so many conditions in shouldShow, we could move this to a util.

🎀 👀 🎀 C+ reviewed.

melvin-bot · 2023-06-27T10:23:28Z

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

melvin-bot · 2023-06-27T17:24:36Z

📣 @dukenv0307 You have been assigned to this job by @tylerkaraszewski!
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 📖

tylerkaraszewski · 2023-06-27T17:24:43Z

Giving this one to @dukenv0307

melvin-bot · 2023-07-10T20:36:22Z

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

melvin-bot · 2023-07-10T20:36:24Z

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

mananjadhav · 2023-07-11T15:51:57Z

@flaviadefaria @CortneyOfstad The regression isn't from the PR. The offending PR for this issue is here.

I've posted a comment here. @tylerkaraszewski I don't think any action is required here and I also don't think we'll need any regression test here? wdyt?

mananjadhav · 2023-07-11T15:52:11Z

@CortneyOfstad I'll raise a payout request for this one NewDot.

dangrous · 2023-07-11T20:38:34Z

Technically we should probably block this on the backend as well, although it's more of an annoyance than anything else. cc @thienlnam and I guess @tylerkaraszewski as the internal teammember on this issue

mananjadhav · 2023-07-12T15:35:36Z

Technically we should probably block this on the backend as well

@dangrous Agreed. Do we want to create another issue in the Web repo? or we'll track using this one?

@CortneyOfstad I've raised a request for my side. Can you please help with the payout for @dukenv0307 ?

dangrous · 2023-07-12T19:28:57Z

Yeah I can make a separate issue!

CortneyOfstad · 2023-07-13T12:16:20Z

@mananjadhav sounds good and to confirm, there is a $500 bonus as well? Just reviewing the contract in Upwork and wanted to confirm for @dukenv0307. TIA!

anmurali · 2023-07-13T12:20:08Z

Paid Manan after confirming here.

CortneyOfstad · 2023-07-14T14:28:04Z

@mananjadhav bump on the comment above!

mananjadhav · 2023-07-15T03:41:35Z

Yes @CortneyOfstad there's a bonus as well.

CortneyOfstad · 2023-07-18T11:41:41Z

Tackling payment now 👍

CortneyOfstad · 2023-07-18T11:44:20Z

@dukenv0307 sent the contract to you in UpWork — let me know as soon as you accept it and I can get that paid to you ASAP. Thanks for your patience, and sorry for the delay!

dukenv0307 · 2023-07-18T12:15:38Z

@CortneyOfstad accepted, thank you!

CortneyOfstad · 2023-07-18T12:34:24Z

Contract paid and completed! Closing!

hungvu193 · 2023-07-18T12:36:49Z

Contract paid and completed! Closing!

@CortneyOfstad you forgot me 🥲

CortneyOfstad · 2023-07-18T12:42:03Z

So I'm a bit confused. @mananjadhav can you provide some context?

hungvu193 · 2023-07-18T12:44:37Z

Ah I'm the reporter

CortneyOfstad · 2023-07-18T12:47:54Z

OMG, I am so sorry! It's been a while since I've had to do this, so I'm a bit rusty! 😂

Paying you now!! So sorry!!

hungvu193 · 2023-07-18T12:52:01Z

OMG, I am so sorry! It's been a while since I've had to do this, so I'm a bit rusty! 😂

Paying you now!! So sorry!!

No worries 🤣

CortneyOfstad · 2023-07-18T12:52:52Z

All paid!

mananjadhav · 2023-07-18T14:38:55Z

Thanks @CortneyOfstad. Can we close this now?

CortneyOfstad · 2023-07-18T15:15:58Z

Yep — closing now!

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

melvin-bot bot assigned abekkala Jun 21, 2023

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

melvin-bot bot changed the title ~~Users are able to flag their own comment~~ [$1000] Users are able to flag their own comment Jun 21, 2023

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

melvin-bot bot assigned mananjadhav Jun 21, 2023

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

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

melvin-bot bot assigned tylerkaraszewski Jun 27, 2023

tylerkaraszewski assigned dukenv0307 Jun 27, 2023

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

melvin-bot bot assigned CortneyOfstad Jul 10, 2023

mananjadhav mentioned this issue Jul 11, 2023

Create FlagCommentPage #19412

Merged

57 tasks

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

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

CortneyOfstad closed this as completed Jul 18, 2023

CortneyOfstad reopened this Jul 18, 2023

CortneyOfstad closed this as completed Jul 18, 2023

[HOLD for payment 2023-07-10] [$1000] Users are able to flag their own comment #21212

[HOLD for payment 2023-07-10] [$1000] Users are able to flag their own comment #21212

Comments

kbecciv commented Jun 21, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

melvin-bot bot commented Jun 21, 2023

melvin-bot bot commented Jun 21, 2023

Bug0 Triage Checklist (Main S/O)

kbecciv 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)

dukenv0307 commented Jun 21, 2023 • edited Loading

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?

Result

melvin-bot bot commented Jun 21, 2023

melvin-bot bot commented Jun 21, 2023

melvin-bot bot commented Jun 21, 2023

ygshbht commented Jun 21, 2023 • edited Loading

Proposal

abekkala commented Jun 22, 2023

ygshbht commented Jun 22, 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?

abekkala commented Jun 26, 2023 • edited Loading

mananjadhav commented Jun 27, 2023

hungvu193 commented Jun 27, 2023 • edited Loading

dukenv0307 commented Jun 27, 2023

mananjadhav commented Jun 27, 2023

melvin-bot bot commented Jun 27, 2023

melvin-bot bot commented Jun 27, 2023

tylerkaraszewski commented Jun 27, 2023

melvin-bot bot commented Jul 10, 2023

melvin-bot bot commented Jul 10, 2023

Bug0 Triage Checklist (Main S/O)

mananjadhav commented Jul 11, 2023

mananjadhav commented Jul 11, 2023

dangrous commented Jul 11, 2023

mananjadhav commented Jul 12, 2023

dangrous commented Jul 12, 2023

CortneyOfstad commented Jul 13, 2023 • edited Loading

anmurali commented Jul 13, 2023

CortneyOfstad commented Jul 14, 2023

mananjadhav commented Jul 15, 2023

CortneyOfstad commented Jul 18, 2023

CortneyOfstad commented Jul 18, 2023

dukenv0307 commented Jul 18, 2023 • edited Loading

CortneyOfstad commented Jul 18, 2023

hungvu193 commented Jul 18, 2023

CortneyOfstad commented Jul 18, 2023

hungvu193 commented Jul 18, 2023

CortneyOfstad commented Jul 18, 2023

hungvu193 commented Jul 18, 2023

CortneyOfstad commented Jul 18, 2023

mananjadhav commented Jul 18, 2023

CortneyOfstad commented Jul 18, 2023

kbecciv commented Jun 21, 2023 •

edited by melvin-bot bot

Loading

dukenv0307 commented Jun 21, 2023 •

edited

Loading

ygshbht commented Jun 21, 2023 •

edited

Loading

abekkala commented Jun 26, 2023 •

edited

Loading

hungvu193 commented Jun 27, 2023 •

edited

Loading

CortneyOfstad commented Jul 13, 2023 •

edited

Loading

dukenv0307 commented Jul 18, 2023 •

edited

Loading