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-04-05] [Wave Collect] [Ideal Nav] Issues with the bold (unread) status on Workspace Item #35682

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

Comments

@hayata-suenaga
Copy link
Contributor

hayata-suenaga commented Feb 2, 2024

Action Performed:

The workspace item on the Workspace Switcher should be bold when there are any unread chat/report in that workspace. This indicator, however, isn't working consistently.

Sometimes, the workspace item is bold even when there is no unread chat in that workspace. And sometimes, it's not bold when there is a unread chat/report.

The following is the reproduction step for the latter issue.

  1. Create a workspace if you don't have one yet.
  2. Go to a workspace (not Expensify one).
  3. Right click (or long press) a chat and click Mark as unread on the popover that appears
  4. Click the Workspace Switcher.
  5. Look for the current workspace name. Confirm that the workspace name is bold.
  6. Go back to the workspace.
  7. Right click the same chat and click Mark as read
  8. Go back to Workspace Switcher
  9. Confirm the current workspace is not bold
Screen.Recording.2024-01-22.at.7.26.44.PM.mov

Workaround:

N/A

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

Vides are attached above.

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0183bc22eb2779a4b0
  • Upwork Job ID: 1763580448153239552
  • Last Price Increase: 2024-03-01
@hayata-suenaga hayata-suenaga added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Feb 2, 2024
@hayata-suenaga hayata-suenaga self-assigned this Feb 2, 2024
@hayata-suenaga hayata-suenaga moved this to Release 1: Ideal Nav & Collect Simplfied Profile, Members, Categories, Workflows (approvals) in [#whatsnext] Wave 08 - Collect Plan Admins Feb 2, 2024
Copy link

melvin-bot bot commented Feb 2, 2024

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

@hayata-suenaga
Copy link
Contributor Author

@mateuuszzzzz I believe you're working on this regression? can you comment on this GH issue so that I can assign you?

@Christinadobrzyn
Copy link
Contributor

Ah good catch! I can reproduce this - we'll see if @mateuuszzzzz is working on this!

@melvin-bot melvin-bot bot added the Overdue label Feb 5, 2024
@cdOut
Copy link
Contributor

cdOut commented Feb 5, 2024

Hello, I'll be working on this one and take it from @mateuuszzzzz.

@trjExpensify
Copy link
Contributor

Nice, assigned you @cdOut!

@ikevin127
Copy link
Contributor

While debugging a similar issue (#35766) which was closed as a dupe of this one I noticed the following:

  • within the ReportUtils.isUnread function we're checking the lastReadTime which if undefined will fallback to empty string
  • in WorkspacesSettingsUtils we're using Onyx.connect to get allReports
  • sometimes for certain workspaces -> reports within that workspace will initially have the lastReadTime as undefined
  • undefined lastReadTime defaults to empty string which when compared to an existing lastVisibleActionCreated date string, isUnread always returns true

Considering these, I tend to think that the RC here might come from the Onyx.connect -> allReports data which might first initialize some report variables as undefined - while shortly after to become defined but the function doesn't register the data change and remains with initial isUnread status.

Hope this helps w/ debugging ✌️

@cdOut
Copy link
Contributor

cdOut commented Feb 5, 2024

Interesting, thanks for sharing the info @ikevin127!

@Christinadobrzyn
Copy link
Contributor

update - we're working on debugging/PR for this!

@situchan
Copy link
Contributor

situchan commented Feb 6, 2024

I am interested in reviewing this since I was C+ in dupe issue

@Christinadobrzyn
Copy link
Contributor

Christinadobrzyn commented Feb 6, 2024

hey just a quick question here... testing this again and talking over this issue with @cdOut... I'm not sure if the steps in the OP are correct or are the issue we're trying to fix.

What I see when testing:

  1. Create a workspace if you don't have one yet.
  2. Go to a workspace (not Expensify one).
  3. Right click (or long press) a chat and click Mark as unread on the popover that appears
  4. Click the Workspace Switcher.
  5. Verify that the workspace is bold
  6. Go back to the workspace and right click (or long press) to mark as read
  7. Verify that the workspace is not bold
2024-02-06_13-43-21.mp4
  1. I don't think the checkmark matters here, the checkmark is the default workspace, yeah?
  2. Is the green dot behaviour incorrect? I think that might be the issue - shouldn't the green dot show for unread and not show for read?

So isn't this behaviour correct? @hayata-suenaga @trjExpensify @situchan

@hayata-suenaga
Copy link
Contributor Author

@Christinadobrzyn I'm sorry. I mistyped the step #6. You're correct. We want to check if the workspace name is bold or not.

There is another issue that is handling RBR and GBR.

@Christinadobrzyn
Copy link
Contributor

Ah okay, thanks @hayata-suenaga so if that's the case, I think this might be fixed? Could you test again and let me know if the bold isn't working? TY! Or I can ask QA to test again if needed!

@trjExpensify
Copy link
Contributor

I still seem to have a phantom unread workspace on v1.4.37-2:

image

I've clicked into all of the chats on that workspace and can't seem to clear it.

@cdOut
Copy link
Contributor

cdOut commented Feb 7, 2024

I still seem to have a phantom unread workspace on v1.4.37-2:
image

I've clicked into all of the chats on that workspace and can't seem to clear it.

That's exactly the problem that I was also able to replicate and was working on a fix on this, just wanted to clarify that that's the only issue appearing currently with bold.
Thanks for retesting and specifying the issue!

@trjExpensify
Copy link
Contributor

Dopeeee! Any idea what it is? 🕵️

@cdOut
Copy link
Contributor

cdOut commented Feb 8, 2024

The issue appears to be occuring on old accounts which already had different reports created but due to updates to different flows they were hidden from the users view without ever gaining the lastReadTime property, which we use to determine whether we should use bold for the workspaces in the switcher.

I do have an old account that has a hidden report in the Everything section of the switcher, but I'd appreciate if I could log into the account you used for the video @trjExpensify with a hidden report in a user created workspace so I can test that case also. I've messaged you on slack about it.

@trjExpensify
Copy link
Contributor

Chatted about this earlier. I can't provide access to my real Expensify account where this is shown in the vid. I'll test an adhoc build though when ready!

@melvin-bot melvin-bot bot added the Overdue label Feb 12, 2024
@cdOut
Copy link
Contributor

cdOut commented Feb 12, 2024

As an update I will be creating a PR for this today or tomorrow.

Copy link

melvin-bot bot commented Mar 29, 2024

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:

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

@Christinadobrzyn
Copy link
Contributor

Payouts due:

  • Contributor+: $500 @situchan - are you paid through Upwork? or NewDot?

Looks like the upwork job is closed so I can create a new one if needed.

Do we need a regression test for this?

@Christinadobrzyn Christinadobrzyn added Daily KSv2 and removed Weekly KSv2 labels Apr 4, 2024
@hayata-suenaga
Copy link
Contributor Author

@situchan, I feel like we need a regression test for this one. What do you think?

Copy link

melvin-bot bot commented Apr 5, 2024

Payment Summary

Upwork Job

BugZero Checklist (@Christinadobrzyn)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1763580448153239552/hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@melvin-bot melvin-bot bot added the Overdue label Apr 8, 2024
@Christinadobrzyn
Copy link
Contributor

I think @situchan might be ooo based on Slack.

@situchan can you accept this Upwork offer - https://www.upwork.com/nx/wm/offer/101773908

Can you also provide a regression test for us? Thanks!

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Apr 8, 2024
@Christinadobrzyn
Copy link
Contributor

nudging @situchan in slack about the offer

@melvin-bot melvin-bot bot removed the Overdue label Apr 11, 2024
@hayata-suenaga
Copy link
Contributor Author

@Christinadobrzyn I heard that @situchan is OOO for a while. I think we might have to wait a little bit until they come back 😄

@Christinadobrzyn Christinadobrzyn added Weekly KSv2 and removed Daily KSv2 labels Apr 12, 2024
@Christinadobrzyn
Copy link
Contributor

Ah okay! I'll move this to weekly since payment due is all that's left.

@melvin-bot melvin-bot bot added the Overdue label Apr 22, 2024
@Christinadobrzyn
Copy link
Contributor

holding for @situchan to accept this Upwork offer - https://www.upwork.com/nx/wm/offer/101773908

@melvin-bot melvin-bot bot removed the Overdue label Apr 24, 2024
@melvin-bot melvin-bot bot added the Overdue label May 2, 2024
@Christinadobrzyn
Copy link
Contributor

Dmd @situchan to see if they are back and can accept the offer

@melvin-bot melvin-bot bot removed the Overdue label May 3, 2024
@Christinadobrzyn
Copy link
Contributor

moving this to monthly since payment is all that's needed. @situchan let me know when you're back b/c I'll need to make a new offer

@Christinadobrzyn Christinadobrzyn added Monthly KSv2 and removed Weekly KSv2 labels May 8, 2024
@situchan
Copy link
Contributor

Sorry for late. I am back. Can you please send new offer?

@Christinadobrzyn
Copy link
Contributor

@situchan here's the new offer - https://www.upwork.com/nx/wm/offer/102347085

Thanks!

@Christinadobrzyn
Copy link
Contributor

Paid @situchan in upwork based on this payment summary - #35682 (comment)

Closing!

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. Engineering Internal Requires API changes or must be handled by Expensify staff Monthly KSv2
Projects
No open projects
Archived in project
Status: Release 1: Ideal Nav & Collect Simplfied Profile, Members, Categories, Workflows (approvals)
Development

No branches or pull requests

6 participants