Return a dictionary in subscriptions/list instead of a tuple.

This will give us flexibility in the future to add new properties to the
list.

In order to support that, we now do a list comprehension rather than just
returning the gather_subscriptions list in get_stream_colors.

(imported from commit a3c0f749a3320f647440f800105942434da08111)

zephyr/lib/actions.py

@@ -373,9 +373,10 @@ def gather_subscriptions(user_profile):
     with_color = StreamColor.objects.filter(subscription__in = subs).select_related()
     no_color   = subs.exclude(id__in = with_color.values('subscription_id')).select_related()
 
-    result = [(get_display_recipient(sc.subscription.recipient), sc.color)
+    result = [{'name': get_display_recipient(sc.subscription.recipient),
-        for sc in with_color]
+               'color': sc.color} for sc in with_color]
-    result.extend((get_display_recipient(sub.recipient), StreamColor.DEFAULT_STREAM_COLOR)
+    result.extend({'name': get_display_recipient(sub.recipient),
-        for sub in no_color)
+                   'color': StreamColor.DEFAULT_STREAM_COLOR} for sub in no_color)
+
 
     return sorted(result)

zephyr/static/js/subs.js

@@ -254,11 +254,11 @@ exports.setup_page = function () {
 
         var sub_rows = [];
         our_subs.sort(function (a, b) {
-            return a[0].localeCompare(b[0]);
+            return a.name.localeCompare(b.name);
         });
         our_subs.forEach(function (elem) {
-            var stream_name = elem[0];
+            var stream_name = elem.name;
-            var sub = create_sub(stream_name, {color: elem[1], subscribed: true});
+            var sub = create_sub(stream_name, {color: elem.color, subscribed: true});
             stream_info[stream_name.toLowerCase()] = sub;
             sub_rows.push(sub);
         });

zephyr/tests.py

@@ -508,8 +508,8 @@ class SubscriptionPropertiesTest(AuthedTestCase):
         for stream, color in json["stream_colors"]:
             self.assertIsInstance(color,  str)
             self.assertIsInstance(stream, str)
-            self.assertIn((stream, color), subs)
+            self.assertIn({'name': stream, 'color': color}, subs)
-            subs.remove((stream, color))
+            subs.remove({'name': stream, 'color': color})
         self.assertFalse(subs)
 
     def test_set_stream_color(self):
@@ -521,7 +521,9 @@ class SubscriptionPropertiesTest(AuthedTestCase):
         self.login(test_email)
 
         old_subs = gather_subscriptions(self.get_user_profile(test_email))
-        stream_name, old_color = old_subs[0]
+        sub = old_subs[0]
+        stream_name = sub['name']
+        old_color = sub['color']
         new_color = "#ffffff" # TODO: ensure that this is different from old_color
         result = self.client.post("/json/subscriptions/property",
                                   {"property": "stream_colors",
@@ -531,10 +533,10 @@ class SubscriptionPropertiesTest(AuthedTestCase):
         self.assert_json_success(result)
 
         new_subs = gather_subscriptions(self.get_user_profile(test_email))
-        self.assertIn((stream_name, new_color), new_subs)
+        self.assertIn({'name': stream_name, 'color': new_color}, new_subs)
 
-        old_subs.remove((stream_name, old_color))
+        old_subs.remove({'name': stream_name, 'color': old_color})
-        new_subs.remove((stream_name, new_color))
+        new_subs.remove({'name': stream_name, 'color': new_color})
         self.assertEqual(old_subs, new_subs)
 
     def test_set_color_missing_stream_name(self):
@@ -618,15 +620,15 @@ class SubscriptionAPITest(AuthedTestCase):
         self.assert_json_success(result)
         json = simplejson.loads(result.content)
         self.assertIn("subscriptions", json)
-        for stream, color in json["subscriptions"]:
+        for stream in json["subscriptions"]:
-            self.assertIsInstance(stream, str)
+            self.assertIsInstance(stream['name'], str)
-            self.assertIsInstance(color, str)
+            self.assertIsInstance(stream['color'], str)
             # check that the stream name corresponds to an actual stream
             try:
-                Stream.objects.get(name__iexact=stream, realm=self.realm)
+                Stream.objects.get(name__iexact=stream['name'], realm=self.realm)
             except Stream.DoesNotExist:
                 self.fail("stream does not exist")
-        list_streams = [stream for stream, color in json["subscriptions"]]
+        list_streams = [stream['name'] for stream in json["subscriptions"]]
         # also check that this matches the list of your subscriptions
         self.assertItemsEqual(list_streams, self.streams)
 
@@ -1450,7 +1452,7 @@ class GetSubscribersTest(AuthedTestCase):
         """
         get_subscribers returns the list of subscribers.
         """
-        stream_name = gather_subscriptions(self.user_profile)[0][0]
+        stream_name = gather_subscriptions(self.user_profile)[0]['name']
         self.make_successful_subscriber_request(stream_name)
 
     def test_nonsubscriber(self):

zephyr/views.py

@@ -1151,7 +1151,7 @@ class SubscriptionProperties(object):
             raise RequestVariableMissingError(property)
 
     def get_stream_colors(self, request, user_profile):
-        return json_success({"stream_colors": gather_subscriptions(user_profile)})
+        return json_success({"stream_colors": [(sub["name"], sub["color"]) for sub in gather_subscriptions(user_profile)]})
 
     def post_stream_colors(self, request, user_profile):
         stream_name = self.request_property(request.POST, "stream_name")