streams: Fix stream color synchronization.

Previously, when a new stream was created on a client other than the
current one, the browser would first receive the "stream_created"
event, and make up a client-side display color at that time to use in
the "stream settings" view (it doesn't yet know the color that was
selected when the user was actually subscribed, because it doesn't
even know yet that the user is being subscribed to this stream), and
then moments after it'll receive a "susbcribe" event letting the
client know that the user is subscribed (and specifying the color to
use).

However, due to an argument not being passed through properly and a
missing rerender, we were not properly updating either the data
structures or doing a stream colors rerender in order to show the new
color.

This fixes the issue reported in
https://chat.zulip.org/#narrow/stream/48-mobile/subject/stream.20colors/near/660170
This commit is contained in:
Tim Abbott
2018-11-09 16:57:10 -08:00
parent fe931d9b66
commit 2c06615909
3 changed files with 21 additions and 9 deletions

View File

@@ -134,6 +134,7 @@ run_test('marked_subscribed', () => {
// Test undefined error
with_overrides(function (override) {
var errors = 0;
override('stream_color.update_stream_color', noop);
override('blueslip.error', function () {
errors += 1;
});
@@ -170,6 +171,7 @@ run_test('marked_subscribed', () => {
// Test unread count update
with_overrides(function (override) {
global.with_stub(function (stub) {
override('stream_color.update_stream_color', noop);
override('message_util.do_unread_count_updates', stub.f);
stream_events.mark_subscribed(frontend, [], '');
var args = stub.get_args('messages');
@@ -180,15 +182,19 @@ run_test('marked_subscribed', () => {
set_global('message_util', { do_unread_count_updates: noop });
// Test jQuery event
global.with_stub(function (stub) {
$(document).on('subscription_add_done.zulip', stub.f);
stream_events.mark_subscribed(frontend, [], '');
var args = stub.get_args('event');
assert.equal(args.event.sub.stream_id, 1);
with_overrides(function (override) {
override('stream_color.update_stream_color', noop);
global.with_stub(function (stub) {
$(document).on('subscription_add_done.zulip', stub.f);
stream_events.mark_subscribed(frontend, [], '');
var args = stub.get_args('event');
assert.equal(args.event.sub.stream_id, 1);
});
});
// Test bookend update
with_overrides(function (override) {
override('stream_color.update_stream_color', noop);
override('narrow_state.is_for_stream_id', function () {
return true;
});
@@ -204,8 +210,11 @@ run_test('marked_subscribed', () => {
set_global('narrow_state', { is_for_stream_id: noop });
// Test setting color
stream_events.mark_subscribed(frontend, [], 'blue');
assert.equal(frontend.color, 'blue');
with_overrides(function (override) {
override('stream_color.update_stream_color', noop);
stream_events.mark_subscribed(frontend, [], 'blue');
assert.equal(frontend.color, 'blue');
});
// Test assigning generated color
with_overrides(function (override) {
@@ -219,6 +228,7 @@ run_test('marked_subscribed', () => {
});
global.with_stub(function (stub) {
override('stream_color.update_stream_color', noop);
override('subs.set_color', stub.f);
stream_events.mark_subscribed(frontend, [], undefined);
var args = stub.get_args('id', 'color');
@@ -230,6 +240,7 @@ run_test('marked_subscribed', () => {
// Test assigning subscriber emails
with_overrides(function (override) {
override('stream_color.update_stream_color', noop);
global.with_stub(function (stub) {
override('stream_data.set_subscribers', stub.f);
var user_ids = [15, 20, 25];

View File

@@ -296,7 +296,7 @@ exports.dispatch_normal_event = function dispatch_normal_event(event) {
var sub = stream_data.get_sub_by_id(rec.stream_id);
if (sub) {
stream_data.update_stream_email_address(sub, rec.email_address);
stream_events.mark_subscribed(sub, rec.subscribers);
stream_events.mark_subscribed(sub, rec.subscribers, rec.color);
} else {
blueslip.error('Subscribing to unknown stream with ID ' + rec.stream_id);
}

View File

@@ -100,8 +100,9 @@ exports.mark_subscribed = function (sub, subscribers, color) {
}
// If the backend sent us a color, use that
if (color !== undefined) {
if (color !== undefined && sub.color !== color) {
sub.color = color;
stream_color.update_stream_color(sub, color, {update_historical: true});
} else if (sub.color === undefined) {
// If the backend didn't, and we have a color already, send
// the backend that color. It's not clear this code path is