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

Opening notification doesn't navigate to conversation, if in another account #4630

Open
gnprice opened this issue Apr 9, 2021 · 0 comments

Comments

@gnprice
Copy link
Member

gnprice commented Apr 9, 2021

When you open a notification, the app opens up and should navigate to the conversation the notification is from. Typically we do.

But if the notification is in a different account from the one you were last using (i.e. the "active account"), then we don't manage this. We go and switch to the right account -- but then we don't go on to navigate to the conversation, as we otherwise would.

The relevant code for this is in narrowToNotification, in notificationActions.js.

A reason this isn't simple to fix is that after we've just switched accounts, we won't have data from the server: in particular the list of users, which we use to map emails (from 1:1 PM notifications) to user IDs (which we use to identify the narrow in order to narrow to it.) Similarly we won't have the data to map stream names to stream IDs -- for now we still use stream names to identify stream and topic narrows, but we're going to change that (#4333), and the server does not send stream IDs in push notifications.

(I'll file a zulip/zulip issue for that last bit Filed that last bit as zulip/zulip#18067, but we'll have to handle existing server versions for a good while after it's fixed.)

This means we need to hang on to this notification's data, and the TODO item of navigating ASAP to its conversation, until we eventually complete the initial fetch for the new account so that we can actually do so. This waiting has no impact from a user perspective -- we can't usefully render any of the rest of the app's UI until we have that data, so we'll be showing a loading screen anyway. But it will require some care arranging the control flow so that we come back to that TODO as soon as the fetch is complete.

Probably the simplest way to do this within the current architecture is to add another call at the end of doInitialFetch. Then, say, narrowToNotification can put something on a list of callbacks to be invoked at that point, which checks that the expected account is now active and then proceeds to finish decoding the notification and navigate to its conversation.

(This is one of the pieces that remained from #2295.)

AkashDhiman added a commit to AkashDhiman/zulip-mobile that referenced this issue Jun 3, 2021
This action will be responsible to generate a narrow from a valid
navigation deep link url, and then navigate to it.

Limitation of current Implementation:
- if the link points to a different account, we only switch the
account and don't actually navigate to it. (similar to zulip#4630)
AkashDhiman added a commit to AkashDhiman/zulip-mobile that referenced this issue Jun 3, 2021
This action will be responsible to generate a narrow from a valid
navigation deep link url, and then navigate to it.

Limitation of current Implementation:
- if the link points to a different account, we only switch the
account and don't actually navigate to it. (similar to zulip#4630)
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

1 participant