Fix crossed notify toggles for project watches
The submit and new changes buttons were cross-wired, causing the one
to impact the other's value. Fix that, and while we are at it clarify
the names involved so there is less confusion going on.
Change-Id: Ie1b082e35b7a95913a7feb92739f79220a24a983
Signed-off-by: Shawn O. Pearce <sop@google.com>
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/MyWatchesTable.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/MyWatchesTable.java
index be808fa..b3bd0cb 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/MyWatchesTable.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/MyWatchesTable.java
@@ -134,9 +134,9 @@
table.setWidget(row, 1, new CheckBox());
table.setWidget(row, 2, fp);
- addNotifyButton(AccountProjectWatch.Type.NEW_CHANGES, info, row, 3);
- addNotifyButton(AccountProjectWatch.Type.COMMENTS, info, row, 4);
- addNotifyButton(AccountProjectWatch.Type.SUBMITS, info, row, 5);
+ addNotifyButton(AccountProjectWatch.NotifyType.NEW_CHANGES, info, row, 3);
+ addNotifyButton(AccountProjectWatch.NotifyType.ALL_COMMENTS, info, row, 4);
+ addNotifyButton(AccountProjectWatch.NotifyType.SUBMITTED_CHANGES, info, row, 5);
final FlexCellFormatter fmt = table.getFlexCellFormatter();
fmt.addStyleName(row, 1, Gerrit.RESOURCES.css().iconCell());
@@ -148,7 +148,7 @@
setRowItem(row, info);
}
- protected void addNotifyButton(final AccountProjectWatch.Type type,
+ protected void addNotifyButton(final AccountProjectWatch.NotifyType type,
final AccountProjectWatchInfo info, final int row, final int col) {
final CheckBox cbox = new CheckBox();
@@ -157,13 +157,16 @@
public void onClick(final ClickEvent event) {
final boolean oldVal = info.getWatch().isNotify(type);
info.getWatch().setNotify(type, cbox.getValue());
+ cbox.setEnabled(false);
Util.ACCOUNT_SVC.updateProjectWatch(info.getWatch(),
new GerritCallback<VoidResult>() {
public void onSuccess(final VoidResult result) {
+ cbox.setEnabled(true);
}
@Override
public void onFailure(final Throwable caught) {
+ cbox.setEnabled(true);
info.getWatch().setNotify(type, oldVal);
cbox.setValue(oldVal);
super.onFailure(caught);
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountProjectWatch.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountProjectWatch.java
index 6713d8f..c18ae82 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountProjectWatch.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountProjectWatch.java
@@ -21,8 +21,8 @@
/** An {@link Account} interested in a {@link Project}. */
public final class AccountProjectWatch {
- public enum Type {
- NEW_CHANGES, SUBMITS, COMMENTS
+ public enum NotifyType {
+ NEW_CHANGES, ALL_COMMENTS, SUBMITTED_CHANGES
}
public static final String FILTER_ALL = "*";
@@ -124,46 +124,32 @@
return FILTER_ALL.equals(key.filter.get()) ? null : key.filter.get();
}
- public boolean isNotifyNewChanges() {
- return notifyNewChanges;
- }
+ public boolean isNotify(final NotifyType type) {
+ switch (type) {
+ case NEW_CHANGES:
+ return notifyNewChanges;
- public void setNotifyNewChanges(final boolean a) {
- notifyNewChanges = a;
- }
+ case ALL_COMMENTS:
+ return notifyAllComments;
- public boolean isNotifyAllComments() {
- return notifyAllComments;
- }
-
- public void setNotifyAllComments(final boolean a) {
- notifyAllComments = a;
- }
-
- public boolean isNotifySubmittedChanges() {
- return notifySubmittedChanges;
- }
-
- public void setNotifySubmittedChanges(final boolean a) {
- notifySubmittedChanges = a;
- }
-
- public boolean isNotify(final Type type) {
- switch(type) {
- case NEW_CHANGES: return notifySubmittedChanges;
- case SUBMITS: return notifyNewChanges;
- case COMMENTS: return notifyAllComments;
+ case SUBMITTED_CHANGES:
+ return notifySubmittedChanges;
}
return false;
}
- public void setNotify(final Type type, final boolean v) {
- switch(type) {
- case NEW_CHANGES: notifySubmittedChanges = v;
+ public void setNotify(final NotifyType type, final boolean v) {
+ switch (type) {
+ case NEW_CHANGES:
+ notifyNewChanges = v;
break;
- case SUBMITS: notifyNewChanges = v;
+
+ case ALL_COMMENTS:
+ notifyAllComments = v;
break;
- case COMMENTS: notifyAllComments = v;
+
+ case SUBMITTED_CHANGES:
+ notifySubmittedChanges = v;
break;
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
index 71712af..0377291 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
@@ -24,6 +24,7 @@
import com.google.gerrit.reviewdb.PatchSetApproval;
import com.google.gerrit.reviewdb.PatchSetInfo;
import com.google.gerrit.reviewdb.StarredChange;
+import com.google.gerrit.reviewdb.AccountProjectWatch.NotifyType;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListEntry;
@@ -299,7 +300,7 @@
// BCC anyone else who has interest in this project's changes
//
for (final AccountProjectWatch w : getWatches()) {
- if (w.isNotifyAllComments()) {
+ if (w.isNotify(NotifyType.ALL_COMMENTS)) {
add(RecipientType.BCC, w.getAccountId());
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/CreateChangeSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/CreateChangeSender.java
index 18bfe976..c14ff1b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/CreateChangeSender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/CreateChangeSender.java
@@ -19,6 +19,7 @@
import com.google.gerrit.reviewdb.AccountGroupMember;
import com.google.gerrit.reviewdb.AccountProjectWatch;
import com.google.gerrit.reviewdb.Change;
+import com.google.gerrit.reviewdb.AccountProjectWatch.NotifyType;
import com.google.gerrit.server.ssh.SshInfo;
import com.google.gwtorm.client.OrmException;
import com.google.inject.Inject;
@@ -61,7 +62,7 @@
// BCC anyone who has interest in this project's changes
//
for (final AccountProjectWatch w : getWatches()) {
- if (w.isNotifyNewChanges()) {
+ if (w.isNotify(NotifyType.NEW_CHANGES)) {
if (owners.contains(w.getAccountId())) {
add(RecipientType.TO, w.getAccountId());
} else {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java
index 40f4790..b2a1c44 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java
@@ -23,6 +23,7 @@
import com.google.gerrit.reviewdb.Branch;
import com.google.gerrit.reviewdb.Change;
import com.google.gerrit.reviewdb.PatchSetApproval;
+import com.google.gerrit.reviewdb.AccountProjectWatch.NotifyType;
import com.google.gwtorm.client.OrmException;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
@@ -149,7 +150,7 @@
// BCC anyone else who has interest in this project's changes
//
for (final AccountProjectWatch w : getWatches()) {
- if (w.isNotifySubmittedChanges()) {
+ if (w.isNotify(NotifyType.SUBMITTED_CHANGES)) {
add(RecipientType.BCC, w.getAccountId());
}
}
