Unify logic for validating draft deletions

This change makes validating draft deletions the same
as draft publishings.

Change-Id: I9ef00d6382c60ce28c1c002fc2c4733ac129e716
This commit is contained in:
Conley Owens
2012-02-06 08:31:47 -08:00
parent 388ba60191
commit d88539c11c
5 changed files with 21 additions and 21 deletions

View File

@@ -178,11 +178,14 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel implements O
populateActions(detail); populateActions(detail);
} }
} }
if (detail.getPatchSet().isDraft() && changeDetail.canPublish()) { if (detail.getPatchSet().isDraft()) {
populatePublishAction(); if (changeDetail.canPublish()) {
} populatePublishAction();
if (canDeletePatchSet(detail)) { }
populateDeleteDraftPatchSetAction(); if (changeDetail.canDeleteDraft() &&
changeDetail.getPatchSets().size() > 1) {
populateDeleteDraftPatchSetAction();
}
} }
} }
populateDiffAllActions(detail); populateDiffAllActions(detail);
@@ -708,17 +711,6 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel implements O
changeScreen.update(result); changeScreen.update(result);
} }
private boolean canDeletePatchSet(PatchSetDetail detail) {
if (!detail.getPatchSet().isDraft()) {
return false;
}
// If the draft PS is the only one in a draft change, just delete the change.
if (changeDetail.getPatchSets().size() <= 1) {
return false;
}
return true;
}
public PatchSet getPatchSet() { public PatchSet getPatchSet() {
return patchSet; return patchSet;
} }

View File

@@ -124,7 +124,7 @@ public class ChangeDetailFactory extends Handler<ChangeDetail> {
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.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(control.canDelete(db));
detail.setStarred(control.getCurrentUser().getStarredChanges().contains( detail.setStarred(control.getCurrentUser().getStarredChanges().contains(
changeId)); changeId));

View File

@@ -61,11 +61,11 @@ class DeleteDraftChange extends Handler<VoidResult> {
final Change.Id changeId = patchSetId.getParentKey(); final Change.Id changeId = patchSetId.getParentKey();
final ChangeControl control = changeControlFactory.validateFor(changeId); final ChangeControl control = changeControlFactory.validateFor(changeId);
if (!control.isOwner()) { if (!control.canDelete(db)) {
throw new NoSuchChangeException(changeId); throw new NoSuchChangeException(changeId);
} }
ChangeUtil.deleteDraftChange(patchSetId, gitManager, replication, db); ChangeUtil.deleteDraftChange(patchSetId, gitManager, replication, db);
return VoidResult.INSTANCE; return VoidResult.INSTANCE;
} }
} }

View File

@@ -189,8 +189,16 @@ public class ChangeControl {
; ;
} }
/** Can this user publish this change? */
public boolean canPublish(final ReviewDb db) throws OrmException { public boolean canPublish(final ReviewDb db) throws OrmException {
return isOwner() && isVisible(db); return change.getStatus() == Change.Status.DRAFT && isOwner()
&& isVisible(db);
}
/** Can this user delete this change? */
public boolean canDelete(final ReviewDb db) throws OrmException {
return change.getStatus() == Change.Status.DRAFT && isOwner()
&& isVisible(db);
} }
/** Can this user restore this change? */ /** Can this user restore this change? */

View File

@@ -255,7 +255,7 @@ public class ReviewCommand extends BaseCommand {
ReviewResult result = publishDraftFactory.create(patchSetId).call(); ReviewResult result = publishDraftFactory.create(patchSetId).call();
handleReviewResultErrors(result); handleReviewResultErrors(result);
} else if (deleteDraftPatchSet) { } else if (deleteDraftPatchSet) {
if (changeControl.isOwner() && changeControl.isVisible(db)) { if (changeControl.canDelete(db)) {
try { try {
ChangeUtil.deleteDraftPatchSet(patchSetId, gitManager, replication, patchSetInfoFactory, db); ChangeUtil.deleteDraftPatchSet(patchSetId, gitManager, replication, patchSetInfoFactory, db);
} catch (PatchSetInfoNotAvailableException e) { } catch (PatchSetInfoNotAvailableException e) {