-
-
Notifications
You must be signed in to change notification settings - Fork 665
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
Separate Android notification data by account #4633
Comments
AkashDhiman
added a commit
to AkashDhiman/zulip-mobile
that referenced
this issue
Jul 1, 2021
`ByConversationMap` was originally intended to store conversations based on `Identity` as key, so that we would be able to group conversations by realmId easily. Here we are removing it, and introducing a new data class in the following commit that will store realmId and messages along with notificationId of that particular conversation. A reason to do this is that structurally we are identifying unique notification based on its title not by its `Identity`. So its easy to create a direct mapping of UI notification with the `ConversationMap` data structure this way. Extra information about the conversation (eg: realmId) will be included in the value of ConversationMap (see the next commit). Skips: zulip#4633
AkashDhiman
added a commit
to AkashDhiman/zulip-mobile
that referenced
this issue
Aug 4, 2021
`ByConversationMap` was originally intended to store conversations based on `Identity` as key, so that we would be able to group conversations by realmId easily. Here we are removing it, in favor of using `NotificationManager#getActiveNotifications` in the following commits. Notifications will persist automatically if we are to do this, and we don't have to be concerned with handling logic for storing past notifications, Android SDK will abstract it away for us. This will eventually be followed by removal of`ConversationMap`, and other associated code. Skips: zulip#4633
AkashDhiman
added a commit
to AkashDhiman/zulip-mobile
that referenced
this issue
Aug 4, 2021
`ByConversationMap` was originally intended to store conversations based on `Identity` as key, so that we would be able to group conversations by realmId easily. Here we are removing it, in favor of using `NotificationManager#getActiveNotifications` in the following commits. Notifications will persist automatically if we are to do this, and we don't have to be concerned with handling logic for storing past notifications, Android SDK will abstract it away for us. This will eventually be followed by removal of`ConversationMap`, and other associated code. Skips: zulip#4633
AkashDhiman
added a commit
to AkashDhiman/zulip-mobile
that referenced
this issue
Aug 5, 2021
`ByConversationMap` was originally intended to store conversations based on `Identity` as key, so that we would be able to group conversations by realmId easily. Here we are removing it, in favor of using `NotificationManager#getActiveNotifications` in the following commits. Notifications will persist automatically if we are to do this, and we don't have to be concerned with handling logic for storing past notifications, Android SDK will abstract it away for us. This will eventually be followed by removal of`ConversationMap`, and other associated code. Skips: zulip#4633
gnprice
pushed a commit
that referenced
this issue
Aug 10, 2021
`ByConversationMap` was originally intended to store conversations based on `Identity` as key, so that we would be able to group conversations by realmId easily. Here we are removing it, in favor of using `NotificationManager#getActiveNotifications` in the following commits. Notifications will persist automatically if we are to do this, and we don't have to be concerned with handling logic for storing past notifications, Android SDK will abstract it away for us. This will eventually be followed by removal of`ConversationMap`, and other associated code. Skips: #4633
This was accomplished as part of #4842. We actually stopped keeping our own data structure here, leaving it to the OS, and we arranged there for the data to be appropriately separate. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently we maintain the following data structure in our Android-native code that handles notifications:
As those comments foreshadow, we should separate out this data by which account/identity the user received it for. As is, because we don't, we end up showing conversations in different accounts jumbled together in a single summarized notification.
This will then be a building block for #2691.
We might also first use the separated data structure to start showing separate notifications per account within our current style of notification; that'll be worthwhile just if the work consists mostly of refactors we need anyway for #2691.
The text was updated successfully, but these errors were encountered: