opendev/gerrit
Code Issues Proposed changes

Add user pref to enable manual reviewing

Browse Source 
Add a checkbox to the preferences header on the diff
screen which allows a user to specify whether they
want manual-reviewing enabled or disabled.  Previously,
every file was auto marked reviewed when a user first
displayed it.  The new manual mode prevents this auto
marking and only marks a file reviewed when the user
explicitly clicks on the reviewed button.

Using the manual mode allows users to better decide
when they feel they have reviewed a file.  This mode
is particularly useful for larger reviews when a
user may need to jump around and look at many other
files while commenting on a specific one.  This
prevents their jumping around from file to file from
marking the files as reviewed.  The intention is to
allow the reviewed status to provide more meaning
than simply "I have seen the file" (which in many
cases might even be a stretch).

Change-Id: Iaedbcd717ef8a587a78c56e3d1b5f3fcc86a6d21
This commit is contained in:
Martin Fick
2011-11-23 10:30:29 -07:00
committed by Shawn O. Pearce
parent b7b202049d
commit e5490f1f5a
8 changed files with 107 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScreen.java
View File

@@ -189,6 +189,13 @@ public abstract class PatchScreen extends Screen implements
}
private void update(AccountDiffPreference dp) {
// Did the user just turn on auto-review?
if (!reviewed.getValue() && prefs.getOld().isManualReview()
&& !dp.isManualReview()) {
reviewed.setValue(true);
setReviewedByCurrentUser(true);
}
if (lastScript != null && canReuse(dp, lastScript)) {
lastScript.setDiffPrefs(dp);
RpcStatus.INSTANCE.onRpcStart(null);
@@ -450,10 +457,20 @@ public abstract class PatchScreen extends Screen implements
bottomNav.display(patchIndex, getPatchScreenType(), fileList);
}
// Mark this file reviewed as soon we display the diff screen
if (Gerrit.isSignedIn() && isFirst) {
reviewed.setValue(true);
setReviewedByCurrentUser(true /* reviewed */);
if (Gerrit.isSignedIn()) {
boolean isReviewed = false;
if (isFirst && !prefs.get().isManualReview()) {
isReviewed = true;
setReviewedByCurrentUser(isReviewed);
} else {
for (Patch p : patchSetDetail.getPatches()) {
if (p.getKey().equals(patchKey)) {
isReviewed = p.isReviewedByCurrentUser();
break;
}
}
}
reviewed.setValue(isReviewed);
}
intralineFailure = isFirst && script.hasIntralineFailure();

5
gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScriptSettingsPanel.java
View File

@@ -78,6 +78,9 @@ public class PatchScriptSettingsPanel extends Composite implements
@UiField
CheckBox showTabs;
@UiField
CheckBox manualReview;
@UiField
CheckBox skipDeleted;
@@ -212,6 +215,7 @@ public class PatchScriptSettingsPanel extends Composite implements
skipUncommented.setValue(dp.isSkipUncommented());
expandAllComments.setValue(dp.isExpandAllComments());
retainHeader.setValue(dp.isRetainHeader());
manualReview.setValue(dp.isManualReview());
}
@UiHandler("update")
@@ -243,6 +247,7 @@ public class PatchScriptSettingsPanel extends Composite implements
dp.setSkipUncommented(skipUncommented.getValue());
dp.setExpandAllComments(expandAllComments.getValue());
dp.setRetainHeader(retainHeader.getValue());
dp.setManualReview(manualReview.getValue());
listenablePrefs.set(dp);
}

13
gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScriptSettingsPanel.ui.xml
View File

@@ -147,20 +147,29 @@ limitations under the License.
</g:CheckBox>
</td>
<td valign='bottom' rowspan='2'>
<g:CheckBox
ui:field='manualReview'
text='Manual Review'
tabIndex='13'>
<ui:attribute name='text'/>
</g:CheckBox>
</td>
<td rowspan='2'>
<br/>
<g:Button
ui:field='update'
text='Update'
styleName='{style.updateButton}'
tabIndex='13'>
tabIndex='14'>
<ui:attribute name='text'/>
</g:Button>
<g:Button
ui:field='save'
text='Save'
styleName='{style.updateButton}'
tabIndex='14'>
tabIndex='15'>
<ui:attribute name='text'/>
</g:Button>
</td>

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/ListenableAccountDiffPreference.java
View File

@@ -21,7 +21,7 @@ import com.google.gerrit.reviewdb.AccountDiffPreference;
import com.google.gwtjsonrpc.client.VoidResult;
public class ListenableAccountDiffPreference
extends ListenableValue<AccountDiffPreference> {
extends ListenableOldValue<AccountDiffPreference> {
public ListenableAccountDiffPreference() {
reset();

30
gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/ListenableOldValue.java Normal file
View File

@@ -0,0 +1,30 @@
// Copyright (C) 2012 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.client.ui;
public class ListenableOldValue<T> extends ListenableValue<T> {
private T oldValue;
public T getOld() {
return oldValue;
}
public void set(final T value) {
oldValue = get();
super.set(value);
oldValue = null; // allow it to be gced before the next event
}
}

13
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountDiffPreference.java
View File

@@ -65,6 +65,7 @@ public class AccountDiffPreference {
p.setIntralineDifference(true);
p.setShowTabs(true);
p.setContext(DEFAULT_CONTEXT);
p.setManualReview(false);
return p;
}
@@ -108,6 +109,9 @@ public class AccountDiffPreference {
@Column(id = 13)
protected boolean retainHeader;
@Column(id = 14)
protected boolean manualReview;
protected AccountDiffPreference() {
}
@@ -129,6 +133,7 @@ public class AccountDiffPreference {
this.expandAllComments = p.expandAllComments;
this.context = p.context;
this.retainHeader = p.retainHeader;
this.manualReview = p.manualReview;
}
public Account.Id getAccountId() {
@@ -233,4 +238,12 @@ public class AccountDiffPreference {
public void setRetainHeader(boolean retain) {
retainHeader = retain;
}
public boolean isManualReview() {
return manualReview;
}
public void setManualReview(boolean manual) {
manualReview = manual;
}
}

2
gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
View File

@@ -32,7 +32,7 @@ import java.util.List;
/** A version of the database schema. */
public abstract class SchemaVersion {
/** The current schema version. */
public static final Class<Schema_61> C = Schema_61.class;
public static final Class<Schema_62> C = Schema_62.class;
public static class Module extends AbstractModule {
@Override

25
gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_62.java Normal file
View File

@@ -0,0 +1,25 @@
// Copyright (C) 2011 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.schema;
import com.google.inject.Inject;
import com.google.inject.Provider;
public class Schema_62 extends SchemaVersion {
@Inject
Schema_62(Provider<Schema_61> prior) {
super(prior);
}
}