Skip to content

[CRITICAL] Incorrect previousReportActionID for whispers from workspace invite #34982

Closed

Description

@roryabraham

Problem

Inviting someone to a workspace will lead the inviter to repeatedly attempt to load whisper actions they can’t access (see the first image from Taras’ post here)

Screenshot 2024-01-25 at 10 58 07

Reproduction:

Must be tested in connection with #30269

  • Create a workspace as User A.
  • Invite User B.
  • Navigate to the created Workspace Chat as User B.
  • Observe User B has a whisper message.
  • Send a few messagse as User B inside the workspace chat.
  • Navigate to Chat as User A.
  • Observe User A does not see any whisper messages nor have them in their Onyx data.
  • Look at Onyx data as User A and observe the first comment from User B has a previousReportActionID equal to the whisper message meant only for User B.
  • User A will see a continuous loading state because we see a “gap” and we are trying to backfill a “missing report action” (i.e. the whisper) that User A should not be able to access.

Expectation:

  • Continuous loading does not happen and we are able to see the CREATED action for this workspace report instead of an infinite load.

Important note: This is but one example, but we are assuming the same problem would exist anywhere a whisper exists.

Observations:

Deleted comments do not suffer the same problem because all users can still access the report action itself.
Whispers can be “hidden” from the front end if the current user’s accountID is not present in the reportAction.message.whisperedTo accountID array.

Solution:

Stop filtering out whisper messages on a per user level and treat them similar to deleted report actions (i.e. everyone can access them so they pose no problems for Comment Linking).

More context here about this decision is in this Slack thread.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01d5dec33974b22a01
  • Upwork Job ID: 1749864653925597184
  • Last Price Increase: 2024-01-23

Activity

  1. added
    InternalRequires API changes or must be handled by Expensify staff
    BugSomething is broken. Auto assigns a BugZero manager.
    on Jan 23, 2024
  2. melvin-bot commented on Jan 23, 2024

    @melvin-bot

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

  3. melvin-bot commented on Jan 23, 2024

    @melvin-bot
  4. melvin-bot commented on Jan 23, 2024

    @melvin-bot

    Triggered auto assignment to Contributor Plus for review of internal employee PR - @fedirjh (Internal)

  5. marcaaron commented on Jan 23, 2024

    @marcaaron
    Contributor

    I can grab this one.

  6. self-assigned this
    on Jan 23, 2024
  7. marcaaron commented on Jan 23, 2024

    @marcaaron
    Contributor

    Following the trail and it should be getting the previous action from here in Auth.

    This appears to work fine for me locally. @roryabraham do you have any more information about this?

  8. marcaaron commented on Jan 24, 2024

    @marcaaron
    Contributor

    Gonna swap this one for #34982 for now as I don't quite understand what we are trying to fix here and the other one is much clearer to me.

  9. 55 remaining items

  10. cristipaval commented on Feb 19, 2024

    @cristipaval
    Contributor

    I hope that the App PR will hit production today.
    Web-E PR is approved and ready to merge once the App PR is into prod.
    I'll make sure that Auth PR is also ready to merge in the meantime. I mean, it's been ready, but I'll push to get reviews and feedback before Web-E gets into prod.

  11. melvin-bot commented on Feb 20, 2024

    @melvin-bot

    @johncschuster @cristipaval this issue is now 4 weeks old and preventing us from maintaining WAQ. This should now be your highest priority. Please post below what your plan is to get a PR in review ASAP. Thanks!

  12. cristipaval commented on Feb 21, 2024

    @cristipaval
    Contributor

    This is HOLD this week, we'll merge Web-E PR after the merge freeze

  13. cristipaval commented on Feb 26, 2024

    @cristipaval
    Contributor

    We'll deprecate the App version today and merge the Web-E PR

  14. cristipaval commented on Feb 26, 2024

    @cristipaval
    Contributor

    Web-E PR is merged

  15. cristipaval commented on Feb 27, 2024

    @cristipaval
    Contributor

    Update: Web-E PR is in staging. I'm actively addressing all the feedback on the Auth PR and doing my best to have it deployed tomorrow.

  16. cristipaval commented on Mar 1, 2024

    @cristipaval
    Contributor

    This is now in production

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

BugSomething is broken. Auto assigns a BugZero manager.DailyKSv2EngineeringInternalRequires API changes or must be handled by Expensify staffReviewingHas a PR in review

Type

No type

Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions