Use user_ids as message destinations

[prah23]

There are 4 commits to this PR:

• The first commit refactors update_recipient_emails() to update_recipients(), also updating the recipients' user_ids now, which is a preparatory step for the third commit. A test has also been added for this method.
• The second commit adds a test fixture for user_id_email_dict as a prep commit for the third commit.
• The third commit migrates from using delivery emails as message destinations to using user_ids instead. Tests updated.
• The fourth commit deprecates the emails parameter from private_box_view(). Tests updated.

The abstraction between the narrow model and the Composition API type is maintained by modifying the corresponding connections to the narrow model such that the delivery emails are still in use in the narrow model currently, making this a message-destination-only change.

This change also serves as a prelude to the larger scale narrow model refactoring, which needs Model and narrow structure abstraction.

This would accommodate the first point under #965.

[preetmishra]

@prah23 Thanks for taking this up. This seems to work well and the changes look good to me as well! 👍

With user_ids in place and given that we could build back recipient_emails from the user_ids, should we drop the recipient_email attribute in boxes.py?

We could potentially squash the changes pre-merge. I have left a few minor in-line comments.

[prah23]

Thanks for the review, and the educative point about the AAA pattern, @preetmishra!
I've incorporated your suggestions and have added a 4th commit deprecating the emails parameter from private_box_view.

[preetmishra]

@prah23 Thanks for the update! The last commit is a good cleanup. We could label it as a refactor:. Overall, the PR looks good to me! 👍

There are two concerns that I would like to point out:

• When opening a saved draft with an invalid user email, the To: field renders empty.
• When sending a PM to an invalid recipient, the footer message gets updated to "Cannot send message without specifying recipients." than "Invalid recipient(s) - ...".

This is likely because you're skipping out invalid emails while creating the user_ids from the regex grepped emails in update_recipients().

[prah23]

This PR is now up-to-date @preetmishra.

Thanks to #937, both of your concerns should no longer persist since we restrict the user from sending a message and saving a draft too if the recipients are invalid. Only if the recipients are valid, they will be updated in self.recipient_user_ids and self.recipient_emails and then the relevant action will proceed.

[prah23]

I've made a couple of changes since the #937 merge, I'm leaving my thoughts on these below, please let me know if you think they need changes.

[preetmishra]

@prah23 Thanks for carrying this forward and resolving the issues that were related elsewhere! 👍 This works well locally.

@neiljp Other than the minor in-line comments, this looks good to me.

[prah23]

Thanks for the reviews, @preetmishra!
Updated as per review.

[zulipbot]

Hello @zulip/server-refactoring members, this pull request was labeled with the "area: refactoring" label, so you may want to check it out!

[neiljp]

@prah23 I made a minor test change and commit text change and am merging this now - thanks! 🎉

Other than the ongoing migration towards ids and improving the narrow structure, I expect we'll want to wrap the access to the user_id_email_dict with a method, but this refactoring is useful to have right now and we can build on this 👍