Merge changes from topic "issue-8381" into stable-2.14
* changes: IndexChangeIT: Test index of change after its owner loses access ChangeData: Prevent false 'Merge Conflict' for user's open review
This commit is contained in:
commit
0dfab17b69
@ -14,7 +14,21 @@
|
||||
|
||||
package com.google.gerrit.acceptance.rest.change;
|
||||
|
||||
import static com.google.common.truth.Truth.assertThat;
|
||||
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
|
||||
|
||||
import com.google.gerrit.acceptance.AbstractDaemonTest;
|
||||
import com.google.gerrit.acceptance.PushOneCommit;
|
||||
import com.google.gerrit.acceptance.TestAccount;
|
||||
import com.google.gerrit.common.data.Permission;
|
||||
import com.google.gerrit.extensions.common.ChangeInfo;
|
||||
import com.google.gerrit.reviewdb.client.AccountGroup;
|
||||
import com.google.gerrit.reviewdb.client.Project;
|
||||
import com.google.gerrit.server.git.ProjectConfig;
|
||||
import com.google.gerrit.server.project.Util;
|
||||
import java.util.List;
|
||||
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
|
||||
import org.eclipse.jgit.junit.TestRepository;
|
||||
import org.junit.Test;
|
||||
|
||||
public class IndexChangeIT extends AbstractDaemonTest {
|
||||
@ -30,4 +44,62 @@ public class IndexChangeIT extends AbstractDaemonTest {
|
||||
blockRead("refs/heads/master");
|
||||
userRestSession.post("/changes/" + changeId + "/index/").assertNotFound();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void indexChangeAfterOwnerLosesVisibility() throws Exception {
|
||||
// Create a test group with 2 users as members
|
||||
TestAccount user2 = accounts.user2();
|
||||
String group = createGroup("test");
|
||||
gApi.groups().id(group).addMembers("admin", "user", user2.username);
|
||||
|
||||
// Create a project and restrict its visibility to the group
|
||||
Project.NameKey p = createProject("p");
|
||||
ProjectConfig cfg = projectCache.checkedGet(p).getConfig();
|
||||
Util.allow(
|
||||
cfg,
|
||||
Permission.READ,
|
||||
groupCache.get(new AccountGroup.NameKey(group)).getGroupUUID(),
|
||||
"refs/*");
|
||||
Util.block(cfg, Permission.READ, REGISTERED_USERS, "refs/*");
|
||||
saveProjectConfig(p, cfg);
|
||||
|
||||
// Clone it and push a change as a regular user
|
||||
TestRepository<InMemoryRepository> repo = cloneProject(p, user);
|
||||
PushOneCommit push = pushFactory.create(db, user.getIdent(), repo);
|
||||
PushOneCommit.Result result = push.to("refs/for/master");
|
||||
result.assertOkStatus();
|
||||
assertThat(result.getChange().change().getOwner()).isEqualTo(user.id);
|
||||
String changeId = result.getChangeId();
|
||||
|
||||
// User can see the change and it is mergeable
|
||||
setApiUser(user);
|
||||
List<ChangeInfo> changes = gApi.changes().query(changeId).get();
|
||||
assertThat(changes).hasSize(1);
|
||||
assertThat(changes.get(0).mergeable).isNotNull();
|
||||
|
||||
// Other user can see the change and it is mergeable
|
||||
setApiUser(user2);
|
||||
changes = gApi.changes().query(changeId).get();
|
||||
assertThat(changes).hasSize(1);
|
||||
assertThat(changes.get(0).mergeable).isTrue();
|
||||
|
||||
// Remove the user from the group so they can no longer see the project
|
||||
setApiUser(admin);
|
||||
gApi.groups().id(group).removeMembers("user");
|
||||
|
||||
// User can no longer see the change
|
||||
setApiUser(user);
|
||||
changes = gApi.changes().query(changeId).get();
|
||||
assertThat(changes).isEmpty();
|
||||
|
||||
// Reindex the change
|
||||
setApiUser(admin);
|
||||
gApi.changes().id(changeId).index();
|
||||
|
||||
// Other user can still see the change and it is still mergeable
|
||||
setApiUser(user2);
|
||||
changes = gApi.changes().query(changeId).get();
|
||||
assertThat(changes).hasSize(1);
|
||||
assertThat(changes.get(0).mergeable).isTrue();
|
||||
}
|
||||
}
|
||||
|
@ -1071,7 +1071,8 @@ public class ChangeData {
|
||||
}
|
||||
PatchSet ps = currentPatchSet();
|
||||
try {
|
||||
if (ps == null || !changeControl().isPatchVisible(ps, db)) {
|
||||
if (ps == null
|
||||
|| (!changeControl().isOwner() && !changeControl().isPatchVisible(ps, db))) {
|
||||
return null;
|
||||
}
|
||||
} catch (OrmException e) {
|
||||
|
Loading…
Reference in New Issue
Block a user