Merge "Change logic for choosing focus target in reply dialog"

polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts

diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
index 29604c3..ccdcc15 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
@@ -108,6 +108,7 @@
 import {customElement, property, state, query} from 'lit/decorators';
 import {subscribe} from '../../lit/subscription-controller';
 import {configModelToken} from '../../../models/config/config-model';
+import {hasHumanReviewer, isOwner} from '../../../utils/change-util';
 
 const STORAGE_DEBOUNCE_INTERVAL_MS = 400;
 
@@ -1415,19 +1416,9 @@
   }
 
   chooseFocusTarget() {
-    // If we are the owner and the reviewers field is empty, focus on that.
-    if (
-      this.account &&
-      this.change &&
-      this.change.owner &&
-      this.account._account_id === this.change.owner._account_id &&
-      (!this.reviewers || this.reviewers?.length === 0)
-    ) {
-      return FocusTarget.REVIEWERS;
-    }
-
-    // Default to BODY.
-    return FocusTarget.BODY;
+    if (!isOwner(this.change, this.account)) return FocusTarget.BODY;
+    if (hasHumanReviewer(this.change)) return FocusTarget.BODY;
+    return FocusTarget.REVIEWERS;
   }
 
   isOwner(account?: AccountInfo, change?: ChangeInfo) {

polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts

diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
index d7fdfda..67190de 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
@@ -28,6 +28,7 @@
   createCommentThread,
   createDraft,
   createRevision,
+  createServiceUserWithId,
 } from '../../../test/test-data-generators';
 import {
   pressAndReleaseKeyOn,
@@ -1624,28 +1625,22 @@
 
   test('chooseFocusTarget', () => {
     element.account = undefined;
-    assert.strictEqual(element.chooseFocusTarget(), element.FocusTarget.BODY);
+    assert.equal(element.chooseFocusTarget(), element.FocusTarget.BODY);
 
-    element.account = {_account_id: 1 as AccountId};
-    assert.strictEqual(element.chooseFocusTarget(), element.FocusTarget.BODY);
+    element.account = element.change!.owner;
+    assert.equal(element.chooseFocusTarget(), element.FocusTarget.REVIEWERS);
 
-    element.change!.owner = {_account_id: 2 as AccountId};
-    assert.strictEqual(element.chooseFocusTarget(), element.FocusTarget.BODY);
+    element.change!.reviewers.REVIEWER = [createAccountWithId(314)];
+    assert.equal(element.chooseFocusTarget(), element.FocusTarget.BODY);
 
-    element.change!.owner._account_id = 1 as AccountId;
-    assert.strictEqual(
-      element.chooseFocusTarget(),
-      element.FocusTarget.REVIEWERS
-    );
+    element.change!.reviewers.REVIEWER = [createServiceUserWithId(314)];
+    assert.equal(element.chooseFocusTarget(), element.FocusTarget.REVIEWERS);
 
-    element.reviewers = [];
-    assert.strictEqual(
-      element.chooseFocusTarget(),
-      element.FocusTarget.REVIEWERS
-    );
-
-    element.reviewers.push({});
-    assert.strictEqual(element.chooseFocusTarget(), element.FocusTarget.BODY);
+    element.change!.reviewers.REVIEWER = [
+      createAccountWithId(314),
+      createServiceUserWithId(314),
+    ];
+    assert.equal(element.chooseFocusTarget(), element.FocusTarget.BODY);
   });
 
   test('only send labels that have changed', async () => {

polygerrit-ui/app/test/test-data-generators.ts

diff --git a/polygerrit-ui/app/test/test-data-generators.ts b/polygerrit-ui/app/test/test-data-generators.ts
index e7e85c2..81ca830 100644
--- a/polygerrit-ui/app/test/test-data-generators.ts
+++ b/polygerrit-ui/app/test/test-data-generators.ts
@@ -72,6 +72,7 @@
 } from '../types/common';
 import {
   AccountsVisibility,
+  AccountTag,
   AppTheme,
   AuthType,
   ChangeStatus,
@@ -188,6 +189,13 @@
   };
 }
 
+export function createServiceUserWithId(id = 5): AccountInfo {
+  return {
+    ...createAccountWithId(id),
+    tags: [AccountTag.SERVICE_USER],
+  };
+}
+
 export function createAccountDetailWithId(id = 5): AccountDetailInfo {
   return {
     _account_id: id as AccountId,

polygerrit-ui/app/utils/change-util.ts

diff --git a/polygerrit-ui/app/utils/change-util.ts b/polygerrit-ui/app/utils/change-util.ts
index d6d2f7d..cf0ff53 100644
--- a/polygerrit-ui/app/utils/change-util.ts
+++ b/polygerrit-ui/app/utils/change-util.ts
@@ -14,6 +14,7 @@
 } from '../types/common';
 import {ParsedChangeInfo} from '../types/types';
 import {ChangeStates} from '../elements/shared/gr-change-status/gr-change-status';
+import {isServiceUser} from './account-util';
 
 // This can be wrong! See WARNING above
 interface ChangeStatusesOptions {
@@ -249,6 +250,11 @@
   );
 }
 
+export function hasHumanReviewer(change?: ChangeInfo): boolean {
+  const reviewers = change?.reviewers.REVIEWER ?? [];
+  return reviewers.some(r => !isServiceUser(r));
+}
+
 export function isRemovableReviewer(
   change?: ChangeInfo,
   reviewer?: AccountInfo