Merge "Emit value change event even when no items are matched"

3 files changed, 16 insertions, 13 deletions

diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts
index eef2a440a2..04887adbe0 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts
+++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts
@@ -74,8 +74,6 @@ export interface AdminSubsectionLink {
 
 @customElement('gr-admin-view')
 export class GrAdminView extends LitElement {
-  private account?: AccountDetailInfo;
-
   @state()
   view?: GerritView;
 
@@ -459,13 +457,18 @@ export class GrAdminView extends LitElement {
   async reload() {
     try {
       this.reloading = true;
+      // There is async barrier inside reload function, we need to clear
+      // subsectionLinks now, because the element might render while waiting for
+      // RestApi responses breaking the invariant that this.view is part of
+      // subsectionLinks if non-empty.
+      this.subsectionLinks = [];
       const promises: [Promise<AccountDetailInfo | undefined>, Promise<void>] =
         [
           this.restApiService.getAccount(),
           this.getPluginLoader().awaitPluginsLoaded(),
         ];
       const result = await Promise.all(promises);
-      this.account = result[0];
+      const account = result[0];
       let options: AdminNavLinksOption | undefined = undefined;
       if (this.repoName) {
         options = {repoName: this.repoName};
@@ -484,7 +487,7 @@ export class GrAdminView extends LitElement {
       }
 
       const res = await getAdminLinks(
-        this.account,
+        account,
         () =>
           this.restApiService.getAccountCapabilities().then(capabilities => {
             if (!capabilities) {
@@ -501,7 +504,6 @@ export class GrAdminView extends LitElement {
         : '';
 
       if (!res.expandedSection) {
-        this.subsectionLinks = [];
         return;
       }
       this.subsectionLinks = [res.expandedSection]
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts
index 0a2e4f2458..8cf4e057d5 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts
@@ -279,9 +279,12 @@ suite('gr-admin-view tests', () => {
     const groupId = '1' as GroupId;
     element.view = GerritView.GROUP;
     element.groupViewState = {groupId, view: GerritView.GROUP};
+    // Check for reload before update. This would normally be done as part of
+    // subscribe method that updates the view/viewState.
+    assert.isTrue(element.needsReload());
+    element.reload();
     await element.updateComplete;
 
-    assert.isTrue(element.needsReload());
     assert.equal(element.groupId, groupId);
   });
 
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.ts b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.ts
index 6b2cca64a5..3dcba9e338 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.ts
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.ts
@@ -188,7 +188,10 @@ export class GrDropdownList extends LitElement {
 
   protected override willUpdate(changedProperties: PropertyValues): void {
     if (changedProperties.has('value') || changedProperties.has('items')) {
-      this.handleDataChange(changedProperties.has('value'));
+      this.updateText();
+    }
+    if (changedProperties.has('value')) {
+      fireNoBubble(this, 'value-change', {value: this.value});
     }
   }
 
@@ -307,7 +310,7 @@ export class GrDropdownList extends LitElement {
     }, 1);
   }
 
-  private handleDataChange(valueChanged: boolean) {
+  private updateText() {
     if (this.value === undefined || this.items === undefined) {
       return;
     }
@@ -318,11 +321,6 @@ export class GrDropdownList extends LitElement {
     this.text = selectedObj.triggerText
       ? selectedObj.triggerText
       : selectedObj.text;
-    // Only emit the event if the value actually changed and matches one of the
-    // items.
-    if (valueChanged) {
-      fireNoBubble(this, 'value-change', {value: this.value});
-    }
   }
 
   /**