paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-06 06:53:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

streams: Store list of stream ids instead of names for sorting.

Browse Source 
We will store list of stream ids to sort streams instead of names.
We have added a compare_function for sorting the list of stream_ids
by comparing stream names.

This change helps us to remove a couple of get_sub calls and using
stream ids instead of name also helps in avoiding bugs caused due
to live update on renaming of stream.
This commit is contained in:
sahil839
2020-07-09 18:38:01 +05:30
committed by Tim Abbott
parent ed96758614
commit dcc24992b9
3 changed files with 36 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
frontend_tests/node_tests/stream_sort.js
View File

@@ -2,7 +2,7 @@ zrequire("stream_data");
zrequire("stream_sort"); zrequire("stream_sort");
run_test("no_subscribed_streams", () => { run_test("no_subscribed_streams", () => {
assert.equal(stream_sort.sort_groups(""), undefined); assert.equal(stream_sort.sort_groups([]), undefined);
assert.equal(stream_sort.first_stream_id(), undefined); assert.equal(stream_sort.first_stream_id(), undefined);
}); });
@@ -44,7 +44,7 @@ stream_data.add_sub(clarinet);
stream_data.add_sub(weaving); stream_data.add_sub(weaving);
function sort_groups(query) { function sort_groups(query) {
const streams = stream_data.subscribed_streams(); const streams = stream_data.subscribed_stream_ids();
return stream_sort.sort_groups(streams, query); return stream_sort.sort_groups(streams, query);
} }
@@ -53,9 +53,9 @@ run_test("basics", (override) => {
// Test sorting into categories/alphabetized // Test sorting into categories/alphabetized
let sorted = sort_groups(""); let sorted = sort_groups("");
assert.deepEqual(sorted.pinned_streams, ["scalene"]); assert.deepEqual(sorted.pinned_streams, [scalene.stream_id]);
assert.deepEqual(sorted.normal_streams, ["clarinet", "fast tortoise"]); assert.deepEqual(sorted.normal_streams, [clarinet.stream_id, fast_tortoise.stream_id]);
assert.deepEqual(sorted.dormant_streams, ["pneumonia"]); assert.deepEqual(sorted.dormant_streams, [pneumonia.stream_id]);
// Test cursor helpers. // Test cursor helpers.
assert.equal(stream_sort.first_stream_id(), scalene.stream_id); assert.equal(stream_sort.first_stream_id(), scalene.stream_id);
@@ -68,7 +68,7 @@ run_test("basics", (override) => {
// Test filtering // Test filtering
sorted = sort_groups("s"); sorted = sort_groups("s");
assert.deepEqual(sorted.pinned_streams, ["scalene"]); assert.deepEqual(sorted.pinned_streams, [scalene.stream_id]);
assert.deepEqual(sorted.normal_streams, []); assert.deepEqual(sorted.normal_streams, []);
assert.deepEqual(sorted.dormant_streams, []); assert.deepEqual(sorted.dormant_streams, []);
@@ -80,17 +80,17 @@ run_test("basics", (override) => {
sorted = sort_groups("PnEuMoNiA"); sorted = sort_groups("PnEuMoNiA");
assert.deepEqual(sorted.pinned_streams, []); assert.deepEqual(sorted.pinned_streams, []);
assert.deepEqual(sorted.normal_streams, []); assert.deepEqual(sorted.normal_streams, []);
assert.deepEqual(sorted.dormant_streams, ["pneumonia"]); assert.deepEqual(sorted.dormant_streams, [pneumonia.stream_id]);
// Test searching part of word // Test searching part of word
sorted = sort_groups("tortoise"); sorted = sort_groups("tortoise");
assert.deepEqual(sorted.pinned_streams, []); assert.deepEqual(sorted.pinned_streams, []);
assert.deepEqual(sorted.normal_streams, ["fast tortoise"]); assert.deepEqual(sorted.normal_streams, [fast_tortoise.stream_id]);
assert.deepEqual(sorted.dormant_streams, []); assert.deepEqual(sorted.dormant_streams, []);
// Test searching stream with spaces // Test searching stream with spaces
sorted = sort_groups("fast t"); sorted = sort_groups("fast t");
assert.deepEqual(sorted.pinned_streams, []); assert.deepEqual(sorted.pinned_streams, []);
assert.deepEqual(sorted.normal_streams, ["fast tortoise"]); assert.deepEqual(sorted.normal_streams, [fast_tortoise.stream_id]);
assert.deepEqual(sorted.dormant_streams, []); assert.deepEqual(sorted.dormant_streams, []);
}); });

7
static/js/stream_list.js
View File

@@ -87,7 +87,7 @@ exports.build_stream_list = function () {
// sidebar rows. Our job here is to build the bigger widget, // sidebar rows. Our job here is to build the bigger widget,
// which largely is a matter of arranging the individual rows in // which largely is a matter of arranging the individual rows in
// the right order. // the right order.
const streams = stream_data.subscribed_streams(); const streams = stream_data.subscribed_stream_ids();
if (streams.length === 0) { if (streams.length === 0) {
return; return;
} }
@@ -103,9 +103,8 @@ exports.build_stream_list = function () {
const parent = $("#stream_filters"); const parent = $("#stream_filters");
const elems = []; const elems = [];
function add_sidebar_li(stream) { function add_sidebar_li(stream_id) {
const sub = stream_data.get_sub(stream); const sidebar_row = exports.stream_sidebar.get_row(stream_id);
const sidebar_row = exports.stream_sidebar.get_row(sub.stream_id);
sidebar_row.update_whether_active(); sidebar_row.update_whether_active();
elems.push(sidebar_row.get_li()); elems.push(sidebar_row.get_li());
} }

49
static/js/stream_sort.js
View File

@@ -8,9 +8,22 @@ let all_streams = [];
exports.get_streams = function () { exports.get_streams = function () {
// Right now this is only used for testing, but we should // Right now this is only used for testing, but we should
// use it for things like hotkeys that cycle through streams. // use it for things like hotkeys that cycle through streams.
return all_streams; const sorted_streams = all_streams.map((stream_id) =>
stream_data.maybe_get_stream_name(stream_id),
);
return sorted_streams;
}; };
function compare_function(a, b) {
const stream_a = stream_data.get_sub_by_id(a);
const stream_b = stream_data.get_sub_by_id(b);
const stream_name_a = stream_a ? stream_a.name : "";
const stream_name_b = stream_b ? stream_b.name : "";
return util.strcmp(stream_name_a, stream_name_b);
}
function filter_streams_by_search(streams, search_term) { function filter_streams_by_search(streams, search_term) {
if (search_term === "") { if (search_term === "") {
return streams; return streams;
@@ -21,7 +34,7 @@ function filter_streams_by_search(streams, search_term) {
const filtered_streams = streams.filter((stream) => const filtered_streams = streams.filter((stream) =>
search_terms.some((search_term) => { search_terms.some((search_term) => {
const lower_stream_name = stream.toLowerCase(); const lower_stream_name = stream_data.get_sub_by_id(stream).name.toLowerCase();
const cands = lower_stream_name.split(" "); const cands = lower_stream_name.split(" ");
cands.push(lower_stream_name); cands.push(lower_stream_name);
return cands.some((name) => name.startsWith(search_term)); return cands.some((name) => name.startsWith(search_term));
@@ -47,7 +60,7 @@ exports.sort_groups = function (streams, search_term) {
const dormant_streams = []; const dormant_streams = [];
for (const stream of streams) { for (const stream of streams) {
const sub = stream_data.get_sub(stream); const sub = stream_data.get_sub_by_id(stream);
const pinned = sub.pin_to_top; const pinned = sub.pin_to_top;
if (pinned) { if (pinned) {
pinned_streams.push(stream); pinned_streams.push(stream);
@@ -58,9 +71,9 @@ exports.sort_groups = function (streams, search_term) {
} }
} }
pinned_streams.sort(util.strcmp); pinned_streams.sort(compare_function);
normal_streams.sort(util.strcmp); normal_streams.sort(compare_function);
dormant_streams.sort(util.strcmp); dormant_streams.sort(compare_function);
const same_as_before = const same_as_before =
previous_pinned !== undefined && previous_pinned !== undefined &&
@@ -84,26 +97,12 @@ exports.sort_groups = function (streams, search_term) {
}; };
}; };
function pos(stream_id) {
const sub = stream_data.get_sub_by_id(stream_id);
const name = sub.name;
const i = all_streams.indexOf(name);
if (i < 0) {
return;
}
return i;
}
function maybe_get_stream_id(i) { function maybe_get_stream_id(i) {
if (i < 0 || i >= all_streams.length) { if (i < 0 || i >= all_streams.length) {
return; return;
} }
const name = all_streams[i]; return all_streams[i];
const stream_id = stream_data.get_stream_id(name);
return stream_id;
} }
exports.first_stream_id = function () { exports.first_stream_id = function () {
@@ -111,9 +110,9 @@ exports.first_stream_id = function () {
}; };
exports.prev_stream_id = function (stream_id) { exports.prev_stream_id = function (stream_id) {
const i = pos(stream_id); const i = all_streams.indexOf(stream_id);
if (i === undefined) { if (i < 0) {
return; return;
} }
@@ -121,9 +120,9 @@ exports.prev_stream_id = function (stream_id) {
}; };
exports.next_stream_id = function (stream_id) { exports.next_stream_id = function (stream_id) {
const i = pos(stream_id); const i = all_streams.indexOf(stream_id);
if (i === undefined) { if (i < 0) {
return; return;
} }