App: [Chat] / Chrome fails to "Ask" for notification privileges

If you haven’t already, check out our contributing guidelines for onboarding!

Platform - version: v1.0.1-488 web on Chrome

Action Performed (reproducible steps):

Navigate to https://expensify.cash/#/signin on web on Chrome browser
Click into the padlock in the URL bar and set notifications to default (see image below)
Reload page
Sign in
Sign out via /settings
Observe that activeClients2 has no value in localStorage whatsoever (see image below)
Sign in again
Observe that activeClients2 still has no value
Leave a comment on a report that is not currently in view from another session as a shared user (this should trigger the prompt to appear)
Observe that no prompt appears because the notification is skipped and this log displays:
[LOCAL_NOTIFICATION] Skipping notification because this client is not the leader

Expected Result: The Ask for notifications prompt should display reliably.

Actual Result: No prompt appears because the notification is skipped because Skipping notification because this client is not the leader

Notes/Photos/Videos:

In addition to posting a proposal here, please make sure to submit via this Upwork job too https://www.upwork.com/jobs/~01b3cfaf1cd42f3481

About this issue

Original URL
State: closed
Created 3 years ago
Comments: 16 (16 by maintainers)

Most upvoted comments

nice work @SameeraMadushan - we’ll issue payment on 18 March if there are no regressions reported 👍🏽

laurenreidexpensify on Mar 11, 2021

Done!

laurenreidexpensify on Mar 10, 2021

Great @SameeraMadushan can you tag me in the PR when you start it? Thanks!

laurenreidexpensify on Mar 10, 2021

Yes it is!

tgolen on Mar 9, 2021

Cool, thanks Marc. I am fine with that. Let’s just use the local variable route to reset the Onyx value after clear() then.

tgolen on Mar 8, 2021

I think another possible solution would be to specify a list of keys that should not be cleared out when Onyx.clear() is called. These keys could be registered as part of Onyx.init() and then a small refactoring of Onyx.clear() would be necessary. What are your thoughts on that approach, Marc?

I think this could be a good future improvement, but not necessarily something we need right now. Looking at the cases where we want to save some data after an Onyx.clear() and most of them are conditional anyway. If we have an error, if we have a reportID then persist after the clear etc.

https://github.com/Expensify/Expensify.cash/blob/c3a6aa86d34404599f5239a1dbd84f96505f0e30/src/libs/actions/SignInRedirect.js#L47-L52

This is the only case where we want a key to persist unconditionally so I think we can wait for the optimization.

marcaaron on Mar 8, 2021

OK, thanks for explaining. I think we’ll need to work on the naming convention a little bit, but the proposal makes more sense now.

I think the onyx library doesn’t support defining items that should not clear right? Seems it’s related to the expensify/react-native-onyx library improvement.

That’s correct. You’d need to implement a change in that library first and then update the version of react-native-onyx in this repo. Still curious for @marcaaron to chime in about that though. I know we’ve talked about it in the past, but maybe it would be more harmful than good.

tgolen on Mar 4, 2021

@SameeraMadushan I am a little bit confused about your proposal. I see both activeClients2 and activeClients, but I think they should all be references to activeClients2, is that correct?

I think overall it’s a pretty good and simple solution.

I think another possible solution would be to specify a list of keys that should not be cleared out when Onyx.clear() is called. These keys could be registered as part of Onyx.init() and then a small refactoring of Onyx.clear() would be necessary. What are your thoughts on that approach, Marc?

tgolen on Mar 4, 2021