refactor: Avoid positional params for MessageList.

We used to have positional parameters for table_name
and filter, but we don't use them for message_list.all
and we're about to replace filter in some cases.

Passing everything in on opts is more consistent and
self-documenting in the calling code, plus lots of
unit tests can get away with passing in `{}` now
for situations where table_name does not matter.

All of our callers pass in muting_enabled, so we
remove the default value for it.  And then the
collapse_messages variable doesn't have to live on
`this` as it's only being passed through down to the
view.
This commit is contained in:
Steve Howell 2018-05-14 13:46:25 +00:00 committed by Tim Abbott
parent f61ecef138
commit a3d3d08a80
7 changed files with 51 additions and 69 deletions

View File

@ -43,14 +43,16 @@ function stub_message_view(list) {
function make_home_msg_list() { function make_home_msg_list() {
var table_name = 'whatever'; var table_name = 'whatever';
var filter = new Filter(); var filter = new Filter();
var opts = {};
var list = new message_list.MessageList(table_name, filter, opts); var list = new message_list.MessageList({
table_name: table_name,
filter: filter,
});
return list; return list;
} }
function make_all_list() { function make_all_list() {
return new message_list.MessageList(); return new message_list.MessageList({});
} }
function reset_lists() { function reset_lists() {
@ -267,10 +269,10 @@ function simulate_narrow() {
return 'operators-stub'; return 'operators-stub';
}; };
var msg_list = new message_list.MessageList( var msg_list = new message_list.MessageList({
'zfilt', table_name: 'zfilt',
filter filter: filter,
); });
set_global('current_msg_list', msg_list); set_global('current_msg_list', msg_list);
return msg_list; return msg_list;

View File

@ -31,10 +31,11 @@ function accept_all_filter() {
} }
run_test('basics', () => { run_test('basics', () => {
var table;
var filter = accept_all_filter(); var filter = accept_all_filter();
var list = new MessageList(table, filter); var list = new MessageList({
filter: filter,
});
var messages = [ var messages = [
{ {
@ -126,9 +127,7 @@ run_test('basics', () => {
}); });
run_test('message_range', () => { run_test('message_range', () => {
var table; var list = new MessageList({});
var filter = {};
var list = new MessageList(table, filter);
var messages = [{id: 30}, {id: 40}, {id: 50}, {id: 60}]; var messages = [{id: 30}, {id: 40}, {id: 50}, {id: 60}];
list.append(messages, true); list.append(messages, true);
@ -140,9 +139,7 @@ run_test('message_range', () => {
}); });
run_test('updates', () => { run_test('updates', () => {
var table; var list = new MessageList({});
var filter = {};
var list = new MessageList(table, filter);
list.view.rerender_the_whole_thing = noop; list.view.rerender_the_whole_thing = noop;
var messages = [ var messages = [
@ -179,10 +176,7 @@ run_test('updates', () => {
}); });
run_test('nth_most_recent_id', () => { run_test('nth_most_recent_id', () => {
var table; var list = new MessageList({});
var filter = {};
var list = new MessageList(table, filter);
list.append([{id:10}, {id:20}, {id:30}]); list.append([{id:10}, {id:20}, {id:30}]);
assert.equal(list.nth_most_recent_id(1), 30); assert.equal(list.nth_most_recent_id(1), 30);
assert.equal(list.nth_most_recent_id(2), 20); assert.equal(list.nth_most_recent_id(2), 20);
@ -191,10 +185,7 @@ run_test('nth_most_recent_id', () => {
}); });
run_test('change_message_id', () => { run_test('change_message_id', () => {
var table; var list = new MessageList({});
var filter = {};
var list = new MessageList(table, filter);
list.append([{id: 10.5, content: "good job"}, {id: 20.5, content: "ok!"}]); list.append([{id: 10.5, content: "good job"}, {id: 20.5, content: "ok!"}]);
list.change_message_id(10.5, 11); list.change_message_id(10.5, 11);
assert.equal(list.get(11).content, "good job"); assert.equal(list.get(11).content, "good job");
@ -204,10 +195,7 @@ run_test('change_message_id', () => {
}); });
run_test('last_sent_by_me', () => { run_test('last_sent_by_me', () => {
var table; var list = new MessageList({});
var filter = {};
var list = new MessageList(table, filter);
var items = [ var items = [
{ {
id: 1, id: 1,
@ -230,10 +218,7 @@ run_test('last_sent_by_me', () => {
}); });
run_test('local_echo', () => { run_test('local_echo', () => {
var table; var list = new MessageList({});
var filter = {};
var list = new MessageList(table, filter);
list.append([{id:10}, {id:20}, {id:30}, {id:20.02}, {id:20.03}, {id:40}, {id:50}, {id:60}]); list.append([{id:10}, {id:20}, {id:30}, {id:20.02}, {id:20.03}, {id:40}, {id:50}, {id:60}]);
list._local_only= {20.02: {id:20.02}, 20.03: {id:20.03}}; list._local_only= {20.02: {id:20.02}, 20.03: {id:20.03}};
@ -255,7 +240,7 @@ run_test('local_echo', () => {
assert.equal(list.closest_id(58), 60); assert.equal(list.closest_id(58), 60);
list = new MessageList(table, filter); list = new MessageList({});
list.append([ list.append([
{id:10}, {id:20}, {id:30}, {id:20.02}, {id:20.03}, {id:40}, {id:10}, {id:20}, {id:30}, {id:20.02}, {id:20.03}, {id:40},
{id:50}, {id: 50.01}, {id: 50.02}, {id:60}]); {id:50}, {id: 50.01}, {id: 50.02}, {id:60}]);
@ -282,10 +267,7 @@ run_test('local_echo', () => {
}); });
run_test('bookend', () => { run_test('bookend', () => {
var table; var list = new MessageList({});
var filter = {};
var list = new MessageList(table, filter);
with_overrides(function (override) { with_overrides(function (override) {
var expected = "translated: You subscribed to stream IceCream"; var expected = "translated: You subscribed to stream IceCream";
@ -362,10 +344,7 @@ run_test('bookend', () => {
}); });
run_test('unmuted_messages', () => { run_test('unmuted_messages', () => {
var table; var list = new MessageList({});
var filter = {};
var list = new MessageList(table, filter);
var unmuted = [ var unmuted = [
{ {
@ -400,10 +379,9 @@ run_test('unmuted_messages', () => {
}); });
run_test('add_remove_rerender', () => { run_test('add_remove_rerender', () => {
var table;
var filter = accept_all_filter(); var filter = accept_all_filter();
var list = new MessageList(table, filter); var list = new MessageList({filter: filter});
var messages = [{id: 1}, {id: 2}, {id: 3}]; var messages = [{id: 1}, {id: 2}, {id: 3}];

View File

@ -370,9 +370,12 @@ run_test('render_windows', () => {
var view = (function make_view() { var view = (function make_view() {
var table_name = 'zfilt'; var table_name = 'zfilt';
var filter = new Filter(); var filter = new Filter();
var opts = {};
var list = new message_list.MessageList(table_name, filter, opts); var list = new message_list.MessageList({
table_name: table_name,
filter: filter,
});
var view = list.view; var view = list.view;
// Stub out functionality that is not core to the rendering window // Stub out functionality that is not core to the rendering window

View File

@ -95,10 +95,10 @@ function test_helper() {
} }
function stub_message_list() { function stub_message_list() {
message_list.MessageList = function (table_name, filter) { message_list.MessageList = function (opts) {
var list = this; var list = this;
this.messages = []; this.messages = [];
this.filter = filter; this.filter = opts.filter;
this.view = { this.view = {
set_message_offset: function (offset) { set_message_offset: function (offset) {
list.view.offset = offset; list.view.offset = offset;

View File

@ -4,18 +4,23 @@ var exports = {};
exports.narrowed = undefined; exports.narrowed = undefined;
exports.MessageList = function (table_name, filter, opts) { exports.MessageList = function (opts) {
_.extend(this, { _.extend(opts, {
collapse_messages: true, collapse_messages: true,
muting_enabled: true, });
}, opts);
var collapse_messages = opts.collapse_messages;
this.muting_enabled = opts.muting_enabled;
var table_name = opts.table_name;
var filter = opts.filter;
this.data = new MessageListData({ this.data = new MessageListData({
muting_enabled: this.muting_enabled, muting_enabled: this.muting_enabled,
filter: filter, filter: filter,
}); });
this.view = new MessageListView(this, table_name, this.collapse_messages); this.view = new MessageListView(this, table_name, collapse_messages);
this.fetch_status = FetchStatus(); this.fetch_status = FetchStatus();
this.table_name = table_name; this.table_name = table_name;
this.narrowed = this.table_name === "zfilt"; this.narrowed = this.table_name === "zfilt";
@ -401,10 +406,9 @@ exports.MessageList.prototype = {
}; };
exports.all = new exports.MessageList( exports.all = new exports.MessageList({
undefined, undefined, muting_enabled: false,
{muting_enabled: false} });
);
// We stop autoscrolling when the user is clearly in the middle of // We stop autoscrolling when the user is clearly in the middle of
// doing something. Be careful, though, if you try to capture // doing something. Be careful, though, if you try to capture

View File

@ -155,16 +155,12 @@ exports.activate = function (raw_operators, opts) {
// Save how far from the pointer the top of the message list was. // Save how far from the pointer the top of the message list was.
exports.save_pre_narrow_offset_for_reload(); exports.save_pre_narrow_offset_for_reload();
var msg_list_opts = { var msg_list = new message_list.MessageList({
table_name: 'zfilt',
filter: narrow_state.get_current_filter(),
collapse_messages: ! narrow_state.get_current_filter().is_search(), collapse_messages: ! narrow_state.get_current_filter().is_search(),
muting_enabled: muting_enabled, muting_enabled: muting_enabled,
}; });
var msg_list = new message_list.MessageList(
'zfilt',
narrow_state.get_current_filter(),
msg_list_opts
);
msg_list.start_time = start_time; msg_list.start_time = start_time;

View File

@ -2,12 +2,11 @@
// global variables from Zulip (everything is being moved into // global variables from Zulip (everything is being moved into
// modules). Please don't add things here. // modules). Please don't add things here.
var home_msg_list = new message_list.MessageList( var home_msg_list = new message_list.MessageList({
'zhome', table_name: 'zhome',
new Filter([{operator: "in", filter: new Filter([{operator: "in", operand: "home"}]),
operand: "home"}]), muting_enabled: true,
{muting_enabled: true} });
);
var current_msg_list = home_msg_list; var current_msg_list = home_msg_list;
if (typeof module !== 'undefined') { if (typeof module !== 'undefined') {