streams: Avoid showing more topics option when it's useless.

This makes the "more topics" option which appears below the list of
known topics in the left sidebar appear only when it's possible there
are actually more topics to be displayed.  Two specific cases it
resolves completely include:

* Newly created realms; this widget was a common source of confusion
  for new organization administrators.
* Newly created streams.

There are still some corner cases this doesn't handle, e.g. if you
just joined a private stream with protected history, but there isn't
as easy a fix for those.

Essentially rewritten by tabbott to fix code duplication and comment
extensively.

Fixes #10265.
This commit is contained in:
Challa Venkata Raghava Reddy 2019-03-13 18:17:57 +05:30 committed by Tim Abbott
parent 2ccf5655da
commit 8623a02d98
5 changed files with 82 additions and 2 deletions

View File

@ -7,6 +7,8 @@ set_global('$', function () {
}); });
set_global('blueslip', global.make_zblueslip()); set_global('blueslip', global.make_zblueslip());
set_global('document', null);
global.stub_out_jquery();
zrequire('color_data'); zrequire('color_data');
zrequire('util'); zrequire('util');
@ -16,6 +18,11 @@ zrequire('people');
zrequire('stream_color'); zrequire('stream_color');
zrequire('stream_data'); zrequire('stream_data');
zrequire('marked', 'third/marked/lib/marked'); zrequire('marked', 'third/marked/lib/marked');
zrequire('FetchStatus', 'js/fetch_status');
zrequire('Filter', 'js/filter');
zrequire('MessageListData', 'js/message_list_data');
zrequire('MessageListView', 'js/message_list_view');
zrequire('message_list');
run_test('basics', () => { run_test('basics', () => {
var denmark = { var denmark = {
@ -808,3 +815,31 @@ run_test('get_invite_stream_data', () => {
}); });
assert.deepEqual(stream_data.get_invite_stream_data(), expected_list); assert.deepEqual(stream_data.get_invite_stream_data(), expected_list);
}); });
run_test('all_topics_in_cache', () => {
// Add a new stream with first_message_id set.
var general = {
name: 'general',
stream_id: 21,
first_message_id: null,
};
var messages = [
{id: 1, stream_id: 21},
{id: 2, stream_id: 21},
{id: 3, stream_id: 21},
];
var sub = stream_data.create_sub_from_server_data('general', general);
assert.equal(stream_data.all_topics_in_cache(sub), false);
message_list.all.data.add_messages(messages);
assert.equal(stream_data.all_topics_in_cache(sub), false);
message_list.all.fetch_status.has_found_newest = () => {return true;};
assert.equal(stream_data.all_topics_in_cache(sub), true);
sub.first_message_id = 0;
assert.equal(stream_data.all_topics_in_cache(sub), false);
sub.first_message_id = 2;
assert.equal(stream_data.all_topics_in_cache(sub), true);
});

View File

@ -6,6 +6,7 @@ set_global('unread', {});
set_global('muting', {}); set_global('muting', {});
set_global('stream_popover', {}); set_global('stream_popover', {});
set_global('templates', {}); set_global('templates', {});
set_global('message_list', {});
zrequire('hash_util'); zrequire('hash_util');
zrequire('stream_data'); zrequire('stream_data');

View File

@ -426,7 +426,6 @@ exports.MessageList.prototype = {
get_last_message_sent_by_me: function () { get_last_message_sent_by_me: function () {
return this.data.get_last_message_sent_by_me(); return this.data.get_last_message_sent_by_me();
}, },
}; };
exports.all = new exports.MessageList({ exports.all = new exports.MessageList({

View File

@ -356,6 +356,43 @@ exports.get_announcement_only = function (stream_name) {
return sub.is_announcement_only; return sub.is_announcement_only;
}; };
exports.all_topics_in_cache = function (sub) {
// Checks whether this browser's cache in message_list.all has all
// messages from a given stream, and thus all historical topics
// for it.
// If the cache isn't initialized, it's a clear false.
if (message_list.all === undefined || message_list.all.empty()) {
return false;
}
// If the cache doesn't have the latest messages, we can't be sure
// we have all topics.
if (!message_list.all.fetch_status.has_found_newest()) {
return false;
}
// This happens before message_list.all is initialized; just
// return false since the cache is empty.
var first_cached_message = message_list.all.first();
if (first_cached_message === undefined) {
return false;
}
if (sub.first_message_id === null) {
// If the stream has no message history, we have it all
// vacuously. This should be a very rare condition, since
// stream creation sends a message.
return true;
}
// Now, we can just compare the first cached message to the first
// message ID in the stream; if it's older, we're good, otherwise,
// we might be missing the oldest topics in this stream in our
// cache.
return first_cached_message.id <= sub.first_message_id;
};
var default_stream_ids = new Dict(); var default_stream_ids = new Dict();
exports.set_realm_default_streams = function (realm_default_streams) { exports.set_realm_default_streams = function (realm_default_streams) {
@ -516,6 +553,7 @@ exports.create_sub_from_server_data = function (stream_name, attrs) {
email_notifications: page_params.enable_stream_email_notifications, email_notifications: page_params.enable_stream_email_notifications,
description: '', description: '',
rendered_description: '', rendered_description: '',
first_message_id: attrs.first_message_id,
}); });
exports.set_subscribers(sub, subscriber_user_ids); exports.set_subscribers(sub, subscriber_user_ids);

View File

@ -116,9 +116,16 @@ exports.widget = function (parent_elem, my_stream_id) {
ul.append(li); ul.append(li);
}); });
// Now, we decide whether we need to show the "more topics"
// widget. We need it if there are at least 5 topics in the
// frontend's cache, or if we (possibly) don't have all
// historical topics in the browser's cache.
var show_more = self.build_more_topics_section(); var show_more = self.build_more_topics_section();
ul.append(show_more); var sub = stream_data.get_sub_by_id(my_stream_id);
if (topic_names.length > max_topics || !stream_data.all_topics_in_cache(sub)) {
ul.append(show_more);
}
return ul; return ul;
}; };