login: Fetch all the data we'll want for an accounts table

[gnprice]

This prepares for #34 by expanding the Account class to contain all the data we'll want to be able to record in an accounts table when each record is first added. That means following up on #39 to fetch more data in the login flow: the user's ID and the server's version information.

NB this requires adding a line to one's credential_fixture.dart. That's the second time we've had to make changes to that file, after #51. I expect it will be the last, as with #34 we'll get to remove that fixture mechanism entirely.

[chrisbobbe]

Thanks! See a few small comments below.

[chrisbobbe]

realmName can be missing, kind of awkwardly. We have this comment in zulip-mobile:

  // When missing, the user requested the root domain of a Zulip server, but
  // there is no realm there. User error.
  //
  // (Also, the server has `ROOT_DOMAIN_LANDING_PAGE = False`, the default.
  // But the mobile client doesn't care about that; we just care that there
  // isn't a realm at the root.)
  //
  // Discussion:
  //   https://chat.zulip.org/#narrow/stream/412-api-documentation/topic/.2Fserver_settings.3A.20.60realm_name.60.2C.20etc.2E/near/1334042
  //
  // TODO(server-future): Expect the error to be encoded in a proper error
  //   response instead of this missing property.
  // TODO(server-future): Then, when all supported servers give that error,
  //   make this property required.

and this bit of logic to handle when it's missing:

  if (realm_name == null) {
    // See comment on realm_name in ApiResponseServerSettings.
    //
    // This error copies the proper error that servers *sometimes* give when
    // the root domain is requested and there's no realm there. [1]
    // Specifically, servers give this when
    // `ROOT_DOMAIN_LANDING_PAGE = True`. That circumstance makes no
    // difference to mobile.
    //
    // [1] Observed empirically. For details, see
    //   https://chat.zulip.org/#narrow/stream/412-api-documentation/topic/.2Fserver_settings.3A.20.60realm_name.60.2C.20etc.2E/near/1332900 .
    throw new ApiError(400, { code: 'BAD_REQUEST', msg: 'Subdomain required', result: 'error' });
  }

Probably we don't need the runtime code right now, but maybe worth a comment mentioning that there's some complexity to be handled until server-future?

[gnprice]

Yeah, my thinking was that this result type would describe only what we get when asking at an actual Zulip realm's domain, not a Zulip server's root domain when there's no realm there. I should make that explicit in dartdoc, because the https://zulip.com/api/get-server-settings doc is written with the opposite focus.

When you talk to a server's root domain when no realm is there, you'll get a response that's missing several of these required fields, and so it'll throw an exception. That seems appropriate if one thinks of this as about a realm — other API endpoints don't work on such a domain either. It's basically the same as one expects if the entered URL doesn't belong to a Zulip server at all.

I'll also leave a TODO comment that as part of #35 it would be good to give feedback when the user enters such a domain. I'm not sure we'll necessarily take care of that in the main round of #35 when we close that issue — in particular, I'm not aware of any existing server one can make
that mistake with other than zulipchat.com, and that one has ROOT_DOMAIN_LANDING_PAGE = True so it fails differently — but if not then we can file a more specific issue for it then.

[chrisbobbe]

zulip-mobile has this comment:

// Current to FL 121.

Should we put a similar comment here? No harm in copying zulip-mobile with 121, I think, since callers don't yet use newer server features.

[gnprice]

I'd like to use those sparingly. In general I'd prefer that the expectation is that things are up to date.

On this get-server-settings endpoint, for example, now that the current feature level is up to 171, that comment makes it look like it might be pretty stale; but in reality the last change it had (*) was at FL 116, so it's still perfectly up to date. I think most of the Zulip API is like this endpoint in that it doesn't change often, and we can pretty well keep it up to date.

The major exception is settings, which the Git history on zulip-mobile reminds me is where we first started applying this "current to" convention in zulip/zulip-mobile@3971c9c47. I think it may well make sense to use it in a couple of spots that have long lists of settings, once we add those here.

One practice we could start in order to systematically catch anything that's out of date: on each server release, take that as a prompt/reminder to go through the API changelog since the last release, find all the updates that are to API surface we describe, and apply those updates where we haven't already. Looking back at the 5.0-to-6.0 changes as an example, I think that'd be a quite manageable task.

(*) Apart from the ignored_parameters_unsupported thing, which is cross-cutting over the whole API and which we're not interested in anyway.

[chrisbobbe]

Ah OK, that makes sense.

[chrisbobbe]

Thanks, LGTM! Please merge at will, then. 🙂

[gnprice]

Thanks for the review!