compose_state: Break out stream_name() into separate getter and setter.

This code is equivalent, because the keep_leading_whitespace parameter
of get_or_set was never used for the stream name.

This addresses an open TODO and makes the code more readable.
This commit is contained in:
evykassirer 2022-10-26 15:22:29 -07:00 committed by Tim Abbott
parent 6200f0f734
commit 7f9989003a
8 changed files with 32 additions and 23 deletions

View File

@ -305,7 +305,7 @@ test_ui("enter_with_preview_open", ({override, override_rewire}) => {
// Test sending a message with content.
compose_state.set_message_type("stream");
compose_state.stream_name("social");
compose_state.set_stream_name("social");
$("#compose-textarea").val("message me");
$("#compose-textarea").hide();
@ -400,7 +400,7 @@ test_ui("finish", ({override, override_rewire, mock_template}) => {
$("#compose .markdown_preview").hide();
$("#compose-textarea").val("foobarfoobar");
compose_state.set_message_type("stream");
compose_state.stream_name("social");
compose_state.set_stream_name("social");
override_rewire(people, "get_by_user_id", () => []);
compose_finished_event_checked = false;
let schedule_message = false;

View File

@ -30,14 +30,14 @@ run_test("has_full_recipient", ({override}) => {
override(compose_pm_pill, "get_emails", () => emails);
compose_state.set_message_type("stream");
compose_state.stream_name("");
compose_state.set_stream_name("");
compose_state.topic("");
assert.equal(compose_state.has_full_recipient(), false);
compose_state.topic("foo");
assert.equal(compose_state.has_full_recipient(), false);
compose_state.stream_name("bar");
compose_state.set_stream_name("bar");
assert.equal(compose_state.has_full_recipient(), true);
compose_state.set_message_type("private");

View File

@ -254,7 +254,7 @@ test_ui("validate", ({override, mock_template}) => {
// test validating stream messages
compose_state.set_message_type("stream");
compose_state.stream_name("");
compose_state.set_stream_name("");
let empty_stream_error_rendered = false;
mock_template("compose_banner/compose_banner.hbs", false, (data) => {
assert.equal(data.classname, compose_banner.CLASSNAMES.missing_stream);
@ -264,7 +264,7 @@ test_ui("validate", ({override, mock_template}) => {
assert.ok(!compose_validate.validate());
assert.ok(empty_stream_error_rendered);
compose_state.stream_name("Denmark");
compose_state.set_stream_name("Denmark");
page_params.realm_mandatory_topics = true;
compose_state.topic("");
let missing_topic_error_rendered = false;
@ -372,7 +372,7 @@ test_ui("validate_stream_message", ({override_rewire, mock_template}) => {
subscribed: true,
};
stream_data.add_sub(sub);
compose_state.stream_name("social");
compose_state.set_stream_name("social");
assert.ok(compose_validate.validate());
assert.ok(!$("#compose-all-everyone").visible());
assert.ok(!$("#compose-send-status").visible());
@ -427,7 +427,7 @@ test_ui("test_validate_stream_message_post_policy_admin_only", ({mock_template})
};
compose_state.topic("topic102");
compose_state.stream_name("stream102");
compose_state.set_stream_name("stream102");
stream_data.add_sub(sub);
let banner_rendered = false;
@ -445,13 +445,13 @@ test_ui("test_validate_stream_message_post_policy_admin_only", ({mock_template})
assert.ok(banner_rendered);
// Reset error message.
compose_state.stream_name("social");
compose_state.set_stream_name("social");
page_params.is_admin = false;
page_params.is_guest = true;
compose_state.topic("topic102");
compose_state.stream_name("stream102");
compose_state.set_stream_name("stream102");
banner_rendered = false;
assert.ok(!compose_validate.validate());
assert.ok(banner_rendered);
@ -471,7 +471,7 @@ test_ui("test_validate_stream_message_post_policy_moderators_only", ({mock_templ
};
compose_state.topic("topic104");
compose_state.stream_name("stream104");
compose_state.set_stream_name("stream104");
stream_data.add_sub(sub);
let banner_rendered = false;
mock_template("compose_banner/compose_banner.hbs", false, (data) => {
@ -487,7 +487,7 @@ test_ui("test_validate_stream_message_post_policy_moderators_only", ({mock_templ
assert.ok(!compose_validate.validate());
assert.ok(banner_rendered);
// Reset error message.
compose_state.stream_name("social");
compose_state.set_stream_name("social");
page_params.is_guest = true;
assert.ok(!compose_validate.validate());
@ -506,7 +506,7 @@ test_ui("test_validate_stream_message_post_policy_full_members_only", ({mock_tem
};
compose_state.topic("topic103");
compose_state.stream_name("stream103");
compose_state.set_stream_name("stream103");
stream_data.add_sub(sub);
let banner_rendered = false;
mock_template("compose_banner/compose_banner.hbs", false, (data) => {
@ -697,7 +697,7 @@ test_ui("warn_if_mentioning_unsubscribed_user", ({override, mock_template}) => {
compose_validate.warn_if_mentioning_unsubscribed_user(mentioned_details);
assert.ok(!new_banner_rendered);
compose_state.stream_name("random");
compose_state.set_stream_name("random");
const sub = {
stream_id: 111,
name: "random",
@ -765,7 +765,7 @@ test_ui("test warn_if_topic_resolved", ({override, mock_template}) => {
stream_data.add_sub(sub);
compose_state.set_message_type("stream");
compose_state.stream_name("Do not exist");
compose_state.set_stream_name("Do not exist");
compose_state.topic(resolved_topic.resolve_name("hello"));
compose_state.message_content("content");
@ -773,7 +773,7 @@ test_ui("test warn_if_topic_resolved", ({override, mock_template}) => {
compose_validate.warn_if_topic_resolved(true);
assert.ok(!error_shown);
compose_state.stream_name("random");
compose_state.set_stream_name("random");
// Show the warning now as stream also exists
error_shown = false;

View File

@ -163,7 +163,7 @@ test("snapshot_message", ({override_rewire}) => {
function set_compose_state() {
compose_state.set_message_type(curr_draft.type);
compose_state.message_content(curr_draft.content);
compose_state.stream_name(curr_draft.stream);
compose_state.set_stream_name(curr_draft.stream);
compose_state.topic(curr_draft.topic);
compose_state.private_message_recipient(curr_draft.private_message_recipient);
}

View File

@ -712,7 +712,7 @@ run_test("narrow_to_compose_target errors", ({disallow_rewire}) => {
// No-op when empty stream.
compose_state.set_message_type("stream");
compose_state.stream_name("");
compose_state.set_stream_name("");
narrow.to_compose_target();
});
@ -726,7 +726,7 @@ run_test("narrow_to_compose_target streams", ({override_rewire}) => {
compose_state.set_message_type("stream");
stream_data.add_sub({name: "ROME", stream_id: 99});
compose_state.stream_name("ROME");
compose_state.set_stream_name("ROME");
// Test with existing topic
compose_state.topic("one");

View File

@ -329,7 +329,7 @@ export function start(msg_type, opts) {
//
// TODO: Move these into a conditional on message_type, using an
// explicit "clear" function for compose_state.
compose_state.stream_name(opts.stream);
compose_state.set_stream_name(opts.stream);
compose_state.topic(opts.topic);
// Set the recipients with a space after each comma, so it looks nice.

View File

@ -36,9 +36,18 @@ function get_or_set(fieldname, keep_leading_whitespace) {
};
}
// TODO: Break out setters and getter into their own functions.
export const stream_name = get_or_set("stream_message_recipient_stream");
export function stream_name() {
return $("#stream_message_recipient_stream").val().trim();
}
export function set_stream_name(newval) {
if (newval !== undefined) {
const $elem = $("#stream_message_recipient_stream");
$elem.val(newval);
}
}
// TODO: Break out setter and getter into their own functions.
export const topic = get_or_set("stream_message_recipient_topic");
// We can't trim leading whitespace in `compose_textarea` because

View File

@ -184,7 +184,7 @@ export function update_stream_name(sub, new_name) {
// Update compose_state if needed
if (compose_state.stream_name() === old_name) {
compose_state.stream_name(new_name);
compose_state.set_stream_name(new_name);
}
// Update navbar if needed