Make PublishAction require ownership & visiblity
Previously, PublishAction allowed reviewers of the draft change to publish. Correct functionality should only allow the change owner to publish. In addition the change visibility has to be checked, because a change is not visible to the change owner if the project state is HIDDEN. In addition, the behaviour in PublishAction was inconsistent with the behaviour in ReviewCommand. This change makes both perform the same check before publishing. Change-Id: Ic306ab6a0dd49f23073f198c9dc8e9eaa90ecccc
This commit is contained in:
@@ -28,6 +28,7 @@ public class ChangeDetail {
|
|||||||
protected AccountInfoCache accounts;
|
protected AccountInfoCache accounts;
|
||||||
protected boolean allowsAnonymous;
|
protected boolean allowsAnonymous;
|
||||||
protected boolean canAbandon;
|
protected boolean canAbandon;
|
||||||
|
protected boolean canPublish;
|
||||||
protected boolean canRestore;
|
protected boolean canRestore;
|
||||||
protected boolean canRevert;
|
protected boolean canRevert;
|
||||||
protected boolean canDeleteDraft;
|
protected boolean canDeleteDraft;
|
||||||
@@ -71,6 +72,14 @@ public class ChangeDetail {
|
|||||||
canAbandon = a;
|
canAbandon = a;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public boolean canPublish() {
|
||||||
|
return canPublish;
|
||||||
|
}
|
||||||
|
|
||||||
|
public void setCanPublish(final boolean a) {
|
||||||
|
canPublish = a;
|
||||||
|
}
|
||||||
|
|
||||||
public boolean canRestore() {
|
public boolean canRestore() {
|
||||||
return canRestore;
|
return canRestore;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -178,7 +178,7 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel implements O
|
|||||||
populateActions(detail);
|
populateActions(detail);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (detail.getPatchSet().isDraft()) {
|
if (detail.getPatchSet().isDraft() && changeDetail.canPublish()) {
|
||||||
populatePublishAction();
|
populatePublishAction();
|
||||||
}
|
}
|
||||||
if (canDeletePatchSet(detail)) {
|
if (canDeletePatchSet(detail)) {
|
||||||
|
|||||||
@@ -122,6 +122,7 @@ public class ChangeDetailFactory extends Handler<ChangeDetail> {
|
|||||||
detail.setAllowsAnonymous(control.forUser(anonymousUser).isVisible(db));
|
detail.setAllowsAnonymous(control.forUser(anonymousUser).isVisible(db));
|
||||||
|
|
||||||
detail.setCanAbandon(change.getStatus() != Change.Status.DRAFT && change.getStatus().isOpen() && control.canAbandon());
|
detail.setCanAbandon(change.getStatus() != Change.Status.DRAFT && change.getStatus().isOpen() && control.canAbandon());
|
||||||
|
detail.setCanPublish(control.canPublish(db));
|
||||||
detail.setCanRestore(change.getStatus() == Change.Status.ABANDONED && control.canRestore());
|
detail.setCanRestore(change.getStatus() == Change.Status.ABANDONED && control.canRestore());
|
||||||
detail.setCanDeleteDraft(change.getStatus() == Change.Status.DRAFT && control.isOwner());
|
detail.setCanDeleteDraft(change.getStatus() == Change.Status.DRAFT && control.isOwner());
|
||||||
detail.setStarred(control.getCurrentUser().getStarredChanges().contains(
|
detail.setStarred(control.getCurrentUser().getStarredChanges().contains(
|
||||||
|
|||||||
@@ -60,11 +60,11 @@ class PublishAction extends Handler<ChangeDetail> {
|
|||||||
final ChangeControl changeControl =
|
final ChangeControl changeControl =
|
||||||
changeControlFactory.validateFor(changeId);
|
changeControlFactory.validateFor(changeId);
|
||||||
|
|
||||||
if (!changeControl.isOwner() && !changeControl.isVisible(db)) {
|
if (!changeControl.canPublish(db)) {
|
||||||
throw new IllegalStateException("Cannot publish patchset");
|
throw new IllegalStateException("Cannot publish patchset");
|
||||||
}
|
}
|
||||||
|
|
||||||
ChangeUtil.publishDraftPatchSet(db, patchSetId);
|
ChangeUtil.publishDraftPatchSet(db, patchSetId);
|
||||||
return changeDetailFactory.create(changeId).call();
|
return changeDetailFactory.create(changeId).call();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -189,6 +189,10 @@ public class ChangeControl {
|
|||||||
;
|
;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public boolean canPublish(final ReviewDb db) throws OrmException {
|
||||||
|
return isOwner() && isVisible(db);
|
||||||
|
}
|
||||||
|
|
||||||
/** Can this user restore this change? */
|
/** Can this user restore this change? */
|
||||||
public boolean canRestore() {
|
public boolean canRestore() {
|
||||||
return canAbandon(); // Anyone who can abandon the change can restore it back
|
return canAbandon(); // Anyone who can abandon the change can restore it back
|
||||||
@@ -526,4 +530,4 @@ public class ChangeControl {
|
|||||||
}
|
}
|
||||||
return list;
|
return list;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -248,7 +248,7 @@ public class ReviewCommand extends BaseCommand {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (publishPatchSet) {
|
if (publishPatchSet) {
|
||||||
if (changeControl.isOwner() && changeControl.isVisible(db)) {
|
if (changeControl.canPublish(db)) {
|
||||||
ChangeUtil.publishDraftPatchSet(db, patchSetId);
|
ChangeUtil.publishDraftPatchSet(db, patchSetId);
|
||||||
} else {
|
} else {
|
||||||
throw error("Not permitted to publish draft patchset");
|
throw error("Not permitted to publish draft patchset");
|
||||||
|
|||||||
Reference in New Issue
Block a user