We now use stream_id as our key to rename streams, which
should prevent a few race conditions long term. (We are
still possibly contending with other events that use
stream_name as a key, so this is not perfect.)
We don't want to prepend new subscribers to our list of
subscribers in the settings page when they hit enter; we
want to wait till we get the event from the server.
This is a fairly new regression that was added when we
live-updated peer subscriber changes.
This fixes and issue where the change_stream_privacy template had its
own duplicate copy of the data-stream-name attribute, which wasn't
updated when streams were renamed.
Fixes#2016.
I make server_events slimmer by not handling a specific
property when subs.update_subscription_properties() should
do all the dispatching (and mostly did).
And then since update_subscription_properties() has
a "sub" already, I can call directly to stream_list code
and remove a function from subs.js. Since I lose the
wrapper function in subs.js, I rename the stream_list
function as part of this commit.
The only code that gets slightly heavier here is that
we have two lines in the 'pin_to_top' case instead of one.
All of the eventual callers to prepend_subscribers()
and format_member_list_elem() call people.get_by_email()
anyway, so now we do it one place. The one exception
was using page_params.fullname, which is awkwardly
different than what we call that variable elsewhere
(fullname vs. full_name).
The startup code in subs.js used to intermingle data
stuff and UI stuff in a loop inside a called function,
which made the code hard to reason about.
Now there is a clear separation of concerns, with these methods
being called in succession:
stream_data.initialize_from_page_params();
stream_list.create_initial_sidebar_rows();
The first method was mostly extracted from subs.js, but I simplified
some things, like not needing to make a copy of the hashes
we were passed in, plus I now garbage collect email_dict. Also,
the code path that initialize_from_page_params() mostly replaces
used to call create_sub(), which fired a trigger, but now it
just does data stuff.
Once the data structure is built up, it's a very simple matter
to build the initial sidebar rows, and that's what the second
method does.
This function will make it easier to unit test upcoming
changes related to stream counts.
This was mostly moving code, but one change is that we
don't call create_subs() in subs.js any more (which would
have been kind of circular dependency), since the only thing
that it did besides calling a more appropriate function
in stream_data.js was to generate a trigger that was
subsequently ignored and possibly a UI trap, as we don't
want to be messing with the stream sidebar when we go into
the stream settings page.
We now simply call exports.create_sub_from_server_data() for
newly encountered unsubscribed streams (which don't belong in
the sidebar anyway.)
This function used to live in subs.js. It's mostly a code move,
but I simplified the logic to determine whether it's subscribed
not to do a lookup into the same data structure that the sub
already came from.
I also added some tests.
Tim Abbott
Brock Whittaker
Steve Howell
sonali0901