per-account 3/n: Sort out remaining selectors #5030
Conversation
|
Thanks, LGTM! Please merge at will. One nit is that "remaining selectors" isn't quite accurate, I think—remember this bit, from your 59d8813:
|
This was a workaround for zulip#4388/zulip#4391. With the introduction of withHaveServerDataGate, we've fixed that bug properly, and this condition should actually be impossible.
Most of its callers are in thunk actions or React components that are clearly per-account. Explicitly comment two that aren't obvious: one component that sounds like it could be cross-account, and one thunk action in outboxActions.
I've just read through all the callers, and confirmed that they're all per-account. Which is what one would hope, given that this selector's interface was already that it'll throw an exception if there isn't an active account. As a bonus, an untruthful error message gets fixed. We've been reporting "Active account not logged in" not only in the case where indeed the active account is not logged in, but also in the case where there is no active account at all. This is NFC other than that change in error message.
All the callers have been converted to the explicitly per-account tryGetThisAuth, with or without tryGetActiveAccountState first. We'll complete the migration of this function's callers shortly, by renaming the new version back to this name tryGetAuth.
This temporarily had the wordier name tryGetThisAuth just to facilitate sorting out its various callers between per-account and global.
This will let us distinguish places where we need a global setting from those where we need a per-account setting.
Most of the places we handle a *SettingsState, it's all local so we don't have or need a type annotation naming what type of object is involved. At one place where we do, though, make that annotation more specific.
As usual, we let the short simple name be kept by the per-account version.
gnprice
force-pushed
the
pr-peraccount-part3
branch
from
3883530 to
34827a6
Compare
|
Thanks for the review! Merged.
True. I guess what I really meant is more like "remaining named selectors", or "remaining selectors in Those small anonymous selectors, we'll get to as part of migrating the React components that they're part of. (I didn't see "remaining" in the commit messages, so I believe this only applies to the PR title.) |
This is the next PR in the series after #5016, #5017, and #5023, produced from the branch described at #5006 (comment).
From the foreshadowing last time:
That's this PR.
Next, continuing what we mentioned last time: