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 issues on mobile #2408

Open
taoeffect opened this issue Oct 30, 2024 · 2 comments
Open

Notifications issues on mobile #2408

taoeffect opened this issue Oct 30, 2024 · 2 comments

Comments

@taoeffect
Copy link
Member

taoeffect commented Oct 30, 2024

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

  • Don't notify me if I'm the one that sent the message to someone or to some channel
  • Don't put "#undefined"
@SebinSong
Copy link
Collaborator

SebinSong commented Oct 31, 2024

@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:

  • the DM was sent while the receiver(u2 here) was not online,
  • Then later u2 launches the app again and immediately sees the notifications emitted.

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)

sideEffect ({ contractID, hash, height, meta, data, innerSigningContractID }, { state, getters }) {
const me = sbp('state/vuex/state').loggedIn.identityContractID
if (me === innerSigningContractID && data.type !== MESSAGE_TYPES.INTERACTIVE) {
return
}

eg2)

sbp('okTurtles.events/on', MESSAGE_RECEIVE_RAW, ({
contractID,
data,
innerSigningContractID,
// If newMessage is undefined, it means that an existing message is being edited
newMessage
}) => {
const getters = sbp('state/vuex/getters')
const rootState = sbp('state/vuex/state')

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.

setupChelonia().then(() => sbp('gi.db/settings/load', SETTING_CURRENT_USER)).then(async (identityContractID) => {
oldIdentityContractID = identityContractID
if (!identityContractID || this.ephemeral.finishedLogin === 'yes') return
await sbp('gi.app/identity/login', { identityContractID })
await sbp('chelonia/contract/wait', identityContractID)
}).catch(async e => {

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 taoeffect assigned corrideat and unassigned SebinSong Oct 31, 2024
@taoeffect
Copy link
Member Author

@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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants