From 791e5de5de48c31ebc450e4991a9636cb3ef4f1f Mon Sep 17 00:00:00 2001 From: sahil839 Date: Tue, 16 Jun 2020 22:12:13 +0530 Subject: [PATCH] api: Remove is_old_stream property from the stream objects. This commit removes is_old_stream property from the stream objects returned by the API. This property was unnecessary and is essentially equivalent to 'stream_weekly_traffic != null'. We compute sub.is_old_stream in stream_data.update_calculated_fields in frontend code and it is used to check whether we have a non-null stream_weekly_traffic or not. Fixes #15181. --- frontend_tests/node_tests/subs.js | 11 ++++------- static/js/stream_data.js | 1 + templates/zerver/api/changelog.md | 6 ++++++ version.py | 2 +- zerver/lib/actions.py | 10 +--------- zerver/lib/events.py | 1 - zerver/models.py | 2 +- zerver/openapi/zulip.yaml | 8 -------- zerver/tests/test_events.py | 1 - 9 files changed, 14 insertions(+), 28 deletions(-) diff --git a/frontend_tests/node_tests/subs.js b/frontend_tests/node_tests/subs.js index 2f6ba38613..0a8a1f417f 100644 --- a/frontend_tests/node_tests/subs.js +++ b/frontend_tests/node_tests/subs.js @@ -15,8 +15,9 @@ set_global('location', { zrequire('subs'); set_global('$', global.make_zjquery()); - -stream_data.update_calculated_fields = () => {}; +set_global('hash_util', { + by_stream_uri: () => {}, +}); run_test('filter_table', () => { const stream_list = $(".streams-list"); @@ -39,7 +40,7 @@ run_test('filter_table', () => { stream_id: 1, description: 'Copenhagen', subscribers: {size: 1}, - is_old_stream: false, + stream_weekly_traffic: null, }, { elem: 'poland', @@ -48,7 +49,6 @@ run_test('filter_table', () => { stream_id: 2, description: 'monday', subscribers: {size: 3}, - is_old_stream: true, stream_weekly_traffic: 13, }, { @@ -58,7 +58,6 @@ run_test('filter_table', () => { stream_id: 3, description: 'college', subscribers: {size: 0}, - is_old_stream: true, stream_weekly_traffic: 0, }, { @@ -68,7 +67,6 @@ run_test('filter_table', () => { stream_id: 4, description: 'programming lang', subscribers: {size: 2}, - is_old_stream: true, stream_weekly_traffic: 6, }, { @@ -78,7 +76,6 @@ run_test('filter_table', () => { stream_id: 5, description: 'california town', subscribers: {size: 2}, - is_old_stream: true, stream_weekly_traffic: 6, }, ]; diff --git a/static/js/stream_data.js b/static/js/stream_data.js index 344d7ce122..a652aae483 100644 --- a/static/js/stream_data.js +++ b/static/js/stream_data.js @@ -461,6 +461,7 @@ exports.update_calculated_fields = function (sub) { !sub.invite_only; sub.preview_url = hash_util.by_stream_uri(sub.stream_id); sub.can_add_subscribers = !page_params.is_guest && (!sub.invite_only || sub.subscribed); + sub.is_old_stream = sub.stream_weekly_traffic !== null; if (sub.rendered_description !== undefined) { sub.rendered_description = sub.rendered_description.replace('

', '').replace('

