Require REVIEWED option to retrieve reviewed flag from /changes/

Setting the reviewed flag in the ChangeInfo JSON requires loading
all ChangeMessage entities for the current patch set, sorting them
by the date, and filtering to find an entry by the calling user.

This information is only valid for the caller.  When viewing another
user's dashboard changes are marked bold if the *caller* has not
reviewed the change, not if the owner of the displayed dashboard has.
This is outright misleading.  On dashboards that are not the callers
highlighting is now disabled and the reviewed flag is not computed.

The reviewed flag and highlighting is not used in search results,
such as All > Open.  Computing the reviewed flag for the signed
in caller is a waste of server CPU resources.  As the queries are
performed sequentially during JSON result construction, this is
also very slow on slower database environments like gerrit-review.

Disable the recursive query logic behind an option in the request,
significantly cutting costs for common queries.

Change-Id: If8acbe430e8803e1d9cfcb747dfe28713c1b6da4

Documentation/rest-api-changes.txt

@@ -48,7 +48,6 @@ Query for open changes of watched projects:
       "status": "NEW",
       "created": "2012-07-17 07:18:30.854000000",
       "updated": "2012-07-17 07:19:27.766000000",
-      "reviewed": true,
       "mergeable": true,
       "_sortkey": "001e7057000006dc",
       "_number": 1756,
@@ -120,7 +119,6 @@ Query that retrieves changes for a user's dashboard:
         "status": "NEW",
         "created": "2012-07-17 07:18:30.854000000",
         "updated": "2012-07-17 07:19:27.766000000",
-        "reviewed": true,
         "mergeable": true,
         "_sortkey": "001e7057000006dc",
         "_number": 1756,
@@ -220,6 +218,12 @@ default. Optional fields are:
   authenticated to obtain the available actions.
 --
 
+[[reviewed]]
+--
+* `REVIEWED`: include the `reviewed` field if the caller is
+  authenticated and has commented on the current revision.
+--
+
 .Request
 ----
   GET /changes/?q=97&o=CURRENT_REVISION&o=CURRENT_COMMIT&o=CURRENT_FILES HTTP/1.0
@@ -351,7 +355,6 @@ describes the change.
     "status": "NEW",
     "created": "2013-02-01 09:59:32.126000000",
     "updated": "2013-02-21 11:16:36.775000000",
-    "reviewed": true,
     "mergeable": true,
     "_sortkey": "0023412400000f7d",
     "_number": 3965,
@@ -401,7 +404,6 @@ describes the change.
     "status": "NEW",
     "created": "2013-02-01 09:59:32.126000000",
     "updated": "2013-02-21 11:16:36.775000000",
-    "reviewed": true,
     "mergeable": true,
     "_sortkey": "0023412400000f7d",
     "_number": 3965,
@@ -640,7 +642,6 @@ describes the abandoned change.
     "status": "ABANDONED",
     "created": "2013-02-01 09:59:32.126000000",
     "updated": "2013-02-21 11:16:36.775000000",
-    "reviewed": true,
     "mergeable": true,
     "_sortkey": "0023412400000f7d",
     "_number": 3965,
@@ -699,7 +700,6 @@ describes the restored change.
     "status": "NEW",
     "created": "2013-02-01 09:59:32.126000000",
     "updated": "2013-02-21 11:16:36.775000000",
-    "reviewed": true,
     "mergeable": true,
     "_sortkey": "0023412400000f7d",
     "_number": 3965,
@@ -847,7 +847,6 @@ describes the reverting change.
     "status": "NEW",
     "created": "2013-02-01 09:59:32.126000000",
     "updated": "2013-02-21 11:16:36.775000000",
-    "reviewed": true,
     "mergeable": true,
     "_sortkey": "0023412400000f7d",
     "_number": 3965,
@@ -912,7 +911,6 @@ describes the submitted/merged change.
     "status": "MERGED",
     "created": "2013-02-01 09:59:32.126000000",
     "updated": "2013-02-21 11:16:36.775000000",
