Remove disabled summarization code

This experiment has been disabled for everyone for a while: if we
bring something like this back, it is not likely to be exactly the same,
and will be different enough to require a different implementation.

As it is, the summarization code was making a few code paths (rendering
especially) more complex, and is worth removing for simplicity's sake.

(imported from commit 6ac8cdc9f7077a5a1da01ab4268aba3db0bc43f8)
This commit is contained in:
Leo Franchi 2014-02-03 16:48:25 -05:00 committed by Jessica McKellar
parent f128ff6d53
commit 748e5b6da6
14 changed files with 6 additions and 349 deletions

View File

@ -55,7 +55,6 @@ exports.fade_at_stream_granularity = page_params.staging;
exports.fade_users_when_composing = true; exports.fade_users_when_composing = true;
exports.mark_read_at_bottom = true; exports.mark_read_at_bottom = true;
exports.propagate_topic_edits = true; exports.propagate_topic_edits = true;
exports.summarize_read_while_narrowed = false;
exports.clicking_notification_causes_narrow = true; exports.clicking_notification_causes_narrow = true;
exports.use_socket = true; exports.use_socket = true;

View File

@ -311,15 +311,6 @@ function process_hotkey(e) {
return true; return true;
} }
if (current_msg_list.on_expandable_row()) {
switch (event_name) {
case 'enter':
ui.expand_summary_row(current_msg_list.selected_row().expectOne());
return true;
}
return false;
}
// Shortcuts that operate on a message // Shortcuts that operate on a message
switch (event_name) { switch (event_name) {
case 'message_actions': case 'message_actions':

View File

@ -69,8 +69,6 @@ function batched_updater(flag, op, immediate) {
} }
exports.send_read = batched_updater('read', 'add'); exports.send_read = batched_updater('read', 'add');
exports.send_summarize_in_stream = batched_updater('summarize_in_stream', 'add');
exports.send_summarize_in_home = batched_updater('summarize_in_home', 'add');
function send_flag(messages, flag_name, set_flag) { function send_flag(messages, flag_name, set_flag) {
var op = set_flag ? 'add' : 'remove'; var op = set_flag ? 'add' : 'remove';

View File

@ -2,8 +2,7 @@
function MessageList(table_name, filter, opts) { function MessageList(table_name, filter, opts) {
_.extend(this, { _.extend(this, {
collapse_messages: true, collapse_messages: true,
muting_enabled: true, muting_enabled: true
summarize_read: false
}, opts); }, opts);
this.view = new MessageListView(this, table_name, this.collapse_messages); this.view = new MessageListView(this, table_name, this.collapse_messages);
@ -23,7 +22,6 @@ function MessageList(table_name, filter, opts) {
this.narrowed = this.table_name === "zfilt"; this.narrowed = this.table_name === "zfilt";
this.num_appends = 0; this.num_appends = 0;
this.min_id_exempted_from_summaries = -1;
return this; return this;
} }
@ -205,10 +203,6 @@ MessageList.prototype = {
return this.get_row(this._selected_id); return this.get_row(this._selected_id);
}, },
on_expandable_row: function MessageList_on_expandable_row() {
return this.view.is_expandable_row(this.selected_row());
},
closest_id: function MessageList_closest_id(id) { closest_id: function MessageList_closest_id(id) {
var items = this._items; var items = this._items;
@ -279,34 +273,6 @@ MessageList.prototype = {
}); });
}, },
is_summarized_message: function (message) {
if (!feature_flags.summarize_read_while_narrowed ||
message === undefined || message.flags === undefined) {
return false;
}
if (message.id >= this.min_id_exempted_from_summaries) {
return false;
}
if (this.summarize_read === 'home') {
return message.flags.indexOf('summarize_in_home') !== -1;
} else if (this.summarize_read === 'stream' ) {
return message.flags.indexOf('summarize_in_stream') !== -1;
} else {
return false;
}
},
summary_adjective: function (message) {
if (_.contains(message.flags, 'force_collapse')) {
return 'collapsed';
} else if (!_.contains(message.flags, 'force_expand')) {
if (this.is_summarized_message(message)) {
return 'read';
}
}
return null;
},
selected_idx: function MessageList_selected_idx() { selected_idx: function MessageList_selected_idx() {
return util.lower_bound(this._items, this._selected_id, return util.lower_bound(this._items, this._selected_id,
function (a, b) { return a.id < b; }); function (a, b) { return a.id < b; });
@ -341,11 +307,6 @@ MessageList.prototype = {
} }
}, },
start_summary_exemption: function MessageList_start_summary_exemption() {
var num_exempt = 8;
this.min_id_exempted_from_summaries = this.nth_most_recent_id(num_exempt);
},
unmuted_messages: function MessageList_unmuted_messages(messages) { unmuted_messages: function MessageList_unmuted_messages(messages) {
return _.reject(messages, function (message) { return _.reject(messages, function (message) {
return muting.is_topic_muted(message.stream, message.subject) && return muting.is_topic_muted(message.stream, message.subject) &&
@ -365,11 +326,6 @@ MessageList.prototype = {
} }
this._items = this._items.concat(viewable_messages); this._items = this._items.concat(viewable_messages);
if (this.num_appends === 0) {
// We can't figure out which messages need to be exempt from
// summarization until we get the first batch of messages.
this.start_summary_exemption();
}
this.num_appends += 1; this.num_appends += 1;
this._add_to_hash(messages); this._add_to_hash(messages);
@ -558,10 +514,6 @@ MessageList.prototype = {
this._selected_id = new_id; this._selected_id = new_id;
} }
if (this.min_id_exempted_from_summaries === old_id) {
this.min_id_exempted_from_summaries = new_id;
}
// If this message is now out of order, re-order and re-render // If this message is now out of order, re-order and re-render
var self = this; var self = this;
setTimeout(function () { setTimeout(function () {

View File

@ -73,9 +73,6 @@ MessageListView.prototype = {
var current_group = []; var current_group = [];
var new_message_groups = []; var new_message_groups = [];
var summary_group = {};
var has_summary = false;
var summary_start_id = 0;
var self = this; var self = this;
if (where === "bottom") { if (where === "bottom") {
@ -109,11 +106,6 @@ MessageListView.prototype = {
var last_row = table.find('div[zid]:last'); var last_row = table.find('div[zid]:last');
last_message_id = rows.id(last_row); last_message_id = rows.id(last_row);
prev = self.get_message(last_message_id); prev = self.get_message(last_message_id);
if (last_row.is('.summary_row')) {
// Don't group with a summary, but don't put separators before the new message
prev = _.pick(prev, 'timestamp', 'historical');
}
} }
function set_template_properties(message) { function set_template_properties(message) {
@ -124,28 +116,6 @@ MessageListView.prototype = {
} }
} }
function finish_summary() {
var first = true;
_.each(summary_group, function (summary_row) {
summary_row.count = summary_row.messages.length;
summary_row.message_ids = _.pluck(summary_row.messages, 'id').join(' ');
if (first) {
summary_row.include_bookend = true;
first = false;
}
set_template_properties(summary_row);
messages_to_render.push(summary_row);
prev = summary_row;
});
prev.include_footer = true;
has_summary = false;
summary_group = {};
prev = _.pick(prev, 'timestamp', 'historical');
}
function finish_group() { function finish_group() {
if (current_group.length > 0) { if (current_group.length > 0) {
var message_ids = _.pluck(current_group, 'id'); var message_ids = _.pluck(current_group, 'id');
@ -162,53 +132,6 @@ MessageListView.prototype = {
add_display_time(message, prev); add_display_time(message, prev);
if (has_summary && message.show_date) {
finish_summary();
}
var summary_adjective = list.summary_adjective(message);
if (summary_adjective) {
if (prev) {
prev.include_footer = true;
}
var key = util.recipient_key(message);
if (summary_group[key] === undefined) {
// Start building a new summary row for messages from this recipient.
//
// Ugly: handlebars renderer only takes messages. We don't want to modify
// the original message, so we make a fake message based on the real one
// that will trigger the right part of the handlebars template and won't
// show the content, date, etc. from the real message.
var fake_message = _.extend(_.pick(message,
'timestamp', 'show_date', 'historical', 'stream',
'is_stream', 'subject', 'display_recipient', 'display_reply_to'), {
is_summary: true,
include_footer: false,
include_bookend: false,
first_message_id: message.id,
summary_adjective: summary_adjective,
messages: [message]
});
if (message.stream) {
fake_message.stream_url = narrow.by_stream_uri(message.stream);
fake_message.topic_url = narrow.by_stream_subject_uri(message.stream, message.subject);
} else {
fake_message.pm_with_url = narrow.pm_with_uri(message.reply_to);
}
summary_group[key] = fake_message;
} else {
summary_group[key].messages.push(message);
}
has_summary = true;
prev = message;
return;
} else if (has_summary) {
finish_summary();
}
if (util.same_recipient(prev, message) && self.collapse_messages && if (util.same_recipient(prev, message) && self.collapse_messages &&
prev.historical === message.historical && !message.show_date) { prev.historical === message.historical && !message.show_date) {
current_group.push(message); current_group.push(message);
@ -297,10 +220,6 @@ MessageListView.prototype = {
prev.include_footer = true; prev.include_footer = true;
} }
if (has_summary) {
finish_summary();
}
if (messages_to_render.length === 0) { if (messages_to_render.length === 0) {
return; return;
} }
@ -323,11 +242,7 @@ MessageListView.prototype = {
var row = $(elem); var row = $(elem);
// Save DOM elements by id into self._rows for O(1) lookup // Save DOM elements by id into self._rows for O(1) lookup
if (row.hasClass('summary_row')) { if (row.hasClass('message_row')) {
_.each(row.attr('data-messages').split(' '), function (id) {
self._rows[id] = elem;
});
} else if (row.hasClass('message_row')) {
self._rows[row.attr('zid')] = elem; self._rows[row.attr('zid')] = elem;
} }
@ -675,10 +590,6 @@ MessageListView.prototype = {
return this.get_row(this.list.selected_id()); return this.get_row(this.list.selected_id());
}, },
is_expandable_row: function MessageListView_is_expandable_row(row) {
return row.hasClass('summary_row');
},
get_message: function MessageListView_get_message(id) { get_message: function MessageListView_get_message(id) {
return this.list.get(id); return this.list.get(id);
}, },

View File

@ -279,14 +279,6 @@ exports.update_messages = function update_messages(events) {
exports.insert_new_messages = function insert_new_messages(messages) { exports.insert_new_messages = function insert_new_messages(messages) {
messages = _.map(messages, add_message_metadata); messages = _.map(messages, add_message_metadata);
if (feature_flags.summarize_read_while_narrowed) {
_.each(messages, function (message) {
if (message.sent_by_me) {
summary.maybe_mark_summarized(message);
}
});
}
// You must add add messages to home_msg_list BEFORE // You must add add messages to home_msg_list BEFORE
// calling unread.process_loaded_messages. // calling unread.process_loaded_messages.
exports.add_messages(messages, home_msg_list, {messages_are_new: true}); exports.add_messages(messages, home_msg_list, {messages_are_new: true});

View File

@ -153,9 +153,7 @@ exports.activate = function (raw_operators, opts) {
blueslip.debug("Narrowed", {operators: _.map(operators, blueslip.debug("Narrowed", {operators: _.map(operators,
function (e) { return e.operator; }), function (e) { return e.operator; }),
trigger: opts ? opts.trigger : undefined, trigger: opts ? opts.trigger : undefined,
previous_id: current_msg_list.selected_id(), previous_id: current_msg_list.selected_id()});
previous_is_summarized: current_msg_list.is_summarized_message(
current_msg_list.get(current_msg_list.selected_id()))});
var had_message_content = compose.has_message_content(); var had_message_content = compose.has_message_content();
@ -230,8 +228,7 @@ exports.activate = function (raw_operators, opts) {
var msg_list = new MessageList('zfilt', current_filter, { var msg_list = new MessageList('zfilt', current_filter, {
collapse_messages: ! current_filter.is_search(), collapse_messages: ! current_filter.is_search(),
muting_enabled: muting_enabled, muting_enabled: muting_enabled
summarize_read: this.summary_enabled()
}); });
msg_list.start_time = start_time; msg_list.start_time = start_time;
@ -447,13 +444,6 @@ exports.deactivate = function () {
empty_ok: true empty_ok: true
}; };
if (feature_flags.summarize_read_while_narrowed) {
// TODO: avoid a full re-render
// Necessary to replace messages read in the narrow with summary blocks
current_msg_list.start_summary_exemption();
current_msg_list.rerender();
}
// We fall back to the closest selected id, if the user has removed a // We fall back to the closest selected id, if the user has removed a
// stream from the home view since leaving it the old selected id might // stream from the home view since leaving it the old selected id might
// no longer be there // no longer be there
@ -626,30 +616,6 @@ exports.muting_enabled = function () {
return (!exports.narrowed_to_topic() && !exports.narrowed_to_search() && !exports.narrowed_to_pms()); return (!exports.narrowed_to_topic() && !exports.narrowed_to_search() && !exports.narrowed_to_pms());
}; };
exports.summary_enabled = function () {
if (!feature_flags.summarize_read_while_narrowed) {
return false;
}
if (current_filter === undefined){
return 'home'; // Home view, but this shouldn't run anyway
}
var operators = current_filter.operators();
if (operators.length === 1 && (
current_filter.has_operand("in", "home") ||
current_filter.has_operand("in", "all"))) {
return 'home';
}
if (operators.length === 1 && (
current_filter.operands("stream").length === 1 ||
current_filter.has_operand("is", "private"))) {
return 'stream';
}
};
return exports; return exports;
}()); }());

View File

@ -54,7 +54,7 @@ exports.get_table = function (table_name) {
exports.get_closest_row = function (element) { exports.get_closest_row = function (element) {
// This gets the closest message row to an element, whether it's // This gets the closest message row to an element, whether it's
// a summary bar, recipient bar, or message. With our current markup, // a recipient bar or message. With our current markup,
// this is the most reliable way to do it. // this is the most reliable way to do it.
return $(element).closest("div.message_row, div.recipient_row"); return $(element).closest("div.message_row, div.recipient_row");
}; };

View File

@ -1,31 +0,0 @@
var summary = (function () {
// This module has helper functions for the August 2013 message
// summarizing experiment.
var exports = {};
function mark_summarized(message) {
if (narrow.narrowed_by_reply()) {
// Narrowed to a topic or PM recipient
message_flags.send_summarize_in_stream(message);
}
if (narrow.active() && !narrow.narrowed_to_search()) {
// Narrowed to anything except a search
message_flags.send_summarize_in_home(message);
}
}
exports.maybe_mark_summarized = function (message) {
if (feature_flags.summarize_read_while_narrowed) {
mark_summarized(message);
}
};
return exports;
}());
if (typeof module !== 'undefined') {
module.exports = summary;
}

View File

@ -862,50 +862,6 @@ exports.collapse = function (row) {
show_more_link(row); show_more_link(row);
}; };
exports.expand_summary_row = function (row) {
var message_ids = row.attr('data-messages').split(' ');
var messages = _.map(message_ids, function (id) {
return current_msg_list.get(id);
});
_.each(messages, function (msg){
msg.flags = _.without(msg.flags, 'force_collapse');
msg.flags.push('force_expand');
});
message_flags.send_force_expand(messages, true);
message_flags.send_force_collapse(messages, false);
//TODO: Avoid a full re-render
home_msg_list.rerender();
if (current_msg_list !== home_msg_list) {
current_msg_list.rerender();
}
current_msg_list.select_id(message_ids[0]);
};
exports.collapse_recipient_group = function (row) {
var message_ids = row.attr('data-messages').split(',');
var messages = _.map(message_ids, function (id) {
return message_store.get(id);
});
_.each(messages, function (msg){
msg.flags = _.without(msg.flags, 'force_expand');
msg.flags.push('force_collapse');
});
message_flags.send_force_expand(messages, false);
message_flags.send_force_collapse(messages, true);
//TODO: Avoid a full re-render
home_msg_list.rerender();
if (current_msg_list !== home_msg_list) {
current_msg_list.rerender();
}
current_msg_list.select_id(message_ids[0]);
};
/* EXPERIMENTS */ /* EXPERIMENTS */
/* This method allows an advanced user to use the console /* This method allows an advanced user to use the console
@ -1246,17 +1202,6 @@ $(function () {
$("#navbar-buttons").addClass("right-userlist"); $("#navbar-buttons").addClass("right-userlist");
} }
if (feature_flags.summarize_read_while_narrowed) {
$("#main_div").on("click", ".summary_row .messages-expand", function (e) {
exports.expand_summary_row($(e.target).closest('.summary_row').expectOne());
e.stopImmediatePropagation();
});
$("#main_div").on("click", ".recipient_row .messages-collapse", function (e) {
exports.collapse_recipient_group($(e.target).closest('.recipient_row').expectOne());
e.stopImmediatePropagation();
});
}
function is_clickable_message_element(target) { function is_clickable_message_element(target) {
return target.is("a") || target.is("img.message_inline_image") || target.is("img.twitter-avatar") || return target.is("a") || target.is("img.message_inline_image") || target.is("img.twitter-avatar") ||
target.is("div.message_length_controller") || target.is("textarea") || target.is("input") || target.is("div.message_length_controller") || target.is("textarea") || target.is("input") ||

View File

@ -229,7 +229,6 @@ exports.mark_messages_as_read = function mark_messages_as_read (messages, option
if (options.from !== "server") { if (options.from !== "server") {
message_flags.send_read(message); message_flags.send_read(message);
} }
summary.maybe_mark_summarized(message);
message.unread = false; message.unread = false;
unread.process_read_message(message, options); unread.process_read_message(message, options);

View File

@ -3,11 +3,7 @@ var all_msg_list = new MessageList(
{muting_enabled: false} {muting_enabled: false}
); );
var home_msg_list = new MessageList('zhome', var home_msg_list = new MessageList('zhome',
new Filter([{operator: "in", operand: "home"}]), new Filter([{operator: "in", operand: "home"}]), {muting_enabled: true}
{
muting_enabled: true,
summarize_read: feature_flags.summarize_read_while_narrowed?'home':false
}
); );
var narrowed_msg_list; var narrowed_msg_list;
var current_msg_list = home_msg_list; var current_msg_list = home_msg_list;

View File

@ -28,65 +28,6 @@
<div class="date_row" data-zid="{{id}}"><div colspan="4">{{{show_date}}}</div></div> <div class="date_row" data-zid="{{id}}"><div colspan="4">{{{show_date}}}</div></div>
{{/if}} {{/if}}
{{#if is_summary}}
<div zid="{{first_message_id}}" data-messages="{{message_ids}}" class="summary_row selectable_row{{#include_footer}} last_message{{/include_footer}}{{^is_stream}} summary_row_private_message{{/is_stream}}">
<div class="message_header message_header_stream right_part" style="box-shadow:inset 9px 0px {{background_color}};">
{{! [+] }}
<i class="messages-expand icon-vector-expand-alt"></i>
{{#if is_stream}}
{{! invite-only lock icon }}
{{#if invite_only}}
<i class="icon-vector-lock" title="This is an invite-only stream"></i>
{{/if}}
{{! stream }}
<a class="message_label_clickable narrows_by_recipient stream_label"
href="{{stream_url}}"
title="Narrow to stream &quot;{{display_recipient}}&quot;">{{display_recipient}}
</a>
{{! > }}
&nbsp;
<i class="icon-vector-narrow icon-vector-small"></i>
<span class="copy-paste-text">&gt;</span>&nbsp;
{{! topic }}
<span class="stream_topic">
<a class="message_label_clickable narrows_by_subject"
href="{{topic_url}}"
title="Narrow to stream &quot;{{display_recipient}}&quot;, topic &quot;{{subject}}&quot;">
{{subject}}
</a>
{{! exterior links (e.g. to a trac ticket) }}
{{#each subject_links}}
<a href="{{this}}" target="_blank">
<i class="icon-vector-external-link-sign"></i>
</a>
{{/each}}
</span>
{{else}}
{{! You and Somebody Else, links to PM narrow}}
<a class="message_label_clickable narrows_by_recipient"
href="{{pm_with_url}}"
title="Narrow to your private messages with {{display_reply_to}}">
You and {{display_reply_to}}
</a>
{{/if}}
{{! "(5 read)" or something similar}}
({{count}} {{summary_adjective}})
</div>
</div>
{{else}}
{{#include_recipient}} {{#include_recipient}}
<div zid="{{id}}" class="recipient_row" data-messages="{{message_ids}}"> <div zid="{{id}}" class="recipient_row" data-messages="{{message_ids}}">
{{#if is_stream}} {{#if is_stream}}
@ -214,7 +155,6 @@
</div> </div>
</div> </div>
{{/if}}
{{/with}} {{/with}}
{{/each}} {{/each}}

View File

@ -513,7 +513,6 @@ JS_SPECS = {
'third/marked/lib/marked.js', 'third/marked/lib/marked.js',
'templates/compiled.js', 'templates/compiled.js',
'js/feature_flags.js', 'js/feature_flags.js',
'js/summary.js',
'js/util.js', 'js/util.js',
'js/dict.js', 'js/dict.js',
'js/localstorage.js', 'js/localstorage.js',