search

zulip / zulip-flutter

Future Zulip client using Flutter
Apache License 2.0
160 stars 133 forks source link

Disallow connecting to unsupported ancient servers #267

Open gnprice opened 1 year ago

gnprice commented 1 year ago

Much like the feature in the zulip-mobile RN app we added as zulip/zulip-mobile#5633 / zulip/zulip-mobile#5102.

Currently this means a minimum server version of 4.0, as documented at https://zulip.readthedocs.io/en/latest/overview/release-lifecycle.html and our README, and as reflected in our API-facing code which ignores all pre-server-4 complications. At some point we'll increase that to 5.0, then in the future to 6.0 and 7.0 and so on.

(A handy link for a record of Zulip Server release dates, for planning when we can bump that minimum: https://blog.zulip.com/tag/major-releases/ .)

gnprice commented 9 months ago

In the UI, this will show up in two contexts: when you try to log into a new account, or when you pick an already logged-in account from the list on the initial screen of the app. In either case, if the server is too old, we'll need to show you an error and take you back.

To picture these contexts, see the first and the fourth of the handy screen captures in the PR that added this feature in zulip-mobile: https://github.com/zulip/zulip-mobile/pull/5633 (The other two are for cases that don't exist in zulip-flutter.)

terpimost commented 9 months ago

I think it is a rare case and user can't do anything about, so the current solution of a system level alert is an appropriate one. I would improve wording by adding "What should you do" information. Please ask your administrator to upgrade to v8 (latest version) and make it a link to release notes.

chrisbobbe commented 7 months ago

Greg proposes (discussion) that we use the feature level to decide if the server is ancient, rather than trying to parse the version string:

It occurs to me now that we should probably switch from making that check [in zulip-mobile] based on zulip_version to doing so based on zulip_feature_level. That's dead simple to parse (it's just an integer); and it's more foolproof for the server to generate too, as it's just quoting a number that's written directly in the server's source tree:

API_FEATURE_LEVEL = 237

Originally we used zulip_version because zulip_feature_level wasn't an option — it was introduced in the 3.0 cycle, and we wanted to tell the difference between 2.1 and 2.0 and 1.9 and 1.8 and so on.

But now our minimum version is one where zulip_feature_level already exists, so that constraint no longer applies.

gnprice commented 7 months ago

A few more details on that:

gnprice commented 3 months ago

This was blocked for a while on #458, which is why I assigned it to myself. Now that #702 is merged, that blocker is resolved.

Looking back at the corresponding zulip-mobile feature in zulip/zulip-mobile#5633, I think we can keep it simpler here: no need to log out of the account, or try to stop notifications. We can just show an error dialog, and pop off the navigation stack any screens that are for that account, so that (unless they've already navigated somewhere else) the user gets taken back to the choose-account screen.

We have an open issue to give the user an option to log out of an account:

Once we have that, if the user does want to log out of the account then they'll be able to do that from the choose-account page.

I think we also don't need the choose-account page to show any special indication on an account that's had this error. We can just put the affected account into the same state as if we hadn't yet tried to load data for it at all: if the user chooses it from that screen, we'll navigate to the account's home screen and try fetching its data again. If the server hasn't been upgraded, it'll fail again the same way, but it's entirely within the user's control whether to try it again and see.

alya commented 3 months ago

That plan sounds good to me!