-    "reviewed": true,
     "_sortkey": "0023412400000f7d",
     "_number": 3965,
     "owner": {
@@ -1220,7 +1218,6 @@ for the current patch set.
     "status": "NEW",
     "created": "2013-02-01 09:59:32.126000000",
     "updated": "2013-02-21 11:16:36.775000000",
-    "reviewed": true,
     "mergeable": true,
     "_sortkey": "0023412400000f7d",
     "_number": 3965,
@@ -2271,7 +2268,6 @@ describes the resulting cherry picked change.
     "status": "NEW",
     "created": "2013-02-01 09:59:32.126000000",
     "updated": "2013-02-21 11:16:36.775000000",
-    "reviewed": true,
     "mergeable": true,
     "_sortkey": "0023412400000f7d",
     "_number": 3965,
@@ -2322,7 +2318,6 @@ describes the change.
     "status": "NEW",
     "created": "2013-02-01 09:59:32.126000000",
     "updated": "2013-02-21 11:16:36.775000000",
-    "reviewed": true,
     "mergeable": true,
     "_sortkey": "0023412400000f7d",
     "_number": 3965,
@@ -2503,6 +2498,7 @@ updated.
 Whether the calling user has starred this change.
 |`reviewed`           |not set if `false`|
 Whether the change was reviewed by the calling user.
+Only set if link:#reviewed[reviewed] is requested.
 |`mergeable`          |optional|
 Whether the change is mergeable. +
 Not set for merged changes.
@@ -2529,7 +2525,7 @@ The reviewers that can be removed by the calling user as a list of
 link:rest-api-accounts.html#account-info[AccountInfo] entities. +
 Only set if link:#detailed-labels[detailed labels] are requested.
 |`messages`|optional|
-Messages associated with the change as a list of 
+Messages associated with the change as a list of
 link:#change-message-info[ChangeMessageInfo] entities. +
 Only set if link:#messages[messages] are requested.
 |`current_revision`   |optional|

gerrit-common/src/main/java/com/google/gerrit/common/changes/ListChangesOption.java

@@ -40,7 +40,10 @@ public enum ListChangesOption {
   MESSAGES(9),
 
   /** Include allowed actions client could perform. */
-  CURRENT_ACTIONS(10);
+  CURRENT_ACTIONS(10),
+
+  /** Set the reviewed boolean for the caller. */
+  REVIEWED(11);
 
   private final int value;
 

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/AccountDashboardScreen.java

@@ -19,6 +19,7 @@ import com.google.gerrit.client.NotFoundScreen;
 import com.google.gerrit.client.rpc.Natives;
 import com.google.gerrit.client.rpc.ScreenLoadCallback;
 import com.google.gerrit.client.ui.Screen;
+import com.google.gerrit.common.changes.ListChangesOption;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gwt.core.client.JsArray;
 import com.google.gwt.event.dom.client.KeyPressEvent;
@@ -26,6 +27,7 @@ import com.google.gwtexpui.globalkey.client.KeyCommand;
 
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.EnumSet;
 
 public class AccountDashboardScreen extends Screen implements ChangeListScreen {
   private final Account.Id ownerId;
@@ -61,7 +63,7 @@ public class AccountDashboardScreen extends Screen implements ChangeListScreen {
 
     outgoing.setTitleText(Util.C.outgoingReviews());
     incoming.setTitleText(Util.C.incomingReviews());
-    incoming.setHighlightUnreviewed(true);
+    incoming.setHighlightUnreviewed(mine);
     closed.setTitleText(Util.C.recentlyClosed());
 
     table.addSection(outgoing);
@@ -83,6 +85,9 @@ public class AccountDashboardScreen extends Screen implements ChangeListScreen {
             display(result);
           }
         },
+        mine
+          ? EnumSet.of(ListChangesOption.REVIEWED)
+          : EnumSet.noneOf(ListChangesOption.class),
         "is:open owner:" + who,
         "is:open reviewer:" + who + " -owner:" + who,
         "is:closed owner:" + who + " -age:4w limit:10");

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeList.java

@@ -28,13 +28,18 @@ public class ChangeList extends JsArray<ChangeInfo> {
 
   /** Run 2 or more queries in a single remote invocation. */
   public static void query(
-      AsyncCallback<JsArray<ChangeList>> callback, String... queries) {
+      AsyncCallback<JsArray<ChangeList>> callback,
+      EnumSet<ListChangesOption> options,
+      String... queries) {
     assert queries.length >= 2; // At least 2 is required for correct result.
     RestApi call = new RestApi(URI);
     for (String q : queries) {
       call.addParameterRaw("q", KeyUtil.encode(q));
     }
-    addOptions(call, EnumSet.of(ListChangesOption.LABELS));
+
+    EnumSet<ListChangesOption> o = EnumSet.of(ListChangesOption.LABELS);
+    o.addAll(options);
+    addOptions(call, o);
     call.get(callback);
   }
 

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/DashboardTable.java

@@ -19,10 +19,12 @@ import com.google.gerrit.client.rpc.GerritCallback;
 import com.google.gerrit.client.rpc.Natives;
 import com.google.gerrit.client.ui.InlineHyperlink;
 import com.google.gerrit.common.PageLinks;
+import com.google.gerrit.common.changes.ListChangesOption;
 import com.google.gwt.core.client.JsArray;
 import com.google.gwt.http.client.URL;
 
 import java.util.ArrayList;
+import java.util.EnumSet;
 import java.util.List;
 import java.util.ListIterator;
 
@@ -103,6 +105,7 @@ public class DashboardTable extends ChangeTable2 {
               finishDisplay();
             }
           },
+          EnumSet.noneOf(ListChangesOption.class),
           queries.toArray(new String[queries.size()]));
     }
   }

gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java

@@ -25,6 +25,7 @@ import static com.google.gerrit.common.changes.ListChangesOption.DETAILED_ACCOUN
 import static com.google.gerrit.common.changes.ListChangesOption.DETAILED_LABELS;
 import static com.google.gerrit.common.changes.ListChangesOption.LABELS;
 import static com.google.gerrit.common.changes.ListChangesOption.MESSAGES;
+import static com.google.gerrit.common.changes.ListChangesOption.REVIEWED;
 
 import com.google.common.base.Joiner;
 import com.google.common.base.Objects;
@@ -260,7 +261,9 @@ public class ChangeJson {
     out.starred = userProvider.get().getStarredChanges().contains(in.getId())
         ? true
         : null;
-    out.reviewed = in.getStatus().isOpen() && isChangeReviewed(cd) ? true : null;
+    out.reviewed = in.getStatus().isOpen()
+        && has(REVIEWED)
+        && isChangeReviewed(cd) ? true : null;
     out.labels = labelsFor(cd, has(LABELS), has(DETAILED_LABELS));
 
     Collection<PatchSet.Id> limited = cd.getLimitedPatchSets();