mirror of https://github.com/zulip/zulip.git
d9740045a5
We eliminate some redundant checks.
We also consistently provide a `subscribers` field
in our stream data with `[]`, even if our users
can't access subscribers. We therefore bump
the API version and tweak the docs. (See further
down for a detailed justification of the change.)
Even though it is sometimes fine to have redundant code
that is defensive in nature, some upcoming changes are gonna
move subscriber-related logic out of build_stream_dict_for_sub
for certain codepaths as part of our effort to streamline
the payload for subscribers within page_params.
So we can't rely on the code that I removed here
inside of build_stream_dict_for_sub.
Anyway, it makes more sense to do these checks explicitly
in the validate function.
The code in build_stream_dict_for_sub was almost effectively
a noop, since the validation function was already preventing
us from getting subscriber info. The only difference it
made was sometimes converting `[]` to `None`, and then
subsequently omitting the subscribers field.
Neither ZT nor the webapp make any distinction between
`[]` or <missing key> for the `subscribers` data in
`page_params`.
The webapp has had this code for a long time (and now
equivalent code elsewhere in this PR):
if (!Object.prototype.hasOwnProperty.call(sub, "subscribers")) {
sub.subscribers = new LazySet([]);
}
The webapp calculates access based on booleans, anyway:
sub.can_access_subscribers =
page_params.is_admin || sub.subscribed ||
(!page_params.is_guest && !sub.invite_only);
And ZT would choke if `subscribers` were missing, except that
it never gets to the relevant code due to other checks:
def get_other_subscribers_in_stream(<snip>):
assert stream_id is not None or stream_name is not None
if stream_id:
assert self.is_user_subscribed_to_stream(stream_id)
return [sub
for sub in self.stream_dict[stream_id]['subscribers']
if sub != self.user_id]
else:
return [sub
for _, stream in self.stream_dict.items()
for sub in stream['subscribers']
if stream['name'] == stream_name
if sub != self.user_id]
You could make a semantic argument that we should prefer
<missing key> to `[]` when subscribers aren't even available, but
we have precedent from the way that `bulk_get_subscriber_user_ids`
has traditionally populated its result:
result: Dict[int, List[int]] =
{stream["id"]: [] for stream in stream_dicts}
If we changed `stream_dicts` to `target_stream_dicts` we
would faciliate a move toward `None`, but it would just cause
headaches for other server code as well as the frontends
(which, to reiterate, already prefer the empty array
for convenience).
|
||
|---|---|---|
| .. | ||
| api | ||
| app | ||
| archive | ||
| emails | ||
| for | ||
| help | ||
| include | ||
| integrations | ||
| tests/markdown | ||
| accounts_accept_terms.html | ||
| accounts_home.html | ||
| accounts_send_confirm.html | ||
| apple-error.md | ||
| apps.html | ||
| base.html | ||
| billing_nav.html | ||
| close_window.html | ||
| compare.html | ||
| config_error.html | ||
| confirm_continue_registration.html | ||
| confirmation_link_expired_error.html | ||
| create_realm.html | ||
| deactivated.html | ||
| desktop_login.html | ||
| desktop_redirect.html | ||
| dev-not-supported-error.md | ||
| dev_env_email_access_details.html | ||
| dev_login.html | ||
| dev_tools.html | ||
| digest_base.html | ||
| documentation_main.html | ||
| email.html | ||
| email_log.html | ||
| faq.html | ||
| features.html | ||
| find_account.html | ||
| footer.html | ||
| for-companies.html | ||
| for-open-source.html | ||
| for-research.html | ||
| for-working-groups-and-communities.html | ||
| github-error.md | ||
| gitlab-error.md | ||
| google-error.md | ||
| gradients.html | ||
| hello.html | ||
| history.html | ||
| insecure_desktop_app.html | ||
| invalid_email.html | ||
| invalid_realm.html | ||
| landing_nav.html | ||
| log_into_subdomain_token_invalid.html | ||
| login.html | ||
| meta_tags.html | ||
| plans.html | ||
| portico-header-dropdown.html | ||
| portico-header.html | ||
| portico-help.html | ||
| portico.html | ||
| portico_signup.html | ||
| privacy.html | ||
| realm_creation_failed.html | ||
| realm_reactivation.html | ||
| realm_reactivation_link_error.html | ||
| realm_redirect.html | ||
| register.html | ||
| reset.html | ||
| reset_confirm.html | ||
| reset_done.html | ||
| reset_emailed.html | ||
| security.html | ||
| security.md | ||
| social_auth_select_email.html | ||
| team.html | ||
| terms.html | ||
| unsubscribe_link_error.html | ||
| unsubscribe_success.html | ||
| unsupported_browser.html | ||
| why-zulip.html | ||
| why-zulip.md | ||
| zulipchat_migration_tos.html | ||