commit 8eddd8975e7c7715e03c55290c8f60c4eb9845e3
author David Ostrovsky <david@ostrovsky.org> Mon Mar 06 08:31:55 2017 +0100
committer David Ostrovsky <david@ostrovsky.org> Mon Mar 06 08:54:07 2017 +0100
tree c2e5785d6de83dcb04afbdab761d82523d7253bd
parent 9eb3b2fd863effb51ebb30b062c1027c90564152

Revert "Always use REVIEW state in ReviewDb"

ReviewDb persistence layer doesn't provide any means to store internal
"reviewer" or "cc" state, like it is possible with NoteDb. Instead,
in ReviewDb code path, the voted reviewers were assigned "reviewer"
state and non voted reviewers were assigned "cc" state. This is
important for implementation of notification channel, that should reach
only specific reviewer state and not all reviewers.

The most prominent examples for this differentiation is adding reviewer
and upload new patch set use cases. Power users and maintainers are
often added to very high amount of changes as reviewers. Not for all of
them they do really care. Until actually voting on those changes
notifications on these events shouldn't be sent to non voted reviewers.

To prevent this, in the mentioned use cases only voted reviewers should
be notified. NoteDb doesn't provide this feature. In NoteDb code path,
the voted reviewer state was never implemented. Instead of promoting any
reviewer state in NoteDb code path to voted reviewer state only, to
provide the battle proven notification firehouse experience in ReviewDb
code path, since Ib9a1f40b69e the internal voted reviewer state was
demoted in ReviewDb code path from voted reviewer state to any state:
voted or non voted reviewer on a change.

This broke all caller sites in ReviewDb code path for:

  changeData.reviewers().byState(ReviewerStateInternal.REVIEWER)

that expected to get a reviewer with non-zero votes. Those caller
sites were not adjusted. The comments are still claiming:

  /** Users who have non-zero approval codes on the change. */

Particularly, this broke "Add Reviewer" and "Upload new patch set" use
cases, that started to add all existing reviewers and not only voted
users to CC for notification for these use cases.

In 2.13 release this change was even not justified, as NoteDb is not
ready for production use anyway. It could be done in one or another form
in later releases, with announcement or, better, with offering of viable
alternative like Work-In-Progress workflow implementation, where users
can have fine granular control of whether notifications are sent or not.

This reverts commit e8da8bfbc3fb853e4a10a34820e1f60527eab093.

Bug: Issue 5190
Change-Id: I1d67e36882b1b4b31d2c2e1a34ce5bbfe5ee733c