commit 75798469c9c491913afaaf45f725b620ec6a7612
author Chris Poucet <poucet@google.com> Mon Dec 12 11:43:38 2022 +0100
committer Chris Poucet <poucet@google.com> Mon Dec 12 11:49:44 2022 +0100
tree db410b7540a9cd9efdaa4bd383b17021d0832dda
parent 25a6e411dc8a58ba2b39329c3f772e27ad0ad20d

Compute owners for file-deletions correctly.

Gerrit FE returns 'path' not 'oldPath' for deleted files. However,
code-owners plugin computes deleted files as 'oldPath' not 'path'.  To
get around this difference in API, we check if oldPath is not set. In
that case, we also check if path exists in the oldPaths returned by the
code-owners plugin

Additionally, uniquify accounts so we don't show duplicates.

Google-Bug-Id: b/200008775
Change-Id: I268397ccf1ed5d89cfe03b64b42b917c67c80233

web/owner-status-column.ts

diff --git a/web/owner-status-column.ts b/web/owner-status-column.ts
index 145047c..66c572c 100644
--- a/web/owner-status-column.ts
+++ b/web/owner-status-column.ts
@@ -95,6 +95,17 @@
   return owners.get(path) ?? [];
 }
 
+export function uniqueAccountId(
+  account: AccountInfo,
+  index: number,
+  accountArray: AccountInfo[]
+) {
+  return (
+    index ===
+    accountArray.findIndex(other => account._account_id === other._account_id)
+  );
+}
+
 const base = CodeOwnersModelMixin(LitElement);
 
 class BaseEl extends base {
@@ -278,9 +289,14 @@
         </gr-tooltip-content>
       `;
     } else if (this.ownedPaths) {
-      const oldOwners = getOwners(this.ownedPaths.oldPathOwners, this.oldPath);
+      let oldOwners = getOwners(this.ownedPaths.oldPathOwners, this.oldPath);
       const newOwners = getOwners(this.ownedPaths.newPathOwners, this.path);
-      const allOwners = oldOwners.concat(newOwners);
+      if (this.oldPath === undefined || this.oldPath === null) {
+        // In case of a file deletion, the Gerrit FE gives 'path' but not 'oldPath'
+        // but code-owners considers a deleted file an oldpath so check the oldpath owners.
+        oldOwners = getOwners(this.ownedPaths.oldPathOwners, this.path);
+      }
+      const allOwners = oldOwners.concat(newOwners).filter(uniqueAccountId);
 
       return html` <gr-avatar-stack
           .accounts=${allOwners.slice(0, 3)}
@@ -307,7 +323,11 @@
     if (!this.ownedPaths) return false;
     if (
       hasPath(this.ownedPaths.newPaths, this.path) ||
-      hasPath(this.ownedPaths.oldPaths, this.oldPath)
+      hasPath(this.ownedPaths.oldPaths, this.oldPath) ||
+      // In case of deletions, the FE gives a path, but code-owners
+      // computes this as being part of the old path.
+      ((this.oldPath === undefined || this.oldPath === null) &&
+        hasPath(this.ownedPaths.oldPaths, this.path))
     )
       return true;
     return false;