commit acc4f6a8b3d06e48146c1c30da9d2d9c7fbc3f35
author Edwin Kempin <ekempin@google.com> Thu Feb 01 09:15:53 2024 +0000
committer Edwin Kempin <ekempin@google.com> Fri Feb 02 09:17:03 2024 +0000
tree 072d25873739ec51ad0ce6a9223b730734b955bb
parent 2bdf49a73d6c7c5d0c51ac913e7248b65f7f65fb

Return reviewer updates for reviewers by email in change details (part 1/2)

ChangeInfo that is returned to the frontend when it requests change
details contains reviewer updates as ReviewerUpdateInfo's.

The frontend relies on getting ReviewerUpdateInfo to show reviewer
updates in the Change Log (the change messages that Gerrit adds for
reviewer updates are filtered out since they are tagged with
"autogenerated:...").

So far Gerrit only returns reviewer updates for reviewers that are
Gerrit accounts but not for reviewers by email. Due to this updates
about reviewers by email are missing in the Change Log.

Reviewer updates are recorded in NoteDb and ChangeNotesParser parses
them from there. At the moment ChangeNotesParser only creates
ReviewerStatusUpdate's for reviewers that are Gerrit accounts (method
parseReviewer), but not for reviewers by email (method
parseReviewerByEmail).

In NoteDb reviewers that are Gerrit accounts are recorded as account
IDs and reviewers by email are recorded as Address strings, consisting
out of a full name and an email ("Full Name <full.name@example.com>")
or only an email ("Full Name <full.name@example.com>").

ReviewerStatusUpdate currently refers to the reviewer by account ID
(field reviewer). To be able to also refer to reviewers by email, we
make the account ID optional (we change the type of reviewer field from
Account.Id to Optional<Account.Id>) and we add a new optional field for
the reviewer address (we add a new reviewerByEmail field of type
Optional<Address>).

ReviewerStatusUpdate's are a part of ChangeNotesState that is converted
into a proto to be cached. This means the changes to
ReviewerStatusUpdate must be reflected in the ReviewerStatusUpdateProto
proto.

In the ChangeNotesStateProto optional fields are accompanied by a "has"
boolean field that tells whether the optional field has been set (see
comment on ChangeNotesStateProto). This means to make the
ReviewerStatusUpdateProto.reviewer field optional we add a boolean
ReviewerStatusUpdateProto.has_reviewer field. For storing reviewers by
email we add a ReviewerStatusUpdateProto.reviewer_by_email field and a
ReviewerStatusUpdateProto.has_reviewer_by_email field.
ChangeNoteState.toReviewerStatusUpdateProto takes care to populate the
new fields.

When rolling out this change at Google we must ensure that:
1. the new code can read old protos
2. there are no new protos that the old code cannot read, until all
   datacenters have been updated with the new code

1. is ensured in ChangeNotesState.toReviewerStatusUpdateList that reads
the reviewer field if neither has_reviewer_by_email nor has_reviewer
have been set, which is the case for old protos.

2. is ensured by not creating any ReviewerStatusUpdate for reviewers by
email yet (see code in ChangeNotesParser.parseReviewerByEmail that is
commented out). This means all ReviewerStatusUpdate's still have the
reviewer field set which is unconditionally read by the old code.

Creating ReviewerStatusUpdate's for reviewers by email will be done by
the follow-up change (uncomment the code in
ChangeNotesParser.parseReviewerByEmail). That change must only be
submitted after this change has been rolled out to all datacenters, so
that all datacenters have the new code and can read the new protos.

Finally, ChangeJson is updated to be able to emit ReviewerUpdateInfo's
for reviewer updates that are done for reviewers by email, and hence
have no account ID. ChangeJson already had code to create AccountInfo's
for reviewers by email that only have the name and email set, but no
account ID. We refactored this a bit so that we can use this code to
create the AccountInfo's in ReviewerUpdateInfo for reviewer updates that
are done for reviewers by email.

A new test in ChangeReviewersByEmailIT, checks that still no reviewer
updates for reviewers by email are returned, but the assertions for when
we can enable this are already prepared as comments. The new test
follows the example of the test that checks reviewer updates for
reviewers that are Gerrit accounts (see
ChangeReviewersIT.addReviewerToReviewerUpdateInfo()).

Release-Notes: skip
Bug: Google b/240676565
Forward-Compatible: checked
Change-Id: If1dfb3c710b5fbd9babddc669cfc85020dab14fd
Signed-off-by: Edwin Kempin <ekempin@google.com>