mirror of https://github.com/zulip/zulip.git
compose_validate: Use correct stream_id value for different context.
This commit addresses the issue of relying on `compose_state` for retrieving the `stream_id` to display warning banners. Previously, warnings were shown for syntax in the message edit box based on whether that syntax would trigger a warning for the draft content (if any) currently in the compose box. We fix this by using a new `get_stream_id_for_textarea` function to obtain the correct `stream_id` value for the check being done. Fixes: #25410.
This commit is contained in:
parent
807b9ae081
commit
3e2d5b3c86
|
@ -59,7 +59,26 @@ export function needs_subscribe_warning(user_id, stream_id) {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
function get_stream_id() {
|
function get_stream_id_for_textarea($textarea) {
|
||||||
|
// Returns the stream ID, if any, associated with the textarea:
|
||||||
|
// The recipient of a message being edited, or the target
|
||||||
|
// recipient of a message being drafted in the compose box.
|
||||||
|
// Returns undefined if the appropriate context is a direct
|
||||||
|
// message conversation.
|
||||||
|
const is_in_editing_area = $textarea.closest(".message_row").length > 0;
|
||||||
|
|
||||||
|
if (is_in_editing_area) {
|
||||||
|
const stream_id_str = $textarea
|
||||||
|
.closest(".recipient_row")
|
||||||
|
.find(".message_header")
|
||||||
|
.attr("data-stream-id");
|
||||||
|
if (stream_id_str === undefined) {
|
||||||
|
// Direct messages don't have a data-stream-id.
|
||||||
|
return undefined;
|
||||||
|
}
|
||||||
|
return Number.parseInt(stream_id_str, 10);
|
||||||
|
}
|
||||||
|
|
||||||
const stream_name = compose_state.stream_name();
|
const stream_name = compose_state.stream_name();
|
||||||
|
|
||||||
if (!stream_name) {
|
if (!stream_name) {
|
||||||
|
@ -70,17 +89,19 @@ function get_stream_id() {
|
||||||
}
|
}
|
||||||
|
|
||||||
export function warn_if_private_stream_is_linked(linked_stream, $textarea) {
|
export function warn_if_private_stream_is_linked(linked_stream, $textarea) {
|
||||||
// For PMs, we currently don't warn about links to private
|
const stream_id = get_stream_id_for_textarea($textarea);
|
||||||
// streams, since you are specifically sharing the existence of
|
|
||||||
// the private stream with someone. One could imagine changing
|
|
||||||
// this policy if user feedback suggested it was useful.
|
|
||||||
if (compose_state.get_message_type() !== "stream") {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
const stream_id = get_stream_id();
|
|
||||||
if (!stream_id) {
|
if (!stream_id) {
|
||||||
// We have an invalid stream name, don't warn about this here as
|
// There are two cases in which the `stream_id` will be
|
||||||
// we show an error to the user when they try to send the message.
|
// omitted, and we want to exclude the warning banner:
|
||||||
|
//
|
||||||
|
// 1. We currently do not warn about links to private streams
|
||||||
|
// in direct messages; it would probably be an improvement to
|
||||||
|
// do so when one of the recipients is not subscribed.
|
||||||
|
//
|
||||||
|
// 2. If we have an invalid stream name, we do not warn about
|
||||||
|
// it here; we will show an error to the user when they try to
|
||||||
|
// send the message.
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -118,10 +139,6 @@ export function warn_if_private_stream_is_linked(linked_stream, $textarea) {
|
||||||
}
|
}
|
||||||
|
|
||||||
export function warn_if_mentioning_unsubscribed_user(mentioned, $textarea) {
|
export function warn_if_mentioning_unsubscribed_user(mentioned, $textarea) {
|
||||||
if (compose_state.get_message_type() !== "stream") {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
// Disable for Zephyr mirroring realms, since we never have subscriber lists there
|
// Disable for Zephyr mirroring realms, since we never have subscriber lists there
|
||||||
if (page_params.realm_is_zephyr_mirror_realm) {
|
if (page_params.realm_is_zephyr_mirror_realm) {
|
||||||
return;
|
return;
|
||||||
|
@ -133,7 +150,7 @@ export function warn_if_mentioning_unsubscribed_user(mentioned, $textarea) {
|
||||||
return; // don't check if @all/@everyone/@stream
|
return; // don't check if @all/@everyone/@stream
|
||||||
}
|
}
|
||||||
|
|
||||||
const stream_id = get_stream_id();
|
const stream_id = get_stream_id_for_textarea($textarea);
|
||||||
|
|
||||||
if (!stream_id) {
|
if (!stream_id) {
|
||||||
return;
|
return;
|
||||||
|
|
|
@ -66,6 +66,15 @@ function test_ui(label, f) {
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function stub_message_row($textarea) {
|
||||||
|
const $stub = $.create("message_row_stub");
|
||||||
|
$textarea.closest = (selector) => {
|
||||||
|
assert.equal(selector, ".message_row");
|
||||||
|
$stub.length = 0;
|
||||||
|
return $stub;
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
test_ui("validate_stream_message_address_info", ({mock_template}) => {
|
test_ui("validate_stream_message_address_info", ({mock_template}) => {
|
||||||
mock_banners();
|
mock_banners();
|
||||||
const sub = {
|
const sub = {
|
||||||
|
@ -622,6 +631,7 @@ test_ui("needs_subscribe_warning", () => {
|
||||||
|
|
||||||
test_ui("warn_if_private_stream_is_linked", ({mock_template}) => {
|
test_ui("warn_if_private_stream_is_linked", ({mock_template}) => {
|
||||||
const $textarea = $("<textarea>").attr("id", "compose-textarea");
|
const $textarea = $("<textarea>").attr("id", "compose-textarea");
|
||||||
|
stub_message_row($textarea);
|
||||||
const test_sub = {
|
const test_sub = {
|
||||||
name: compose_state.stream_name(),
|
name: compose_state.stream_name(),
|
||||||
stream_id: 99,
|
stream_id: 99,
|
||||||
|
@ -676,6 +686,7 @@ test_ui("warn_if_private_stream_is_linked", ({mock_template}) => {
|
||||||
test_ui("warn_if_mentioning_unsubscribed_user", ({override, override_rewire, mock_template}) => {
|
test_ui("warn_if_mentioning_unsubscribed_user", ({override, override_rewire, mock_template}) => {
|
||||||
override_rewire(compose_recipient, "on_compose_select_recipient_update", () => {});
|
override_rewire(compose_recipient, "on_compose_select_recipient_update", () => {});
|
||||||
const $textarea = $("<textarea>").attr("id", "compose-textarea");
|
const $textarea = $("<textarea>").attr("id", "compose-textarea");
|
||||||
|
stub_message_row($textarea);
|
||||||
compose_state.set_stream_name("");
|
compose_state.set_stream_name("");
|
||||||
override(settings_data, "user_can_subscribe_other_users", () => true);
|
override(settings_data, "user_can_subscribe_other_users", () => true);
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue