paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-20 06:28:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

compose: Add data-stream-id field in compose_invite_users.

Browse Source 
This commits add data-stream-id attribute to the compose_invite_users
template. This helps in avoiding the error that occured if user
clicked the link after renaming of stream.

As a result of above changes, the checks for empty and invalid stream
name in compose box are done in warn_if_mentioning_unsubscribed_user
function instead of needs_subscribe_warning function.
This commit is contained in:
sahil839
2020-06-23 01:06:21 +05:30
committed by Tim Abbott
parent f8d1e0f86a
commit c4c1790b00
3 changed files with 53 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

54
frontend_tests/node_tests/compose.js
View File

@@ -1071,17 +1071,6 @@ run_test('needs_subscribe_warning', () => {
assert.equal(compose.needs_subscribe_warning(), false);
compose_state.stream_name('random');
assert.equal(compose.needs_subscribe_warning(), false);
const sub = {
stream_id: 111,
name: 'random',
subscribed: true,
};
stream_data.add_sub(sub);
assert.equal(compose.needs_subscribe_warning(), false);
people.get_by_user_id = function () {
return {
is_bot: true,
@@ -1125,12 +1114,27 @@ run_test('warn_if_mentioning_unsubscribed_user', () => {
test_noop_case(false, true, false);
test_noop_case(false, false, true);
// Test mentioning a user that should gets a warning.
$("#compose_invite_users").hide();
compose_state.set_message_type('stream');
page_params.realm_is_zephyr_mirror_realm = false;
// Test with empty stream name in compose box. It should return noop.
assert.equal(compose_state.stream_name(), "");
compose.warn_if_mentioning_unsubscribed_user(mentioned);
assert.equal($('#compose_invite_users').visible(), false);
compose_state.stream_name('random');
const sub = {
stream_id: 111,
name: 'random',
};
// Test with invalid stream in compose box. It should return noop.
compose.warn_if_mentioning_unsubscribed_user(mentioned);
assert.equal($('#compose_invite_users').visible(), false);
// Test mentioning a user that should gets a warning.
const checks = [
(function () {
let called;
@@ -1148,6 +1152,7 @@ run_test('warn_if_mentioning_unsubscribed_user', () => {
called = true;
assert.equal(template_name, 'compose_invite_users');
assert.equal(context.user_id, 34);
assert.equal(context.stream_id, 111);
assert.equal(context.name, 'Foo Barson');
return 'fake-compose-invite-user-template';
});
@@ -1170,6 +1175,7 @@ run_test('warn_if_mentioning_unsubscribed_user', () => {
full_name: 'Foo Barson',
};
stream_data.add_sub(sub);
compose.warn_if_mentioning_unsubscribed_user(mentioned);
assert.equal($('#compose_invite_users').visible(), true);
@@ -1180,9 +1186,13 @@ run_test('warn_if_mentioning_unsubscribed_user', () => {
let looked_for_existing;
warning_row.data = function (field) {
assert.equal(field, 'user-id');
if (field === 'user-id') {
looked_for_existing = true;
return '34';
}
if (field === 'stream-id') {
return '111';
}
};
const previous_users = $('#compose_invite_users .compose_invite_user');
@@ -1280,23 +1290,17 @@ run_test('on_events', () => {
'.compose_invite_user',
);
// !sub will result false here and we check the failure code path.
blueslip.expect('warn', 'Stream no longer exists: no-stream');
$('#stream_message_recipient_stream').val('no-stream');
helper.container.data = function (field) {
assert.equal(field, 'user-id');
if (field === 'user-id') {
return '34';
}
if (field === 'stream-id') {
return '102';
}
};
$("#compose-textarea").select(noop);
helper.target.prop('disabled', false);
handler(helper.event);
assert(helper.target.attr('disabled'));
assert(!invite_user_to_stream_called);
assert(!helper.container_was_removed());
assert(!$("#compose_invite_users").visible());
assert.equal($('#compose-error-msg').html(), "Stream no longer exists: no-stream");
// !sub will result in true here and we check the success code path.
stream_data.add_sub(subscription);
$('#stream_message_recipient_stream').val('test');

43
static/js/compose.js
View File

@@ -740,10 +740,9 @@ exports.handle_keyup = function (event, textarea) {
rtl.set_rtl_class_for_textarea(textarea);
};
exports.needs_subscribe_warning = function (user_id) {
exports.needs_subscribe_warning = function (user_id, stream_name) {
// This returns true if all of these conditions are met:
// * the user is valid
// * the stream in the compose box is valid
// * the user is not already subscribed to the stream
// * the user has no back-door way to see stream messages
// (i.e. bots on public/private streams)
@@ -753,18 +752,11 @@ exports.needs_subscribe_warning = function (user_id) {
// need it?".
//
// We expect the caller to already have verified that we're
// sending to a stream and trying to mention the user.
// sending to a valid stream and trying to mention the user.
const user = people.get_by_user_id(user_id);
const stream_name = compose_state.stream_name();
if (!stream_name) {
return false;
}
const sub = stream_data.get_sub(stream_name);
if (!sub || !user) {
if (!user) {
return false;
}
@@ -912,7 +904,19 @@ exports.warn_if_mentioning_unsubscribed_user = function (mentioned) {
return; // don't check if @all/@everyone/@stream
}
if (exports.needs_subscribe_warning(user_id)) {
const stream_name = compose_state.stream_name();
if (!stream_name) {
return;
}
const sub = stream_data.get_sub(stream_name);
if (!sub) {
return;
}
if (exports.needs_subscribe_warning(user_id, sub.name)) {
const error_area = $("#compose_invite_users");
const existing_invites_area = $('#compose_invite_users .compose_invite_user');
@@ -921,9 +925,11 @@ exports.warn_if_mentioning_unsubscribed_user = function (mentioned) {
if (!existing_invites.includes(user_id)) {
const context = {
user_id: user_id,
stream_id: sub.stream_id,
name: mentioned.full_name,
can_subscribe_other_users: page_params.can_subscribe_other_users,
};
const new_row = render_compose_invite_users(context);
error_area.append(new_row);
}
@@ -994,6 +1000,7 @@ exports.initialize = function () {
const invite_row = $(event.target).parents('.compose_invite_user');
const user_id = parseInt($(invite_row).data('user-id'), 10);
const stream_id = parseInt($(invite_row).data('stream-id'), 10);
function success() {
const all_invites = $("#compose_invite_users");
@@ -1015,17 +1022,7 @@ exports.initialize = function () {
failure(error.msg);
}
const stream_name = compose_state.stream_name();
const sub = stream_data.get_sub(stream_name);
if (!sub) {
// This should only happen if a stream rename occurs
// before the user clicks. We could prevent this by
// putting a stream id in the link.
const error_msg = 'Stream no longer exists: ' + stream_name;
blueslip.warn(error_msg);
failure(error_msg);
return;
}
const sub = stream_data.get_sub_by_id(stream_id);
stream_edit.invite_user_to_stream([user_id], sub, success, xhr_failure);
});

2
static/templates/compose_invite_users.hbs
View File

@@ -1,4 +1,4 @@
<div class="compose_invite_user" data-user-id="{{user_id}}">
<div class="compose_invite_user" data-user-id="{{user_id}}" data-stream-id="{{stream_id}}">
{{#if can_subscribe_other_users}}
<p>{{#tr this}}<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">