filter: Store reference to _sub instead of _stream_params.

In commit 4f6377d493 we added
`_stream_params` as a way of storing attributes such as stream name
and stream privacy, this involved adding a few calls within functions
that updated these values (in order to maintain consistency).

This commit replaces `_stream_params` with an always consistent `_sub`
object and removes unnecessary `_stream_params` related code. Once the
`_sub` object is available, calls to `stream_data` may be considered
suspicious as they can often be avoided by just picking the desired
attribute off of the `_sub` object.
This commit is contained in:
YashRE42
2020-06-15 22:27:26 +00:00
committed by Tim Abbott
parent b0b53c8543
commit 7ea60ea1ab
3 changed files with 20 additions and 54 deletions

View File

@@ -151,10 +151,12 @@ function message_matches_search_term(message, operator, operand) {
function Filter(operators) { function Filter(operators) {
if (operators === undefined) { if (operators === undefined) {
this._operators = []; this._operators = [];
this._stream_params = undefined; this._sub = undefined;
} else { } else {
this._operators = this.fix_operators(operators); this._operators = this.fix_operators(operators);
this.fix_stream_params(); if (this.has_operator('stream')) {
this._sub = stream_data.get_sub_by_name(this.operands('stream')[0]);
}
} }
} }
@@ -480,7 +482,7 @@ Filter.prototype = {
// this comes first because it has 3 term_types but is not a "complex filter" // this comes first because it has 3 term_types but is not a "complex filter"
if (_.isEqual(term_types, ['stream', 'topic', 'search'])) { if (_.isEqual(term_types, ['stream', 'topic', 'search'])) {
// if stream does not exist, redirect to All // if stream does not exist, redirect to All
if (!this._stream_params) { if (!this._sub) {
return "#"; return "#";
} }
return '/#narrow/stream/' + stream_data.name_to_slug(this.operands('stream')[0]) + '/topic/' + this.operands('topic')[0]; return '/#narrow/stream/' + stream_data.name_to_slug(this.operands('stream')[0]) + '/topic/' + this.operands('topic')[0];
@@ -495,7 +497,7 @@ Filter.prototype = {
switch (term_types[0]) { switch (term_types[0]) {
case 'stream': case 'stream':
// if stream does not exist, redirect to All // if stream does not exist, redirect to All
if (!this._stream_params) { if (!this._sub) {
return "#"; return "#";
} }
return '/#narrow/stream/' + stream_data.name_to_slug(this.operands('stream')[0]); return '/#narrow/stream/' + stream_data.name_to_slug(this.operands('stream')[0]);
@@ -520,27 +522,6 @@ Filter.prototype = {
return "#"; // redirect to All return "#"; // redirect to All
}, },
fix_stream_params: function () {
this._stream_params = this.get_stream_params();
},
get_stream_params: function () {
// we return undefined when we are unable to get these parameters
if (!this.has_operator('stream')) {
return;
}
const stream_name_from_search = this.operands('stream')[0];
const sub = stream_data.get_sub_by_name(stream_name_from_search);
if (!sub) {
return;
}
return {
_stream_name: sub.name,
_is_stream_private: sub.invite_only,
_is_web_public: sub.is_web_public,
};
},
get_icon: function () { get_icon: function () {
// We have special icons for the simple narrows available for the via sidebars. // We have special icons for the simple narrows available for the via sidebars.
const term_types = this.sorted_term_types(); const term_types = this.sorted_term_types();
@@ -549,13 +530,13 @@ Filter.prototype = {
case 'in-all': case 'in-all':
return 'home'; return 'home';
case 'stream': case 'stream':
if (!this._stream_params) { if (!this._sub) {
return 'question-circle-o'; return 'question-circle-o';
} }
if (this._stream_params._is_stream_private) { if (this._sub.invite_only) {
return 'lock'; return 'lock';
} }
if (this._stream_params._is_web_public) { if (this._sub.is_web_public) {
return 'globe'; return 'globe';
} }
return 'hashtag'; return 'hashtag';
@@ -575,10 +556,10 @@ Filter.prototype = {
const term_types = this.sorted_term_types(); const term_types = this.sorted_term_types();
if (term_types.length === 3 && _.isEqual(term_types, ['stream', 'topic', 'search']) || if (term_types.length === 3 && _.isEqual(term_types, ['stream', 'topic', 'search']) ||
term_types.length === 2 && _.isEqual(term_types, ['stream', 'topic'])) { term_types.length === 2 && _.isEqual(term_types, ['stream', 'topic'])) {
if (!this._stream_params) { if (!this._sub) {
return i18n.t('Unknown stream'); return i18n.t('Unknown stream');
} }
return this._stream_params._stream_name; return this._sub.name;
} }
if (term_types.length === 1 || term_types.length === 2 && term_types[1] === 'search') { if (term_types.length === 1 || term_types.length === 2 && term_types[1] === 'search') {
switch (term_types[0]) { switch (term_types[0]) {
@@ -589,10 +570,10 @@ Filter.prototype = {
case 'streams-public': case 'streams-public':
return i18n.t('Public stream messages in organization'); return i18n.t('Public stream messages in organization');
case 'stream': case 'stream':
if (!this._stream_params) { if (!this._sub) {
return i18n.t('Unknown stream'); return i18n.t('Unknown stream');
} }
return this._stream_params._stream_name; return this._sub.name;
case 'is-starred': case 'is-starred':
return i18n.t('Starred messages'); return i18n.t('Starred messages');
case 'is-mentioned': case 'is-mentioned':

View File

@@ -140,20 +140,9 @@ exports.update_stream_name = function (sub, new_name) {
compose_state.stream_name(new_name); compose_state.stream_name(new_name);
} }
// Update navbar stream name if needed // Update navbar if needed
const filter = narrow_state.filter(); const filter = narrow_state.filter();
if (filter && filter.operands("stream")[0] === old_name) { if (filter && filter._sub && filter._sub.stream_id === sub.stream_id) {
// This works, but it relies on `filter.fix_stream_params` masking
// some bad behaviour in the Filter object. In particular, the fact
// that the Filter object relies on the search box which doesn't
// rename the currently focused stream.
//
// This will likely be improved as we migrate to using search pills
// and then a stream ID based representation of the stream in Filter.
// update the stream_params stored in the filter object
filter.fix_stream_params();
// use these to update the navbar
tab_bar.render_title_area(); tab_bar.render_title_area();
} }
}; };
@@ -171,7 +160,7 @@ exports.update_stream_description = function (sub, description, rendered_descrip
// Update navbar if needed // Update navbar if needed
const filter = narrow_state.filter(); const filter = narrow_state.filter();
if (filter && filter.operands("stream")[0] === sub.name) { if (filter && filter._sub && filter._sub.stream_id === sub.stream_id) {
tab_bar.render_title_area(); tab_bar.render_title_area();
} }
}; };
@@ -189,12 +178,9 @@ exports.update_stream_privacy = function (sub, values) {
stream_ui_updates.update_add_subscriptions_elements(sub); stream_ui_updates.update_add_subscriptions_elements(sub);
stream_list.redraw_stream_privacy(sub); stream_list.redraw_stream_privacy(sub);
// Update navbar stream name if needed // Update navbar if needed
const filter = narrow_state.filter(); const filter = narrow_state.filter();
if (filter && filter.operands("stream")[0] === sub.name) { if (filter && filter._sub && filter._sub.stream_id === sub.stream_id) {
// update the stream_params stored in the filter object
filter.fix_stream_params();
// use these to update the navbar
tab_bar.render_title_area(); tab_bar.render_title_area();
} }
}; };

View File

@@ -49,9 +49,8 @@ function make_tab_data(filter) {
exports.colorize_tab_bar = function () { exports.colorize_tab_bar = function () {
const filter = narrow_state.filter(); const filter = narrow_state.filter();
if (filter === undefined || !filter.has_operator('stream') || !filter._stream_params) {return;} if (filter === undefined || !filter._sub) {return;}
const color_for_stream = stream_data.get_color(filter._stream_params._stream_name); $("#tab_list .stream > .fa").css('color', filter._sub.color);
$("#tab_list .stream > .fa").css('color', color_for_stream);
}; };
function append_and_display_title_area(tab_bar_data) { function append_and_display_title_area(tab_bar_data) {