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.
This commit is contained in:
sahil839 2020-06-16 22:12:13 +05:30 committed by Tim Abbott
parent 6950d8d769
commit 791e5de5de
9 changed files with 14 additions and 28 deletions

View File

@ -15,8 +15,9 @@ set_global('location', {
zrequire('subs'); zrequire('subs');
set_global('$', global.make_zjquery()); set_global('$', global.make_zjquery());
set_global('hash_util', {
stream_data.update_calculated_fields = () => {}; by_stream_uri: () => {},
});
run_test('filter_table', () => { run_test('filter_table', () => {
const stream_list = $(".streams-list"); const stream_list = $(".streams-list");
@ -39,7 +40,7 @@ run_test('filter_table', () => {
stream_id: 1, stream_id: 1,
description: 'Copenhagen', description: 'Copenhagen',
subscribers: {size: 1}, subscribers: {size: 1},
is_old_stream: false, stream_weekly_traffic: null,
}, },
{ {
elem: 'poland', elem: 'poland',
@ -48,7 +49,6 @@ run_test('filter_table', () => {
stream_id: 2, stream_id: 2,
description: 'monday', description: 'monday',
subscribers: {size: 3}, subscribers: {size: 3},
is_old_stream: true,
stream_weekly_traffic: 13, stream_weekly_traffic: 13,
}, },
{ {
@ -58,7 +58,6 @@ run_test('filter_table', () => {
stream_id: 3, stream_id: 3,
description: 'college', description: 'college',
subscribers: {size: 0}, subscribers: {size: 0},
is_old_stream: true,
stream_weekly_traffic: 0, stream_weekly_traffic: 0,
}, },
{ {
@ -68,7 +67,6 @@ run_test('filter_table', () => {
stream_id: 4, stream_id: 4,
description: 'programming lang', description: 'programming lang',
subscribers: {size: 2}, subscribers: {size: 2},
is_old_stream: true,
stream_weekly_traffic: 6, stream_weekly_traffic: 6,
}, },
{ {
@ -78,7 +76,6 @@ run_test('filter_table', () => {
stream_id: 5, stream_id: 5,
description: 'california town', description: 'california town',
subscribers: {size: 2}, subscribers: {size: 2},
is_old_stream: true,
stream_weekly_traffic: 6, stream_weekly_traffic: 6,
}, },
]; ];

View File

@ -461,6 +461,7 @@ exports.update_calculated_fields = function (sub) {
!sub.invite_only; !sub.invite_only;
sub.preview_url = hash_util.by_stream_uri(sub.stream_id); 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.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) { if (sub.rendered_description !== undefined) {
sub.rendered_description = sub.rendered_description.replace('<p>', '').replace('</p>', ''); sub.rendered_description = sub.rendered_description.replace('<p>', '').replace('</p>', '');
} }

View File

@ -10,6 +10,12 @@ below features are supported.
## Changes in Zulip 2.2 ## 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** **Feature level 13**
* [`POST /register`](/api/register-queue): Added * [`POST /register`](/api/register-queue): Added

View File

@ -29,7 +29,7 @@ DESKTOP_WARNING_VERSION = "5.2.0"
# #
# Changes should be accompanied by documentation explaining what the # Changes should be accompanied by documentation explaining what the
# new level means in templates/zerver/api/changelog.md. # 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 # 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 # only when going from an old version of the code to a newer version. Bump

View File

@ -2687,7 +2687,6 @@ def notify_subscriptions_added(user_profile: UserProfile,
sub_dict['in_home_view'] = not subscription.is_muted sub_dict['in_home_view'] = not subscription.is_muted
sub_dict['email_address'] = encode_email_address(stream, show_sender=True) 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( sub_dict['stream_weekly_traffic'] = get_average_weekly_stream_traffic(
stream.id, stream.date_created, recent_traffic) stream.id, stream.date_created, recent_traffic)
sub_dict['subscribers'] = stream_user_ids(stream) 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) 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]]] SubHelperT = Tuple[List[Dict[str, Any]], List[Dict[str, Any]], List[Dict[str, Any]]]
def get_web_public_subs(realm: Realm) -> SubHelperT: 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['push_notifications'] = True
stream_dict['email_notifications'] = True stream_dict['email_notifications'] = True
stream_dict['pin_to_top'] = False 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_weekly_traffic = get_average_weekly_stream_traffic(stream.id,
stream.date_created, stream.date_created,
{}) {})
@ -4745,7 +4739,7 @@ def gather_subscriptions_helper(user_profile: UserProfile,
all_streams = get_active_streams(user_profile.realm).select_related( all_streams = get_active_streams(user_profile.realm).select_related(
"realm").values( "realm").values(
*Stream.API_FIELDS, *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", "date_created",
# The realm_id and recipient_id are generally not needed in the API. # The realm_id and recipient_id are generally not needed in the API.
"realm_id", "realm_id",
@ -4817,7 +4811,6 @@ def gather_subscriptions_helper(user_profile: UserProfile,
stream['stream_post_policy'] == Stream.STREAM_POST_POLICY_ADMINS stream['stream_post_policy'] == Stream.STREAM_POST_POLICY_ADMINS
# Add a few computed fields not directly from the data models. # 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_dict['stream_weekly_traffic'] = get_average_weekly_stream_traffic(
stream["id"], stream["date_created"], recent_traffic) stream["id"], stream["date_created"], recent_traffic)
stream_dict['email_address'] = encode_email_address_helper( stream_dict['email_address'] = encode_email_address_helper(
@ -4860,7 +4853,6 @@ def gather_subscriptions_helper(user_profile: UserProfile,
continue continue
stream_dict[field_name] = stream[field_name] 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_dict['stream_weekly_traffic'] = get_average_weekly_stream_traffic(
stream["id"], stream["date_created"], recent_traffic) stream["id"], stream["date_created"], recent_traffic)
# Backwards-compatibility addition of removed field. # Backwards-compatibility addition of removed field.

View File

@ -522,7 +522,6 @@ def apply_event(state: Dict[str, Any],
if include_subscribers: if include_subscribers:
stream_data['subscribers'] = [] stream_data['subscribers'] = []
stream_data['stream_weekly_traffic'] = None stream_data['stream_weekly_traffic'] = None
stream_data['is_old_stream'] = False
stream_data['stream_post_policy'] = Stream.STREAM_POST_POLICY_EVERYONE stream_data['stream_post_policy'] = Stream.STREAM_POST_POLICY_EVERYONE
# Add stream to never_subscribed (if not invite_only) # Add stream to never_subscribed (if not invite_only)
state['never_subscribed'].append(stream_data) state['never_subscribed'].append(stream_data)

View File

@ -1501,7 +1501,7 @@ class Stream(models.Model):
# * "deactivated" streams are filtered from the API entirely. # * "deactivated" streams are filtered from the API entirely.
# * "realm" and "recipient" are not exposed to clients via the API. # * "realm" and "recipient" are not exposed to clients via the API.
# * "date_created" should probably be added here, as it's useful information # * "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 # * message_retention_days should be added here once the feature is
# complete. # complete.
API_FIELDS = [ API_FIELDS = [

View File

@ -2230,14 +2230,6 @@ paths:
Legacy property for if the given stream is muted, with inverted meeting. Legacy property for if the given stream is muted, with inverted meeting.
**Deprecated**; clients should use is_muted where available. **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: is_announcement_only:
type: boolean type: boolean
description: | description: |

View File

@ -2578,7 +2578,6 @@ class EventsRegisterTest(ZulipTestCase):
('history_public_to_subscribers', check_bool), ('history_public_to_subscribers', check_bool),
('pin_to_top', check_bool), ('pin_to_top', check_bool),
('stream_weekly_traffic', check_none_or(check_int)), ('stream_weekly_traffic', check_none_or(check_int)),
('is_old_stream', check_bool),
('wildcard_mentions_notify', check_none_or(check_bool)), ('wildcard_mentions_notify', check_none_or(check_bool)),
] ]
if include_subscribers: if include_subscribers: