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.
This commit is contained in:
Steve Howell
2020-03-21 19:19:30 +00:00
committed by Tim Abbott
parent 25d2e2e122
commit f0c99b42ec
23 changed files with 110 additions and 102 deletions

View File

@@ -123,13 +123,13 @@ const zoe = {
const people = global.people; const people = global.people;
people.add_in_realm(alice); people.add(alice);
people.add_in_realm(fred); people.add(fred);
people.add_in_realm(jill); people.add(jill);
people.add_in_realm(mark); people.add(mark);
people.add_in_realm(norbert); people.add(norbert);
people.add_in_realm(zoe); people.add(zoe);
people.add_in_realm(me); people.add(me);
people.initialize_current_user(me.user_id); people.initialize_current_user(me.user_id);
const real_update_huddles = activity.update_huddles; const real_update_huddles = activity.update_huddles;

View File

@@ -57,14 +57,14 @@ function make_people() {
full_name: `Human ${i}`, full_name: `Human ${i}`,
email: `person${i}@example.com`, email: `person${i}@example.com`,
}; };
people.add_in_realm(person); people.add(person);
} }
people.add_in_realm(bot); people.add(bot);
people.add_in_realm(bot_with_owner); people.add(bot_with_owner);
people.add_in_realm(selma); people.add(selma);
people.add_in_realm(me); people.add(me);
people.add_in_realm(old_user); people.add(old_user);
people.initialize_current_user(me.user_id); people.initialize_current_user(me.user_id);
} }

View File