', ''); } diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index d7b227520b..b89628450b 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -10,6 +10,12 @@ below features are supported. ## Changes in Zulip 2.2 +**Feature level 14** + +* [`GET users/me/subscriptions`](/api/get-subscribed-streams): Removed + the `is_old_stream` field from Stream objects. This field was + always equivalent to `stream_weekly_traffic != null` on the same object. + **Feature level 13** * [`POST /register`](/api/register-queue): Added diff --git a/version.py b/version.py index 6172bd686c..37d69c1992 100644 --- a/version.py +++ b/version.py @@ -29,7 +29,7 @@ DESKTOP_WARNING_VERSION = "5.2.0" # # Changes should be accompanied by documentation explaining what the # new level means in templates/zerver/api/changelog.md. -API_FEATURE_LEVEL = 13 +API_FEATURE_LEVEL = 14 # Bump the minor PROVISION_VERSION to indicate that folks should provision # only when going from an old version of the code to a newer version. Bump diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 9afd5720f2..a6b51641dd 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -2687,7 +2687,6 @@ def notify_subscriptions_added(user_profile: UserProfile, sub_dict['in_home_view'] = not subscription.is_muted sub_dict['email_address'] = encode_email_address(stream, show_sender=True) - sub_dict['is_old_stream'] = is_old_stream(stream.date_created) sub_dict['stream_weekly_traffic'] = get_average_weekly_stream_traffic( stream.id, stream.date_created, recent_traffic) sub_dict['subscribers'] = stream_user_ids(stream) @@ -4680,10 +4679,6 @@ def get_average_weekly_stream_traffic(stream_id: int, stream_date_created: datet return round_to_2_significant_digits(average_weekly_traffic) -def is_old_stream(stream_date_created: datetime.datetime) -> bool: - return (timezone_now() - stream_date_created).days \ - >= STREAM_TRAFFIC_CALCULATION_MIN_AGE_DAYS - SubHelperT = Tuple[List[Dict[str, Any]], List[Dict[str, Any]], List[Dict[str, Any]]] def get_web_public_subs(realm: Realm) -> SubHelperT: @@ -4708,7 +4703,6 @@ def get_web_public_subs(realm: Realm) -> SubHelperT: stream_dict['push_notifications'] = True stream_dict['email_notifications'] = True stream_dict['pin_to_top'] = False - stream_dict['is_old_stream'] = is_old_stream(stream.date_created) stream_weekly_traffic = get_average_weekly_stream_traffic(stream.id, stream.date_created, {}) @@ -4745,7 +4739,7 @@ def gather_subscriptions_helper(user_profile: UserProfile, all_streams = get_active_streams(user_profile.realm).select_related( "realm").values( *Stream.API_FIELDS, - # date_created is used as an input for the is_old_stream computed field. + # date_created is used as an input for the stream_weekly_traffic computed field. "date_created", # The realm_id and recipient_id are generally not needed in the API. "realm_id", @@ -4817,7 +4811,6 @@ def gather_subscriptions_helper(user_profile: UserProfile, stream['stream_post_policy'] == Stream.STREAM_POST_POLICY_ADMINS # Add a few computed fields not directly from the data models. - stream_dict['is_old_stream'] = is_old_stream(stream["date_created"]) stream_dict['stream_weekly_traffic'] = get_average_weekly_stream_traffic( stream["id"], stream["date_created"], recent_traffic) stream_dict['email_address'] = encode_email_address_helper( @@ -4860,7 +4853,6 @@ def gather_subscriptions_helper(user_profile: UserProfile, continue stream_dict[field_name] = stream[field_name] - stream_dict['is_old_stream'] = is_old_stream(stream["date_created"]) stream_dict['stream_weekly_traffic'] = get_average_weekly_stream_traffic( stream["id"], stream["date_created"], recent_traffic) # Backwards-compatibility addition of removed field. diff --git a/zerver/lib/events.py b/zerver/lib/events.py index 8b475ae1df..472907cc34 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -522,7 +522,6 @@ def apply_event(state: Dict[str, Any], if include_subscribers: stream_data['subscribers'] = [] stream_data['stream_weekly_traffic'] = None - stream_data['is_old_stream'] = False stream_data['stream_post_policy'] = Stream.STREAM_POST_POLICY_EVERYONE # Add stream to never_subscribed (if not invite_only) state['never_subscribed'].append(stream_data) diff --git a/zerver/models.py b/zerver/models.py index 326d1b01c5..3d0126f56a 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -1501,7 +1501,7 @@ class Stream(models.Model): # * "deactivated" streams are filtered from the API entirely. # * "realm" and "recipient" are not exposed to clients via the API. # * "date_created" should probably be added here, as it's useful information - # to subscribers and is needed to compute is_old_stream. + # to subscribers. # * message_retention_days should be added here once the feature is # complete. API_FIELDS = [ diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 36a2941ae5..e831b00c85 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -2230,14 +2230,6 @@ paths: Legacy property for if the given stream is muted, with inverted meeting. **Deprecated**; clients should use is_muted where available. - is_old_stream: - type: boolean - description: | - Whether the stream is old enough to have data in - `stream_weekly_traffic`. - - **Deprecated**: To me removed. Clients should simply check - whether stream_weekly_traffic is null. is_announcement_only: type: boolean description: | diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 8257696e4b..d805331c9c 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -2578,7 +2578,6 @@ class EventsRegisterTest(ZulipTestCase): ('history_public_to_subscribers', check_bool), ('pin_to_top', check_bool), ('stream_weekly_traffic', check_none_or(check_int)), - ('is_old_stream', check_bool), ('wildcard_mentions_notify', check_none_or(check_bool)), ] if include_subscribers: