From e8afaa8b8e1eab64ab8e83613a02d0f4381c84b9 Mon Sep 17 00:00:00 2001 From: Luke Faraone Date: Tue, 29 Jan 2013 15:40:16 -0500 Subject: [PATCH] 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 | 9 +++++---- zephyr/static/js/subs.js | 6 +++--- zephyr/tests.py | 26 ++++++++++++++------------ zephyr/views.py | 2 +- 4 files changed, 23 insertions(+), 20 deletions(-) diff --git a/zephyr/lib/actions.py b/zephyr/lib/actions.py index 3a1daff51c..b597314453 100644 --- a/zephyr/lib/actions.py +++ b/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) - for sc in with_color] - result.extend((get_display_recipient(sub.recipient), StreamColor.DEFAULT_STREAM_COLOR) - for sub in no_color) + result = [{'name': get_display_recipient(sc.subscription.recipient), + 'color': sc.color} for sc in with_color] + result.extend({'name': get_display_recipient(sub.recipient), + 'color': StreamColor.DEFAULT_STREAM_COLOR} for sub in no_color) + return sorted(result) diff --git a/zephyr/static/js/subs.js b/zephyr/static/js/subs.js index 94ec49cc4d..a573e700c9 100644 --- a/zephyr/static/js/subs.js +++ b/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 sub = create_sub(stream_name, {color: elem[1], subscribed: true}); + var stream_name = elem.name; + var sub = create_sub(stream_name, {color: elem.color, subscribed: true}); stream_info[stream_name.toLowerCase()] = sub; sub_rows.push(sub); }); diff --git a/zephyr/tests.py b/zephyr/tests.py index d966145ad2..1a26a1c972 100644 --- a/zephyr/tests.py +++ b/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) - subs.remove((stream, color)) + self.assertIn({'name': stream, 'color': color}, subs) + 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)) - new_subs.remove((stream_name, new_color)) + old_subs.remove({'name': stream_name, 'color': old_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"]: - self.assertIsInstance(stream, str) - self.assertIsInstance(color, str) + for stream in json["subscriptions"]: + self.assertIsInstance(stream['name'], 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): diff --git a/zephyr/views.py b/zephyr/views.py index 59d8ea24f2..4296712948 100644 --- a/zephyr/views.py +++ b/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")