This prevents a bug where we interpret "2something"
as a modern slug instead of a legacy stream name.
The bug was probably somewhat unlikely to happen in
practice, since it only manifests if 2 is an actual
stream_id.
The maybe_clear_subscribers() function was an artifact of
when we used to attach subscribers to the "sub" records in
stream_data.js. I think it was basically a refactoring
shim, and due to some other recent cleanup, it was only
used in test code.
We also change how we validate stream ids.
Going forward, peer_data just looks up stream_ids with the
normal stream_data API when it's trying to warn about
rogue stream_ids coming in. As I alluded to in an earlier
commit, some of the warning code here might be overly
defensive, but at least it's pretty self-contained.
We also streamline some of the error handling code
by doing everything up front. This will prevent
scenarios where a single bad stream_id/user_id causes a
bunch of the same warnings in an inner loop.
This removes a bit of complexity. If a piece of
settings code needs to render a stream with
subscribers, it just asks for it.
We no longer have the brittle, action-at-a-distance
mechanism of mutating the subscriber count on to
the stream_data version of a sub.
Stream subs are pretty small, so making copies of
them is cheap, and the blueslip timings from the
previous commit can help confirm that.
There is some discussion of putting `subscriber_count`
on the Stream model, which may eventually get us
away from tracking it in `peer_data.js`, but we will
cross that bridge when we get there. See
https://github.com/zulip/zulip/issues/17101 for
more details.
The goal here is to make all our peer_data functions
basically work in id space. Passing a full `sub`
to these functions is a legacy of when subscriber
info was attached to a full stream "sub" object,
but we don't care about anything sub-related
(color, description, name, etc.) when we are
dealing with subscriptions.
When callers pass in stream_id, you can be more
confident in a quick skim of the code that we're
not mutating anything in the "sub".
This de-clutters stream_data a bit. Since our
peer data is our biggest performance concern,
I want to contain any optimizations to a fairly
well-focused module.
The name `peer_data` is a bit of a compromise,
since we already have `subs.js` and we use
`sub` as a variable name for stream records
throughout our code, but it's consistent with
our event nomenclature (peer/add, peer/remove)
and it's short while still being fairly easy
to find with grep.
This sets us up to use better system-wide data structures
for tracking subscribers.
Basically, instead of storing subscriber data on the
"sub" objects in stream_data.js, we instead have a
parallel data structure called stream_subscribers.
We also have stream_create, stream_edit, and friends
use helper functions rather than accessing
sub.subscribers directly.
We now use add_sub only in tests.
The line to defensively initialize subscribers does
not get copied from add_sub, since we know that
create_sub_from_server_data always initializes
subscribers via set_subscribers.
Instead of prohibiting ‘return undefined’ (#8669), we require that a
function must return an explicit value always or never. This prevents
you from forgetting to return a value in some cases. It will also be
important for TypeScript, which distinguishes between undefined and
void.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
ES and TypeScript modules are strict by default and don’t need this
directive. ESLint will remind us to add it to new CommonJS files and
remove it from ES and TypeScript modules.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We add a function subscribed_stream_ids which returns an array
of stream ids of all subscribed streams.
This is a prep commit for changing the logic for sorting streams
to store stream ids instead of names.
Prettier would do this anyway, but it’s separated out for a more
reviewable diff. Generated by ESLint.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Prettier would do this anyway, but it’s separated out for a more
reviewable diff. Generated by ESLint.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This commit changes stream_data.is_user_subscribed to use stream id
instead of stream name.
We are using stream ids so that we can avoid bugs related to live
update after stream rename.
The stream notification settings checkboxes were not checked
even when the notifications were turned on for the stream.
This was happening because we were passing stream name to
receives_notifications instead of stream id.
This commit fixes the bug by passing stream id to
receives_notifications. This change should have been done
in f3604fb while refactoring receives_notifications to use
stream id instead of name.
We change validate_stream_message to check the existence of stream from
the stream name in compose box early and we then pass stream_id or the
obtained sub objects accordingly to other validate functions.
Passing stream_id or sub objects to these functions, enables us to use
stream_id instead of stream name in stream_data.get_subscriber_count.
stream_data.get_stream_post_policy is also removed as we only used it in
validate_stream_message_policy, but we do not need it now as we can get
stream_post_policy directly from sub object obtained by early check of
valid stream name.
This commit changes stream_data.create_sub_from_server_data to use
stream id, instead of stream name, for checking whether subscription
already exists or not. We are using stream ids so that we can avoid
bugs related to live update after stream rename.
This commit changes stream_data.remove_subscriber to use stream id
instead of stream name. We are using stream ids so that we can
avoid bugs related to live update after stream rename.
Thsi commit changes stream_data.add_subscriber to use stream_id
instead of stream name. We are using stream ids so that we can
avoid bugs related to live update after stream rename.
This commit changes receives_notifications function to use
stream_ids instead of stream names. We are using stream ids so
that we can avoid bugs related to live update after stream rename.
This commit adds frontend support for setting and updating message
retention days of a stream from stream settings.
Message retention days can be changed from stream privacy modal of the
stream and can be set from stream_creation_form while creating streams.
Only admins can create streams with message_retention_days value other
than realm_default.
This commit also contains relevant changes to docs.
This commit removes is_old_stream property from the stream objects
returned by the API. This property was unnecessary and is essentially
equivalent to 'stream_weekly_traffic != null'.
We compute sub.is_old_stream in stream_data.update_calculated_fields
in frontend code and it is used to check whether we have a non-null
stream_weekly_traffic or not.
Fixes#15181.
The `wildcard_mentions_notify` key was missing from the initial
sub data when a new stream was created. Thus `wildcard_mentions_notify`
was undefined and `wildcard_mentions_notify_display` was false.
(This key is used to render the data in the templates)
This caused a bug where the wildcard notifications was unchecked
in the stream personal settings and the newly created stream was
displayed in the stream specific notifications table.
The reason for this change is that, this is where `Filter` and
actual tracking of what messages are contiguous lives. This
will be beneficial when we will to move to a model where we
cache `MessageListData` objects for a large number of views.
We shouldn't add redundant data to page_params. Since we already have
page_params.realm_notifications_stream_id, we can use that value instead
of creating page_params.notifications_stream.
We, however, still need the name of the notifications stream to render
it in templates. Thus we create stream_data.get_notifications_stream().
This change adds a toggle widget to the "add streams" page that
lets the user change the sort order of the streams list. So far,
this supports sorting by stream name, by number of subscribers,
or by estimated weekly traffic.
`stream_topic_history` is a more appropriate name as this
module will contain information about last message of a
stream in upcoming commits. Function and variable names
are changed accordingly like:
* topic_history() -> per_stream_history()
* get_recent_names() -> get_recent_topic_names()
* name -> topic_name
We now use `assert.throws()` to test that we're
properly calling `blueslip.fatal`.
In order to not break line coverage here, we have
to remove an unreachable `return` in `stream_data.js`.
Usually we test `fatal` for line coverage reasons.
Most places where we use `blueslip.fatal` fall in
these categories:
* the code is theoretically unreachable, but
we have `blueslip.fatal` for defensive reasons
* we have some upstream bug that we should just
fix
* the code should recover gracefully and just
use blueslip.errors()
It's possible that we should eliminate `blueslip.fatal`
from our API and just throw errors when really important
invariants get broken. This will make it more obvious
to somebody reading the code that we're not going to
continue after the call, and `blueslip` already knows
how to catch exceptions and report them.
This function returns a list of objects to create a
list_render object, and each item contains the streams
whose atleast one notification setting differs from the
default set by the user.
This is done by comparing the global settings in the
`#settings/notifications` page with those settings
present in the subscribed streams.
Work towards #9228.
We now only use `page_params.realm_default_streams` during
initialization, and then after that we use `stream_data`
APIs to get default stream ids and related info. (And
for the event that replace the data, we just update our
internal data structures as well.)
Long term we should have the server just send us ids here,
since we are now hydrating info from stream data in all places.
Steve Howell
Anders Kaseorg
sahil839
Priyank Patel
Ryan Rehman
Rohitt Vashishtha
Roland Crosby
Hashir Sarwar
Steve Howell