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

Notifications - Unseen notifications marked as seen #88344

Closed
Addison-Stavlo opened this issue Mar 8, 2024 · 5 comments
Closed

Notifications - Unseen notifications marked as seen #88344

Addison-Stavlo opened this issue Mar 8, 2024 · 5 comments
Assignees
Labels
[Feature] Notifications Site notifications. [Pri] Normal Triaged To be used when issues have been triaged. [Type] Bug

Comments

@Addison-Stavlo
Copy link
Contributor

Addison-Stavlo commented Mar 8, 2024

Related to slack thread p1709902592755159-slack-C06DN6QQVAQ

The Problem

Notifications are being marked as seen without being opened in a number of places. By "marked as seen" i do not mean "read", but i mean that the orange dot that displays when there are unseen notifications disappears.

Screenshot 2024-03-08 at 12 12 40 PM

To Repro

To repro, you will probably need to trigger some notifications. Getting a comment on one of your blogs should do this. You can probably do this in an incognito tab as a logged out user. If you have a sandbox you can run a wp_cli command for ease of use as described in p2XLIi-JB-p2

  • For simplicity and isolated testing, only have 1 tab with a notification component open. (close extra P2s, test sites, etc.)
  • I uninstalled my Jetpack app and disabled browser notifications in /me/notifications, im not sure if this is necessary but i wanted to ensure no other systems were picking up the notifications in testing these isolated areas.
  • Trigger a new notification by creating a comment.
  • Notice the orange dot appear on the logged in tab you have open.
  • Reload the browser tab, the notification dot should disappear after reloading (sometimes a couple times, sometimes the first time)

Notes

So far this seems to happen everywhere we have notifications, except global state with the new nav sidebar (no idea why?). Here are some of my notes from testing different notification components in isolation:

  • A - /home in default view / simple site (old topbar in calypso)

    • trigger new notif -> dot appears
    • after a couple reloads, the dot is gone 🔴
  • B - in global sidebar (at /sites)

    • trigger new notif -> dot appears
    • after a few reloads, dot continues to be there and wont go away 🟢
    • continued to navigate and reload at global levels (/domains, /me, /reader, back to /sites) notif wont go away 🟢
  • C - in wp-admin for a classic site (core masterbar)

    • new notif -> dot appears
    • after first reload, disappears 🔴
  • D - P2

    • new notif -> dot appears
    • disappears after a couple reloads 🔴
  • E - global sidebar at site level /home

    • new notif -> dot appears
    • disappears after a couple reloads 🔴
  • F - network admin top bar

    • new notif -> dot appears
    • disappears after a couple reloads 🔴

    sometimes its after the first reload, sometimes after a few, but i cant seem to get it to happen with the new global sidebar when staying at the global level with no site selected.

@Addison-Stavlo
Copy link
Contributor Author

Addison-Stavlo commented Mar 8, 2024

Testing more on the old calypso masterbar, p2, and network admin:

  • Tab open, Notification made, dot comes on.
  • Requests to notifications show a last-seen-time that is consistent and doesn't change as this gets rerequested.
Screenshot 2024-03-08 at 12 39 30 PM
  • Reload the tab.
  • Initial requests to notifications have the same last-seen-time as the previous load.
  • A request goes out to mark notifications as seen and update the last-seen-time.
Screenshot 2024-03-08 at 12 39 13 PM Screenshot 2024-03-08 at 12 39 23 PM
  • notif requests after that have the updated last seen time and the dot goes away.
Screenshot 2024-03-08 at 12 39 58 PM
  • When reloading with no new notifications, the /seen never goes out. It seems that these components are always sending a request to /seen when they load IF there is an unseen notification.

Testing on global sidebar at global level: the /seen never gets sent out and this continues to work as expected.

@Addison-Stavlo
Copy link
Contributor Author

Addison-Stavlo commented Mar 8, 2024

@fullofcaffeine I see you are the most recent contributor to the apps/notifications readme, if you might have any ideas about the above or who to ping. We have the post request for /notifications/seen going out when it shouldn't be.

I tried building calypso dev, sync'd the notes build to my sandbox, sandboxed widgets, etc. and have been trying to test around this, but this is weird. I only see 1 place that calls to sendLastSeenTime which seems is responsible for that post request, but I don't see it called when debugging locally. I can see the updateLastSeenTime debugging commands go off in general use, but when the problematic request goes out none of these debug lines fire. Further I can update that function to change the URL to something other than /notifications/seen and build/sync changes, and the call to /notifications/seen still goes off in the network tab. 🤷‍♀️ 😵

I feel like im missing some steps in getting the development environment for this setup correctly.

@Addison-Stavlo
Copy link
Contributor Author

Addison-Stavlo commented Mar 8, 2024

I tried building calypso dev, sync'd the notes build to my sandbox, sandboxed widgets, etc. and have been trying to test around this

It seems like there is something related to sandboxing the selected site? This is a really weird end of day find, as it doesn't seem to make any sense but also is consistent in testing.

  • Testing on /home
  • build and sync the apps/notifications to your sandbox. You don't need any changes, just a sync'd build.
  • sandbox public-api, widgets, AND the site you have selected in my-home
  • I can no longer reproduce the issue.

As soon as i unsandbox the selected site URL, the issue comes back. Similarly, if I remove the sync'd app build from my sandbox the issue comes back as well, even though that sync'd build had no actual code changes.

The part about the selected site is WEIRD - but it does fit with my initial findings above regarding the discrepency between test B and E - its the same component and sidebar... but at the no site selected level there is no bug, and at the site selected level there is...

@Addison-Stavlo
Copy link
Contributor Author

Addison-Stavlo commented Mar 12, 2024

A clarifying Update!

Im finding that the problem within calypso is due to notifications from within the site preview. This explains why my local debugging in calypso wasnt working, and explains the discrepency between B and E in the initial tests. If i remove the site preview component from my home, i can no longer repro the issue.

So the problem seems to be within the standalone section of the notes app. I was focusing deeper inside the Panel section since that is all that calypso uses and the bug was reproducible there. Important note - the problem isn't specifically that notes are loaded in site previews, this problem happens everywhere the standalone notes app is loaded. I will continue to investigate more.

@Addison-Stavlo Addison-Stavlo self-assigned this Mar 13, 2024
@rickmgithub rickmgithub added Triaged To be used when issues have been triaged. [Pri] High and removed [Pri] TBD labels Mar 15, 2024
@github-actions github-actions bot added the [Status] Priority Review Triggered Quality squad has been notified of this issue in #dotcom-triage-alerts label Mar 15, 2024
@rickmgithub rickmgithub added [Pri] Normal and removed [Pri] High [Status] Priority Review Triggered Quality squad has been notified of this issue in #dotcom-triage-alerts labels Mar 15, 2024
@Addison-Stavlo
Copy link
Contributor Author

This should now be resolved with #88463

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Notifications Site notifications. [Pri] Normal Triaged To be used when issues have been triaged. [Type] Bug
Development

No branches or pull requests

2 participants