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

stream_data: Modify is_user_subscribed to use stream id.

Browse Source 
This commit changes stream_data.is_user_subscribed to use stream id
instead of stream name.
We are using stream ids so that we can avoid bugs related to live
update after stream rename.
This commit is contained in:
sahil839
2020-07-09 01:58:35 +05:30
committed by Tim Abbott
parent 525b42cecc
commit a0d2c7db16
7 changed files with 30 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
frontend_tests/node_tests/compose.js
View File

@@ -1138,9 +1138,10 @@ run_test('warn_if_mentioning_unsubscribed_user', () => {
const checks = [ const checks = [
(function () { (function () {
let called; let called;
compose.needs_subscribe_warning = function (user_id) { compose.needs_subscribe_warning = function (user_id, stream_id) {
called = true; called = true;
assert.equal(user_id, 34); assert.equal(user_id, 34);
assert.equal(stream_id, 111);
return true; return true;
}; };
return function () { assert(called); }; return function () { assert(called); };

32
frontend_tests/node_tests/stream_data.js
View File

@@ -206,10 +206,10 @@ run_test('subscribers', () => {
stream_data.set_subscribers(sub, [me.user_id, fred.user_id, george.user_id]); stream_data.set_subscribers(sub, [me.user_id, fred.user_id, george.user_id]);
stream_data.update_calculated_fields(sub); stream_data.update_calculated_fields(sub);
assert(stream_data.is_user_subscribed('Rome', me.user_id)); assert(stream_data.is_user_subscribed(sub.stream_id, me.user_id));
assert(stream_data.is_user_subscribed('Rome', fred.user_id)); assert(stream_data.is_user_subscribed(sub.stream_id, fred.user_id));
assert(stream_data.is_user_subscribed('Rome', george.user_id)); assert(stream_data.is_user_subscribed(sub.stream_id, george.user_id));
assert(!stream_data.is_user_subscribed('Rome', not_fred.user_id)); assert(!stream_data.is_user_subscribed(sub.stream_id, not_fred.user_id));
assert.deepEqual( assert.deepEqual(
potential_subscriber_ids(), potential_subscriber_ids(),
@@ -226,12 +226,12 @@ run_test('subscribers', () => {
user_id: 104, user_id: 104,
}; };
people.add_active_user(brutus); people.add_active_user(brutus);
assert(!stream_data.is_user_subscribed('Rome', brutus.user_id)); assert(!stream_data.is_user_subscribed(sub.stream_id, brutus.user_id));
// add // add
let ok = stream_data.add_subscriber(sub.stream_id, brutus.user_id); let ok = stream_data.add_subscriber(sub.stream_id, brutus.user_id);
assert(ok); assert(ok);
assert(stream_data.is_user_subscribed('Rome', brutus.user_id)); assert(stream_data.is_user_subscribed(sub.stream_id, brutus.user_id));
sub = stream_data.get_sub('Rome'); sub = stream_data.get_sub('Rome');
stream_data.update_subscribers_count(sub); stream_data.update_subscribers_count(sub);
assert.equal(sub.subscriber_count, 1); assert.equal(sub.subscriber_count, 1);
@@ -241,7 +241,7 @@ run_test('subscribers', () => {
// verify that adding an already-added subscriber is a noop // verify that adding an already-added subscriber is a noop
stream_data.add_subscriber(sub.stream_id, brutus.user_id); stream_data.add_subscriber(sub.stream_id, brutus.user_id);
assert(stream_data.is_user_subscribed('Rome', brutus.user_id)); assert(stream_data.is_user_subscribed(sub.stream_id, brutus.user_id));
sub = stream_data.get_sub('Rome'); sub = stream_data.get_sub('Rome');
stream_data.update_subscribers_count(sub); stream_data.update_subscribers_count(sub);
assert.equal(sub.subscriber_count, 1); assert.equal(sub.subscriber_count, 1);
@@ -249,7 +249,7 @@ run_test('subscribers', () => {
// remove // remove
ok = stream_data.remove_subscriber(sub.stream_id, brutus.user_id); ok = stream_data.remove_subscriber(sub.stream_id, brutus.user_id);
assert(ok); assert(ok);
assert(!stream_data.is_user_subscribed('Rome', brutus.user_id)); assert(!stream_data.is_user_subscribed(sub.stream_id, brutus.user_id));
sub = stream_data.get_sub('Rome'); sub = stream_data.get_sub('Rome');
stream_data.update_subscribers_count(sub); stream_data.update_subscribers_count(sub);
assert.equal(sub.subscriber_count, 0); assert.equal(sub.subscriber_count, 0);
@@ -257,7 +257,7 @@ run_test('subscribers', () => {
// verify that checking subscription with undefined user id // verify that checking subscription with undefined user id
blueslip.expect('warn', 'Undefined user_id passed to function is_user_subscribed'); blueslip.expect('warn', 'Undefined user_id passed to function is_user_subscribed');
assert.equal(stream_data.is_user_subscribed('Rome', undefined), undefined); assert.equal(stream_data.is_user_subscribed(sub.stream_id, undefined), undefined);
// Verify noop for bad stream when removing subscriber // Verify noop for bad stream when removing subscriber
const bad_stream_id = 999999; const bad_stream_id = 999999;
@@ -269,7 +269,7 @@ run_test('subscribers', () => {
blueslip.expect('warn', 'We tried to remove invalid subscriber: 104'); blueslip.expect('warn', 'We tried to remove invalid subscriber: 104');
ok = stream_data.remove_subscriber(sub.stream_id, brutus.user_id); ok = stream_data.remove_subscriber(sub.stream_id, brutus.user_id);
assert(!ok); assert(!ok);
assert(!stream_data.is_user_subscribed('Rome', brutus.user_id)); assert(!stream_data.is_user_subscribed(sub.stream_id, brutus.user_id));
sub = stream_data.get_sub('Rome'); sub = stream_data.get_sub('Rome');
stream_data.update_subscribers_count(sub); stream_data.update_subscribers_count(sub);
assert.equal(sub.subscriber_count, 0); assert.equal(sub.subscriber_count, 0);
@@ -280,27 +280,27 @@ run_test('subscribers', () => {
stream_data.add_sub(sub); stream_data.add_sub(sub);
stream_data.add_subscriber(sub.stream_id, brutus.user_id); stream_data.add_subscriber(sub.stream_id, brutus.user_id);
sub.subscribed = true; sub.subscribed = true;
assert(stream_data.is_user_subscribed('Rome', brutus.user_id)); assert(stream_data.is_user_subscribed(sub.stream_id, brutus.user_id));
// Verify that we noop and don't crash when unsubscribed. // Verify that we noop and don't crash when unsubscribed.
sub.subscribed = false; sub.subscribed = false;
stream_data.update_calculated_fields(sub); stream_data.update_calculated_fields(sub);
ok = stream_data.add_subscriber(sub.stream_id, brutus.user_id); ok = stream_data.add_subscriber(sub.stream_id, brutus.user_id);
assert(ok); assert(ok);
assert.equal(stream_data.is_user_subscribed('Rome', brutus.user_id), true); assert.equal(stream_data.is_user_subscribed(sub.stream_id, brutus.user_id), true);
stream_data.remove_subscriber(sub.stream_id, brutus.user_id); stream_data.remove_subscriber(sub.stream_id, brutus.user_id);
assert.equal(stream_data.is_user_subscribed('Rome', brutus.user_id), false); assert.equal(stream_data.is_user_subscribed(sub.stream_id, brutus.user_id), false);
stream_data.add_subscriber(sub.stream_id, brutus.user_id); stream_data.add_subscriber(sub.stream_id, brutus.user_id);
assert.equal(stream_data.is_user_subscribed('Rome', brutus.user_id), true); assert.equal(stream_data.is_user_subscribed(sub.stream_id, brutus.user_id), true);
blueslip.expect( blueslip.expect(
'warn', 'warn',
'We got a is_user_subscribed call for a non-existent or inaccessible stream.', 2); 'We got a is_user_subscribed call for a non-existent or inaccessible stream.', 2);
sub.invite_only = true; sub.invite_only = true;
stream_data.update_calculated_fields(sub); stream_data.update_calculated_fields(sub);
assert.equal(stream_data.is_user_subscribed('Rome', brutus.user_id), undefined); assert.equal(stream_data.is_user_subscribed(sub.stream_id, brutus.user_id), undefined);
stream_data.remove_subscriber(sub.stream_id, brutus.user_id); stream_data.remove_subscriber(sub.stream_id, brutus.user_id);
assert.equal(stream_data.is_user_subscribed('Rome', brutus.user_id), undefined); assert.equal(stream_data.is_user_subscribed(sub.stream_id, brutus.user_id), undefined);
// Verify that we don't crash and return false for a bad stream. // Verify that we don't crash and return false for a bad stream.
blueslip.expect( blueslip.expect(

6
static/js/compose.js
View File

@@ -746,7 +746,7 @@ exports.handle_keyup = function (event, textarea) {
rtl.set_rtl_class_for_textarea(textarea); rtl.set_rtl_class_for_textarea(textarea);
}; };
exports.needs_subscribe_warning = function (user_id, stream_name) { exports.needs_subscribe_warning = function (user_id, stream_id) {
// This returns true if all of these conditions are met: // This returns true if all of these conditions are met:
// * the user is valid // * the user is valid
// * the user is not already subscribed to the stream // * the user is not already subscribed to the stream
@@ -772,7 +772,7 @@ exports.needs_subscribe_warning = function (user_id, stream_name) {
return false; return false;
} }
if (stream_data.is_user_subscribed(stream_name, user_id)) { if (stream_data.is_user_subscribed(stream_id, user_id)) {
// If our user is already subscribed // If our user is already subscribed
return false; return false;
} }
@@ -917,7 +917,7 @@ exports.warn_if_mentioning_unsubscribed_user = function (mentioned) {
return; return;
} }
if (exports.needs_subscribe_warning(user_id, sub.name)) { if (exports.needs_subscribe_warning(user_id, sub.stream_id)) {
const error_area = $("#compose_invite_users"); const error_area = $("#compose_invite_users");
const existing_invites_area = $('#compose_invite_users .compose_invite_user'); const existing_invites_area = $('#compose_invite_users .compose_invite_user');

2
static/js/compose_fade.js
View File

@@ -102,7 +102,7 @@ exports.would_receive_message = function (user_id) {
return true; return true;
} }
return stream_data.is_user_subscribed(focused_recipient.stream, user_id); return stream_data.is_user_subscribed(focused_recipient.stream_id, user_id);
} }
// PM, so check if the given email is in the recipients list. // PM, so check if the given email is in the recipients list.

4
static/js/stream_data.js
View File

@@ -683,8 +683,8 @@ exports.remove_subscriber = function (stream_id, user_id) {
return true; return true;
}; };
exports.is_user_subscribed = function (stream_name, user_id) { exports.is_user_subscribed = function (stream_id, user_id) {
const sub = exports.get_sub(stream_name); const sub = exports.get_sub_by_id(stream_id);
if (typeof sub === 'undefined' || !sub.can_access_subscribers) { if (typeof sub === 'undefined' || !sub.can_access_subscribers) {
// If we don't know about the stream, or we ourselves cannot access subscriber list, // If we don't know about the stream, or we ourselves cannot access subscriber list,
// so we return undefined (treated as falsy if not explicitly handled). // so we return undefined (treated as falsy if not explicitly handled).

2
static/js/stream_events.js
View File

@@ -136,7 +136,7 @@ exports.remove_deactivated_user_from_all_streams = function (user_id) {
const all_subs = stream_data.get_unsorted_subs(); const all_subs = stream_data.get_unsorted_subs();
for (const sub of all_subs) { for (const sub of all_subs) {
if (stream_data.is_user_subscribed(sub.name, user_id)) { if (stream_data.is_user_subscribed(sub.stream_id, user_id)) {
stream_data.remove_subscriber(sub.stream_id, user_id); stream_data.remove_subscriber(sub.stream_id, user_id);
subs.rerender_subscriptions_settings(sub); subs.rerender_subscriptions_settings(sub);
} }

10
static/js/typeahead_helper.js
View File

@@ -191,7 +191,7 @@ exports.compare_people_for_relevance = function (
person_a, person_a,
person_b, person_b,
tertiary_compare, tertiary_compare,
current_stream) { current_stream_id) {
// give preference to "all", "everyone" or "stream" // give preference to "all", "everyone" or "stream"
// We use is_broadcast for a quick check. It will // We use is_broadcast for a quick check. It will
@@ -209,9 +209,9 @@ exports.compare_people_for_relevance = function (
// Now handle actual people users. // Now handle actual people users.
// give preference to subscribed users first // give preference to subscribed users first
if (current_stream !== undefined) { if (current_stream_id !== undefined) {
const a_is_sub = stream_data.is_user_subscribed(current_stream, person_a.user_id); const a_is_sub = stream_data.is_user_subscribed(current_stream_id, person_a.user_id);
const b_is_sub = stream_data.is_user_subscribed(current_stream, person_b.user_id); const b_is_sub = stream_data.is_user_subscribed(current_stream_id, person_b.user_id);
if (a_is_sub && !b_is_sub) { if (a_is_sub && !b_is_sub) {
return -1; return -1;
@@ -257,7 +257,7 @@ exports.sort_people_for_relevance = function (objs, current_stream_name, current
stream_id, stream_id,
current_topic, current_topic,
), ),
current_stream.name, current_stream.stream_id,
)); ));
} }