okTurtles / group-income

A decentralized and private (end-to-end encrypted) financial safety net for you and your friends.
https://groupincome.org
GNU Affero General Public License v3.0
331 stars 44 forks source link

Notifications issues on mobile #2408

Open taoeffect opened 3 weeks ago

taoeffect commented 3 weeks ago

Problem

When I opened up the PWA on mobile I got this notification. But it's a message I DM'd to someone else, so I shouldn't be notified and also it shouldn't say "undefined":

Solution

SebinSong commented 3 weeks ago

@taoeffect

Observations

I investigated this issue for some time and found out that this also happens in desktop browsers too.

And this(notification displayed with missing data) seems to only happen when:


Possible causes

From my investigation, It appears that the root-cause is that some sideEffects of 'chatroom.js' contracts are executed too soon before user-settings are fully loaded to the Vuex store.

As you are aware too, there are multiple places in the source-code where the app uses sbp('state/vuex/state') and/or sbp('state/vuex/getters') to determine whether a message notification needs to be sent or not. For examples,

eg1)

https://github.com/okTurtles/group-income/blob/3b142e94445de8ab49b435e8d4bde7129f3aed53/frontend/model/contracts/chatroom.js#L323-L328

eg2)

https://github.com/okTurtles/group-income/blob/3b142e94445de8ab49b435e8d4bde7129f3aed53/frontend/controller/actions/chatroom.js#L17-L25

and so on. Something like sbp('state/vuex/state').loggedIn.identityContractID here is actually based on the assumption that the user-setting data has been completely loaded to the Vuex store by the time the app executes this line.

But then what I've observed is these are executed before sbp('gi.db/settings/load', SETTING_CURRENT_USER) and await sbp('gi.app/identity/login', { identityContractID }) operations in main.js (reference below) are completed.

https://github.com/okTurtles/group-income/blob/3b142e94445de8ab49b435e8d4bde7129f3aed53/frontend/main.js#L301-L306

below is a devtool console.log screenshot showing messageReceivePostEffect(...) is executed earlier than sbp('gi.db/settings/load', SETTING_CURRENT_USER) operation is done.

image

So when we have const me = sbp('state/vuex/state').loggedIn.identityContractID, a check like me === innerSigningContractID would become undefined === innerSigningContractID because sbp('state/vuex/state').loggedIn is evaluated to false.

It's not clear since when this has been the case as I'm not the expert in this area. It seems like the complete fix is to make sure all operations related to sbp('gi.db/settings/load', SETTING_CURRENT_USER) block in main.js are done early enough that things like sbp('state/vuex/state') in contract sideEffects fetch the expected data. If you agree with this reasoning, I feel this fix should be taken care of by someone who has more thorough expertise in this area than myself..!

What's your thoughts?

Thanks,

taoeffect commented 3 weeks ago

@SebinSong Thanks for having a look at this! You may be correct, but since you're unsure and because @corrideat is working on notifications related things anyway, I've gone ahead and reassigned him to this issue to have a look. Your investigation I'm sure will be useful to him, so thanks again for posting that!