From 025b8c19aeb6608bb2998ab36818b8bc1ba0fab5 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Mon, 27 Nov 2017 09:13:57 -0800 Subject: [PATCH] Simplify code to warn about private stream links. This change does a few things: * I use "early return" to make the code a bit flatter and easier to comment. * I added more comments. * I removed some unneeded passing of `invite_only` into the template. --- frontend_tests/node_tests/compose.js | 1 - frontend_tests/node_tests/templates.js | 1 - static/js/compose.js | 34 +++++++++++++++++--------- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/frontend_tests/node_tests/compose.js b/frontend_tests/node_tests/compose.js index be1f729d6d..8a2ad8d691 100644 --- a/frontend_tests/node_tests/compose.js +++ b/frontend_tests/node_tests/compose.js @@ -1089,7 +1089,6 @@ function test_with_mock_socket(test_params) { templates.render = function (template_name, context) { called = true; assert.equal(template_name, 'compose_private_stream_alert'); - assert.equal(context.invite_only, true); assert.equal(context.stream_name, 'Denmark'); return 'fake-compose_private_stream_alert-template'; }; diff --git a/frontend_tests/node_tests/templates.js b/frontend_tests/node_tests/templates.js index d4887757a2..ab9b71d186 100644 --- a/frontend_tests/node_tests/templates.js +++ b/frontend_tests/node_tests/templates.js @@ -521,7 +521,6 @@ function render(template_name, args) { (function compose_private_stream_alert() { var args = { stream_name: 'Denmark', - invite_only: true, }; var html = render('compose_private_stream_alert', args); assert($(html).hasClass('compose_private_stream_alert')); diff --git a/static/js/compose.js b/static/js/compose.js index 9fb305c45f..20721fd04c 100644 --- a/static/js/compose.js +++ b/static/js/compose.js @@ -640,24 +640,34 @@ exports.initialize = function () { // Show a warning if a private stream is linked $(document).on('streamname_completed.zulip', function (event, data) { + // For PMs, we don't warn about links to private streams, since + // you are often specifically encouraging somebody to subscribe + // to the stream over PMs. if (compose_state.get_message_type() !== 'stream') { return; } - if (data !== undefined && data.stream !== undefined) { - var invite_only = data.stream.invite_only; - var stream_name = data.stream.name; - - if (invite_only) { - var warning_area = $("#compose_private_stream_alert"); - var context = { stream_name: stream_name, invite_only: invite_only }; - var new_row = templates.render("compose_private_stream_alert", context); - - warning_area.append(new_row); - warning_area.show(); - } + if (data === undefined || data.stream === undefined) { + blueslip.error('Invalid options passed into handler.'); + return; } + // data.stream refers to the stream we're linking to in + // typeahead. If it's not invite-only, then it's public, and + // there is no need to warn about it, since all users can already + // see all the public streams. + if (!data.stream.invite_only) { + return; + } + + var stream_name = data.stream.name; + + var warning_area = $("#compose_private_stream_alert"); + var context = { stream_name: stream_name }; + var new_row = templates.render("compose_private_stream_alert", context); + + warning_area.append(new_row); + warning_area.show(); }); $("#compose_private_stream_alert").on('click', '.compose_private_stream_alert_close', function (event) {