[WIP] messages: Add a prompt for muted streams and topics. #2721

trueskawka · 2016-12-15T19:57:14Z

Purpose

This PR creates a prompt that appears when a user tries to send a message to a stream or topic they've muted and asks them if they want to unmute it (yes/no) - in response to #2083.

Description

This is mostly based on existing functionality for a similar prompt for @all/@everyone mentions. I've considered rewriting them into a single function, but decided against it, since we have a util.is_all_or_everyone_mentioned and we're using it both when the @mention is completed and when the user sends a message - with muted streams/topics I'm not sure we want to check them as soon as the user unfocuses the field (but we can also rewrite it to support this).

The main differences are thus:

the stream/topic muted status is checked when the user tries to send a message,
there is no additional error prompt, since the util.is_all_or_everyone_mentioned additional prompt works with checking the status when the @mention is completed,
agreeing to this prompt changes muted/unmuted status of a stream/topic.

WIP

@timabbott @showell

Is this the implementation we're looking for in terms of usability and consistency?
Node tests are failing, since I was unable to gracefully import js/subs and js/muting (now I've realized I probably should use var muting = require('js/muting.js');). Is this the right place to test this feature or should I move it to node_tests/muting.js? I guess we could argue both approaches, as this is a feature both for composing messages and muting.
We currently don't have Casper tests for muting features and I was wondering if it should be a part of this PR, or should we think about creating a whole group of Casper tests for muting.
This PR requires refactoring and refining.

Create a prompt that asks user if they want to unmute a stream or topic they want to post a message to. Fixes zulip#2083.

showell · 2016-12-16T16:59:23Z

For node testing, I would mock out the 'subs.js' calls by doing something like this:

set_global('subs', {});

And then in specific tests you can do something like this:

var toggle_arg;
subs.toggle_home = function (arg) {toggle_arg = arg;};
do_stuff();
var expected_arg = <whatever>;
assert.equal(toggle_args, expected_args);

showell · 2016-12-16T17:04:16Z

Regarding Casper tests, obviously they would be nice to have, but I think they could be done as a follow-up issue. We do want to add as much node testing as possible here, so I would focus on structuring the code so that it's easy to test some of the individual moving parts with node tests. I've only kind of skimmed the code so far, but it seems reasonably well structured, but perhaps as you go deeper on the node tests, you'll find areas where you may want to move responsibilities to node-friendly modules.

showell · 2016-12-16T17:06:45Z

static/js/compose.js

+        if (user_acknowledged_muted_context === undefined ||
+            user_acknowledged_muted_context === false) {
+            unmute_context = [ "stream", stream_name ];
+            // this is only checked on send, whereas @all/@everyone mention is checked


Is this comment a copy/paste error?

timabbott · 2017-02-12T07:51:38Z

@trueskawka this PR seems to have gotten lost in the chaos of GCI; want to rebase it and address Steve's comments so we can merge it?

zulipbot · 2017-07-31T18:07:15Z

Hello @trueskawka, a Zulip maintainer reviewed this pull request over a week ago, but you haven't updated your pull request since. Please take a look at the requested changes and update your pull request accordingly.