From 5b8e217bf4ab812f0ccd1445ef6e1f810f0b3733 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Tue, 31 Jan 2017 11:44:51 -0800 Subject: [PATCH] Add people.update_email(). The function people.update_email() is not yet connected to anything, but it sets the stage for upcoming changes. When emails get updated, fundamentally we just update the appropriate person object and add a new key to people_dict. We sort of get a shim for free--old email lookups will continue to work--but we add blueslip warnings for stale lookups. --- frontend_tests/node_tests/people.js | 50 +++++++++++++++++++++++++++++ static/js/people.js | 43 +++++++++++++++++++------ 2 files changed, 84 insertions(+), 9 deletions(-) diff --git a/frontend_tests/node_tests/people.js b/frontend_tests/node_tests/people.js index 1470b9d876..9c9ca33e9c 100644 --- a/frontend_tests/node_tests/people.js +++ b/frontend_tests/node_tests/people.js @@ -5,6 +5,7 @@ add_dependencies({ global.stub_out_jquery(); var people = require("js/people.js"); +set_global('blueslip', {}); var _ = global._; @@ -238,3 +239,52 @@ people.init(); var email = people.slug_to_emails(slug); assert.equal(email, 'debbie71@example.com'); }()); + +initialize(); + +(function test_updates() { + var old_email = 'FOO@example.com'; + var new_email = 'bar@example.com'; + var user_id = 502; + + var person = { + email: old_email, + user_id: user_id, + full_name: 'Foo Barson', + }; + people.add_in_realm(person); + + // Do sanity checks on our data. + assert.equal(people.get_by_email(old_email).user_id, user_id); + assert.equal(people.realm_get(old_email).user_id, user_id); + assert (!people.is_cross_realm_email(old_email)); + + assert.equal(people.get_by_email(new_email), undefined); + + // DO THE EMAIL UPDATE HERE. + people.update_email(user_id, new_email); + + // Now look up using the new email. + assert.equal(people.get_by_email(new_email).user_id, user_id); + assert.equal(people.realm_get(new_email).user_id, user_id); + assert (!people.is_cross_realm_email(new_email)); + + var all_people = people.get_all_persons(); + assert.equal(all_people.length, 2); + + person = _.filter(all_people, function (p) { + return (p.email === new_email); + })[0]; + assert.equal(person.full_name, 'Foo Barson'); + + // Test shim where we can still retrieve user info using the + // old email. + var warning; + global.blueslip.warn = function (w) { + warning = w; + }; + + person = people.get_by_email(old_email); + assert(/Obsolete email.*FOO.*bar/.test(warning)); + assert.equal(person.user_id, user_id); +}()); diff --git a/static/js/people.js b/static/js/people.js index e0464b8b28..30e8a16738 100644 --- a/static/js/people.js +++ b/static/js/people.js @@ -14,11 +14,13 @@ var my_user_id; // can easily clear data. exports.init = function () { // The following three Dicts point to the same objects - // All people we've seen + // (all people we've seen), but people_dict can have duplicate + // keys related to email changes. We want to deprecate + // people_dict over time and always do lookups by user_id. people_dict = new Dict({fold_case: true}); people_by_name_dict = new Dict({fold_case: true}); people_by_user_id_dict = new Dict(); - // People in this realm + realm_people_dict = new Dict(); cross_realm_dict = new Dict(); // keyed by user_id pm_recipient_count_dict = new Dict(); @@ -35,12 +37,35 @@ exports.get_person_from_user_id = function (user_id) { return people_by_user_id_dict.get(user_id); }; -exports.get_by_email = function get_by_email(email) { - return people_dict.get(email); +exports.get_by_email = function (email) { + var person = people_dict.get(email); + + if (!person) { + return undefined; + } + + if (person.email.toLowerCase() !== email.toLowerCase()) { + blueslip.warn( + 'Obsolete email passed to get_by_email: ' + email + + ' new email = ' + person.email + ); + } + + return person; +}; + +exports.update_email = function (user_id, new_email) { + var person = people_by_user_id_dict.get(user_id); + person.email = new_email; + people_dict.set(new_email, person); + + // For legacy reasons we don't delete the old email + // keys in our dictionaries, so that reverse lookups + // still work correctly. }; exports.get_user_id = function (email) { - var person = people_dict.get(email); + var person = people.get_by_email(email); if (person === undefined) { // Our blueslip reporting here is a bit complicated, but // there are known race conditions after reload, and we @@ -115,7 +140,7 @@ exports.user_ids_string_to_emails_string = function (user_ids_string) { exports.emails_strings_to_user_ids_string = function (emails_string) { var emails = emails_string.split(','); var user_ids = _.map(emails, function (email) { - var person = people_dict.get(email); + var person = people.get_by_email(email); if (person) { return person.user_id; } @@ -226,7 +251,7 @@ exports.small_avatar_url = function (message) { }; exports.realm_get = function realm_get(email) { - var person = people_dict.get(email); + var person = people.get_by_email(email); if (!person) { return undefined; } @@ -234,7 +259,7 @@ exports.realm_get = function realm_get(email) { }; exports.get_all_persons = function () { - return people_dict.values(); + return people_by_user_id_dict.values(); }; exports.get_realm_persons = function () { @@ -242,7 +267,7 @@ exports.get_realm_persons = function () { }; exports.is_cross_realm_email = function (email) { - var person = people_dict.get(email); + var person = people.get_by_email(email); if (!person) { return undefined; }