opendev/gerrit
Code Issues Proposed changes

Disallow creating change edits on closed changes

Browse Source 
Change-Id: If5aed6dbbc77ea51d83c9aab0cc8afd32b9d1973
This commit is contained in:
Changcheng Xiao
2018-04-05 17:58:05 +02:00
parent 87f3ac3a8f
commit b08c27c6e9
3 changed files with 46 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
java/com/google/gerrit/server/edit/ChangeEditModifier.java
View File

@@ -398,6 +398,14 @@ public class ChangeEditModifier {
if (!currentUser.get().isIdentifiedUser()) {
throw new AuthException("Authentication required");
}
Change c = notes.getChange();
if (!c.getStatus().isOpen()) {
throw new ResourceConflictException(
String.format(
"change %s is %s", c.getChangeId(), c.getStatus().toString().toLowerCase()));
}
try {
permissionBackend
.currentUser()

21
javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
View File

@@ -706,6 +706,27 @@ public class ChangeEditIT extends AbstractDaemonTest {
createEmptyEditFor(r1.getChangeId());
}
@Test
public void editCannotBeCreatedOnMergedChange() throws Exception {
ChangeInfo change = gApi.changes().id(changeId).get();
gApi.changes().id(changeId).current().review(ReviewInput.approve());
gApi.changes().id(changeId).current().submit();
exception.expect(ResourceConflictException.class);
exception.expectMessage(String.format("change %s is merged", change._number));
createArbitraryEditFor(changeId);
}
@Test
public void editCannotBeCreatedOnAbandonedChange() throws Exception {
ChangeInfo change = gApi.changes().id(changeId).get();
gApi.changes().id(changeId).abandon();
exception.expect(ResourceConflictException.class);
exception.expectMessage(String.format("change %s is abandoned", change._number));
createArbitraryEditFor(changeId);
}
private void createArbitraryEditFor(String changeId) throws Exception {
createEmptyEditFor(changeId);
arbitrarilyModifyEditOf(changeId);

32
javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
View File

@@ -254,7 +254,7 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
public void uploadPackSubsetOfBranchesVisibleWithEdit() throws Exception {
allow("refs/heads/master", Permission.READ, REGISTERED_USERS);
Change c = notesFactory.createChecked(db, project, c1.getId()).getChange();
Change c = notesFactory.createChecked(db, project, c3.getId()).getChange();
String changeId = c.getKey().get();
// Admin's edit is not visible.
@@ -273,7 +273,8 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
r3 + "meta",
"refs/heads/master",
"refs/tags/master-tag",
"refs/users/01/1000001/edit-" + c1.getId() + "/1");
"refs/tags/branch-tag",
"refs/users/01/1000001/edit-" + c3.getId() + "/1");
}
@Test
@@ -281,21 +282,21 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
allow("refs/heads/master", Permission.READ, REGISTERED_USERS);
allow("refs/*", Permission.VIEW_PRIVATE_CHANGES, REGISTERED_USERS);
Change change1 = notesFactory.createChecked(db, project, c1.getId()).getChange();
String changeId1 = change1.getKey().get();
Change change2 = notesFactory.createChecked(db, project, c2.getId()).getChange();
String changeId2 = change2.getKey().get();
Change change3 = notesFactory.createChecked(db, project, c3.getId()).getChange();
String changeId3 = change3.getKey().get();
Change change4 = notesFactory.createChecked(db, project, c4.getId()).getChange();
String changeId4 = change4.getKey().get();
// Admin's edit on change1 is visible.
// Admin's edit on change3 is visible.
setApiUser(admin);
gApi.changes().id(changeId1).edit().create();
gApi.changes().id(changeId3).edit().create();
// Admin's edit on change2 is not visible since user cannot see the change.
gApi.changes().id(changeId2).edit().create();
// Admin's edit on change4 is not visible since user cannot see the change.
gApi.changes().id(changeId4).edit().create();
// User's edit is visible.
setApiUser(user);
gApi.changes().id(changeId1).edit().create();
gApi.changes().id(changeId3).edit().create();
assertUploadPackRefs(
"HEAD",
@@ -305,8 +306,9 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
r3 + "meta",
"refs/heads/master",
"refs/tags/master-tag",
"refs/users/00/1000000/edit-" + c1.getId() + "/1",
"refs/users/01/1000001/edit-" + c1.getId() + "/1");
"refs/tags/branch-tag",
"refs/users/00/1000000/edit-" + c3.getId() + "/1",
"refs/users/01/1000001/edit-" + c3.getId() + "/1");
}
@Test
@@ -315,7 +317,7 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
deny("refs/heads/master", Permission.READ, REGISTERED_USERS);
allow("refs/heads/branch", Permission.READ, REGISTERED_USERS);
String changeId = c1.change().getKey().get();
String changeId = c3.change().getKey().get();
setApiUser(admin);
gApi.changes().id(changeId).edit().create();
setApiUser(user);
@@ -336,7 +338,7 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
// See comment in subsetOfBranchesVisibleNotIncludingHead.
"refs/tags/master-tag",
// All edits are visible due to accessDatabase capability.
"refs/users/00/1000000/edit-" + c1.getId() + "/1");
"refs/users/00/1000000/edit-" + c3.getId() + "/1");
}
@Test