Skip to content

Commit

Permalink
streams: Render and store the stream description from the backend.
Browse files Browse the repository at this point in the history
This commit does the following three things:
    1. Update stream model to accomodate rendered description.
    2. Render and save the stream rendered description on update.
    3. Render and save stream descriptions on creation.

Further, the stream's rendered description is also sent whenever the
stream's description is being sent.

This is preparatory work for eliminating the use of the
non-authoritative marked.js markdown parser for stream descriptions.
  • Loading branch information
Hypro999 authored and timabbott committed Feb 2, 2019
1 parent 022c8be commit 73d26c8
Show file tree
Hide file tree
Showing 10 changed files with 70 additions and 7 deletions.
1 change: 1 addition & 0 deletions tools/linter_lib/custom_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,7 @@ def build_custom_checkers(by_lang):
'zerver/migrations/0032_verify_all_medium_avatar_images.py',
'zerver/migrations/0060_move_avatars_to_be_uid_based.py',
'zerver/migrations/0104_fix_unreads.py',
'zerver/migrations/0206_stream_rendered_description.py',
'pgroonga/migrations/0002_html_escape_subject.py',
]),
'description': "Don't import models or other code in migrations; see docs/subsystems/schema-migrations.md",
Expand Down
1 change: 1 addition & 0 deletions zerver/data_import/import_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@ def build_stream(date_created: Any, realm_id: int, name: str,
name=name,
deactivated=deactivated,
description=description,
# We don't set rendered_description here; it'll be added on import
date_created=date_created,
invite_only=invite_only,
id=stream_id)
Expand Down
12 changes: 10 additions & 2 deletions zerver/lib/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1732,6 +1732,8 @@ def create_stream_if_needed(realm: Realm,
)

