From d3a7aa3a3718b27fffadb4d1b74bf03f95e016f3 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Sat, 13 May 2017 08:41:10 -0700 Subject: [PATCH] Have get_stream_li() take a stream_id. Rather than having get_stream_li() look up stream id using stream name, we force the callers to pass in the stream id. This adds an extra line to most of the callers for now, but this will eventually change as we fix some of the callers to have their callers pass in stream_id. In places where we now call stream_data.get_stream_id() to get the stream id, we will be more resilient toward stream renamings, at least until the next reload, since stream_data.get_stream_id() can resolve old names that are stored when we process stream-rename events. --- frontend_tests/node_tests/stream_list.js | 2 +- static/js/navigate.js | 6 +++-- static/js/stream_list.js | 28 +++++++++++++----------- static/js/stream_muting.js | 2 +- 4 files changed, 21 insertions(+), 17 deletions(-) diff --git a/frontend_tests/node_tests/stream_list.js b/frontend_tests/node_tests/stream_list.js index d607cc3de4..31ea7587b6 100644 --- a/frontend_tests/node_tests/stream_list.js +++ b/frontend_tests/node_tests/stream_list.js @@ -74,7 +74,7 @@ function clear_filters() { var html = $("body").html(); global.write_test_output("test_create_sidebar_row", html); - var li = stream_list.get_stream_li('social'); + var li = stream_list.get_stream_li(social.stream_id); assert.equal(li.attr('data-name'), 'social'); assert.equal(li.find('a.stream-name').text().trim(), 'social'); assert(li.find('.arrow').find("i").hasClass("icon-vector-chevron-down")); diff --git a/static/js/navigate.js b/static/js/navigate.js index af1186ab59..23db8b126b 100644 --- a/static/js/navigate.js +++ b/static/js/navigate.js @@ -118,8 +118,10 @@ exports.page_down = function () { exports.cycle_stream = function (direction) { var currentStream; var nextStream; - if (narrow_state.stream() !== undefined) { - currentStream = stream_list.get_stream_li(narrow_state.stream()); + var stream_name = narrow.stream(); + if (stream_name !== undefined) { + var stream_id = stream_data.get_stream_id(stream_name); + currentStream = stream_list.get_stream_li(stream_id); } switch (direction) { case 'forward': diff --git a/static/js/stream_list.js b/static/js/stream_list.js index f7720b25c4..ccfa5c976a 100644 --- a/static/js/stream_list.js +++ b/static/js/stream_list.js @@ -136,9 +136,8 @@ function get_filter_li(type, name) { return iterate_to_find("#" + type + "_filters > li", name); } -exports.get_stream_li = function (stream_name) { - var sub = stream_data.get_sub(stream_name); - return $("#stream_sidebar_" + sub.stream_id); +exports.get_stream_li = function (stream_id) { + return $("#stream_sidebar_" + stream_id); }; function zoom_in() { @@ -190,8 +189,8 @@ function reset_to_unnarrowed(narrowed_within_same_stream) { } } -exports.set_in_home_view = function (stream, in_home) { - var li = exports.get_stream_li(stream); +exports.set_in_home_view = function (stream_id, in_home) { + var li = exports.get_stream_li(stream_id); if (in_home) { li.removeClass("out_of_home_view"); } else { @@ -256,9 +255,9 @@ exports.create_sidebar_row = function (sub) { }; exports.redraw_stream_privacy = function (stream_name) { - var li = exports.get_stream_li(stream_name); - var div = li.find('.stream-privacy'); var sub = stream_data.get_sub(stream_name); + var li = exports.get_stream_li(sub.stream_id); + var div = li.find('.stream-privacy'); var color = stream_data.get_color(stream_name); var dark_background = stream_color.get_color_class(color); @@ -277,15 +276,17 @@ function set_count(type, name, count) { } function set_stream_unread_count(stream_name, count) { - var unread_count_elem = exports.get_stream_li(stream_name); + var stream_id = stream_data.get_stream_id(stream_name); + var unread_count_elem = exports.get_stream_li(stream_id); update_count_in_dom(unread_count_elem, count); } -function rebuild_recent_topics(stream) { +function rebuild_recent_topics(stream_name) { // TODO: Call rebuild_recent_topics less, not on every new // message. - var stream_li = exports.get_stream_li(stream); - topic_list.rebuild(stream_li, stream); + var stream_id = stream_data.get_stream_id(stream_name); + var stream_li = exports.get_stream_li(stream_id); + topic_list.rebuild(stream_li, stream_name); } exports.update_streams_sidebar = function () { @@ -350,7 +351,7 @@ exports.refresh_pinned_or_unpinned_stream = function (sub) { // a topic, we may be literally trying to get it out of // our sight. if (sub.pin_to_top) { - var stream_li = exports.get_stream_li(sub.name); + var stream_li = exports.get_stream_li(sub.stream_id); exports.scroll_to_active_stream(stream_li); } }; @@ -398,7 +399,8 @@ $(function () { var op_stream = event.filter.operands('stream'); if (op_stream.length !== 0 && stream_data.is_subscribed(op_stream[0])) { var stream_name = op_stream[0]; - var stream_li = exports.get_stream_li(stream_name); + var stream_id = stream_data.get_stream_id(stream_name); + var stream_li = exports.get_stream_li(stream_id); var op_subject = event.filter.operands('topic'); if (op_subject.length === 0) { stream_li.addClass('active-filter'); diff --git a/static/js/stream_muting.js b/static/js/stream_muting.js index 619d0f3978..7a788cb121 100644 --- a/static/js/stream_muting.js +++ b/static/js/stream_muting.js @@ -48,7 +48,7 @@ exports.update_in_home_view = function (sub, value) { } }, 0); - stream_list.set_in_home_view(sub.name, sub.in_home_view); + stream_list.set_in_home_view(sub.stream_id, sub.in_home_view); var not_in_home_view_checkbox = $(".subscription_settings[data-stream-id='" + sub.stream_id + "'] #sub_setting_not_in_home_view .sub_setting_control"); not_in_home_view_checkbox.prop('checked', !value);