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

[HOLD for Payment 2024-06-21] Flag message - "Only visible to you" disappears after flagging the whisper, and then reappears #42557

Closed
6 tasks done
lanitochka17 opened this issue May 23, 2024 · 25 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff

Comments

@lanitochka17
Copy link

lanitochka17 commented May 23, 2024

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.75-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

Precondition:

  • User is invited to a workspace
  1. Go to staging.new.expensify.com
  2. Go to workspace chat
  3. Right click on the welcome whisper message
  4. Click Flag as offensive
  5. Select Spam

Expected Result:

"Only visible to you" will remain above the welcome message after flagging the whisper

Actual Result:

Only visible to you" disappears after flagging the whisper, and then reappears

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

Bug6489769_1716490934768.bandicam_2024-05-24_02-58-34-780.mp4

View all open jobs on GitHub

@lanitochka17 lanitochka17 added DeployBlockerCash This issue or pull request should block deployment DeployBlocker Indicates it should block deploying the API labels May 23, 2024
Copy link

melvin-bot bot commented May 23, 2024

Triggered auto assignment to @NikkiWines (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

Copy link

melvin-bot bot commented May 23, 2024

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open Staging deploy checklist to see the list of PRs included in this release, then work quickly on the following:

  1. If you find which PR caused the issue/bug, you can reassign it to the person responsible for it.
    • If the author is OOO or won’t get online before the daily deploy is due, you are responsible for finding the best fix/path forward. Don’t hesitate to ask for help!
  2. Try to reproduce the issue, if the bug is on production, remove the DeployBlocker label but stay assigned to fix it (or find out which PR broke it to get help from the author).
    • You can adjust the urgency of the issue to better represent the gravity of the bug.
    • If the issue is super low priority, feel free to un-assign yourself.
    • Be careful with PHP warnings, sometimes it is more complex than just adding a null coalescing operator as they might be uncovering some bigger bug.
    • If it was a one-off issue that requires no action (for example, Bedrock was down or it is a duplicated issue), you can close it.

Remember rule #2: Never un-assign yourself from a real DeployBlocker unless you are 100% sure someone else is assigned and will take care of it.

Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@lanitochka17
Copy link
Author

@NikkiWines FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

@NikkiWines
Copy link
Contributor

Given the testing steps, seems like it might be tied to #41622. @dangrous could you confirm?

@dangrous
Copy link
Contributor

Hm yeah looks like it might be a missed piece from that implementation. Maybe isWhisper is no longer returning true when it's flagged? Not sure why though - I'm happy to look into it, though, since it's my code, or we can push external?

@NikkiWines
Copy link
Contributor

If you want to look, I'd be happy to review your PR 🤝

@dangrous
Copy link
Contributor

Cool, I can self-assign this now and take a look when I'm back in on Tuesday... is that too late? I don't think this needs to be a deploy blocker, imo.

@mountiny mountiny added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 DeployBlocker Indicates it should block deploying the API labels May 23, 2024
@mountiny
Copy link
Contributor

Given the fact the state of the app eventually gets to the correct state I dont think this has to be a blocker so removing the labels. Also assigned Daniel to follow up on this one. Thank you!

@melvin-bot melvin-bot bot added the Overdue label May 27, 2024
Copy link

melvin-bot bot commented May 27, 2024

@dangrous, @NikkiWines Whoops! This issue is 2 days overdue. Let's get this updated quick!

@dangrous
Copy link
Contributor

will look today!

@dangrous
Copy link
Contributor

Okay I've figured out why - #40020 (comment) - but I'm not sure why that code exists in the first place. I can fix it by just deleting the check for pendingActions in isWhisper but need to make sure that that won't cause side effects.

@melvin-bot melvin-bot bot added the Overdue label May 30, 2024
Copy link

melvin-bot bot commented May 31, 2024

@dangrous, @NikkiWines Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@NikkiWines
Copy link
Contributor

Unassigning myself, since @dangrous is looking into this!

@melvin-bot melvin-bot bot removed the Overdue label Jun 3, 2024
@dangrous
Copy link
Contributor

dangrous commented Jun 6, 2024

Based on @ishpaul777's testing I think we can move forward with removing that check. I'll make a draft PR, and then include testing for both this issue as well as the testing steps from #40020 and we can go ahead with that for now (while we're waiting to hear back from the contributor)

Copy link

melvin-bot bot commented Jun 11, 2024

@dangrous Eep! 4 days overdue now. Issues have feelings too...

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Overdue Daily KSv2 labels Jun 11, 2024
@ikevin127
Copy link
Contributor

⚠️ Automation failed for this issue -> this should be on [HOLD for Payment 2024-06-21] according to yesterday's production deploy from #43198 (comment). I require payment here for C+ review of PR #43198.

@dangrous Could you please add the HOLD to the issue's title and the Bug label to get a BZ team member assigned for handling payment here ?

@dangrous dangrous changed the title Flag message - "Only visible to you" disappears after flagging the whisper, and then reappears [HOLD for Payment 2024-06-21] Flag message - "Only visible to you" disappears after flagging the whisper, and then reappears Jun 18, 2024
@dangrous dangrous added the Bug Something is broken. Auto assigns a BugZero manager. label Jun 18, 2024
Copy link

melvin-bot bot commented Jun 18, 2024

Triggered auto assignment to @stephanieelliott (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jun 18, 2024
@ikevin127
Copy link
Contributor

ikevin127 commented Jun 21, 2024

⚠️ This is missing Awaiting Payment label which might cause payment delay!
Also Stephanie seems to be ⛔ OOO today.

cc @dangrous @stephanieelliott

@mallenexpensify mallenexpensify added Internal Requires API changes or must be handled by Expensify staff Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review labels Jun 21, 2024
@mallenexpensify
Copy link
Contributor

Assigned @ikevin127 , they reviewed the PR

@ikevin127 , can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~0182f10ddb5995af64

@ikevin127
Copy link
Contributor

Accepted, thank you!

@mallenexpensify
Copy link
Contributor

Contributor+: @ikevin127 paid $250 via Upwork.

Looks like we need the BZ checklist filled here, can you do so @ikevin127 ?

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:

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

@mallenexpensify mallenexpensify added Internal Requires API changes or must be handled by Expensify staff and removed Internal Requires API changes or must be handled by Expensify staff labels Jun 21, 2024
@ikevin127
Copy link
Contributor

Regression Test Proposal

  1. Create a workspace.
  2. Invite another account to that workspace.
  3. On the invited account, go to the workspace chat.
  4. Right click on the welcome whisper message.
  5. Click Flag as offensive and select Spam.
  6. Verify that "👁️ Only visible to you" above the welcome whisper message maintains visibility at all times.

Do we agree 👍 or 👎.

@mallenexpensify
Copy link
Contributor

Thanks @ikevin127 , TestRail GH created

Should be all good here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff
Projects
None yet
Development

No branches or pull requests

7 participants