From f0c99b42ece307d1b8f6b6be7431d1c5621ebf36 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Sat, 21 Mar 2020 19:19:30 +0000 Subject: [PATCH] Rename people.add_in_realm to people.add(). We had this API: people.add_in_realm = full-fledged user people.add = not necessarily in realm Now the API is this: people.add = full-fledged user people._add_user = internal API for cross-realm bots and deactivated users I think in most of our tests the distinction between people.add() and people.add_in_realm() was just an accident of history and didn't reflect any real intention. And if I had to guess the intention in 99% of the cases, folks probably thought they were just creating ordinary, active users in the current realm. In places where the distinction was obviously important (because a test failed), I deactivated the user via `people.deactivate`. For the 'basics' test in the people test suite, I clean up the test setup for Isaac. Before this commit I was adding him first as a non-realm user then as a full-fledged user, but this was contrived and confusing, and we didn't really need it for test coverage purposes. --- frontend_tests/node_tests/activity.js | 14 +++--- frontend_tests/node_tests/buddy_data.js | 12 ++--- frontend_tests/node_tests/compose.js | 2 +- frontend_tests/node_tests/compose_actions.js | 6 +-- frontend_tests/node_tests/compose_fade.js | 6 +-- frontend_tests/node_tests/compose_ui.js | 4 +- .../node_tests/composebox_typeahead.js | 21 ++++----- frontend_tests/node_tests/hash_util.js | 2 +- frontend_tests/node_tests/message_store.js | 10 ++--- frontend_tests/node_tests/narrow.js | 12 ++--- frontend_tests/node_tests/narrow_state.js | 2 +- frontend_tests/node_tests/people.js | 44 +++++++++---------- frontend_tests/node_tests/pm_list.js | 8 ++-- frontend_tests/node_tests/popovers.js | 4 +- frontend_tests/node_tests/presence.js | 10 ++--- frontend_tests/node_tests/reactions.js | 6 +-- frontend_tests/node_tests/stream_data.js | 12 ++--- frontend_tests/node_tests/top_left_corner.js | 4 +- frontend_tests/node_tests/typeahead_helper.js | 2 +- frontend_tests/node_tests/unread.js | 6 +-- frontend_tests/node_tests/user_pill.js | 4 +- static/js/people.js | 19 +++++--- static/js/server_events_dispatch.js | 2 +- 23 files changed, 110 insertions(+), 102 deletions(-) diff --git a/frontend_tests/node_tests/activity.js b/frontend_tests/node_tests/activity.js index 500e635558..63a197cb55 100644 --- a/frontend_tests/node_tests/activity.js +++ b/frontend_tests/node_tests/activity.js @@ -123,13 +123,13 @@ const zoe = { const people = global.people; -people.add_in_realm(alice); -people.add_in_realm(fred); -people.add_in_realm(jill); -people.add_in_realm(mark); -people.add_in_realm(norbert); -people.add_in_realm(zoe); -people.add_in_realm(me); +people.add(alice); +people.add(fred); +people.add(jill); +people.add(mark); +people.add(norbert); +people.add(zoe); +people.add(me); people.initialize_current_user(me.user_id); const real_update_huddles = activity.update_huddles; diff --git a/frontend_tests/node_tests/buddy_data.js b/frontend_tests/node_tests/buddy_data.js index 5af1afe636..6c3d8440b8 100644 --- a/frontend_tests/node_tests/buddy_data.js +++ b/frontend_tests/node_tests/buddy_data.js @@ -57,14 +57,14 @@ function make_people() { full_name: `Human ${i}`, email: `person${i}@example.com`, }; - people.add_in_realm(person); + people.add(person); } - people.add_in_realm(bot); - people.add_in_realm(bot_with_owner); - people.add_in_realm(selma); - people.add_in_realm(me); - people.add_in_realm(old_user); + people.add(bot); + people.add(bot_with_owner); + people.add(selma); + people.add(me); + people.add(old_user); people.initialize_current_user(me.user_id); } diff --git a/frontend_tests/node_tests/compose.js b/frontend_tests/node_tests/compose.js index ab5f70caac..09a345f6c1 100644 --- a/frontend_tests/node_tests/compose.js +++ b/frontend_tests/node_tests/compose.js @@ -247,7 +247,7 @@ run_test('validate', () => { assert.equal($('#compose-error-msg').html(), i18n.t('Please specify at least one valid recipient', {})); - people.add_in_realm(bob); + people.add(bob); compose_state.private_message_recipient('bob@example.com'); assert(compose.validate()); diff --git a/frontend_tests/node_tests/compose_actions.js b/frontend_tests/node_tests/compose_actions.js index e2760a13ed..77608da19f 100644 --- a/frontend_tests/node_tests/compose_actions.js +++ b/frontend_tests/node_tests/compose_actions.js @@ -235,7 +235,7 @@ run_test('respond_to_message', () => { email: 'alice@example.com', full_name: 'Alice', }; - people.add_in_realm(person); + people.add(person); let msg = { type: 'private', @@ -296,13 +296,13 @@ run_test('reply_with_mention', () => { email: 'bob1@example.com', full_name: 'Bob Roberts', }; - people.add_in_realm(bob_1); + people.add(bob_1); const bob_2 = { user_id: 40, email: 'bob2@example.com', full_name: 'Bob Roberts', }; - people.add_in_realm(bob_2); + people.add(bob_2); reply_with_mention(opts); assert.equal($('#stream_message_recipient_stream').val(), 'devel'); diff --git a/frontend_tests/node_tests/compose_fade.js b/frontend_tests/node_tests/compose_fade.js index 09c7860ec4..220709dc49 100644 --- a/frontend_tests/node_tests/compose_fade.js +++ b/frontend_tests/node_tests/compose_fade.js @@ -23,11 +23,11 @@ const bob = { full_name: 'Bob', }; -people.add_in_realm(me); +people.add(me); people.initialize_current_user(me.user_id); -people.add_in_realm(alice); -people.add_in_realm(bob); +people.add(alice); +people.add(bob); run_test('set_focused_recipient', () => { diff --git a/frontend_tests/node_tests/compose_ui.js b/frontend_tests/node_tests/compose_ui.js index 14447b950c..5e7ebfd2c0 100644 --- a/frontend_tests/node_tests/compose_ui.js +++ b/frontend_tests/node_tests/compose_ui.js @@ -20,8 +20,8 @@ const bob = { full_name: 'Bob', }; -global.people.add_in_realm(alice); -global.people.add_in_realm(bob); +global.people.add(alice); +global.people.add(bob); const noop = function () {}; diff --git a/frontend_tests/node_tests/composebox_typeahead.js b/frontend_tests/node_tests/composebox_typeahead.js index 0b54d4be4b..5f5f24f935 100644 --- a/frontend_tests/node_tests/composebox_typeahead.js +++ b/frontend_tests/node_tests/composebox_typeahead.js @@ -240,17 +240,18 @@ const harry = { email: 'harry@zulip.com', }; -global.people.add_in_realm(alice); -global.people.add_in_realm(hamlet); -global.people.add_in_realm(othello); -global.people.add_in_realm(cordelia); -global.people.add_in_realm(lear); -global.people.add_in_realm(twin1); -global.people.add_in_realm(twin2); -global.people.add_in_realm(gael); -global.people.add_in_realm(hal); -global.people.add_in_realm(harry); +global.people.add(alice); +global.people.add(hamlet); +global.people.add(othello); +global.people.add(cordelia); +global.people.add(lear); +global.people.add(twin1); +global.people.add(twin2); +global.people.add(gael); +global.people.add(hal); +global.people.add(harry); global.people.add(deactivated_user); +global.people.deactivate(deactivated_user); const hamletcharacters = { name: "hamletcharacters", diff --git a/frontend_tests/node_tests/hash_util.js b/frontend_tests/node_tests/hash_util.js index 37efbcbaaf..990cfe37d8 100644 --- a/frontend_tests/node_tests/hash_util.js +++ b/frontend_tests/node_tests/hash_util.js @@ -15,7 +15,7 @@ const hamlet = { full_name: 'Hamlet', }; -people.add_in_realm(hamlet); +people.add(hamlet); const frontend = { stream_id: 99, diff --git a/frontend_tests/node_tests/message_store.js b/frontend_tests/node_tests/message_store.js index 0dd68a7cae..1463733683 100644 --- a/frontend_tests/node_tests/message_store.js +++ b/frontend_tests/node_tests/message_store.js @@ -58,11 +58,11 @@ const denise = { full_name: 'Denise ', }; -people.add_in_realm(me); -people.add_in_realm(alice); -people.add_in_realm(bob); -people.add_in_realm(cindy); -people.add_in_realm(denise); +people.add(me); +people.add(alice); +people.add(bob); +people.add(cindy); +people.add(denise); global.people.initialize_current_user(me.user_id); diff --git a/frontend_tests/node_tests/narrow.js b/frontend_tests/node_tests/narrow.js index d9e2356e99..d6928a407c 100644 --- a/frontend_tests/node_tests/narrow.js +++ b/frontend_tests/node_tests/narrow.js @@ -104,7 +104,7 @@ run_test('show_empty_narrow_message', () => { narrow.show_empty_narrow_message(); assert($('#non_existing_user').visible()); - people.add_in_realm(alice); + people.add(alice); set_filter([['pm-with', ['alice@example.com', 'Yo']]]); narrow.show_empty_narrow_message(); assert($('#non_existing_users').visible()); @@ -202,8 +202,8 @@ run_test('show_invalid_narrow_message', () => { assert($('#empty_search_narrow_message').visible()); assert.equal(display.text(), 'translated: You are searching for messages that belong to more than one topic, which is not possible.'); - people.add_in_realm(ray); - people.add_in_realm(alice); + people.add(ray); + people.add(alice); set_filter([['sender', 'alice@example.com'], ['sender', 'ray@example.com']]); narrow.show_empty_narrow_message(); @@ -281,9 +281,9 @@ run_test('narrow_to_compose_target', () => { // --- Tests for PMs --- global.compose_state.get_message_type = () => 'private'; - people.add_in_realm(ray); - people.add_in_realm(alice); - people.add_in_realm(me); + people.add(ray); + people.add(alice); + people.add(me); // Test with valid person global.compose_state.private_message_recipient = () => 'alice@example.com'; diff --git a/frontend_tests/node_tests/narrow_state.js b/frontend_tests/node_tests/narrow_state.js index 67a7762478..a23982b43e 100644 --- a/frontend_tests/node_tests/narrow_state.js +++ b/frontend_tests/node_tests/narrow_state.js @@ -150,7 +150,7 @@ run_test('set_compose_defaults', () => { full_name: 'John Doe', }; people.add(john); - people.add_in_realm(john); + people.add(john); set_filter([['pm-with', 'john@doe.com']]); pm_test = narrow_state.set_compose_defaults(); diff --git a/frontend_tests/node_tests/people.js b/frontend_tests/node_tests/people.js index 9287aecb35..8487877265 100644 --- a/frontend_tests/node_tests/people.js +++ b/frontend_tests/node_tests/people.js @@ -37,7 +37,7 @@ const isaac = { function initialize() { people.init(); - people.add_in_realm(me); + people.add(me); people.initialize_current_user(me.user_id); } @@ -78,16 +78,16 @@ run_test('basics', () => { assert(people.is_valid_full_name_and_user_id(full_name, 32)); assert(people.is_known_user_id(32)); - assert.equal(people.get_active_human_count(), 1); + assert.equal(people.get_active_human_count(), 2); assert.equal(people.get_user_id_from_name(full_name), 32); let person = people.get_by_email(email); assert.equal(person.full_name, full_name); - person = people.get_active_user_for_email(email); + + person = people.get_active_user_for_email('nobody@example.com'); assert(!person); - people.add_in_realm(isaac); - assert.equal(people.get_active_human_count(), 2); + person = people.get_active_user_for_email(email); assert.equal(person.email, email); @@ -113,7 +113,7 @@ run_test('basics', () => { full_name: 'Bot Botson', is_bot: true, }; - people.add_in_realm(bot_botson); + people.add(bot_botson); assert.equal(people.is_active_user_for_popover(bot_botson.user_id), true); // get_realm_users() will include our active bot, @@ -153,7 +153,7 @@ run_test('basics', () => { assert.equal(people.is_my_user_id(undefined), false); // Reactivating issac - people.add_in_realm(isaac); + people.add(isaac); const active_humans = people.get_active_humans(); assert.equal(active_humans.length, 2); assert.deepEqual( @@ -194,7 +194,7 @@ run_test('bot_custom_profile_data', () => { full_name: 'Bot', is_bot: true, }; - people.add_in_realm(bot); + people.add(bot); assert.equal(people.get_custom_profile_data(31, 3), null); }); @@ -320,9 +320,9 @@ run_test('get_people_for_stream_create', () => { user_id: 204, full_name: 'Bob van Roberts', }; - people.add_in_realm(alice1); - people.add_in_realm(bob); - people.add_in_realm(alice2); + people.add(alice1); + people.add(bob); + people.add(alice2); assert.equal(people.get_active_human_count(), 4); const others = people.get_people_for_stream_create(); @@ -379,12 +379,12 @@ run_test('filtered_users', () => { full_name: 'Nooaah Emerson', }; - people.add_in_realm(charles); - people.add_in_realm(maria); - people.add_in_realm(ashton); - people.add_in_realm(linus); - people.add_in_realm(noah); - people.add_in_realm(plain_noah); + people.add(charles); + people.add(maria); + people.add(ashton); + people.add(linus); + people.add(noah); + people.add(plain_noah); const search_term = 'a'; const users = people.get_people_for_stream_create(); @@ -445,8 +445,8 @@ run_test('multi_user_methods', () => { full_name: 'whatever 402', }; - people.add_in_realm(emp401); - people.add_in_realm(emp402); + people.add(emp401); + people.add(emp402); let emails_string = people.user_ids_string_to_emails_string('402,401'); assert.equal(emails_string, 'emp401@example.com,emp402@example.com'); @@ -653,7 +653,7 @@ run_test('maybe_incr_recipient_count', () => { const maria_recip = { id: maria.user_id, }; - people.add_in_realm(maria); + people.add(maria); let message = { type: 'private', @@ -724,7 +724,7 @@ run_test('get_people_for_search_bar', () => { full_name: 'James Jones', user_id: 1000 + i, }; - people.add_in_realm(person); + people.add(person); } const big_results = people.get_people_for_search_bar('James'); @@ -755,7 +755,7 @@ run_test('updates', () => { user_id: user_id, full_name: 'Foo Barson', }; - people.add_in_realm(person); + people.add(person); // Do sanity checks on our data. assert.equal(people.get_by_email(old_email).user_id, user_id); diff --git a/frontend_tests/node_tests/pm_list.js b/frontend_tests/node_tests/pm_list.js index a7ab385b67..82543fe047 100644 --- a/frontend_tests/node_tests/pm_list.js +++ b/frontend_tests/node_tests/pm_list.js @@ -50,10 +50,10 @@ const bot_test = { is_admin: false, is_bot: true, }; -global.people.add_in_realm(alice); -global.people.add_in_realm(bob); -global.people.add_in_realm(me); -global.people.add_in_realm(bot_test); +global.people.add(alice); +global.people.add(bob); +global.people.add(me); +global.people.add(bot_test); global.people.initialize_current_user(me.user_id); run_test('close', () => { diff --git a/frontend_tests/node_tests/popovers.js b/frontend_tests/node_tests/popovers.js index 61892b32d9..4e6dfdab57 100644 --- a/frontend_tests/node_tests/popovers.js +++ b/frontend_tests/node_tests/popovers.js @@ -75,8 +75,8 @@ const e = { function initialize_people() { people.init(); - people.add_in_realm(me); - people.add_in_realm(alice); + people.add(me); + people.add(alice); people.initialize_current_user(me.user_id); } diff --git a/frontend_tests/node_tests/presence.js b/frontend_tests/node_tests/presence.js index 01a7bfa52d..19609d50cf 100644 --- a/frontend_tests/node_tests/presence.js +++ b/frontend_tests/node_tests/presence.js @@ -42,11 +42,11 @@ const bot = { is_bot: true, }; -people.add_in_realm(me); -people.add_in_realm(alice); -people.add_in_realm(fred); -people.add_in_realm(zoe); -people.add_in_realm(bot); +people.add(me); +people.add(alice); +people.add(fred); +people.add(zoe); +people.add(bot); people.initialize_current_user(me.user_id); run_test('my user', () => { diff --git a/frontend_tests/node_tests/reactions.js b/frontend_tests/node_tests/reactions.js index b857dd82c3..b55f42ffaf 100644 --- a/frontend_tests/node_tests/reactions.js +++ b/frontend_tests/node_tests/reactions.js @@ -69,9 +69,9 @@ const cali = { user_id: 7, full_name: 'Cali', }; -people.add_in_realm(alice); -people.add_in_realm(bob); -people.add_in_realm(cali); +people.add(alice); +people.add(bob); +people.add(cali); const message = { id: 1001, diff --git a/frontend_tests/node_tests/stream_data.js b/frontend_tests/node_tests/stream_data.js index bd59fa9db5..d7e3d6887d 100644 --- a/frontend_tests/node_tests/stream_data.js +++ b/frontend_tests/node_tests/stream_data.js @@ -28,7 +28,7 @@ const settings_config = zrequire("settings_config"); const me = { email: 'me@zulip.com', full_name: 'Current User', - user_id: 81, + user_id: 100, }; // set up user data @@ -182,9 +182,9 @@ run_test('subscribers', () => { full_name: 'George', user_id: 103, }; - people.add_in_realm(fred); - people.add_in_realm(not_fred); - people.add_in_realm(george); + people.add(fred); + people.add(not_fred); + people.add(george); function potential_subscriber_ids() { const users = stream_data.potential_subscribers(sub); @@ -194,14 +194,16 @@ run_test('subscribers', () => { assert.deepEqual( potential_subscriber_ids(), [ + me.user_id, fred.user_id, not_fred.user_id, george.user_id, ] ); - stream_data.set_subscribers(sub, [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); + assert(stream_data.is_user_subscribed('Rome', me.user_id)); assert(stream_data.is_user_subscribed('Rome', fred.user_id)); assert(stream_data.is_user_subscribed('Rome', george.user_id)); assert(!stream_data.is_user_subscribed('Rome', not_fred.user_id)); diff --git a/frontend_tests/node_tests/top_left_corner.js b/frontend_tests/node_tests/top_left_corner.js index 1012070d5f..d0715e78cb 100644 --- a/frontend_tests/node_tests/top_left_corner.js +++ b/frontend_tests/node_tests/top_left_corner.js @@ -35,8 +35,8 @@ run_test('narrowing', () => { full_name: 'Bob Patel', }; - people.add_in_realm(alice); - people.add_in_realm(bob); + people.add(alice); + people.add(bob); pm_expanded = false; filter = new Filter([ diff --git a/frontend_tests/node_tests/typeahead_helper.js b/frontend_tests/node_tests/typeahead_helper.js index ce93e9219c..a93cec34a6 100644 --- a/frontend_tests/node_tests/typeahead_helper.js +++ b/frontend_tests/node_tests/typeahead_helper.js @@ -202,7 +202,7 @@ const matches = [ ]; for (const person of matches) { - global.people.add_in_realm(person); + global.people.add(person); } function get_typeahead_result(query, current_stream, current_topic) { diff --git a/frontend_tests/node_tests/unread.js b/frontend_tests/node_tests/unread.js index e285b51b19..9d5861343c 100644 --- a/frontend_tests/node_tests/unread.js +++ b/frontend_tests/node_tests/unread.js @@ -398,7 +398,7 @@ run_test('private_messages', () => { user_id: 999, full_name: 'Any Body', }; - people.add_in_realm(anybody); + people.add(anybody); const message = { id: 15, @@ -429,14 +429,14 @@ run_test('private_messages', () => { user_id: 101, full_name: 'Alice', }; - people.add_in_realm(alice); + people.add(alice); const bob = { email: 'bob@example.com', user_id: 102, full_name: 'Bob', }; - people.add_in_realm(bob); + people.add(bob); assert.equal(unread.num_unread_for_person(alice.user_id.toString()), 0); assert.equal(unread.num_unread_for_person(bob.user_id.toString()), 0); diff --git a/frontend_tests/node_tests/user_pill.js b/frontend_tests/node_tests/user_pill.js index 9bc0d0fd8f..f224c9f426 100644 --- a/frontend_tests/node_tests/user_pill.js +++ b/frontend_tests/node_tests/user_pill.js @@ -32,8 +32,8 @@ const isaac_item = { }; run_test('setup', () => { - people.add_in_realm(alice); - people.add_in_realm(isaac); + people.add(alice); + people.add(isaac); }); run_test('create_item', () => { diff --git a/static/js/people.js b/static/js/people.js index 753c581215..01743f85b5 100644 --- a/static/js/people.js +++ b/static/js/people.js @@ -981,7 +981,12 @@ exports.get_mention_syntax = function (full_name, user_id, silent) { return mention; }; -exports.add = function add(person) { +exports._add_user = function add(person) { + /* + This is common code to add any user, even + users who may be deactivated or outside + our realm (like cross-realm bots). + */ if (person.user_id) { people_by_user_id_dict.set(person.user_id, person); } else { @@ -999,9 +1004,9 @@ exports.add = function add(person) { people_by_name_dict.set(person.full_name, person); }; -exports.add_in_realm = function (person) { +exports.add = function (person) { active_user_dict.set(person.user_id, person); - exports.add(person); + exports._add_user(person); }; exports.deactivate = function (person) { @@ -1056,7 +1061,7 @@ exports.extract_people_from_message = function (message) { exports.report_late_add(user_id, person.email); - exports.add({ + exports._add_user({ email: person.email, user_id: user_id, full_name: person.full_name, @@ -1190,16 +1195,16 @@ exports.is_my_user_id = function (user_id) { exports.initialize = function (my_user_id, params) { for (const person of params.realm_users) { - exports.add_in_realm(person); + exports.add(person); } for (const person of params.realm_non_active_users) { - exports.add(person); + exports._add_user(person); } for (const person of params.cross_realm_bots) { if (!people_dict.has(person.email)) { - exports.add(person); + exports._add_user(person); } cross_realm_dict.set(person.user_id, person); } diff --git a/static/js/server_events_dispatch.js b/static/js/server_events_dispatch.js index 1f7f566b04..f073b5d1b6 100644 --- a/static/js/server_events_dispatch.js +++ b/static/js/server_events_dispatch.js @@ -250,7 +250,7 @@ exports.dispatch_normal_event = function dispatch_normal_event(event) { case 'realm_user': if (event.op === 'add') { - people.add_in_realm(event.person); + people.add(event.person); } else if (event.op === 'remove') { people.deactivate(event.person); stream_events.remove_deactivated_user_from_all_streams(event.person.user_id);