diff --git a/frontend_tests/node_tests/stream_data.js b/frontend_tests/node_tests/stream_data.js index 1338a2f25c439..d13e96f837721 100644 --- a/frontend_tests/node_tests/stream_data.js +++ b/frontend_tests/node_tests/stream_data.js @@ -433,3 +433,26 @@ var people = global.people; assert(!stream_data.get_sub('Canada')); assert(!stream_data.get_sub_by_id(canada.stream_id)); }()); + +(function test_rename_sub() { + stream_data.clear_subscriptions(); + var id = 42; + var sub = { + name: 'Denmark', + subscribed: true, + color: 'red', + stream_id: id, + }; + stream_data.add_sub('Denmark', sub); + stream_data.rename_sub(sub, 'Sweden'); + sub = stream_data.get_sub('Sweden'); + assert.deepEqual(sub.old_names, ['Denmark']); + + var new_name = stream_data.new_name_for_stream('Denmark'); + assert.equal(new_name, 'Sweden'); + + new_name = stream_data.new_name_for_stream('DENMARK'); + assert.equal(new_name, 'Sweden'); +}()); + + diff --git a/static/js/filter.js b/static/js/filter.js index 3ea8a3a8a0e3e..992d6c74c9bba 100644 --- a/static/js/filter.js +++ b/static/js/filter.js @@ -347,17 +347,25 @@ Filter.prototype = { }); }, - filter_with_new_topic: function Filter_filter_with_new_topic(new_topic) { + filter_with_new: function Filter_filter_with_new(field, new_value) { var terms = _.map(this._operators, function (term) { var new_term = _.clone(term); - if (new_term.operator === 'topic' && !new_term.negated) { - new_term.operand = new_topic; + if (new_term.operator === field && !new_term.negated) { + new_term.operand = new_value; } return new_term; }); return new Filter(terms); }, + filter_with_new_topic: function Filter_filter_with_new_topic(new_topic) { + return this.filter_with_new('topic', new_topic); + }, + + filter_with_new_stream: function Filter_filter_with_new_stream(new_stream) { + return this.filter_with_new('stream', new_stream); + }, + has_topic: function Filter_has_topic(stream_name, topic) { return this.has_operand('stream', stream_name) && this.has_operand('topic', topic); }, diff --git a/static/js/hashchange.js b/static/js/hashchange.js index 1bd975cde9e7b..876e1a1133e5e 100644 --- a/static/js/hashchange.js +++ b/static/js/hashchange.js @@ -6,7 +6,7 @@ var exports = {}; var expected_hash; var changing_hash = false; -function set_hash(hash) { +function set_hash(hash, replace_state) { var location = window.location; if (history.pushState) { @@ -24,18 +24,22 @@ function set_hash(hash) { // Build a full URL to not have same origin problems var url = location.protocol + '//' + location.host + pathname + hash; - history.pushState(null, null, url); + if (replace_state) { + history.replaceState(null, null, url); + } else { + history.pushState(null, null, url); + } } else { location.hash = hash; } } -exports.changehash = function (newhash) { +exports.changehash = function (newhash, replace_state) { if (changing_hash) { return; } $(document).trigger($.Event('zuliphashchange.zulip')); - set_hash(newhash); + set_hash(newhash, replace_state); favicon.reset(); }; @@ -61,12 +65,12 @@ exports.operators_to_hash = function (operators) { return hash; }; -exports.save_narrow = function (operators) { +exports.save_narrow = function (operators, replace_state) { if (changing_hash) { return; } var new_hash = exports.operators_to_hash(operators); - exports.changehash(new_hash); + exports.changehash(new_hash, replace_state); }; exports.parse_narrow = function (hash) { diff --git a/static/js/narrow.js b/static/js/narrow.js index f91ea4553c82c..5f957bc76caa2 100644 --- a/static/js/narrow.js +++ b/static/js/narrow.js @@ -48,6 +48,8 @@ exports.narrow_title = "home"; exports.activate = function (raw_operators, opts) { var start_time = new Date(); var was_narrowed_already = narrow_state.active(); + var following_alias = false; + // most users aren't going to send a bunch of a out-of-narrow messages // and expect to visit a list of narrows, so let's get these out of the way. notifications.clear_compose_notifications(); @@ -56,6 +58,19 @@ exports.activate = function (raw_operators, opts) { return exports.deactivate(); } var filter = new Filter(raw_operators); + var old_stream_name = filter.operands("stream")[0]; + if (old_stream_name && !stream_data.get_sub(old_stream_name)) { + var stream_name = stream_data.new_name_for_stream(old_stream_name); + if (stream_name) { + filter = filter.filter_with_new_stream(stream_name); + + // Even if the caller thinks we shouldn't change the hash + // right now, we want to make sure the hash gets updated + // (see call to `hashchange.save_narrow` change below) + following_alias = true; + } + } + var operators = filter.operators(); // Take the most detailed part of the narrow to use as the title. @@ -234,11 +249,18 @@ exports.activate = function (raw_operators, opts) { ui.show_loading_more_messages_indicator(); } - // Put the narrow operators in the URL fragment. - // Disabled when the URL fragment was the source - // of this narrow. + // Put the narrow operators in the URL fragment. Disabled when the URL + // fragment was the initiating source of this narrow. if (opts.change_hash) { hashchange.save_narrow(operators); + } else if (following_alias) { + // We want this to update the hash on page load if someone followed + // a link to a renamed stream. Since hashchange will refuse us right now + // (it's already tracking a page load), we process on a future tick. + // We set the "replace_state" parameter true to avoid polluting history. + setTimeout(function () { + hashchange.save_narrow(operators, true); + }); } // Put the narrow operators in the search bar. diff --git a/static/js/stream_data.js b/static/js/stream_data.js index f2c66922b1cc6..687b4d0dc6ccc 100644 --- a/static/js/stream_data.js +++ b/static/js/stream_data.js @@ -25,6 +25,11 @@ exports.is_active = function (sub) { exports.rename_sub = function (sub, new_name) { var old_name = sub.name; sub.name = new_name; + if (sub.old_names) { + sub.old_names.push(old_name); + } else { + sub.old_names = [old_name]; + } stream_info.del(old_name); stream_info.set(new_name, sub); }; @@ -57,6 +62,22 @@ exports.get_sub = function (stream_name) { return stream_info.get(stream_name); }; +exports.new_name_for_stream = function (old_stream_name) { + old_stream_name = old_stream_name.toLowerCase(); + var new_name = _.chain(stream_info.items()) + .filter(function (si) { + var stream = si[1]; + return stream.old_names && stream.old_names.map(function (name) { + return name.toLowerCase(); + }).indexOf(old_stream_name) !== -1; + }).map(function (si) { + var stream_name = si[0]; + return stream_name; + }).value()[0]; + + return new_name || null; +}; + exports.get_sub_by_id = function (stream_id) { return subs_by_stream_id.get(stream_id); }; diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index fbd0221002133..3c47149c8f111 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -28,7 +28,7 @@ render_markdown, ) from zerver.lib.realm_icon import realm_icon_url -from zerver.models import Realm, RealmEmoji, Stream, UserProfile, UserActivity, RealmDomain, \ +from zerver.models import Realm, RealmEmoji, Stream, StreamAlias, UserProfile, UserActivity, RealmDomain, \ Subscription, Recipient, Message, Attachment, UserMessage, RealmAuditLog, UserHotspot, \ Client, DefaultStream, UserPresence, Referral, PushDeviceToken, MAX_SUBJECT_LENGTH, \ MAX_MESSAGE_LENGTH, get_client, get_stream, get_recipient, get_huddle, \ @@ -1023,6 +1023,7 @@ def create_stream_if_needed(realm, stream_name, invite_only=False, stream_descri 'description': stream_description, 'invite_only': invite_only}) if created: + StreamAlias.objects.filter(old_name=stream.name).delete() Recipient.objects.create(type_id=stream.id, type=Recipient.STREAM) if not invite_only: event = dict(type="stream", op="create", @@ -2098,6 +2099,8 @@ def do_change_stream_invite_only(stream, invite_only): def do_rename_stream(stream, new_name, log=True): # type: (Stream, Text, bool) -> Dict[str, Text] old_name = stream.name + StreamAlias.objects.filter(old_name=new_name).delete() + StreamAlias(old_name=old_name, stream=stream).save() stream.name = new_name stream.save(update_fields=["name"]) @@ -2923,6 +2926,7 @@ def gather_subscriptions_helper(user_profile, include_subscribers=True): sub_dicts = Subscription.objects.select_related("recipient").filter( user_profile = user_profile, recipient__type = Recipient.STREAM).values( + "id", "recipient__type_id", "in_home_view", "color", "desktop_notifications", "audible_notifications", "active", "pin_to_top") @@ -2931,6 +2935,19 @@ def gather_subscriptions_helper(user_profile, include_subscribers=True): "realm").values("id", "name", "invite_only", "realm_id", "email_token", "description") + renames = StreamAlias.objects.all() + rename_map = {} # type: Dict[str, List[Any]] + for r in renames: + if r.stream.id not in rename_map: + rename_map[r.stream.id] = [] + rename_map[r.stream.id].append(r.old_name) + + for s in all_streams: + if s['id'] in rename_map: + s['old_names'] = rename_map[s['id']] + else: + s['old_names'] = [] + stream_dicts = [stream for stream in all_streams if stream['id'] in stream_ids] stream_hash = {} for stream in stream_dicts: @@ -2981,6 +2998,7 @@ def gather_subscriptions_helper(user_profile, include_subscribers=True): 'pin_to_top': sub["pin_to_top"], 'stream_id': stream["id"], 'description': stream["description"], + 'old_names': stream["old_names"], 'email_address': encode_email_address_helper(stream["name"], stream["email_token"])} if subscribers is not None: stream_dict['subscribers'] = subscribers @@ -3001,6 +3019,7 @@ def gather_subscriptions_helper(user_profile, include_subscribers=True): for stream in never_subscribed_streams: if not stream['invite_only']: stream_dict = {'name': stream['name'], + 'old_names': stream['old_names'], 'invite_only': stream['invite_only'], 'stream_id': stream['id'], 'description': stream['description']} diff --git a/zerver/lib/events.py b/zerver/lib/events.py index 088d1bdb093f6..d71f624154721 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -294,7 +294,10 @@ def our_person(p): # the state var here, for the benefit of the JS code. for obj in state['subscriptions']: if obj['name'].lower() == event['name'].lower(): + if event['property'] == 'name': + obj['old_names'].append(obj['name']) obj[event['property']] = event['value'] + # Also update the pure streams data for stream in state['streams']: if stream['name'].lower() == event['name'].lower(): @@ -346,6 +349,8 @@ def name(sub): if event['op'] == "add": added_names = set(map(name, event["subscriptions"])) + for e in event['subscriptions']: + e['old_names'] = [] was_added = lambda s: name(s) in added_names # add the new subscriptions diff --git a/zerver/migrations/0080_streamalias.py b/zerver/migrations/0080_streamalias.py new file mode 100644 index 0000000000000..190a1f2aebd5b --- /dev/null +++ b/zerver/migrations/0080_streamalias.py @@ -0,0 +1,26 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.10.5 on 2017-05-11 17:04 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion +import zerver.lib.str_utils + + +class Migration(migrations.Migration): + + dependencies = [ + ('zerver', '0079_remove_old_scheduled_jobs'), + ] + + operations = [ + migrations.CreateModel( + name='StreamAlias', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('old_name', models.CharField(max_length=60)), + ('stream', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='zerver.Stream')), + ], + bases=(zerver.lib.str_utils.ModelReprMixin, models.Model), + ), + ] diff --git a/zerver/models.py b/zerver/models.py index 6983c950ab737..2614e6126cbae 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -851,12 +851,18 @@ def to_dict(self): # type: () -> Dict[str, Any] return dict(name=self.name, stream_id=self.id, + old_names=[], description=self.description, invite_only=self.invite_only) post_save.connect(flush_stream, sender=Stream) post_delete.connect(flush_stream, sender=Stream) +class StreamAlias(ModelReprMixin, models.Model): + MAX_NAME_LENGTH = 60 + old_name = models.CharField(max_length=MAX_NAME_LENGTH) # type: Text + stream = models.ForeignKey(Stream) # type: Stream + # The Recipient table is used to map Messages to the set of users who # received the message. It is implemented as a set of triples (id, # type_id, type). We have 3 types of recipients: Huddles (for group diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 15025c7d29def..ea7054fc63ede 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -1488,12 +1488,11 @@ def do_test_subscribe_events(self, include_subscribers): ('desktop_notifications', check_bool), ('audible_notifications', check_bool), ('stream_id', check_int), + ('old_names', check_list(check_string)), ] if include_subscribers: subscription_fields.append(('subscribers', check_list(check_int))) # type: ignore - subscription_schema_checker = check_list( - check_dict(subscription_fields), # TODO: Can this be converted to check_dict_only? - ) + subscription_schema_checker = check_list(check_dict(subscription_fields)) # type: ignore stream_create_schema_checker = self.check_events_dict([ ('type', equals('stream')), ('op', equals('create')), diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 36c000d442342..3917486c87114 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -1609,7 +1609,7 @@ def test_multi_user_subscription(self): streams_to_sub, dict(principals=ujson.dumps([email1, email2])), ) - self.assert_length(queries, 67) + self.assert_length(queries, 68) self.assert_length(events, 9) for ev in [x for x in events if x['event']['type'] not in ('message', 'stream')]: @@ -2375,7 +2375,7 @@ def test_subscriber(self): def test_gather_subscriptions(self): # type: () -> None """ - gather_subscriptions returns correct results with only 3 queries + gather_subscriptions returns correct results with only 5 queries """ streams = ["stream_%s" % i for i in range(10)] for stream_name in streams: @@ -2401,7 +2401,7 @@ def test_gather_subscriptions(self): if not sub["name"].startswith("stream_"): continue self.assertTrue(len(sub["subscribers"]) == len(users_to_subscribe)) - self.assert_length(queries, 4) + self.assert_length(queries, 5) @slow("common_subscribe_to_streams is slow") def test_never_subscribed_streams(self): @@ -2434,7 +2434,7 @@ def test_never_subscribed_streams(self): if stream_dict["name"].startswith("stream_"): self.assertFalse(stream_dict['name'] == "stream_invite_only_1") self.assertTrue(len(stream_dict["subscribers"]) == len(users_to_subscribe)) - self.assert_length(queries, 3) + self.assert_length(queries, 4) @slow("common_subscribe_to_streams is slow") def test_gather_subscriptions_mit(self): @@ -2465,7 +2465,7 @@ def test_gather_subscriptions_mit(self): self.assertTrue(len(sub["subscribers"]) == len(users_to_subscribe)) else: self.assertTrue(len(sub["subscribers"]) == 0) - self.assert_length(queries, 4) + self.assert_length(queries, 5) def test_nonsubscriber(self): # type: () -> None