From 7509f73f02d6a777205c531fef4a7a06eac82380 Mon Sep 17 00:00:00 2001
From: Steve Howell <showell30@yahoo.com>
Date: Sun, 30 Oct 2016 09:33:23 -0700
Subject: [PATCH] Clean up stream renaming in the JS code.

We now use stream_id as our key to rename streams, which
should prevent a few race conditions long term.  (We are
still possibly contending with other events that use
stream_name as a key, so this is not perfect.)
---
 frontend_tests/node_tests/stream_data.js | 11 +++++------
 static/js/stream_data.js                 | 10 ++++++++++
 static/js/stream_list.js                 |  4 ++--
 static/js/subs.js                        | 11 ++++-------
 4 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/frontend_tests/node_tests/stream_data.js b/frontend_tests/node_tests/stream_data.js
index 99bccf9094..36f0a1a503 100644
--- a/frontend_tests/node_tests/stream_data.js
+++ b/frontend_tests/node_tests/stream_data.js
@@ -62,12 +62,6 @@ var stream_data = require('js/stream_data.js');
 
     assert(stream_data.in_home_view('social'));
     assert(!stream_data.in_home_view('denmark'));
-
-    // Deleting a subscription makes you unsubscribed from the perspective of
-    // the client.
-    // Deleting a subscription is case-insensitive.
-    stream_data.delete_sub('SOCIAL');
-    assert(!stream_data.is_subscribed('social'));
 }());
 
 (function test_get_by_id() {
@@ -84,6 +78,11 @@ var stream_data = require('js/stream_data.js');
     assert.equal(sub.color, 'red');
     sub = stream_data.get_sub_by_id(id);
     assert.equal(sub.color, 'red');
+
+    stream_data.rename_sub(id, 'Sweden');
+    sub = stream_data.get_sub_by_id(id);
+    assert.equal(sub.color, 'red');
+    assert.equal(sub.name, 'Sweden');
 }());
 
 (function test_subscribers() {
diff --git a/static/js/stream_data.js b/static/js/stream_data.js
index 63a49cb6d1..99756bca3a 100644
--- a/static/js/stream_data.js
+++ b/static/js/stream_data.js
@@ -20,6 +20,16 @@ exports.is_active = function (stream_name) {
     return recent_topics.has(stream_name);
 };
 
+exports.rename_sub = function (stream_id, new_name) {
+    var sub = subs_by_stream_id.get(stream_id);
+    var old_name = sub.name;
+    sub.name = new_name;
+    stream_info.del(old_name);
+    stream_info.set(new_name, sub);
+
+    return sub;
+};
+
 exports.add_sub = function (stream_name, sub) {
     if (!_.has(sub, 'subscribers')) {
         sub.subscribers = Dict.from_array([], {fold_case: true});
diff --git a/static/js/stream_list.js b/static/js/stream_list.js
index 30117073c4..a551e627a5 100644
--- a/static/js/stream_list.js
+++ b/static/js/stream_list.js
@@ -524,8 +524,8 @@ exports.update_dom_with_unread_counts = function (counts) {
     animate_mention_changes(counts.mentioned_message_count);
 };
 
-exports.rename_stream = function (sub) {
-    sub.sidebar_li = build_stream_sidebar_row(sub.name);
+exports.rename_stream = function (sub, new_name) {
+    sub.sidebar_li = build_stream_sidebar_row(new_name);
     exports.build_stream_list(); // big hammer
 };
 
diff --git a/static/js/subs.js b/static/js/subs.js
index 383a59ec48..319049d074 100644
--- a/static/js/subs.js
+++ b/static/js/subs.js
@@ -150,15 +150,12 @@ function update_stream_pin(sub, value) {
     sub.pin_to_top = value;
 }
 
-function update_stream_name(sub, new_name) {
+function update_stream_name(stream_id, old_name, new_name) {
     // Rename the stream internally.
-    var old_name = sub.name;
-    stream_data.delete_sub(old_name);
-    sub.name = new_name;
-    stream_data.add_sub(new_name, sub);
+    var sub = stream_data.rename_sub(stream_id, new_name);
 
     // Update the left sidebar.
-    stream_list.rename_stream(sub);
+    stream_list.rename_stream(sub, new_name);
 
     // Update the subscriptions page
     var sub_settings_selector = '.stream-row[data-stream-id=' + sub.stream_id + ']';
@@ -576,7 +573,7 @@ exports.update_subscription_properties = function (stream_name, property, value)
         update_stream_audible_notifications(sub, value);
         break;
     case 'name':
-        update_stream_name(sub, value);
+        update_stream_name(sub.stream_id, sub.name, value);
         break;
     case 'description':
         update_stream_description(sub, value);