From c22a1d1f234c3e56f12bc93b979db12b084428d1 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Tue, 24 Apr 2018 23:05:57 +0000 Subject: [PATCH] refactor: Simplify return values for would_receive_message(). Instead of treating false differently from undefined, our function is now a regular boolean function, and we limit our code comments to the one corner case where the true/false decision is kind of arbitrary and possibly confusing. --- frontend_tests/node_tests/compose_fade.js | 8 ++++---- static/js/compose_fade.js | 22 ++++++---------------- 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/frontend_tests/node_tests/compose_fade.js b/frontend_tests/node_tests/compose_fade.js index d233d684d5..755f947193 100644 --- a/frontend_tests/node_tests/compose_fade.js +++ b/frontend_tests/node_tests/compose_fade.js @@ -60,10 +60,10 @@ people.add_in_realm(bob); compose_fade.set_focused_recipient('stream'); - assert(compose_fade.would_receive_message('me@example.com')); - assert(compose_fade.would_receive_message('alice@example.com')); - assert(!compose_fade.would_receive_message('bob@example.com')); - assert.equal(compose_fade.would_receive_message('nonrealmuser@example.com'), undefined); + assert.equal(compose_fade.would_receive_message('me@example.com'), true); + assert.equal(compose_fade.would_receive_message('alice@example.com'), true); + assert.equal(compose_fade.would_receive_message('bob@example.com'), false); + assert.equal(compose_fade.would_receive_message('nonrealmuser@example.com'), true); var good_msg = { type: 'stream', diff --git a/static/js/compose_fade.js b/static/js/compose_fade.js index 038364982a..f32d7416f7 100644 --- a/static/js/compose_fade.js +++ b/static/js/compose_fade.js @@ -96,22 +96,14 @@ function fade_messages() { } exports.would_receive_message = function (email) { - // Given the current focused_recipient, this function returns true if - // the user in question would definitely receive this message, false if - // they would definitely not receive this message, and undefined if we - // don't know (e.g. the recipient is a stream we're not subscribed to). - // - // The distinction between undefined and true is historical. We really - // only ever fade stuff if would_receive_message() returns false; i.e. - // we are **sure** that you would **not** receive the message. - if (focused_recipient.type === 'stream') { var user = people.get_active_user_for_email(email); var sub = stream_data.get_sub(focused_recipient.stream); if (!sub || !user) { // If the stream or user isn't valid, there is no risk of a mix - // yet, so don't fade. - return; + // yet, so we sort of "lie" and say they would receive a + // message. + return true; } return stream_data.is_user_subscribed(focused_recipient.stream, user.user_id); @@ -138,12 +130,10 @@ function update_user_row_when_fading(li, conf) { var email = people.get_person_from_user_id(user_id).email; var would_receive = exports.would_receive_message(email); - if (would_receive === false) { - conf.fade(li); - } else { - // would_receive is either true (so definitely don't fade) or - // undefined (in which case we don't presume to fade) + if (would_receive) { conf.unfade(li); + } else { + conf.fade(li); } }