compose_state: Rename compost_state.recipient to be about PMs only.

The compose_state.recipient field was only actually the recipient for
the message if it was a private_message_recipient (in the sense of
other code); we store the stream in compose_state.stream instead.

As a result, the name was quite confusing, resulting in the
possibility of problematic correctness bugs where code assumes this
field has a valid value for stream messages.  Fix this by changing it
to compose_state.private_message_recipient for clarity.
This commit is contained in:
Tim Abbott
2019-12-02 08:53:55 -08:00
parent 65270419b9
commit ea7c6d395f
13 changed files with 34 additions and 33 deletions

View File

@@ -212,7 +212,7 @@ exports.pm_recipient = {
expect: function (expected_value) {
var displayed_recipients = casper.evaluate(function () {
return compose_state.recipient();
return compose_state.private_message_recipient();
});
casper.test.assertEquals(displayed_recipients, expected_value);
},

View File

@@ -332,7 +332,7 @@ reset_setup();
run_test('presence_list_full_update', () => {
$('.user-list-filter').focus();
compose_state.recipient = () => fred.email;
compose_state.private_message_recipient = () => fred.email;
compose_fade.set_focused_recipient("private");
let user_ids = activity.build_user_sidebar();

View File

@@ -223,25 +223,25 @@ run_test('validate', () => {
// test validating private messages
compose_state.set_message_type('private');
compose_state.recipient('');
compose_state.private_message_recipient('');
assert(!compose.validate());
assert.equal($('#compose-error-msg').html(), i18n.t('Please specify at least one valid recipient'));
initialize_pm_pill();
add_content_to_compose_box();
compose_state.recipient('foo@zulip.com');
compose_state.private_message_recipient('foo@zulip.com');
assert(!compose.validate());
assert.equal($('#compose-error-msg').html(), i18n.t('Please specify at least one valid recipient', {}));
compose_state.recipient('foo@zulip.com,alice@zulip.com');
compose_state.private_message_recipient('foo@zulip.com,alice@zulip.com');
assert(!compose.validate());
assert.equal($('#compose-error-msg').html(), i18n.t('Please specify at least one valid recipient', {}));
people.add_in_realm(bob);
compose_state.recipient('bob@example.com');
compose_state.private_message_recipient('bob@example.com');
assert(compose.validate());
page_params.realm_is_zephyr_mirror_realm = true;
@@ -271,7 +271,7 @@ run_test('get_invalid_recipient_emails', () => {
page_params.cross_realm_bots = [feedback_bot];
page_params.user_id = 30;
people.initialize();
compose_state.recipient('feedback@example.com');
compose_state.private_message_recipient('feedback@example.com');
assert.deepEqual(compose.get_invalid_recipient_emails(), []);
});
@@ -588,7 +588,7 @@ run_test('send_message', () => {
compose_state.topic('');
compose_state.set_message_type('private');
page_params.user_id = 101;
compose_state.recipient = function () {
compose_state.private_message_recipient = function () {
return 'alice@example.com';
};
@@ -779,7 +779,7 @@ run_test('finish', () => {
$("#markdown_preview").hide();
$("#compose-textarea").val('foobarfoobar');
compose_state.set_message_type('private');
compose_state.recipient = function () {
compose_state.private_message_recipient = function () {
return 'bob@example.com';
};
@@ -1666,7 +1666,7 @@ run_test('create_message_object', () => {
global.compose_state.get_message_type = function () {
return 'private';
};
compose_state.recipient = function () {
compose_state.private_message_recipient = function () {
return 'alice@example.com, bob@example.com';
};

View File

@@ -34,7 +34,7 @@ const quote_and_reply = compose_actions.quote_and_reply;
const compose_state = global.compose_state;
compose_state.recipient = (function () {
compose_state.private_message_recipient = (function () {
let recipient;
return function (arg) {
@@ -201,7 +201,7 @@ run_test('start', () => {
assert_hidden('#stream-message');
assert_visible('#private-message');
assert.equal(compose_state.recipient(), 'foo@example.com');
assert.equal(compose_state.private_message_recipient(), 'foo@example.com');
assert.equal($('#compose-textarea').val(), 'hello');
assert.equal(compose_state.get_message_type(), 'private');
assert(compose_state.composing());
@@ -241,7 +241,7 @@ run_test('respond_to_message', () => {
};
respond_to_message(opts);
assert.equal(compose_state.recipient(), 'alice@example.com');
assert.equal(compose_state.private_message_recipient(), 'alice@example.com');
// Test stream
msg = {

View File

@@ -163,7 +163,7 @@ run_test('snapshot_message', () => {
global.compose_state.message_content = function () {
return draft.content;
};
global.compose_state.recipient = function () {
global.compose_state.private_message_recipient = function () {
return draft.private_message_recipient;
};
global.compose_state.stream_name = function () {

View File

@@ -318,7 +318,7 @@ run_test('narrow_to_compose_target', () => {
people.add_in_realm(me);
// Test with valid person
global.compose_state.recipient = () => 'alice@example.com';
global.compose_state.private_message_recipient = () => 'alice@example.com';
args.called = false;
narrow.to_compose_target();
assert.equal(args.called, true);
@@ -327,7 +327,7 @@ run_test('narrow_to_compose_target', () => {
]);
// Test with valid persons
global.compose_state.recipient = () => 'alice@example.com,ray@example.com';
global.compose_state.private_message_recipient = () => 'alice@example.com,ray@example.com';
args.called = false;
narrow.to_compose_target();
assert.equal(args.called, true);
@@ -336,7 +336,7 @@ run_test('narrow_to_compose_target', () => {
]);
// Test with some inavlid persons
global.compose_state.recipient = () => 'alice@example.com,random,ray@example.com';
global.compose_state.private_message_recipient = () => 'alice@example.com,random,ray@example.com';
args.called = false;
narrow.to_compose_target();
assert.equal(args.called, true);
@@ -345,7 +345,7 @@ run_test('narrow_to_compose_target', () => {
]);
// Test with all inavlid persons
global.compose_state.recipient = () => 'alice,random,ray';
global.compose_state.private_message_recipient = () => 'alice,random,ray';
args.called = false;
narrow.to_compose_target();
assert.equal(args.called, true);
@@ -354,7 +354,7 @@ run_test('narrow_to_compose_target', () => {
]);
// Test with no persons
global.compose_state.recipient = () => '';
global.compose_state.private_message_recipient = () => '';
args.called = false;
narrow.to_compose_target();
assert.equal(args.called, true);

View File

@@ -176,7 +176,7 @@ function create_message_object() {
if (message.type === "private") {
// TODO: this should be collapsed with the code in composebox_typeahead.js
const recipient = compose_state.recipient();
const recipient = compose_state.private_message_recipient();
const emails = util.extract_pm_recipients(recipient);
message.to = emails;
message.reply_to = recipient;
@@ -372,7 +372,7 @@ exports.do_post_send_tasks = function () {
};
exports.update_email = function (user_id, new_email) {
let reply_to = compose_state.recipient();
let reply_to = compose_state.private_message_recipient();
if (!reply_to) {
return;
@@ -380,11 +380,12 @@ exports.update_email = function (user_id, new_email) {
reply_to = people.update_email_in_reply_to(reply_to, user_id, new_email);
compose_state.recipient(reply_to);
compose_state.private_message_recipient(reply_to);
};
exports.get_invalid_recipient_emails = function () {
const private_recipients = util.extract_pm_recipients(compose_state.recipient());
const private_recipients = util.extract_pm_recipients(
compose_state.private_message_recipient());
const invalid_recipients = _.reject(private_recipients, people.is_valid_email_for_compose);
return invalid_recipients;
@@ -561,7 +562,7 @@ function validate_stream_message() {
// The function checks whether the recipients are users of the realm or cross realm users (bots
// for now)
function validate_private_message() {
if (compose_state.recipient().length === 0) {
if (compose_state.private_message_recipient().length === 0) {
compose_error(i18n.t("Please specify at least one valid recipient"), $("#private_message_recipient"));
return false;
} else if (page_params.realm_is_zephyr_mirror_realm) {

View File

@@ -190,7 +190,7 @@ function same_recipient_as_before(msg_type, opts) {
opts.stream === compose_state.stream_name() &&
opts.topic === compose_state.topic() ||
msg_type === "private" &&
opts.private_message_recipient === compose_state.recipient());
opts.private_message_recipient === compose_state.private_message_recipient());
}
exports.update_placeholder_text = function (opts) {
@@ -231,7 +231,7 @@ exports.start = function (msg_type, opts) {
compose_state.topic(opts.topic);
// Set the recipients with a space after each comma, so it looks nice.
compose_state.recipient(opts.private_message_recipient.replace(/,\s*/g, ", "));
compose_state.private_message_recipient(opts.private_message_recipient.replace(/,\s*/g, ", "));
// If the user opens the compose box, types some text, and then clicks on a
// different stream/topic, we want to keep the text in the compose box

View File

@@ -27,7 +27,7 @@ exports.set_focused_recipient = function (msg_type) {
} else {
// Normalize the recipient list so it matches the one used when
// adding the message (see message_store.add_message_metadata()).
const reply_to = util.normalize_recipients(compose_state.recipient());
const reply_to = util.normalize_recipients(compose_state.private_message_recipient());
focused_recipient.reply_to = reply_to;
focused_recipient.to_user_ids = people.reply_to_to_user_ids_string(reply_to);
}
@@ -73,7 +73,7 @@ function fade_messages() {
if (current_msg_list !== expected_msg_list ||
!compose_state.composing() ||
compose_state.recipient() !== expected_recipient) {
compose_state.private_message_recipient() !== expected_recipient) {
return;
}
@@ -88,7 +88,7 @@ function fade_messages() {
}
floating_recipient_bar.update();
}, 0, current_msg_list, compose_state.recipient());
}, 0, current_msg_list, compose_state.private_message_recipient());
}
exports.would_receive_message = function (email) {

View File

@@ -41,7 +41,7 @@ exports.topic = get_or_set('stream_message_recipient_topic');
// We can't trim leading whitespace in `compose_textarea` because
// of the indented syntax for multi-line code blocks.
exports.message_content = get_or_set('compose-textarea', true);
exports.recipient = function (value) {
exports.private_message_recipient = function (value) {
if (typeof value === "string") {
compose_pm_pill.set_from_emails(value);
} else {

View File

@@ -74,7 +74,7 @@ exports.snapshot_message = function () {
content: compose_state.message_content(),
};
if (message.type === "private") {
const recipient = compose_state.recipient();
const recipient = compose_state.private_message_recipient();
message.reply_to = recipient;
message.private_message_recipient = recipient;
} else {

View File

@@ -709,7 +709,7 @@ exports.to_compose_target = function () {
}
if (compose_state.get_message_type() === 'private') {
const recipient_string = compose_state.recipient();
const recipient_string = compose_state.private_message_recipient();
const emails = util.extract_pm_recipients(recipient_string);
const invalid = _.reject(emails, people.is_valid_email_for_compose);
// If there are no recipients or any recipient is

View File

@@ -27,7 +27,7 @@ function preserve_state(send_after_reload, save_pointer, save_narrow, save_compo
url += "+topic=" + encodeURIComponent(compose_state.topic());
} else if (msg_type === 'private') {
url += "+msg_type=private";
url += "+recipient=" + encodeURIComponent(compose_state.recipient());
url += "+recipient=" + encodeURIComponent(compose_state.private_message_recipient());
}
if (msg_type) {