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

Refactor activity center count fetching #16023

Closed
jrainville opened this issue Aug 7, 2024 · 1 comment · Fixed by #16152
Closed

Refactor activity center count fetching #16023

jrainville opened this issue Aug 7, 2024 · 1 comment · Fixed by #16152
Assignees
Milestone

Comments

@jrainville
Copy link
Member

Description

Currently, in the activity center view, we call the module, which in turn calls the controller, the service and then status-go, each time the QML code wants to know the count.

See here:

proc unreadActivityCenterNotificationsCount*(self: View): int {.slot.} =
self.unreadCount = self.delegate.unreadActivityCenterNotificationsCount()
return self.unreadCount

This is doubled with the fact that the service call is sync:

proc getActivityCenterNotificationsCounters(self: Service, activityTypes: seq[int], readType: ActivityCenterReadType): Table[int, int] =
try:
let response = backend.activityCenterNotificationsCount(
backend.ActivityCenterCountRequest(
activityTypes: activityTypes,
readType: readType.int,
)
)

We should refactor this code so that status-go is only called when needed.
We should also make it so that it's async.

So the view should have a copy of the count and serve only that to the QML.

When we have potential updates, we call the async call to check the count and then update the view with the new count.

Or even better, have status-go be the emitter at all times (except the first app load). Meaning that when the count of status-go is updated but external or user events, there is a response or signal from status-go that includes the count currently.
However, the difficulty with this option is that we would then need to save the current AC setting in status-go (All, Mentions, Replies, etc.)

Do note that the count of unread AC notifs is also used in the main module to see if we have notification to show the red dot:

method onActivityNotificationsUpdated[T](self: Module[T]) =
self.checkIfWeHaveNotifications()

@jrainville
Copy link
Member Author

Moving this 2.31 as I just realized, that just like the wakuext_getLatestVerificationRequestFrom, this gets called WAY too much
app_20240819_132019.log

@jrainville jrainville modified the milestones: 2.32.0 Beta, 2.31.0 Beta Aug 19, 2024
@jrainville jrainville self-assigned this Aug 19, 2024
jrainville added a commit that referenced this issue Aug 19, 2024
Fixes #16023

Caches the values of the number of notifications and hasUnseen in the view so that we access status-go only when there is an update or on app start.
Also, uses the response value from user actions in the AC to retrieve the hasUnseen value directly instead of re-fetching.
Finally, fixes an issue where marking notifs as read/unread wouldn't update the count
@jrainville jrainville moved this to In Progress in Status Desktop/Mobile Board Aug 19, 2024
@jrainville jrainville moved this from In Progress to Code Review in Status Desktop/Mobile Board Aug 19, 2024
@github-project-automation github-project-automation bot moved this from Code Review to Done in Status Desktop/Mobile Board Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants