recipient: Use IDs in isSameRecipient; cut out normalizeRecipients!

The hairy logic in normalizeRecipients was one of the last few cases
of the confusion around PM recipients we're seeking to remove for zulip#4035.

It also represents one of the many places we're in the process of
converting from using emails to using numeric user IDs.

The case where this could potentially change behavior is where we
have several messages that are in the same PM conversation, but the
`display_recipient` property on different messages disagrees about
the email of some participant (e.g. because that user changed their
email between when we learned about one message and when we learned
about the other.)  The old code would fail to notice the messages
were in the same conversation, and so when showing them
consecutively in an interleaved narrow (like all-messages or a
search result), we'd show a new recipient bar between them.

With this fix, we correctly identify the messages as belonging to
the same conversation.

src/utils/__tests__/recipient-test.js

@@ -1,30 +1,8 @@
 import {
-  normalizeRecipients,
   normalizeRecipientsAsUserIds,
   normalizeRecipientsAsUserIdsSansMe,
   isSameRecipient,
 } from '../recipient';
-import * as logging from '../logging';
-
-describe('normalizeRecipients', () => {
-  test('joins emails from recipients, sorted, trimmed, not including missing ones', () => {
-    const recipients = [
-      { email: '' },
-      { email: 'abc@example.com' },
-      { email: 'xyz@example.com' },
-      { email: '  def@example.com  ' },
-    ];
-    const expectedResult = 'abc@example.com,def@example.com,xyz@example.com';
-
-    logging.error.mockReturnValue();
-
-    const normalized = normalizeRecipients(recipients);
-    expect(normalized).toEqual(expectedResult);
-
-    expect(logging.error.mock.calls).toHaveLength(2);
-    logging.error.mockReset();
-  });
-});
 
 describe('normalizeRecipientsAsUserIds', () => {
   test('joins user IDs from recipients, sorted', () => {

src/utils/recipient.js

@@ -1,8 +1,8 @@
 /* @flow strict-local */
 import invariant from 'invariant';
+import isEqual from 'lodash.isequal';
 
 import type { PmRecipientUser, Message, Outbox, User, UserOrBot } from '../types';
-import * as logging from './logging';
 
 /** The stream name a stream message was sent to.  Throws if a PM. */
 export const streamNameOfStreamMessage = (message: Message | Outbox): string => {
@@ -109,33 +109,6 @@ const filterRecipientsAsUserIds = (
     ? [...recipients]
     : recipients.filter(r => r !== ownUserId).sort((a, b) => a - b);
 
-/** PRIVATE -- exported only for tests. */
-export const normalizeRecipients = (recipients: $ReadOnlyArray<{ +email: string, ... }>) => {
-  const emails = recipients.map(r => r.email);
-
-  if (emails.some(e => e.trim() !== e)) {
-    // This should never happen -- it makes the email address invalid.  If
-    // there's some user input that might be accepted like this, it should
-    // be turned into a valid email address long before this point.  We
-    // include this defensive logic only out of an abundance of caution
-    // because we had it, with no logging, for a long time.
-    logging.error('normalizeRecipients: got email with whitespace', { emails });
-  }
-  if (emails.some(e => !e)) {
-    // This should similarly never happen -- it means we got an
-    // unrecoverably bogus email address in here.  We carry on hoping, or
-    // pretending, that it just shouldn't have been in the list at all.
-    logging.error('normalizeRecipients: got empty email', { emails });
-  }
-  // Both of these fudge conditions should really go away.  We can do that
-  // after we've had a release out in the wild with the above logging for at
-  // least a few weeks, and seen no reports of them actually happening.
-  // Until then, conservatively keep fudging like we have for a long time.
-  const massagedEmails = emails.map(e => e.trim()).filter(Boolean);
-
-  return massagedEmails.sort().join(',');
-};
-
 export const normalizeRecipientsAsUserIds = (recipients: number[]) =>
   recipients.sort((a, b) => a - b).join(',');
 
@@ -314,9 +287,9 @@ export const isSameRecipient = (
 
   switch (message1.type) {
     case 'private':
-      return (
-        normalizeRecipients(recipientsOfPrivateMessage(message1)).toLowerCase()
-        === normalizeRecipients(recipientsOfPrivateMessage(message2)).toLowerCase()
+      return isEqual(
+        recipientsOfPrivateMessage(message1).map(r => r.id),
+        recipientsOfPrivateMessage(message2).map(r => r.id),
       );
     case 'stream':
       return (