From 28b6af5c4b8b60871a9ee34f7c0377d5b6f87ef7 Mon Sep 17 00:00:00 2001
From: sahil839 <sahilbatra839@gmail.com>
Date: Sun, 21 Jun 2020 18:53:43 +0530
Subject: [PATCH] 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.
---
 frontend_tests/node_tests/stream_data.js |  9 +++++----
 static/js/stream_data.js                 | 22 +++++++++++-----------
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/frontend_tests/node_tests/stream_data.js b/frontend_tests/node_tests/stream_data.js
index 3bd203fe14..cae937d05f 100644
--- a/frontend_tests/node_tests/stream_data.js
+++ b/frontend_tests/node_tests/stream_data.js
@@ -864,17 +864,18 @@ run_test('create_sub', () => {
         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.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.throws(() => {
         stream_data.create_sub_from_server_data('Canada', canada);
     }, {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.equal(antarctica_sub.color, '#76ce90');
 });
@@ -1062,7 +1063,7 @@ run_test('all_topics_in_cache', () => {
         {id: 2, 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);
 
diff --git a/static/js/stream_data.js b/static/js/stream_data.js
index 32e5e3541e..6ea06448d7 100644
--- a/static/js/stream_data.js
+++ b/static/js/stream_data.js
@@ -679,27 +679,28 @@ exports.is_user_subscribed = function (stream_name, user_id) {
 exports.create_streams = function (streams) {
     for (const stream of streams) {
         // We handle subscriber stuff in other events.
+
         const attrs = {
             subscribers: [],
             subscribed: false,
             ...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) {
-    let sub = exports.get_sub(stream_name);
-    if (sub !== undefined) {
-        // We've already created this subscription, no need to continue.
-        return sub;
-    }
-
+exports.create_sub_from_server_data = function (attrs) {
     if (!attrs.stream_id) {
         // fail fast (blueslip.fatal will throw an error on our behalf)
         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,
     // 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
@@ -712,7 +713,7 @@ exports.create_sub_from_server_data = function (stream_name, attrs) {
     delete attrs.subscribers;
 
     sub = {
-        name: stream_name,
+        name: attrs.name,
         render_subscribers: !page_params.realm_is_zephyr_mirror_realm || attrs.invite_only === true,
         subscribed: true,
         newly_subscribed: false,
@@ -916,11 +917,10 @@ exports.initialize = function (params) {
 
     function populate_subscriptions(subs, subscribed, previously_subscribed) {
         subs.forEach(function (sub) {
-            const stream_name = sub.name;
             sub.subscribed = subscribed;
             sub.previously_subscribed = previously_subscribed;
 
-            exports.create_sub_from_server_data(stream_name, sub);
+            exports.create_sub_from_server_data(sub);
         });
     }