Merge "Add temporary api for interaction with find-owners plugin"
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/ChangedFiles.java b/java/com/google/gerrit/plugins/codeowners/backend/ChangedFiles.java
index 4fdab65..c96af17 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/ChangedFiles.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/ChangedFiles.java
@@ -200,10 +200,14 @@
RevCommit baseCommit = revCommit.getParentCount() > 0 ? revCommit.getParent(0) : null;
logger.atFine().log("baseCommit = %s", baseCommit != null ? baseCommit.name() : "n/a");
+ // Detecting renames is expensive (since it requires Git to load and compare file contents of
+ // added and deleted files) and can significantly increase the latency for changes that touch
+ // large files. To avoid this latency we do not enable the rename detection on the
+ // DiffFormater. As a result of this renamed files will be returned as 2 ChangedFile's, one
+ // for the deletion of the old path and one for the addition of the new path.
try (DiffFormatter diffFormatter = new DiffFormatter(DisabledOutputStream.INSTANCE)) {
diffFormatter.setReader(revWalk.getObjectReader(), repoConfig);
diffFormatter.setDiffComparator(RawTextComparator.DEFAULT);
- diffFormatter.setDetectRenames(true);
List<DiffEntry> diffEntries = diffFormatter.scan(baseCommit, revCommit);
ImmutableSet<ChangedFile> changedFiles =
diffEntries.stream().map(ChangedFile::create).collect(toImmutableSet());
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
index 8361d9b..1c5e87f 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
@@ -122,30 +122,39 @@
* @param changeNotes the change notes for which the owned files should be returned
* @param patchSet the patch set for which the owned files should be returned
* @param accountId account ID of the code owner for which the owned files should be returned
+ * @param limit the max number of owned paths that should be returned (0 = unlimited)
* @return the paths of the files in the given patch set that are owned by the specified account
* @throws ResourceConflictException if the destination branch of the change no longer exists
*/
public ImmutableList<Path> getOwnedPaths(
- ChangeNotes changeNotes, PatchSet patchSet, Account.Id accountId)
+ ChangeNotes changeNotes, PatchSet patchSet, Account.Id accountId, int limit)
throws ResourceConflictException {
try (Timer0.Context ctx = codeOwnerMetrics.computeOwnedPaths.start()) {
logger.atFine().log(
- "compute owned paths for account %d (project = %s, change = %d, patch set = %d)",
+ "compute owned paths for account %d (project = %s, change = %d, patch set = %d,"
+ + " limit = %d)",
accountId.get(),
changeNotes.getProjectName(),
changeNotes.getChangeId().get(),
- patchSet.id().get());
- return getFileStatusesForAccount(changeNotes, patchSet, accountId)
- .flatMap(
- fileCodeOwnerStatus ->
- Stream.of(
- fileCodeOwnerStatus.newPathStatus(), fileCodeOwnerStatus.oldPathStatus())
- .filter(Optional::isPresent)
- .map(Optional::get))
- .filter(pathCodeOwnerStatus -> pathCodeOwnerStatus.status() == CodeOwnerStatus.APPROVED)
- .map(PathCodeOwnerStatus::path)
- .sorted(comparing(Path::toString))
- .collect(toImmutableList());
+ patchSet.id().get(),
+ limit);
+ Stream<Path> ownedPaths =
+ getFileStatusesForAccount(changeNotes, patchSet, accountId)
+ .flatMap(
+ fileCodeOwnerStatus ->
+ Stream.of(
+ fileCodeOwnerStatus.newPathStatus(),
+ fileCodeOwnerStatus.oldPathStatus())
+ .filter(Optional::isPresent)
+ .map(Optional::get))
+ .filter(
+ pathCodeOwnerStatus -> pathCodeOwnerStatus.status() == CodeOwnerStatus.APPROVED)
+ .map(PathCodeOwnerStatus::path)
+ .sorted(comparing(Path::toString));
+ if (limit > 0) {
+ ownedPaths = ownedPaths.limit(limit);
+ }
+ return ownedPaths.collect(toImmutableList());
} catch (IOException | PatchListNotAvailableException e) {
throw new CodeOwnersInternalServerErrorException(
String.format(
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersOnAddReviewer.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersOnAddReviewer.java
index 9352e97..e6a55ee 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersOnAddReviewer.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersOnAddReviewer.java
@@ -96,8 +96,8 @@
CodeOwnersPluginConfigSnapshot codeOwnersConfig =
codeOwnersPluginConfiguration.getProjectConfig(projectName);
- if (codeOwnersConfig.isDisabled(event.getChange().branch)
- || codeOwnersConfig.getMaxPathsInChangeMessages() <= 0) {
+ int maxPathsInChangeMessages = codeOwnersConfig.getMaxPathsInChangeMessages();
+ if (codeOwnersConfig.isDisabled(event.getChange().branch) || maxPathsInChangeMessages <= 0) {
return;
}
@@ -108,7 +108,8 @@
updateFactory -> {
try (BatchUpdate batchUpdate =
updateFactory.create(projectName, userProvider.get(), TimeUtil.nowTs())) {
- batchUpdate.addOp(changeId, new Op(event.getReviewers()));
+ batchUpdate.addOp(
+ changeId, new Op(event.getReviewers(), maxPathsInChangeMessages));
batchUpdate.execute();
}
return null;
@@ -124,9 +125,11 @@
private class Op implements BatchUpdateOp {
private final List<AccountInfo> reviewers;
+ private final int limit;
- Op(List<AccountInfo> reviewers) {
+ Op(List<AccountInfo> reviewers, int limit) {
this.reviewers = reviewers;
+ this.limit = limit;
}
@Override
@@ -158,9 +161,10 @@
ImmutableList<Path> ownedPaths;
try {
+ // limit + 1, so that we can show an indicator if there are more than <limit> files.
ownedPaths =
codeOwnerApprovalCheck.getOwnedPaths(
- changeNotes, changeNotes.getCurrentPatchSet(), reviewerAccountId);
+ changeNotes, changeNotes.getCurrentPatchSet(), reviewerAccountId, limit + 1);
} catch (RestApiException e) {
logger.atFine().withCause(e).log(
"Couldn't compute owned paths of change %s for account %s",
@@ -181,15 +185,11 @@
"%s who was added as reviewer owns the following files:\n",
reviewerAccount.getName()));
- int maxPathsInChangeMessage =
- codeOwnersPluginConfiguration.getProjectConfig(projectName).getMaxPathsInChangeMessages();
- if (ownedPaths.size() <= maxPathsInChangeMessage) {
+ if (ownedPaths.size() <= limit) {
appendPaths(message, ownedPaths.stream());
} else {
- // -1 so that we never show "(1 more files)"
- int limit = maxPathsInChangeMessage - 1;
appendPaths(message, ownedPaths.stream().limit(limit));
- message.append(String.format("(%s more files)\n", ownedPaths.size() - limit));
+ message.append("(more files)\n");
}
return Optional.of(message.toString());
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerApproval.java b/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerApproval.java
index 101cedd..e9e2c39 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerApproval.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerApproval.java
@@ -77,7 +77,9 @@
Map<String, Short> approvals) {
CodeOwnersPluginConfigSnapshot codeOwnersConfig =
codeOwnersPluginConfiguration.getProjectConfig(changeNotes.getProjectName());
- if (codeOwnersConfig.isDisabled(changeNotes.getChange().getDest().branch())) {
+ int maxPathsInChangeMessage = codeOwnersConfig.getMaxPathsInChangeMessages();
+ if (codeOwnersConfig.isDisabled(changeNotes.getChange().getDest().branch())
+ || maxPathsInChangeMessage <= 0) {
return Optional.empty();
}
@@ -97,7 +99,13 @@
try (Timer0.Context ctx = codeOwnerMetrics.extendChangeMessageOnPostReview.start()) {
return buildMessageForCodeOwnerApproval(
- user, changeNotes, patchSet, oldApprovals, approvals, requiredApproval);
+ user,
+ changeNotes,
+ patchSet,
+ oldApprovals,
+ approvals,
+ requiredApproval,
+ maxPathsInChangeMessage);
}
}
@@ -107,21 +115,16 @@
PatchSet patchSet,
Map<String, Short> oldApprovals,
Map<String, Short> approvals,
- RequiredApproval requiredApproval) {
- CodeOwnersPluginConfigSnapshot codeOwnersConfig =
- codeOwnersPluginConfiguration.getProjectConfig(changeNotes.getProjectName());
- int maxPathsInChangeMessage = codeOwnersConfig.getMaxPathsInChangeMessages();
- if (maxPathsInChangeMessage <= 0) {
- return Optional.empty();
- }
-
+ RequiredApproval requiredApproval,
+ int limit) {
LabelVote newVote = getNewVote(requiredApproval, approvals);
ImmutableList<Path> ownedPaths;
try {
+ // limit + 1, so that we can show an indicator if there are more than <limit> files.
ownedPaths =
codeOwnerApprovalCheck.getOwnedPaths(
- changeNotes, changeNotes.getCurrentPatchSet(), user.getAccountId());
+ changeNotes, changeNotes.getCurrentPatchSet(), user.getAccountId(), limit + 1);
} catch (RestApiException e) {
logger.atFine().withCause(e).log(
"Couldn't compute owned paths of change %s for account %s",
@@ -147,7 +150,9 @@
}
boolean hasImplicitApprovalByUser =
- codeOwnersConfig.areImplicitApprovalsEnabled()
+ codeOwnersPluginConfiguration
+ .getProjectConfig(changeNotes.getProjectName())
+ .areImplicitApprovalsEnabled()
&& patchSet.uploader().equals(user.getAccountId());
boolean noLongerExplicitlyApproved = false;
@@ -213,13 +218,11 @@
return Optional.empty();
}
- if (ownedPaths.size() <= maxPathsInChangeMessage) {
+ if (ownedPaths.size() <= limit) {
appendPaths(message, ownedPaths.stream());
} else {
- // -1 so that we never show "(1 more files)"
- int limit = maxPathsInChangeMessage - 1;
appendPaths(message, ownedPaths.stream().limit(limit));
- message.append(String.format("(%s more files)\n", ownedPaths.size() - limit));
+ message.append("(more files)\n");
}
if (hasImplicitApprovalByUser && noLongerExplicitlyApproved) {
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/GetOwnedPaths.java b/java/com/google/gerrit/plugins/codeowners/restapi/GetOwnedPaths.java
index 305b31a..0430f94 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/GetOwnedPaths.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/GetOwnedPaths.java
@@ -66,7 +66,7 @@
ImmutableList<Path> ownedPaths =
codeOwnerApprovalCheck.getOwnedPaths(
- revisionResource.getNotes(), revisionResource.getPatchSet(), accountId);
+ revisionResource.getNotes(), revisionResource.getPatchSet(), accountId, /* limit= */ 0);
OwnedPathsInfo ownedPathsInfo = new OwnedPathsInfo();
ownedPathsInfo.ownedPaths = ownedPaths.stream().map(Path::toString).collect(toImmutableList());
diff --git a/java/com/google/gerrit/plugins/codeowners/testing/ChangedFileSubject.java b/java/com/google/gerrit/plugins/codeowners/testing/ChangedFileSubject.java
index 805eccd..069122c 100644
--- a/java/com/google/gerrit/plugins/codeowners/testing/ChangedFileSubject.java
+++ b/java/com/google/gerrit/plugins/codeowners/testing/ChangedFileSubject.java
@@ -17,10 +17,13 @@
import static com.google.common.truth.Truth.assertAbout;
import static com.google.gerrit.truth.OptionalSubject.optionals;
+import com.google.common.collect.ImmutableList;
import com.google.common.truth.FailureMetadata;
import com.google.common.truth.Subject;
import com.google.gerrit.plugins.codeowners.common.ChangedFile;
+import com.google.gerrit.truth.ListSubject;
import com.google.gerrit.truth.OptionalSubject;
+import java.util.Collection;
/** {@link Subject} for doing assertions on {@link ChangedFile}s. */
public class ChangedFileSubject extends Subject {
@@ -34,6 +37,12 @@
return assertAbout(changedFiles()).that(changedFile);
}
+ /** Starts fluent chain to do assertions on a collection of {@link ChangedFile}s. */
+ public static ListSubject<ChangedFileSubject, ChangedFile> assertThatCollection(
+ Collection<ChangedFile> changedFiles) {
+ return ListSubject.assertThat(ImmutableList.copyOf(changedFiles), changedFiles());
+ }
+
/** Creates subject factory for mapping {@link ChangedFile}s to {@link ChangedFileSubject}s. */
public static Subject.Factory<ChangedFileSubject, ChangedFile> changedFiles() {
return ChangedFileSubject::new;
diff --git a/java/com/google/gerrit/plugins/codeowners/testing/FileCodeOwnerStatusSubject.java b/java/com/google/gerrit/plugins/codeowners/testing/FileCodeOwnerStatusSubject.java
index 03948f1..3a81908 100644
--- a/java/com/google/gerrit/plugins/codeowners/testing/FileCodeOwnerStatusSubject.java
+++ b/java/com/google/gerrit/plugins/codeowners/testing/FileCodeOwnerStatusSubject.java
@@ -20,11 +20,13 @@
import static com.google.gerrit.plugins.codeowners.testing.PathCodeOwnerStatusSubject.pathCodeOwnerStatuses;
import static com.google.gerrit.truth.OptionalSubject.optionals;
+import com.google.common.collect.ImmutableList;
import com.google.common.truth.FailureMetadata;
import com.google.common.truth.Subject;
import com.google.gerrit.plugins.codeowners.backend.FileCodeOwnerStatus;
import com.google.gerrit.truth.ListSubject;
import com.google.gerrit.truth.OptionalSubject;
+import java.util.Collection;
import java.util.stream.Stream;
/** {@link Subject} for doing assertions on {@link FileCodeOwnerStatus}es. */
@@ -39,6 +41,13 @@
fileCodeOwnerStatuses.collect(toImmutableList()), fileCodeOwnerStatuses());
}
+ /** Starts fluent chain to do assertions on a collection of {@link FileCodeOwnerStatus}es. */
+ public static ListSubject<FileCodeOwnerStatusSubject, FileCodeOwnerStatus> assertThatCollection(
+ Collection<FileCodeOwnerStatus> fileCodeOwnerStatuses) {
+ return ListSubject.assertThat(
+ ImmutableList.copyOf(fileCodeOwnerStatuses), fileCodeOwnerStatuses());
+ }
+
private static Factory<FileCodeOwnerStatusSubject, FileCodeOwnerStatus> fileCodeOwnerStatuses() {
return FileCodeOwnerStatusSubject::new;
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersOnAddReviewerIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersOnAddReviewerIT.java
index 576d2fe..91c40db 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersOnAddReviewerIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersOnAddReviewerIT.java
@@ -210,6 +210,7 @@
String path2 = "foo/baz.bar";
String path3 = "bar/foo.baz";
String path4 = "bar/baz.foo";
+ String path5 = "baz/foo.bar";
String changeId =
createChange(
"Test Change",
@@ -221,6 +222,8 @@
path3,
"file content",
path4,
+ "file content",
+ path5,
"file content"))
.getChangeId();
@@ -233,8 +236,9 @@
"%s who was added as reviewer owns the following files:\n"
+ "* %s\n"
+ "* %s\n"
- + "(2 more files)\n",
- user.fullName(), path4, path3));
+ + "* %s\n"
+ + "(more files)\n",
+ user.fullName(), path4, path3, path5));
}
@Test
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerStatusIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerStatusIT.java
index 64b24f8..531f118 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerStatusIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerStatusIT.java
@@ -78,6 +78,100 @@
}
@Test
+ public void getStatusForRenamedFile() throws Exception {
+ TestAccount user2 = accountCreator.user2();
+
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/bar")
+ .addCodeOwnerEmail(user.email())
+ .create();
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/baz")
+ .addCodeOwnerEmail(user2.email())
+ .create();
+
+ String oldPath = "foo/bar/abc.txt";
+ String newPath = "foo/baz/abc.txt";
+ String changeId = createChangeWithFileRename(oldPath, newPath);
+
+ CodeOwnerStatusInfo codeOwnerStatus =
+ changeCodeOwnersApiFactory.change(changeId).getCodeOwnerStatus();
+ assertThat(codeOwnerStatus).hasFileCodeOwnerStatusesThat().hasSize(2);
+ FileCodeOwnerStatusInfoSubject fileCodeOwnerStatusInfoSubject1 =
+ assertThat(codeOwnerStatus).hasFileCodeOwnerStatusesThat().element(0);
+ fileCodeOwnerStatusInfoSubject1.hasChangeTypeThat().isEqualTo(ChangeType.DELETED);
+ fileCodeOwnerStatusInfoSubject1.hasOldPathStatusThat().value().hasPathThat().isEqualTo(oldPath);
+ fileCodeOwnerStatusInfoSubject1
+ .hasOldPathStatusThat()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+ FileCodeOwnerStatusInfoSubject fileCodeOwnerStatusInfoSubject2 =
+ assertThat(codeOwnerStatus).hasFileCodeOwnerStatusesThat().element(1);
+ fileCodeOwnerStatusInfoSubject2.hasChangeTypeThat().isEqualTo(ChangeType.ADDED);
+ fileCodeOwnerStatusInfoSubject2.hasNewPathStatusThat().value().hasPathThat().isEqualTo(newPath);
+ fileCodeOwnerStatusInfoSubject2
+ .hasNewPathStatusThat()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+
+ // Add a reviewer that is a code owner of the old path.
+ gApi.changes().id(changeId).addReviewer(user.email());
+
+ codeOwnerStatus = changeCodeOwnersApiFactory.change(changeId).getCodeOwnerStatus();
+ assertThat(codeOwnerStatus).hasFileCodeOwnerStatusesThat().hasSize(2);
+ fileCodeOwnerStatusInfoSubject1 =
+ assertThat(codeOwnerStatus).hasFileCodeOwnerStatusesThat().element(0);
+ fileCodeOwnerStatusInfoSubject1.hasChangeTypeThat().isEqualTo(ChangeType.DELETED);
+ fileCodeOwnerStatusInfoSubject1.hasOldPathStatusThat().value().hasPathThat().isEqualTo(oldPath);
+ fileCodeOwnerStatusInfoSubject1
+ .hasOldPathStatusThat()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.PENDING);
+ fileCodeOwnerStatusInfoSubject2 =
+ assertThat(codeOwnerStatus).hasFileCodeOwnerStatusesThat().element(1);
+ fileCodeOwnerStatusInfoSubject2.hasChangeTypeThat().isEqualTo(ChangeType.ADDED);
+ fileCodeOwnerStatusInfoSubject2.hasNewPathStatusThat().value().hasPathThat().isEqualTo(newPath);
+ fileCodeOwnerStatusInfoSubject2
+ .hasNewPathStatusThat()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+
+ // Add a reviewer that is a code owner of the new path.
+ gApi.changes().id(changeId).addReviewer(user2.email());
+
+ codeOwnerStatus = changeCodeOwnersApiFactory.change(changeId).getCodeOwnerStatus();
+ assertThat(codeOwnerStatus).hasFileCodeOwnerStatusesThat().hasSize(2);
+ fileCodeOwnerStatusInfoSubject1 =
+ assertThat(codeOwnerStatus).hasFileCodeOwnerStatusesThat().element(0);
+ fileCodeOwnerStatusInfoSubject1.hasChangeTypeThat().isEqualTo(ChangeType.DELETED);
+ fileCodeOwnerStatusInfoSubject1.hasOldPathStatusThat().value().hasPathThat().isEqualTo(oldPath);
+ fileCodeOwnerStatusInfoSubject1
+ .hasOldPathStatusThat()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.PENDING);
+ fileCodeOwnerStatusInfoSubject2 =
+ assertThat(codeOwnerStatus).hasFileCodeOwnerStatusesThat().element(1);
+ fileCodeOwnerStatusInfoSubject2.hasChangeTypeThat().isEqualTo(ChangeType.ADDED);
+ fileCodeOwnerStatusInfoSubject2.hasNewPathStatusThat().value().hasPathThat().isEqualTo(newPath);
+ fileCodeOwnerStatusInfoSubject2
+ .hasNewPathStatusThat()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.PENDING);
+ }
+
+ @Test
public void getCodeOwnerStatusIfCodeOwnersFunctionalityIsDisabled() throws Exception {
disableCodeOwnersForProject(project);
String path = "foo/bar.baz";
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerApprovalIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerApprovalIT.java
index 971f8e9..8a8b9c4 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerApprovalIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerApprovalIT.java
@@ -808,6 +808,7 @@
String path2 = "foo/baz.bar";
String path3 = "bar/foo.baz";
String path4 = "bar/baz.foo";
+ String path5 = "baz/foo.bar";
String changeId =
createChange(
"Test Change",
@@ -819,6 +820,8 @@
path3,
"file content",
path4,
+ "file content",
+ path5,
"file content"))
.getChangeId();
@@ -833,8 +836,9 @@
+ " %s:\n"
+ "* %s\n"
+ "* %s\n"
- + "(2 more files)\n",
- admin.fullName(), path4, path3));
+ + "* %s\n"
+ + "(more files)\n",
+ admin.fullName(), path4, path3, path5));
}
@Test
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/ChangedFilesTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/ChangedFilesTest.java
index 3cf1aaf..34e0cb2 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/ChangedFilesTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/ChangedFilesTest.java
@@ -17,6 +17,7 @@
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.plugins.codeowners.testing.ChangedFileSubject.assertThat;
+import static com.google.gerrit.plugins.codeowners.testing.ChangedFileSubject.assertThatCollection;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static org.junit.Assert.fail;
@@ -35,6 +36,7 @@
import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersTest;
import com.google.gerrit.plugins.codeowners.common.ChangedFile;
import com.google.gerrit.plugins.codeowners.common.MergeCommitStrategy;
+import com.google.gerrit.plugins.codeowners.testing.ChangedFileSubject;
import com.google.gerrit.plugins.codeowners.util.JgitPath;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.RevisionResource;
@@ -135,13 +137,21 @@
gApi.changes().id(changeId).current().files();
+ // A renamed file is reported as addition of new path + deletion of old path. This is because
+ // ChangedFiles uses a DiffFormatter without rename detection (because rename detection requires
+ // loading the file contents which is too expensive).
ImmutableSet<ChangedFile> changedFilesSet = changedFiles.compute(getRevisionResource(changeId));
- assertThat(changedFilesSet).hasSize(1);
- ChangedFile changedFile = Iterables.getOnlyElement(changedFilesSet);
- assertThat(changedFile).hasNewPath().value().isEqualTo(Paths.get(newPath));
- assertThat(changedFile).hasOldPath().value().isEqualTo(Paths.get(oldPath));
- assertThat(changedFile).isRename();
- assertThat(changedFile).isNoDeletion();
+ assertThat(changedFilesSet).hasSize(2);
+ ChangedFileSubject changedFile1 = assertThatCollection(changedFilesSet).element(0);
+ changedFile1.hasNewPath().value().isEqualTo(Paths.get(newPath));
+ changedFile1.hasOldPath().isEmpty();
+ changedFile1.isNoRename();
+ changedFile1.isNoDeletion();
+ ChangedFileSubject changedFile2 = assertThatCollection(changedFilesSet).element(1);
+ changedFile2.hasNewPath().isEmpty();
+ changedFile2.hasOldPath().value().isEqualTo(Paths.get(oldPath));
+ changedFile2.isNoRename();
+ changedFile2.isDeletion();
}
@Test
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckTest.java
index f2c55e4..d23f647 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckTest.java
@@ -18,6 +18,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowLabel;
import static com.google.gerrit.plugins.codeowners.testing.FileCodeOwnerStatusSubject.assertThat;
+import static com.google.gerrit.plugins.codeowners.testing.FileCodeOwnerStatusSubject.assertThatCollection;
import static com.google.gerrit.plugins.codeowners.testing.FileCodeOwnerStatusSubject.assertThatStream;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
@@ -187,24 +188,29 @@
requestScopeOperations.setApiUser(user2.id());
recommend(changeId);
- Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
- FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
- assertThatStream(fileCodeOwnerStatuses).onlyElement();
- fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
- fileCodeOwnerStatusSubject
+ ImmutableList<FileCodeOwnerStatus> fileCodeOwnerStatuses =
+ codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId)).collect(toImmutableList());
+ assertThatCollection(fileCodeOwnerStatuses).hasSize(2);
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject1 =
+ assertThatCollection(fileCodeOwnerStatuses).element(0);
+ fileCodeOwnerStatusSubject1.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
+ fileCodeOwnerStatusSubject1
.hasNewPathStatus()
.value()
.hasStatusThat()
.isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
- fileCodeOwnerStatusSubject.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
- fileCodeOwnerStatusSubject
+ fileCodeOwnerStatusSubject1.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isNoDeletion();
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject2 =
+ assertThatCollection(fileCodeOwnerStatuses).element(1);
+ fileCodeOwnerStatusSubject2.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
+ fileCodeOwnerStatusSubject2
.hasOldPathStatus()
.value()
.hasStatusThat()
.isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
- fileCodeOwnerStatusSubject.hasChangedFile().isRename();
- fileCodeOwnerStatusSubject.hasChangedFile().isNoDeletion();
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject2.hasChangedFile().isDeletion();
}
@Test
@@ -321,24 +327,29 @@
requestScopeOperations.setApiUser(user2.id());
recommend(changeId);
- Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
- FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
- assertThatStream(fileCodeOwnerStatuses).onlyElement();
- fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
- fileCodeOwnerStatusSubject
- .hasNewPathStatus()
- .value()
- .hasStatusThat()
- .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
- fileCodeOwnerStatusSubject.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
- fileCodeOwnerStatusSubject
+ ImmutableList<FileCodeOwnerStatus> fileCodeOwnerStatuses =
+ codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId)).collect(toImmutableList());
+ assertThatCollection(fileCodeOwnerStatuses).hasSize(2);
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject1 =
+ assertThatCollection(fileCodeOwnerStatuses).element(0);
+ fileCodeOwnerStatusSubject1.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
+ fileCodeOwnerStatusSubject1
.hasOldPathStatus()
.value()
.hasStatusThat()
.isEqualTo(CodeOwnerStatus.PENDING);
- fileCodeOwnerStatusSubject.hasChangedFile().isRename();
- fileCodeOwnerStatusSubject.hasChangedFile().isNoDeletion();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isDeletion();
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject2 =
+ assertThatCollection(fileCodeOwnerStatuses).element(1);
+ fileCodeOwnerStatusSubject2.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
+ fileCodeOwnerStatusSubject2
+ .hasNewPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoDeletion();
}
@Test
@@ -358,24 +369,29 @@
requestScopeOperations.setApiUser(user2.id());
recommend(changeId);
- Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
- FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
- assertThatStream(fileCodeOwnerStatuses).onlyElement();
- fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
- fileCodeOwnerStatusSubject
- .hasNewPathStatus()
- .value()
- .hasStatusThat()
- .isEqualTo(CodeOwnerStatus.PENDING);
- fileCodeOwnerStatusSubject.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
- fileCodeOwnerStatusSubject
+ ImmutableList<FileCodeOwnerStatus> fileCodeOwnerStatuses =
+ codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId)).collect(toImmutableList());
+ assertThatCollection(fileCodeOwnerStatuses).hasSize(2);
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject1 =
+ assertThatCollection(fileCodeOwnerStatuses).element(0);
+ fileCodeOwnerStatusSubject1.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
+ fileCodeOwnerStatusSubject1
.hasOldPathStatus()
.value()
.hasStatusThat()
.isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
- fileCodeOwnerStatusSubject.hasChangedFile().isRename();
- fileCodeOwnerStatusSubject.hasChangedFile().isNoDeletion();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isDeletion();
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject2 =
+ assertThatCollection(fileCodeOwnerStatuses).element(1);
+ fileCodeOwnerStatusSubject2.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
+ fileCodeOwnerStatusSubject2
+ .hasNewPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.PENDING);
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoDeletion();
}
@Test
@@ -473,24 +489,29 @@
requestScopeOperations.setApiUser(user.id());
recommend(changeId);
- Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
- FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
- assertThatStream(fileCodeOwnerStatuses).onlyElement();
- fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
- fileCodeOwnerStatusSubject
- .hasNewPathStatus()
- .value()
- .hasStatusThat()
- .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
- fileCodeOwnerStatusSubject.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
- fileCodeOwnerStatusSubject
+ ImmutableList<FileCodeOwnerStatus> fileCodeOwnerStatuses =
+ codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId)).collect(toImmutableList());
+ assertThatCollection(fileCodeOwnerStatuses).hasSize(2);
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject1 =
+ assertThatCollection(fileCodeOwnerStatuses).element(0);
+ fileCodeOwnerStatusSubject1.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
+ fileCodeOwnerStatusSubject1
.hasOldPathStatus()
.value()
.hasStatusThat()
.isEqualTo(CodeOwnerStatus.APPROVED);
- fileCodeOwnerStatusSubject.hasChangedFile().isRename();
- fileCodeOwnerStatusSubject.hasChangedFile().isNoDeletion();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isDeletion();
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject2 =
+ assertThatCollection(fileCodeOwnerStatuses).element(1);
+ fileCodeOwnerStatusSubject2.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
+ fileCodeOwnerStatusSubject2
+ .hasNewPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoDeletion();
}
@Test
@@ -506,24 +527,29 @@
requestScopeOperations.setApiUser(user.id());
recommend(changeId);
- Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
- FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
- assertThatStream(fileCodeOwnerStatuses).onlyElement();
- fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
- fileCodeOwnerStatusSubject
- .hasNewPathStatus()
- .value()
- .hasStatusThat()
- .isEqualTo(CodeOwnerStatus.APPROVED);
- fileCodeOwnerStatusSubject.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
- fileCodeOwnerStatusSubject
+ ImmutableList<FileCodeOwnerStatus> fileCodeOwnerStatuses =
+ codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId)).collect(toImmutableList());
+ assertThatCollection(fileCodeOwnerStatuses).hasSize(2);
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject1 =
+ assertThatCollection(fileCodeOwnerStatuses).element(0);
+ fileCodeOwnerStatusSubject1.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
+ fileCodeOwnerStatusSubject1
.hasOldPathStatus()
.value()
.hasStatusThat()
.isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
- fileCodeOwnerStatusSubject.hasChangedFile().isRename();
- fileCodeOwnerStatusSubject.hasChangedFile().isNoDeletion();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isDeletion();
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject2 =
+ assertThatCollection(fileCodeOwnerStatuses).element(1);
+ fileCodeOwnerStatusSubject2.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
+ fileCodeOwnerStatusSubject2
+ .hasNewPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.APPROVED);
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoDeletion();
}
@Test
@@ -672,18 +698,13 @@
String changeId = createChangeWithFileRename(oldPath, newPath);
amendChange(user, changeId);
- Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
- FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
- assertThatStream(fileCodeOwnerStatuses).onlyElement();
- fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
- fileCodeOwnerStatusSubject
- .hasNewPathStatus()
- .value()
- .hasStatusThat()
- .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
- fileCodeOwnerStatusSubject.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
- fileCodeOwnerStatusSubject
+ ImmutableList<FileCodeOwnerStatus> fileCodeOwnerStatuses =
+ codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId)).collect(toImmutableList());
+ assertThatCollection(fileCodeOwnerStatuses).hasSize(2);
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject1 =
+ assertThatCollection(fileCodeOwnerStatuses).element(0);
+ fileCodeOwnerStatusSubject1.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
+ fileCodeOwnerStatusSubject1
.hasOldPathStatus()
.value()
.hasStatusThat()
@@ -691,8 +712,18 @@
implicitApprovalsEnabled
? CodeOwnerStatus.APPROVED
: CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
- fileCodeOwnerStatusSubject.hasChangedFile().isRename();
- fileCodeOwnerStatusSubject.hasChangedFile().isNoDeletion();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isDeletion();
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject2 =
+ assertThatCollection(fileCodeOwnerStatuses).element(1);
+ fileCodeOwnerStatusSubject2.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
+ fileCodeOwnerStatusSubject2
+ .hasNewPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoDeletion();
}
@Test
@@ -719,12 +750,23 @@
String changeId = createChangeWithFileRename(oldPath, newPath);
amendChange(user, changeId);
- Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
- FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
- assertThatStream(fileCodeOwnerStatuses).onlyElement();
- fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
- fileCodeOwnerStatusSubject
+ ImmutableList<FileCodeOwnerStatus> fileCodeOwnerStatuses =
+ codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId)).collect(toImmutableList());
+ assertThatCollection(fileCodeOwnerStatuses).hasSize(2);
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject1 =
+ assertThatCollection(fileCodeOwnerStatuses).element(0);
+ fileCodeOwnerStatusSubject1.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
+ fileCodeOwnerStatusSubject1
+ .hasOldPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+ fileCodeOwnerStatusSubject1.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isDeletion();
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject2 =
+ assertThatCollection(fileCodeOwnerStatuses).element(1);
+ fileCodeOwnerStatusSubject2.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
+ fileCodeOwnerStatusSubject2
.hasNewPathStatus()
.value()
.hasStatusThat()
@@ -732,14 +774,8 @@
implicitApprovalsEnabled
? CodeOwnerStatus.APPROVED
: CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
- fileCodeOwnerStatusSubject.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
- fileCodeOwnerStatusSubject
- .hasOldPathStatus()
- .value()
- .hasStatusThat()
- .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
- fileCodeOwnerStatusSubject.hasChangedFile().isRename();
- fileCodeOwnerStatusSubject.hasChangedFile().isNoDeletion();
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoDeletion();
}
@Test
diff --git a/resources/Documentation/rest-api.md b/resources/Documentation/rest-api.md
index c0e608b..29d5fd8 100644
--- a/resources/Documentation/rest-api.md
+++ b/resources/Documentation/rest-api.md
@@ -615,17 +615,6 @@
"path": "docs/todo.txt",
"status": "PENDING"
}
- },
- {
- "change_type": "RENAMED",
- "old_path_status" {
- "path": "user-introduction.txt",
- "status": "INSUFFICIENT_REVIEWERS"
- },
- "new_path_status" {
- "path": "docs/user-intro.md",
- "status": "APPROVED"
- }
}
]
}
@@ -949,7 +938,7 @@
| Field Name | | Description |
| ------------- | -------- | ----------- |
-| `change_type` | optional | The type of the file modification. Can be `ADDED`, `MODIFIED`, `DELETED`, `RENAMED` or `COPIED`. Not set if `MODIFIED`.
+| `change_type` | optional | The type of the file modification. Can be `ADDED`, `MODIFIED`, `DELETED`, `RENAMED` or `COPIED`. Not set if `MODIFIED`. Renamed files might appear as separate addition and deletion or with type=RENAMED. Copied files might appear as addition or with type=COPIED.
| `old_path_status` | optional | The code owner status for the old path as [PathCodeOwnerStatusInfo](#path-code-owner-status-info) entity. Only set if `change_type` is `DELETED` or `RENAMED`.
| `new_path_status` | optional | The code owner status for the new path as [PathCodeOwnerStatusInfo](#path-code-owner-status-info) entity. Not set if `change_type` is `DELETED`.
diff --git a/ui/owner-status-column.js b/ui/owner-status-column.js
index 31ad654..b3d92fb 100644
--- a/ui/owner-status-column.js
+++ b/ui/owner-status-column.js
@@ -116,6 +116,7 @@
static get properties() {
return {
path: String,
+ oldPath: String,
patchRange: Object,
hidden: {
type: Boolean,
@@ -169,7 +170,7 @@
static get observers() {
return [
- 'computeStatusIcon(model.status, path)',
+ 'computeStatusIcon(model.status, path, oldPath)',
];
}
@@ -178,8 +179,8 @@
this.modelLoader.loadStatus();
}
- computeStatusIcon(modelStatus, path) {
- if ([modelStatus, path].includes(undefined)) return;
+ computeStatusIcon(modelStatus, path, oldPath) {
+ if ([modelStatus, path, oldPath].includes(undefined)) return;
if (MAGIC_FILES.includes(path)) return;
const codeOwnerStatusMap = modelStatus.codeOwnerStatusMap;
@@ -191,10 +192,10 @@
const status = statusItem.status;
let oldPathStatus = null;
- if (statusItem.oldPath) {
- const oldStatusItem = codeOwnerStatusMap.get(statusItem.oldPath);
+ if (oldPath !== path) {
+ const oldStatusItem = codeOwnerStatusMap.get(oldPath);
if (!oldStatusItem) {
- // should not happen
+ this.status = STATUS_CODE.ERROR;
} else {
oldPathStatus = oldStatusItem.status;
}