if created:
stream.rendered_description = bugdown_convert(stream.description)
stream.save(update_fields=["rendered_description"])
Recipient.objects.create(type_id=stream.id, type=Recipient.STREAM)
if stream.is_public():
send_stream_creation_event(stream, active_non_guest_user_ids(stream.realm_id))
Expand Down Expand Up @@ -2577,6 +2579,7 @@ def notify_subscriptions_added(user_profile: UserProfile,
push_notifications=subscription.push_notifications,
email_notifications=subscription.email_notifications,
description=stream.description,
rendered_description=stream.rendered_description,
pin_to_top=subscription.pin_to_top,
is_old_stream=is_old_stream(stream.date_created),
stream_weekly_traffic=get_average_weekly_stream_traffic(
Expand Down Expand Up @@ -3415,7 +3418,8 @@ def do_rename_stream(stream: Stream,

def do_change_stream_description(stream: Stream, new_description: str) -> None:
stream.description = new_description
stream.save(update_fields=['description'])
stream.rendered_description = bugdown_convert(new_description)
stream.save(update_fields=['description', 'rendered_description'])

event = dict(
type='stream',
Expand All @@ -3424,6 +3428,7 @@ def do_change_stream_description(stream: Stream, new_description: str) -> None:
name=stream.name,
stream_id=stream.id,
value=new_description,
rendered_description=stream.rendered_description
)
send_event(stream.realm, event, can_access_stream_user_ids(stream))

Expand Down Expand Up @@ -4401,6 +4406,7 @@ def get_next_color() -> str:
'pin_to_top': False,
'stream_id': stream.id,
'description': stream.description,
'rendered_description': stream.rendered_description,
'is_old_stream': is_old_stream(stream.date_created),
'stream_weekly_traffic': get_average_weekly_stream_traffic(stream.id,
stream.date_created,
Expand Down Expand Up @@ -4438,7 +4444,7 @@ def gather_subscriptions_helper(user_profile: UserProfile,

all_streams = get_active_streams(user_profile.realm).select_related(
"realm").values("id", "name", "invite_only", "is_announcement_only", "realm_id",
"email_token", "description", "date_created",
"email_token", "description", "rendered_description", "date_created",
"history_public_to_subscribers")

stream_dicts = [stream for stream in all_streams if stream['id'] in stream_ids]
Expand Down Expand Up @@ -4503,6 +4509,7 @@ def gather_subscriptions_helper(user_profile: UserProfile,
'pin_to_top': sub["pin_to_top"],
'stream_id': stream["id"],
'description': stream["description"],
'rendered_description': stream["rendered_description"],
'is_old_stream': is_old_stream(stream["date_created"]),
'stream_weekly_traffic': get_average_weekly_stream_traffic(stream["id"],
stream["date_created"],
Expand Down Expand Up @@ -4536,6 +4543,7 @@ def gather_subscriptions_helper(user_profile: UserProfile,
stream["date_created"],
recent_traffic),
'description': stream['description'],
'rendered_description': stream["rendered_description"],
'history_public_to_subscribers': stream['history_public_to_subscribers']}
if is_public or user_profile.is_realm_admin:
subscribers = subscriber_map[stream["id"]]
Expand Down
2 changes: 2 additions & 0 deletions zerver/lib/bulk_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from zerver.models import Realm, Stream, UserProfile, Huddle, \
Subscription, Recipient, Client, RealmAuditLog, get_huddle_hash
from zerver.lib.create_user import create_user_profile
from zerver.lib.bugdown import convert as bugdown_convert

def bulk_create_users(realm: Realm,
users_raw: Set[Tuple[str, str, str, bool]],
Expand Down Expand Up @@ -75,6 +76,7 @@ def bulk_create_streams(realm: Realm,
realm=realm,
name=name,
description=options["description"],
rendered_description=bugdown_convert(options["description"]),
invite_only=options.get("invite_only", False),
is_announcement_only=options.get("is_announcement_only", False),
history_public_to_subscribers=options["history_public_to_subscribers"],
Expand Down
4 changes: 4 additions & 0 deletions zerver/lib/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -476,12 +476,16 @@ def apply_event(state: Dict[str, Any],
for obj in state['subscriptions']:
if obj['name'].lower() == event['name'].lower():
obj[event['property']] = event['value']
if event['property'] == "description":
obj['rendered_description'] = event['rendered_description']
# Also update the pure streams data
for stream in state['streams']:
if stream['name'].lower() == event['name'].lower():
prop = event['property']
if prop in stream:
stream[prop] = event['value']
if prop == 'description':
stream['rendered_description'] = event['rendered_description']
elif event['op'] == "occupy":
state['streams'] += event['streams']
elif event['op'] == "vacate":
Expand Down
7 changes: 6 additions & 1 deletion zerver/lib/import_realm.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from zerver.lib.export import DATE_FIELDS, \
Record, TableData, TableName, Field, Path
from zerver.lib.message import do_render_markdown, RealmAlertWords
from zerver.lib.bugdown import version as bugdown_version
from zerver.lib.bugdown import version as bugdown_version, convert as bugdown_convert
from zerver.lib.upload import random_name, sanitize_name, \
guess_type, BadImageError
from zerver.lib.utils import generate_api_key, process_list_in_batches
Expand Down Expand Up @@ -742,6 +742,11 @@ def do_import_realm(import_dir: Path, subdomain: str, processes: int=1) -> Realm
# Stream objects are created by Django.
fix_datetime_fields(data, 'zerver_stream')
re_map_foreign_keys(data, 'zerver_stream', 'realm', related_table="realm")
# Handle rendering of stream descriptions for import from non-Zulip
for stream in data['zerver_stream']:
if 'rendered_description' in stream:
continue
stream["rendered_description"] = bugdown_convert(stream["description"])
bulk_import_model(data, Stream)

realm.notifications_stream_id = notifications_stream_id
Expand Down
35 changes: 35 additions & 0 deletions zerver/migrations/0206_stream_rendered_description.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.db import migrations, models
from django.apps import apps
from django.db.models import F

from django.db.migrations.state import StateApps
from django.db.backends.postgresql_psycopg2.schema import DatabaseSchemaEditor

from zerver.lib.bugdown import convert as bugdown_convert

def render_all_stream_descriptions(apps: StateApps, schema_editor: DatabaseSchemaEditor) -> None:
Stream = apps.get_model('zerver', 'Stream')
all_streams = Stream.objects.exclude(description='')
for stream in all_streams:
stream.rendered_description = bugdown_convert(stream.description)
stream.save(update_fields=["rendered_description"])


class Migration(migrations.Migration):

dependencies = [
('zerver', '0205_remove_realmauditlog_requires_billing_update'),
]

operations = [
migrations.AddField(
model_name='stream',
name='rendered_description',
field=models.TextField(default=''),
),
migrations.RunPython(render_all_stream_descriptions,
reverse_code=migrations.RunPython.noop),
]
2 changes: 2 additions & 0 deletions zerver/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1124,6 +1124,7 @@ class Stream(models.Model):
date_created = models.DateTimeField(default=timezone_now) # type: datetime.datetime
deactivated = models.BooleanField(default=False) # type: bool
description = models.CharField(max_length=MAX_DESCRIPTION_LENGTH, default=u'') # type: str
rendered_description = models.TextField(default=u'') # type: str

invite_only = models.NullBooleanField(default=False) # type: Optional[bool]
history_public_to_subscribers = models.BooleanField(default=False) # type: bool
Expand Down Expand Up @@ -1172,6 +1173,7 @@ def to_dict(self) -> Dict[str, Any]:
name=self.name,
stream_id=self.id,
description=self.description,
rendered_description=self.rendered_description,
invite_only=self.invite_only,
is_announcement_only=self.is_announcement_only,
history_public_to_subscribers=self.history_public_to_subscribers
Expand Down
4 changes: 4 additions & 0 deletions zerver/tests/test_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -1313,6 +1313,7 @@ def test_default_stream_groups_events(self) -> None:
('description', check_string),
('streams', check_list(check_dict_only([
('description', check_string),
('rendered_description', check_string),
('invite_only', check_bool),
('is_announcement_only', check_bool),
('name', check_string),
Expand Down Expand Up @@ -2190,6 +2191,7 @@ def do_test_subscribe_events(self, include_subscribers: bool) -> None:
subscription_fields = [
('color', check_string),
('description', check_string),
('rendered_description', check_string),
('email_address', check_string),
('invite_only', check_bool),
('is_announcement_only', check_bool),
Expand Down Expand Up @@ -2218,6 +2220,7 @@ def do_test_subscribe_events(self, include_subscribers: bool) -> None:
('stream_id', check_int),
('invite_only', check_bool),
('description', check_string),
('rendered_description', check_string),
]))),
])
add_schema_checker = self.check_events_dict([
Expand Down Expand Up @@ -2252,6 +2255,7 @@ def do_test_subscribe_events(self, include_subscribers: bool) -> None:
('op', equals('update')),
('property', equals('description')),
('value', check_string),
('rendered_description', check_string),
('stream_id', check_int),
('name', check_string),
])
Expand Down
9 changes: 5 additions & 4 deletions zerver/tests/test_subs.py
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,7 @@ def test_change_stream_description(self) -> None:
type='stream',
property='description',
value='Test description',
rendered_description='<p>Test description</p>',
stream_id=stream_id,
name='stream_name1'
))
Expand Down Expand Up @@ -2186,7 +2187,7 @@ def test_multi_user_subscription(self) -> None:
streams_to_sub,
dict(principals=ujson.dumps([user1.email, user2.email])),
)
self.assert_length(queries, 43)
self.assert_length(queries, 45)

self.assert_length(events, 7)
for ev in [x for x in events if x['event']['type'] not in ('message', 'stream')]:
Expand Down Expand Up @@ -2947,7 +2948,7 @@ def test_subscriptions_query_count(self) -> None:
[new_streams[0]],
dict(principals=ujson.dumps([user1.email, user2.email])),
)
self.assert_length(queries, 43)
self.assert_length(queries, 45)

# Test creating private stream.
with queries_captured() as queries:
Expand All @@ -2957,7 +2958,7 @@ def test_subscriptions_query_count(self) -> None:
dict(principals=ujson.dumps([user1.email, user2.email])),
invite_only=True,
)
self.assert_length(queries, 38)
self.assert_length(queries, 40)

# Test creating a public stream with announce when realm has a notification stream.
notifications_stream = get_stream(self.streams[0], self.test_realm)
Expand All @@ -2972,7 +2973,7 @@ def test_subscriptions_query_count(self) -> None:
principals=ujson.dumps([user1.email, user2.email])
)
)
self.assert_length(queries, 53)
self.assert_length(queries, 55)

class GetPublicStreamsTest(ZulipTestCase):

Expand Down

0 comments on commit 73d26c8

Please sign in to comment.