Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

compose: Migrate compose_invite_users to use compose_banner. 

This is part several updates for #22524.

Testing note: I removed the tests test_compose_invite_users_clicked
and test_compose_invite_close_clicked, since they heavily relied on
the old way of rendering banners and were too UI-focused (instead of
logic focused) for me to feel like it was worth testing that the
banners removed when clicking the buttons.
This commit is contained in:
evykassirer 2022-08-19 17:51:13 -07:00 committed by Tim Abbott
parent 89230ba554
commit ab5d088188
10 changed files with 81 additions and 220 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

82
frontend_tests/node_tests/compose.js
View File

@ -41,7 +41,6 @@ const resize = mock_esm("../../static/js/resize");
const sent_messages = mock_esm("../../static/js/sent_messages");
const server_events = mock_esm("../../static/js/server_events");
const stream_settings_ui = mock_esm("../../static/js/stream_settings_ui");
const subscriber_api = mock_esm("../../static/js/subscriber_api");
const transmit = mock_esm("../../static/js/transmit");
const upload = mock_esm("../../static/js/upload");
@ -365,7 +364,7 @@ test_ui("finish", ({override, override_rewire, mock_template}) => {
$("#compose-textarea").val("");
const res = compose.finish();
assert.equal(res, false);
assert.ok(!$("#compose_invite_users").visible());
assert.ok(!$("#compose_banners .recipient_not_subscribed").visible());
assert.ok(!$("#compose-send-button .loader").visible());
assert.ok(show_button_spinner_called);
})();
@ -624,85 +623,6 @@ test_ui("on_events", ({override, override_rewire}) => {
assert.ok(!$("#compose-send-status").visible());
})();
(function test_compose_invite_users_clicked() {
const handler = $("#compose_invite_users").get_on_handler("click", ".compose_invite_link");
const subscription = {
stream_id: 102,
name: "test",
subscribed: true,
};
const mentioned = {
full_name: "Foo Barson",
email: "foo@bar.com",
user_id: 34,
};
people.add_active_user(mentioned);
override(subscriber_api, "add_user_ids_to_stream", (user_ids, sub, success) => {
assert.deepEqual(user_ids, [mentioned.user_id]);
assert.equal(sub, subscription);
success(); // This will check success callback path.
});
const helper = setup_parents_and_mock_remove(
"compose_invite_users",
"compose_invite_link",
".compose_invite_user",
);
helper.$container.data = (field) => {
switch (field) {
case "user-id":
return "34";
case "stream-id":
return "102";
/* istanbul ignore next */
default:
throw new Error(`Unknown field ${field}`);
}
};
helper.$target.prop("disabled", false);
// !sub will result in true here and we check the success code path.
stream_data.add_sub(subscription);
$("#stream_message_recipient_stream").val("test");
let all_invite_children_called = false;
$("#compose_invite_users").children = () => {
all_invite_children_called = true;
return [];
};
$("#compose_invite_users").show();
handler(helper.event);
assert.ok(helper.container_was_removed());
assert.ok(!$("#compose_invite_users").visible());
assert.ok(all_invite_children_called);
})();
(function test_compose_invite_close_clicked() {
const handler = $("#compose_invite_users").get_on_handler("click", ".compose_invite_close");
const helper = setup_parents_and_mock_remove(
"compose_invite_users_close",
"compose_invite_close",
".compose_invite_user",
);
let all_invite_children_called = false;
$("#compose_invite_users").children = () => {
all_invite_children_called = true;
return [];
};
$("#compose_invite_users").show();
handler(helper.event);
assert.ok(helper.container_was_removed());
assert.ok(all_invite_children_called);
assert.ok(!$("#compose_invite_users").visible());
})();
(function test_compose_not_subscribed_clicked() {
const handler = $("#compose-send-status").get_on_handler("click", ".sub_unsub_button");
const subscription = {

104
frontend_tests/node_tests/compose_validate.js
View File

@ -401,7 +401,6 @@ test_ui("validate_stream_message", ({override_rewire, mock_template}) => {
}),
);
wildcards_not_allowed_rendered = true;
return "wildcard_warning_stub";
});
override_rewire(compose_validate, "wildcard_mention_allowed", () => false);
assert.ok(!compose_validate.validate());
@ -673,22 +672,30 @@ 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, mock_template}) => {
override(settings_data, "user_can_subscribe_other_users", () => true);
let mentioned_details = {
email: "foo@bar.com",
};
$("#compose_invite_users .compose_invite_user").length = 0;
let new_banner_rendered = false;
mock_template("compose_banner/not_subscribed_warning.hbs", false, (data) => {
assert.equal(data.classname, compose_error.CLASSNAMES.recipient_not_subscribed);
assert.equal(data.user_id, 34);
assert.equal(data.stream_id, 111);
assert.equal(data.name, "Foo Barson");
new_banner_rendered = true;
});
function test_noop_case(is_private, is_zephyr_mirror, is_broadcast) {
new_banner_rendered = false;
const msg_type = is_private ? "private" : "stream";
compose_state.set_message_type(msg_type);
page_params.realm_is_zephyr_mirror_realm = is_zephyr_mirror;
mentioned_details.is_broadcast = is_broadcast;
compose_validate.warn_if_mentioning_unsubscribed_user(mentioned_details);
assert.equal($("#compose_invite_users").visible(), false);
assert.ok(!new_banner_rendered);
}
test_noop_case(true, false, false);
@ -700,9 +707,10 @@ test_ui("warn_if_mentioning_unsubscribed_user", ({override, override_rewire, moc
page_params.realm_is_zephyr_mirror_realm = false;
// Test with empty stream name in compose box. It should return noop.
new_banner_rendered = false;
assert.equal(compose_state.stream_name(), "");
compose_validate.warn_if_mentioning_unsubscribed_user(mentioned_details);
assert.equal($("#compose_invite_users").visible(), false);
assert.ok(!new_banner_rendered);
compose_state.stream_name("random");
const sub = {
@ -711,93 +719,39 @@ test_ui("warn_if_mentioning_unsubscribed_user", ({override, override_rewire, moc
};
// Test with invalid stream in compose box. It should return noop.
new_banner_rendered = false;
compose_validate.warn_if_mentioning_unsubscribed_user(mentioned_details);
assert.equal($("#compose_invite_users").visible(), false);
assert.ok(!new_banner_rendered);
// Test mentioning a user that should gets a warning.
const checks = [
(function () {
let called;
override_rewire(compose_validate, "needs_subscribe_warning", (user_id, stream_id) => {
called = true;
assert.equal(user_id, 34);
assert.equal(stream_id, 111);
return true;
});
return function () {
assert.ok(called);
};
})(),
(function () {
let called;
mock_template("compose_invite_users.hbs", false, (context) => {
called = true;
assert.equal(context.user_id, 34);
assert.equal(context.stream_id, 111);
assert.equal(context.name, "Foo Barson");
return "fake-compose-invite-user-template";
});
return function () {
assert.ok(called);
};
})(),
(function () {
let called;
$("#compose_invite_users").append = (html) => {
called = true;
assert.equal(html, "fake-compose-invite-user-template");
};
return function () {
assert.ok(called);
};
})(),
];
mentioned_details = {
email: "foo@bar.com",
user_id: 34,
full_name: "Foo Barson",
};
people.add_active_user(mentioned_details);
stream_data.add_sub(sub);
new_banner_rendered = false;
$("#compose_banners .recipient_not_subscribed").length = 0;
compose_validate.warn_if_mentioning_unsubscribed_user(mentioned_details);
assert.equal($("#compose_invite_users").visible(), true);
for (const f of checks) {
f();
}
assert.ok(new_banner_rendered);
// Simulate that the row was added to the DOM.
const $warning_row = $("<warning-row-stub>");
let looked_for_existing;
$warning_row.data = (field) => {
switch (field) {
case "user-id":
looked_for_existing = true;
return "34";
/* istanbul ignore next */
case "stream-id":
return "111";
/* istanbul ignore next */
default:
throw new Error(`Unknown field ${field}`);
}
};
const $previous_users = $("#compose_invite_users .compose_invite_user");
$previous_users.length = 1;
$previous_users[0] = $warning_row;
$("#compose_invite_users").hide();
const $warning_row = $("#compose_banners .recipient_not_subscribed");
$warning_row.data = (key) =>
({
"user-id": "34",
"stream-id": "111",
}[key]);
$("#compose_banners .recipient_not_subscribed").length = 1;
$("#compose_banners .recipient_not_subscribed")[0] = $warning_row;
// Now try to mention the same person again. The template should
// not render.
new_banner_rendered = false;
compose_validate.warn_if_mentioning_unsubscribed_user(mentioned_details);
assert.equal($("#compose_invite_users").visible(), true);
assert.ok(looked_for_existing);
assert.ok(!new_banner_rendered);
});
test_ui("test warn_if_topic_resolved", ({override, mock_template}) => {

29
static/js/compose.js
View File

@ -74,8 +74,7 @@ export function update_video_chat_button_display() {
}
export function clear_invites() {
$("#compose_invite_users").hide();
$("#compose_invite_users").empty();
$(`#compose_banners .${compose_error.CLASSNAMES.recipient_not_subscribed}`).remove();
}
export function clear_private_stream_alert() {
@ -495,21 +494,19 @@ export function initialize() {
},
);
$("#compose_invite_users").on("click", ".compose_invite_link", (event) => {
$("#compose_banners").on(
"click",
`.${compose_error.CLASSNAMES.recipient_not_subscribed} .compose_banner_action_button`,
(event) => {
event.preventDefault();
const $invite_row = $(event.target).parents(".compose_invite_user");
const $invite_row = $(event.target).parents(".compose_banner");
const user_id = Number.parseInt($invite_row.data("user-id"), 10);
const stream_id = Number.parseInt($invite_row.data("stream-id"), 10);
function success() {
const $all_invites = $("#compose_invite_users");
$invite_row.remove();
if ($all_invites.children().length === 0) {
$all_invites.hide();
}
}
function failure(error_msg) {
@ -530,18 +527,8 @@ export function initialize() {
const sub = sub_store.get(stream_id);
subscriber_api.add_user_ids_to_stream([user_id], sub, success, xhr_failure);
});
$("#compose_invite_users").on("click", ".compose_invite_close", (event) => {
const $invite_row = $(event.target).parents(".compose_invite_user");
const $all_invites = $("#compose_invite_users");
$invite_row.remove();
if ($all_invites.children().length === 0) {
$all_invites.hide();
}
});
},
);
$("#compose_private_stream_alert").on(
"click",

1
static/js/compose_error.ts
View File

@ -12,6 +12,7 @@ export const ERROR = "error";
export const CLASSNAMES = {
// warnings
topic_resolved: "topic_resolved",
recipient_not_subscribed: "recipient_not_subscribed",
// errors
empty_message: "empty_message",
wildcards_not_allowed: "wildcards_not_allowed",

22
static/js/compose_validate.js
View File

@ -3,7 +3,7 @@ import $ from "jquery";
import * as resolved_topic from "../shared/js/resolved_topic";
import render_compose_all_everyone from "../templates/compose_all_everyone.hbs";
import render_compose_banner from "../templates/compose_banner/compose_banner.hbs";
import render_compose_invite_users from "../templates/compose_invite_users.hbs";
import render_not_subscribed_warning from "../templates/compose_banner/not_subscribed_warning.hbs";
import render_compose_not_subscribed from "../templates/compose_not_subscribed.hbs";
import render_compose_private_stream_alert from "../templates/compose_private_stream_alert.hbs";
@ -140,26 +140,32 @@ export function warn_if_mentioning_unsubscribed_user(mentioned) {
}
if (needs_subscribe_warning(user_id, sub.stream_id)) {
const $error_area = $("#compose_invite_users");
const $existing_invites_area = $("#compose_invite_users .compose_invite_user");
const $existing_invites_area = $(
`#compose_banners .${compose_error.CLASSNAMES.recipient_not_subscribed}`,
);
const existing_invites = Array.from($existing_invites_area, (user_row) =>
Number.parseInt($(user_row).data("user-id"), 10),
);
const can_subscribe_other_users = settings_data.user_can_subscribe_other_users();
if (!existing_invites.includes(user_id)) {
const context = {
user_id,
stream_id: sub.stream_id,
banner_type: compose_error.WARNING,
button_text: can_subscribe_other_users
? $t({defaultMessage: "Subscribe them"})
: null,
can_subscribe_other_users,
name: mentioned.full_name,
can_subscribe_other_users: settings_data.user_can_subscribe_other_users(),
classname: compose_error.CLASSNAMES.recipient_not_subscribed,
};
const new_row = render_compose_invite_users(context);
$error_area.append(new_row);
const new_row = render_not_subscribed_warning(context);
$("#compose_banners").append(new_row);
}
$error_area.show();
}
}

4
static/styles/compose.css
View File

@ -381,7 +381,6 @@
}
}
.compose_invite_user,
.compose_private_stream_alert,
.compose-all-everyone,
.compose_not_subscribed {
@ -397,7 +396,6 @@
position: absolute;
}
.compose_invite_user_controls .compose_invite_close,
.compose_private_stream_alert_controls .compose_private_stream_alert_close {
display: inline-block;
position: absolute;
@ -407,13 +405,11 @@
}
.compose-all-everyone-controls,
.compose_invite_user_controls,
.compose_private_stream_alert_controls {
float: right;
position: relative;
}
.compose_invite_user p,
.compose_not_subscribed p {
margin: 5px 20px;
display: inline-block;

1
static/templates/compose.hbs
View File

@ -51,7 +51,6 @@
<span class="compose-send-status-close">&times;</span>
<span id="compose-error-msg"></span>
</div>
<div id="compose_invite_users" class="alert home-error-bar"></div>
<div id="compose-all-everyone" class="alert home-error-bar"></div>
<div id="compose_not_subscribed" class="alert home-error-bar"></div>
<div id="compose_private_stream_alert" class="alert home-error-bar"></div>

1
static/templates/compose_banner/compose_banner.hbs
View File

@ -1,5 +1,6 @@
<div
class="compose_banner {{banner_type}} {{classname}}"
{{#if user_id}}data-user-id="{{user_id}}"{{/if}}
{{#if stream_id}}data-stream-id="{{stream_id}}"{{/if}}
{{#if topic_name}}data-topic-name="{{topic_name}}"{{/if}}>
{{#if banner_text}}

7
static/templates/compose_banner/not_subscribed_warning.hbs Normal file
View File

@ -0,0 +1,7 @@
{{#> compose_banner }}
{{#if can_subscribe_other_users}}
<p>{{#tr}}<strong>{name}</strong> is not subscribed to this stream. They will not be notified unless you subscribe them.{{/tr}}</p>
{{else}}
<p>{{#tr}}<strong>{name}</strong> is not subscribed to this stream. They will not be notified if you mention them.{{/tr}}</p>
{{/if}}
{{/compose_banner}}

10
static/templates/compose_invite_users.hbs
View File

@ -1,10 +0,0 @@
<div class="compose_invite_user" data-user-id="{{user_id}}" data-stream-id="{{stream_id}}">
{{#if can_subscribe_other_users}}
<p>{{#tr}}<strong>{name}</strong> is not subscribed to this stream. They will not be notified unless you subscribe them.{{/tr}}</p>
<div class="compose_invite_user_controls">
<button class="btn btn-warning compose_invite_link" >{{t "Subscribe them" }}</button><button type="button" class="compose_invite_close close">&times;</button>
</div>
{{else}}
<p>{{#tr}}<strong>{name}</strong> is not subscribed to this stream. They will not be notified if you mention them.{{/tr}}</p>
{{/if}}
</div>