tree 354215cea0c1ea63c2210369b92c936f610d1800
parent 76079daca916a5530a5c7f4532a73511716d96c4
author Edwin Kempin <ekempin@google.com> 1610355211 +0100
committer Edwin Kempin <ekempin@google.com> 1610719959 +0100

CodeOwnersOnPostReview: Fix handling of unchanged code owner approval

The detection of unchanged approvals in CodeOwnersOnPostReview didn't
work. If a label is unchanged its value in the oldApprovals map is null
(a key for the label name is present, but its value is null), but so far
we wrongly checked that no key is present.

Due to this we wrongly extended the change message if the exact same
code owner approval was applied again. In this case the change message
said that by reapplying the vote the files were now code-owner approved
by the user, which wrongly implied that they were not code-owner
approved by the user before.

If a vote is reapplied Gerrit core omits it from the change message:
a) Code-Review+1 is voted when Code-Review+1 is already present:
   no change message is added
b) Code-Review+1 and Other+1 are voted when Code-Review+1 is already
   present:
   the change message says that Other+1 was added, but doesn't mention
   Code-Review+1
c) Code-Review+1 is voted and a comment is added when Code-Review+1 is
   already present:
   the change message says that a comment was added, but doesn't mention
   Code-Review+1

If the reapplying of the label is not mentioned by Gerrit core, we
shouldn't extend the change message in this case.

I considered that the change message should say that the files are still
approved (as we do when an approval is upgraded, e.g. from Code-Review+1
to Code-Review+2), but doing this is:
a) confusing if at the same time Gerrit doesn't even mentioned that this
   vote was applied
b) not possible with the current extension point, as it doesn't allow us
   to distinguish between the user applying the same code-owner approval
   again and the user applying an unrelated approval and not touching
   the existing code-owner approval (in both cases oldApprovals would
   contain null for the code owner approval, and approvals would contain
   its current value).

This increases the test coverage for CodeOwnersOnPostReview from 91.2%
to 92.2%.

Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I9ac2e188139710e769b79f04fba924fcb45aaec5