stream_data: Modify create_sub_from_server_data to use stream id.

This commit changes stream_data.create_sub_from_server_data to use
stream id, instead of stream name, for checking whether subscription
already exists or not. We are using stream ids so that we can avoid
bugs related to live update after stream rename.
This commit is contained in:
sahil839 2020-06-21 18:53:43 +05:30 committed by Tim Abbott
parent b99320ffa0
commit 28b6af5c4b
2 changed files with 16 additions and 15 deletions

View File

@ -864,17 +864,18 @@ run_test('create_sub', () => {
return '#bd86e5'; return '#bd86e5';
}; };
const india_sub = stream_data.create_sub_from_server_data('India', india); const india_sub = stream_data.create_sub_from_server_data(india);
assert(india_sub); assert(india_sub);
assert.equal(india_sub.color, '#bd86e5'); assert.equal(india_sub.color, '#bd86e5');
const new_sub = stream_data.create_sub_from_server_data('India', india); // make sure sub doesn't get created twice const new_sub = stream_data.create_sub_from_server_data(india);
// make sure sub doesn't get created twice
assert.equal(india_sub, new_sub); assert.equal(india_sub, new_sub);
assert.throws(() => { assert.throws(() => {
stream_data.create_sub_from_server_data('Canada', canada); stream_data.create_sub_from_server_data('Canada', canada);
}, {message: 'We cannot create a sub without a stream_id'}); }, {message: 'We cannot create a sub without a stream_id'});
const antarctica_sub = stream_data.create_sub_from_server_data('Antarctica', antarctica); const antarctica_sub = stream_data.create_sub_from_server_data(antarctica);
assert(antarctica_sub); assert(antarctica_sub);
assert.equal(antarctica_sub.color, '#76ce90'); assert.equal(antarctica_sub.color, '#76ce90');
}); });
@ -1062,7 +1063,7 @@ run_test('all_topics_in_cache', () => {
{id: 2, stream_id: 21}, {id: 2, stream_id: 21},
{id: 3, stream_id: 21}, {id: 3, stream_id: 21},
]; ];
const sub = stream_data.create_sub_from_server_data('general', general); const sub = stream_data.create_sub_from_server_data(general);
assert.equal(stream_data.all_topics_in_cache(sub), false); assert.equal(stream_data.all_topics_in_cache(sub), false);

View File

@ -679,27 +679,28 @@ exports.is_user_subscribed = function (stream_name, user_id) {
exports.create_streams = function (streams) { exports.create_streams = function (streams) {
for (const stream of streams) { for (const stream of streams) {
// We handle subscriber stuff in other events. // We handle subscriber stuff in other events.
const attrs = { const attrs = {
subscribers: [], subscribers: [],
subscribed: false, subscribed: false,
...stream, ...stream,
}; };
exports.create_sub_from_server_data(stream.name, attrs); exports.create_sub_from_server_data(attrs);
} }
}; };
exports.create_sub_from_server_data = function (stream_name, attrs) { exports.create_sub_from_server_data = function (attrs) {
let sub = exports.get_sub(stream_name);
if (sub !== undefined) {
// We've already created this subscription, no need to continue.
return sub;
}
if (!attrs.stream_id) { if (!attrs.stream_id) {
// fail fast (blueslip.fatal will throw an error on our behalf) // fail fast (blueslip.fatal will throw an error on our behalf)
blueslip.fatal("We cannot create a sub without a stream_id"); blueslip.fatal("We cannot create a sub without a stream_id");
} }
let sub = exports.get_sub_by_id(attrs.stream_id);
if (sub !== undefined) {
// We've already created this subscription, no need to continue.
return sub;
}
// Our internal data structure for subscriptions is mostly plain dictionaries, // Our internal data structure for subscriptions is mostly plain dictionaries,
// so we just reuse the attrs that are passed in to us, but we encapsulate how // so we just reuse the attrs that are passed in to us, but we encapsulate how
// we handle subscribers. We defensively remove the `subscribers` field from // we handle subscribers. We defensively remove the `subscribers` field from
@ -712,7 +713,7 @@ exports.create_sub_from_server_data = function (stream_name, attrs) {
delete attrs.subscribers; delete attrs.subscribers;
sub = { sub = {
name: stream_name, name: attrs.name,
render_subscribers: !page_params.realm_is_zephyr_mirror_realm || attrs.invite_only === true, render_subscribers: !page_params.realm_is_zephyr_mirror_realm || attrs.invite_only === true,
subscribed: true, subscribed: true,
newly_subscribed: false, newly_subscribed: false,
@ -916,11 +917,10 @@ exports.initialize = function (params) {
function populate_subscriptions(subs, subscribed, previously_subscribed) { function populate_subscriptions(subs, subscribed, previously_subscribed) {
subs.forEach(function (sub) { subs.forEach(function (sub) {
const stream_name = sub.name;
sub.subscribed = subscribed; sub.subscribed = subscribed;
sub.previously_subscribed = previously_subscribed; sub.previously_subscribed = previously_subscribed;
exports.create_sub_from_server_data(stream_name, sub); exports.create_sub_from_server_data(sub);
}); });
} }