@@ -247,7 +247,7 @@ run_test('validate', () => {
assert.equal($('#compose-error-msg').html(), i18n.t('Please specify at least one valid recipient', {})); 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'); compose_state.private_message_recipient('bob@example.com');
assert(compose.validate()); assert(compose.validate());

View File

@@ -235,7 +235,7 @@ run_test('respond_to_message', () => {
email: 'alice@example.com', email: 'alice@example.com',
full_name: 'Alice', full_name: 'Alice',
}; };
people.add_in_realm(person); people.add(person);
let msg = { let msg = {
type: 'private', type: 'private',
@@ -296,13 +296,13 @@ run_test('reply_with_mention', () => {
email: 'bob1@example.com', email: 'bob1@example.com',
full_name: 'Bob Roberts', full_name: 'Bob Roberts',
}; };
people.add_in_realm(bob_1); people.add(bob_1);
const bob_2 = { const bob_2 = {
user_id: 40, user_id: 40,
email: 'bob2@example.com', email: 'bob2@example.com',
full_name: 'Bob Roberts', full_name: 'Bob Roberts',
}; };
people.add_in_realm(bob_2); people.add(bob_2);
reply_with_mention(opts); reply_with_mention(opts);
assert.equal($('#stream_message_recipient_stream').val(), 'devel'); assert.equal($('#stream_message_recipient_stream').val(), 'devel');

View File

@@ -23,11 +23,11 @@ const bob = {
full_name: 'Bob', full_name: 'Bob',
}; };
people.add_in_realm(me); people.add(me);
people.initialize_current_user(me.user_id); people.initialize_current_user(me.user_id);
people.add_in_realm(alice); people.add(alice);
people.add_in_realm(bob); people.add(bob);
run_test('set_focused_recipient', () => { run_test('set_focused_recipient', () => {

View File

@@ -20,8 +20,8 @@ const bob = {
full_name: 'Bob', full_name: 'Bob',
}; };
global.people.add_in_realm(alice); global.people.add(alice);
global.people.add_in_realm(bob); global.people.add(bob);
const noop = function () {}; const noop = function () {};

View File

@@ -240,17 +240,18 @@ const harry = {
email: 'harry@zulip.com', email: 'harry@zulip.com',
}; };
global.people.add_in_realm(alice); global.people.add(alice);
global.people.add_in_realm(hamlet); global.people.add(hamlet);
global.people.add_in_realm(othello); global.people.add(othello);
global.people.add_in_realm(cordelia); global.people.add(cordelia);
global.people.add_in_realm(lear); global.people.add(lear);
global.people.add_in_realm(twin1); global.people.add(twin1);
global.people.add_in_realm(twin2); global.people.add(twin2);
global.people.add_in_realm(gael); global.people.add(gael);
global.people.add_in_realm(hal); global.people.add(hal);
global.people.add_in_realm(harry); global.people.add(harry);
global.people.add(deactivated_user); global.people.add(deactivated_user);
global.people.deactivate(deactivated_user);
const hamletcharacters = { const hamletcharacters = {
name: "hamletcharacters", name: "hamletcharacters",

View File

@@ -15,7 +15,7 @@ const hamlet = {
full_name: 'Hamlet', full_name: 'Hamlet',
}; };
people.add_in_realm(hamlet); people.add(hamlet);
const frontend = { const frontend = {
stream_id: 99, stream_id: 99,

View File

@@ -58,11 +58,11 @@ const denise = {
full_name: 'Denise ', full_name: 'Denise ',
}; };
people.add_in_realm(me); people.add(me);
people.add_in_realm(alice); people.add(alice);
people.add_in_realm(bob); people.add(bob);
people.add_in_realm(cindy); people.add(cindy);
people.add_in_realm(denise); people.add(denise);
global.people.initialize_current_user(me.user_id); global.people.initialize_current_user(me.user_id);

View File

@@ -104,7 +104,7 @@ run_test('show_empty_narrow_message', () => {
narrow.show_empty_narrow_message(); narrow.show_empty_narrow_message();
assert($('#non_existing_user').visible()); assert($('#non_existing_user').visible());
people.add_in_realm(alice); people.add(alice);
set_filter([['pm-with', ['alice@example.com', 'Yo']]]); set_filter([['pm-with', ['alice@example.com', 'Yo']]]);
narrow.show_empty_narrow_message(); narrow.show_empty_narrow_message();
assert($('#non_existing_users').visible()); assert($('#non_existing_users').visible());
@@ -202,8 +202,8 @@ run_test('show_invalid_narrow_message', () => {
assert($('#empty_search_narrow_message').visible()); 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.'); 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(ray);
people.add_in_realm(alice); people.add(alice);
set_filter([['sender', 'alice@example.com'], ['sender', 'ray@example.com']]); set_filter([['sender', 'alice@example.com'], ['sender', 'ray@example.com']]);
narrow.show_empty_narrow_message(); narrow.show_empty_narrow_message();
@@ -281,9 +281,9 @@ run_test('narrow_to_compose_target', () => {
// --- Tests for PMs --- // --- Tests for PMs ---
global.compose_state.get_message_type = () => 'private'; global.compose_state.get_message_type = () => 'private';
people.add_in_realm(ray); people.add(ray);
people.add_in_realm(alice); people.add(alice);
people.add_in_realm(me); people.add(me);
// Test with valid person // Test with valid person
global.compose_state.private_message_recipient = () => 'alice@example.com'; global.compose_state.private_message_recipient = () => 'alice@example.com';

View File

@@ -150,7 +150,7 @@ run_test('set_compose_defaults', () => {
full_name: 'John Doe', full_name: 'John Doe',
}; };
people.add(john); people.add(john);
people.add_in_realm(john); people.add(john);
set_filter([['pm-with', 'john@doe.com']]); set_filter([['pm-with', 'john@doe.com']]);
pm_test = narrow_state.set_compose_defaults(); pm_test = narrow_state.set_compose_defaults();

View File

@@ -37,7 +37,7 @@ const isaac = {
function initialize() { function initialize() {
people.init(); people.init();
people.add_in_realm(me); people.add(me);
people.initialize_current_user(me.user_id); 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_valid_full_name_and_user_id(full_name, 32));
assert(people.is_known_user_id(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); assert.equal(people.get_user_id_from_name(full_name), 32);
let person = people.get_by_email(email); let person = people.get_by_email(email);
assert.equal(person.full_name, full_name); 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); assert(!person);
people.add_in_realm(isaac);
assert.equal(people.get_active_human_count(), 2);
person = people.get_active_user_for_email(email); person = people.get_active_user_for_email(email);
assert.equal(person.email, email); assert.equal(person.email, email);
@@ -113,7 +113,7 @@ run_test('basics', () => {
full_name: 'Bot Botson', full_name: 'Bot Botson',
is_bot: true, 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); assert.equal(people.is_active_user_for_popover(bot_botson.user_id), true);
// get_realm_users() will include our active bot, // get_realm_users() will include our active bot,
@@ -153,7 +153,7 @@ run_test('basics', () => {
assert.equal(people.is_my_user_id(undefined), false); assert.equal(people.is_my_user_id(undefined), false);
// Reactivating issac // Reactivating issac
people.add_in_realm(isaac); people.add(isaac);
const active_humans = people.get_active_humans(); const active_humans = people.get_active_humans();
assert.equal(active_humans.length, 2); assert.equal(active_humans.length, 2);
assert.deepEqual( assert.deepEqual(
@@ -194,7 +194,7 @@ run_test('bot_custom_profile_data', () => {
full_name: 'Bot', full_name: 'Bot',
is_bot: true, is_bot: true,
}; };
people.add_in_realm(bot); people.add(bot);
assert.equal(people.get_custom_profile_data(31, 3), null); assert.equal(people.get_custom_profile_data(31, 3), null);
}); });
@@ -320,9 +320,9 @@ run_test('get_people_for_stream_create', () => {
user_id: 204, user_id: 204,
full_name: 'Bob van Roberts', full_name: 'Bob van Roberts',
}; };
people.add_in_realm(alice1); people.add(alice1);
people.add_in_realm(bob); people.add(bob);
people.add_in_realm(alice2); people.add(alice2);
assert.equal(people.get_active_human_count(), 4); assert.equal(people.get_active_human_count(), 4);
const others = people.get_people_for_stream_create(); const others = people.get_people_for_stream_create();
@@ -379,12 +379,12 @@ run_test('filtered_users', () => {
full_name: 'Nooaah Emerson', full_name: 'Nooaah Emerson',
}; };
people.add_in_realm(charles); people.add(charles);
people.add_in_realm(maria); people.add(maria);
people.add_in_realm(ashton); people.add(ashton);
people.add_in_realm(linus); people.add(linus);
people.add_in_realm(noah); people.add(noah);
people.add_in_realm(plain_noah); people.add(plain_noah);
const search_term = 'a'; const search_term = 'a';
const users = people.get_people_for_stream_create(); const users = people.get_people_for_stream_create();
@@ -445,8 +445,8 @@ run_test('multi_user_methods', () => {
full_name: 'whatever 402', full_name: 'whatever 402',
}; };
people.add_in_realm(emp401); people.add(emp401);
people.add_in_realm(emp402); people.add(emp402);
let emails_string = people.user_ids_string_to_emails_string('402,401'); let emails_string = people.user_ids_string_to_emails_string('402,401');
assert.equal(emails_string, 'emp401@example.com,emp402@example.com'); assert.equal(emails_string, 'emp401@example.com,emp402@example.com');
@@ -653,7 +653,7 @@ run_test('maybe_incr_recipient_count', () => {
const maria_recip = { const maria_recip = {
id: maria.user_id, id: maria.user_id,
}; };
people.add_in_realm(maria); people.add(maria);
let message = { let message = {
type: 'private', type: 'private',
@@ -724,7 +724,7 @@ run_test('get_people_for_search_bar', () => {
full_name: 'James Jones', full_name: 'James Jones',
user_id: 1000 + i, user_id: 1000 + i,
}; };
people.add_in_realm(person); people.add(person);
} }
const big_results = people.get_people_for_search_bar('James'); const big_results = people.get_people_for_search_bar('James');
@@ -755,7 +755,7 @@ run_test('updates', () => {
user_id: user_id, user_id: user_id,
full_name: 'Foo Barson', full_name: 'Foo Barson',
}; };
people.add_in_realm(person); people.add(person);
// Do sanity checks on our data. // Do sanity checks on our data.
assert.equal(people.get_by_email(old_email).user_id, user_id); assert.equal(people.get_by_email(old_email).user_id, user_id);

View File

@@ -50,10 +50,10 @@ const bot_test = {
is_admin: false, is_admin: false,
is_bot: true, is_bot: true,
}; };
global.people.add_in_realm(alice); global.people.add(alice);
global.people.add_in_realm(bob); global.people.add(bob);
global.people.add_in_realm(me); global.people.add(me);
global.people.add_in_realm(bot_test); global.people.add(bot_test);
global.people.initialize_current_user(me.user_id); global.people.initialize_current_user(me.user_id);
run_test('close', () => { run_test('close', () => {

View File

@@ -75,8 +75,8 @@ const e = {
function initialize_people() { function initialize_people() {
people.init(); people.init();
people.add_in_realm(me); people.add(me);
people.add_in_realm(alice); people.add(alice);
people.initialize_current_user(me.user_id); people.initialize_current_user(me.user_id);
} }

View File

@@ -42,11 +42,11 @@ const bot = {
is_bot: true, is_bot: true,
}; };
people.add_in_realm(me); people.add(me);
people.add_in_realm(alice); people.add(alice);
people.add_in_realm(fred); people.add(fred);
people.add_in_realm(zoe); people.add(zoe);
people.add_in_realm(bot); people.add(bot);
people.initialize_current_user(me.user_id); people.initialize_current_user(me.user_id);
run_test('my user', () => { run_test('my user', () => {

View File

@@ -69,9 +69,9 @@ const cali = {
user_id: 7, user_id: 7,
full_name: 'Cali', full_name: 'Cali',
}; };
people.add_in_realm(alice); people.add(alice);
people.add_in_realm(bob); people.add(bob);
people.add_in_realm(cali); people.add(cali);
const message = { const message = {
id: 1001, id: 1001,

View File

@@ -28,7 +28,7 @@ const settings_config = zrequire("settings_config");
const me = { const me = {
email: 'me@zulip.com', email: 'me@zulip.com',
full_name: 'Current User', full_name: 'Current User',
user_id: 81, user_id: 100,
}; };
// set up user data // set up user data
@@ -182,9 +182,9 @@ run_test('subscribers', () => {
full_name: 'George', full_name: 'George',
user_id: 103, user_id: 103,
}; };
people.add_in_realm(fred); people.add(fred);
people.add_in_realm(not_fred); people.add(not_fred);
people.add_in_realm(george); people.add(george);
function potential_subscriber_ids() { function potential_subscriber_ids() {
const users = stream_data.potential_subscribers(sub); const users = stream_data.potential_subscribers(sub);
@@ -194,14 +194,16 @@ run_test('subscribers', () => {
assert.deepEqual( assert.deepEqual(
potential_subscriber_ids(), potential_subscriber_ids(),
[ [
me.user_id,
fred.user_id, fred.user_id,
not_fred.user_id, not_fred.user_id,
george.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); 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', fred.user_id));
assert(stream_data.is_user_subscribed('Rome', george.user_id)); assert(stream_data.is_user_subscribed('Rome', george.user_id));
assert(!stream_data.is_user_subscribed('Rome', not_fred.user_id)); assert(!stream_data.is_user_subscribed('Rome', not_fred.user_id));

View File

@@ -35,8 +35,8 @@ run_test('narrowing', () => {
full_name: 'Bob Patel', full_name: 'Bob Patel',
}; };
people.add_in_realm(alice); people.add(alice);
people.add_in_realm(bob); people.add(bob);
pm_expanded = false; pm_expanded = false;
filter = new Filter([ filter = new Filter([

View File

@@ -202,7 +202,7 @@ const matches = [
]; ];
for (const person of 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) { function get_typeahead_result(query, current_stream, current_topic) {

View File

@@ -398,7 +398,7 @@ run_test('private_messages', () => {
user_id: 999, user_id: 999,
full_name: 'Any Body', full_name: 'Any Body',
}; };
people.add_in_realm(anybody); people.add(anybody);
const message = { const message = {
id: 15, id: 15,
@@ -429,14 +429,14 @@ run_test('private_messages', () => {
user_id: 101, user_id: 101,
full_name: 'Alice', full_name: 'Alice',
}; };
people.add_in_realm(alice); people.add(alice);
const bob = { const bob = {
email: 'bob@example.com', email: 'bob@example.com',
user_id: 102, user_id: 102,
full_name: 'Bob', 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(alice.user_id.toString()), 0);
assert.equal(unread.num_unread_for_person(bob.user_id.toString()), 0); assert.equal(unread.num_unread_for_person(bob.user_id.toString()), 0);

View File

@@ -32,8 +32,8 @@ const isaac_item = {
}; };
run_test('setup', () => { run_test('setup', () => {
people.add_in_realm(alice); people.add(alice);
people.add_in_realm(isaac); people.add(isaac);
}); });
run_test('create_item', () => { run_test('create_item', () => {

View File

@@ -981,7 +981,12 @@ exports.get_mention_syntax = function (full_name, user_id, silent) {
return mention; 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) { if (person.user_id) {
people_by_user_id_dict.set(person.user_id, person); people_by_user_id_dict.set(person.user_id, person);
} else { } else {
@@ -999,9 +1004,9 @@ exports.add = function add(person) {
people_by_name_dict.set(person.full_name, 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); active_user_dict.set(person.user_id, person);
exports.add(person); exports._add_user(person);
}; };
exports.deactivate = function (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.report_late_add(user_id, person.email);
exports.add({ exports._add_user({
email: person.email, email: person.email,
user_id: user_id, user_id: user_id,
full_name: person.full_name, full_name: person.full_name,
@@ -1190,16 +1195,16 @@ exports.is_my_user_id = function (user_id) {
exports.initialize = function (my_user_id, params) { exports.initialize = function (my_user_id, params) {
for (const person of params.realm_users) { for (const person of params.realm_users) {
exports.add_in_realm(person); exports.add(person);
} }
for (const person of params.realm_non_active_users) { for (const person of params.realm_non_active_users) {
exports.add(person); exports._add_user(person);
} }
for (const person of params.cross_realm_bots) { for (const person of params.cross_realm_bots) {
if (!people_dict.has(person.email)) { if (!people_dict.has(person.email)) {
exports.add(person); exports._add_user(person);
} }
cross_realm_dict.set(person.user_id, person); cross_realm_dict.set(person.user_id, person);
} }

View File

@@ -250,7 +250,7 @@ exports.dispatch_normal_event = function dispatch_normal_event(event) {
case 'realm_user': case 'realm_user':
if (event.op === 'add') { if (event.op === 'add') {
people.add_in_realm(event.person); people.add(event.person);
} else if (event.op === 'remove') { } else if (event.op === 'remove') {
people.deactivate(event.person); people.deactivate(event.person);
stream_events.remove_deactivated_user_from_all_streams(event.person.user_id); stream_events.remove_deactivated_user_from_all_streams(event.person.user_id);