Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dev: Web - User can flag a message on admins only room #22237

Closed
1 of 6 tasks
kbecciv opened this issue Jul 5, 2023 · 20 comments
Closed
1 of 6 tasks

Dev: Web - User can flag a message on admins only room #22237

kbecciv opened this issue Jul 5, 2023 · 20 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Needs Reproduction Reproducible steps needed

Comments

@kbecciv
Copy link

kbecciv commented Jul 5, 2023

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 userA account
  2. Go to the “New room” section
  3. Enter room name and select userA workspace
  4. Select visibility to “Public”
  5. Click on “Create room”
  6. Send some messages in room chat
  7. Go to the room detail
  8. Share code > Copy URL to clipboard
  9. Go back and click on Settings > Who can post
  10. Select Admin only
  11. Open other browser and login with userB account
  12. Join the room by pasting URL
  13. Hover or right click on userB comment
  14. Verify that there's no Flag as offensive option.
  15. Click on 'reply in thread' icon
  16. Hover over the threaded message & click on 'flag as offensive' icon
    17, Click on 'Harassment' link

Expected Result:

'Flag as offensive' option shouldn't be available on in the threaded message

Actual Result:

'flag as offensive' option is available & system throws a technical error message (Auth CreateReprtAction returned an error) on flagging the message with harassment.

Workaround:

Unknown

Platforms:

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

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: Dev 1.3.36-4
Reproducible in staging?: n/a
Reproducible in production?: n/a
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

Screencast.from.2023-07-05.00-17-32.mp4

Expensify/Expensify Issue URL:
Issue reported by: @Natnael-Guchima
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1688506126888419

View all open jobs on GitHub

@kbecciv kbecciv added Daily KSv2 Needs Reproduction Reproducible steps needed Bug Something is broken. Auto assigns a BugZero manager. labels Jul 5, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 5, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jul 5, 2023

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
Copy link
Contributor

It's fixed here: #22099

@Natnael-Guchima
Copy link

It's fixed here: #22099

@hungvu193 flagging from a thread scenario doesn't seem to be handled on the pr you have linked.

@hungvu193
Copy link
Contributor

hungvu193 commented Jul 5, 2023

@Natnael-Guchima Good catch, but I don't see Flag as offensive is blocked from backend inside thread. In case, this is a bug, it seems a regression from my PR (#22099) and I'll create PR to fix it.
Do we also need to block thread inside room from flag as offensive cc @allroundexperts @dangrous

@allroundexperts

This comment was marked as outdated.

@allroundexperts
Copy link
Contributor

I've investigated this further and have found that this is an issue with thread titles only. Not sure if this qualifies as a regression or not (since it was broken before as well), but its surely a case that we missed to think of while working on #22099. @hungvu193 Adding something like the following here, might work.

        isAllowedToComment(isThreadFirstChat(reportAction, reportID) ? getParentReport(reportID) : getReport(reportID))

Can you please spin up a PR for this? Let's try to squash this proactively!

@hungvu193
Copy link
Contributor

I also asked from slack and seems we don't block flag comment inside thread, should we wait for final confirmation? @allroundexperts
https://expensify.slack.com/archives/C01GTK53T8Q/p1688543896061599?thread_ts=1688541444.807659&cid=C01GTK53T8Q

@allroundexperts
Copy link
Contributor

I also asked from slack and seems we don't block flag comment inside thread, should we wait for final confirmation? @allroundexperts https://expensify.slack.com/archives/C01GTK53T8Q/p1688543896061599?thread_ts=1688541444.807659&cid=C01GTK53T8Q

Well, If we decide to block comments inside the threads as well, then yea, lets wait!

@hungvu193
Copy link
Contributor

Yeah, because we can create many nested threads as we can inside the room, so if we decided to block the flag comment then we need to handle all these nested thread. Lets wait for final confirmation 😄

@melvin-bot melvin-bot bot added the Overdue label Jul 7, 2023
@lschurr
Copy link
Contributor

lschurr commented Jul 7, 2023

Are we still working on this one? @allroundexperts - Should it be assigned to @hungvu193?

@melvin-bot melvin-bot bot removed the Overdue label Jul 7, 2023
@allroundexperts
Copy link
Contributor

@lschurr After the discussion here, it seems like we'll need changes on the backend as well as the frontend. We need to block the creation of threads by non-admin members of a room. Right now, the backend does allow the creation of threads, it seems. On the other hand, anyone can flag a comment (even if he is not an admin). For this, we need a backend fix as well.

@melvin-bot
Copy link

melvin-bot bot commented Jul 10, 2023

Triggered auto assignment to @puneetlath (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@lschurr
Copy link
Contributor

lschurr commented Jul 10, 2023

@puneetlath adding Eng for another set of eyes. Should I be adding External to this one or do we need to fix internally?

@melvin-bot melvin-bot bot removed the Overdue label Jul 10, 2023
@puneetlath
Copy link
Contributor

This will need to be fixed internally.

But we need to decide what the ideal behavior should be. Whether we allow a non-admin to flag messages in an admin-only chat. I'm thinking that we should allow a non-admin to flag messages, even though they aren't allowed to post in the channel. That would be a good discussion for you to start in Slack @lschurr. And I would recommend cc'ing @dangrous and @amyevans in that discussion since they worked on the moderations/admin-only features.

@dangrous
Copy link
Contributor

dangrous commented Jul 10, 2023

Ha! We have the opposite bug also reported.

Slack conversation to determine expected behavior is ongoing - hop in! Personally I think logged-in users should be able to flag anything they can see

@Natnael-Guchima
Copy link

Natnael-Guchima commented Jul 10, 2023

They are not opposite. They user roles are different on both reports. On the one you have shared a link to the user role is an admin, and in this issue the user role is member. The admin should be able to do anything since, they have a write access on admins only access type, and the member should be constrained to make some changes since thier access is revoked by the admin.

@lschurr
Copy link
Contributor

lschurr commented Jul 12, 2023

Where are we at on this one @dangrous @puneetlath - Did we decide this issue is a bug or expected behavior?

@puneetlath
Copy link
Contributor

Based on the thread, it looks like it's currently the expected behavior but @jasperhuangg is opening a separate issue to update moderation for some more cases. So I think this is fine to close.

@lschurr
Copy link
Contributor

lschurr commented Jul 12, 2023

Cool, thanks! Going to close.

@lschurr lschurr closed this as completed Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Needs Reproduction Reproducible steps needed
Projects
None yet
Development

No branches or pull requests

7 participants