fixed an issue with user-ids (#348)

Co-authored-by: Shane Weaver <[email protected]>

Author: nmetulev

src/components/mgt-people/mgt-people.ts

@@ -13,7 +13,8 @@ import { getUsersForUserIds } from '../../graph/graph.user';
 import { Providers } from '../../Providers';
 import { ProviderState } from '../../providers/IProvider';
 import '../../styles/fabric-icon-font';
-import '../mgt-person/mgt-person';
+import { arraysAreEqual } from '../../utils/Utils';
+import { IDynamicPerson } from '../mgt-person/mgt-person';
 import { MgtTemplatedComponent } from '../templatedComponent';
 import { PersonCardInteraction } from './../PersonCardInteraction';
 import { styles } from './mgt-people-css';
@@ -38,26 +39,6 @@ export class MgtPeople extends MgtTemplatedComponent {
     return styles;
   }
 
-  /**
-   * containing array of people used in the component.
-   * @type {Array<MgtPersonDetails>}
-   */
-  @property({
-    attribute: 'people',
-    type: Object
-  })
-  public people: Array<MicrosoftGraph.User | MicrosoftGraph.Person | MicrosoftGraph.Contact>;
-
-  /**
-   * developer determined max people shown in component
-   * @type {number}
-   */
-  @property({
-    attribute: 'show-max',
-    type: Number
-  })
-  public showMax: number;
-
   /**
    * determines if agenda events come from specific group
    * @type {string}
@@ -66,7 +47,16 @@ export class MgtPeople extends MgtTemplatedComponent {
     attribute: 'group-id',
     type: String
   })
-  public groupId: string;
+  public get groupId(): string {
+    return this._groupId;
+  }
+  public set groupId(value) {
+    if (this._groupId === value) {
+      return;
+    }
+    this._groupId = value;
+    this.requestStateUpdate(true);
+  }
 
   /**
    * user id array
@@ -76,19 +66,40 @@ export class MgtPeople extends MgtTemplatedComponent {
   @property({
     attribute: 'user-ids',
     converter: (value, type) => {
-      return value.split(',');
+      return value.split(',').map(v => v.trim());
     }
   })
   public get userIds(): string[] {
-    return this.privateUserIds;
+    return this._userIds;
   }
   public set userIds(value: string[]) {
-    const oldValue = this.userIds;
-    this.privateUserIds = value;
-    this.updateUserIds(value);
-    this.requestUpdate('userIds', oldValue);
+    if (arraysAreEqual(this._userIds, value)) {
+      return;
+    }
+    this._userIds = value;
+    this.requestStateUpdate(true);
   }
 
+  /**
+   * containing array of people used in the component.
+   * @type {Array<MgtPersonDetails>}
+   */
+  @property({
+    attribute: 'people',
+    type: Object
+  })
+  public people: IDynamicPerson[];
+
+  /**
+   * developer determined max people shown in component
+   * @type {number}
+   */
+  @property({
+    attribute: 'show-max',
+    type: Number
+  })
+  public showMax: number;
+
   /**
    * Sets how the person-card is invoked
    * Set to PersonCardInteraction.none to not show the card
@@ -109,14 +120,29 @@ export class MgtPeople extends MgtTemplatedComponent {
   })
   public personCardInteraction: PersonCardInteraction = PersonCardInteraction.hover;
 
-  private privateUserIds: string[];
+  private _groupId: string;
+  private _userIds: string[];
 
   constructor() {
     super();
 
     this.showMax = 3;
   }
 
+  /**
+   * Request to reload the state.
+   * Use reload instead of load to ensure loading events are fired.
+   *
+   * @protected
+   * @memberof MgtBaseComponent
+   */
+  protected requestStateUpdate(force?: boolean) {
+    if (force) {
+      this.people = null;
+    }
+    return super.requestStateUpdate(force);
+  }
+
   /**
    * Invoked on each update to perform rendering tasks. This method must return
    * a lit-html TemplateResult. Setting properties inside this method will *not*
@@ -250,34 +276,4 @@ export class MgtPeople extends MgtTemplatedComponent {
       }
     }
   }
-
-  private async updateUserIds(newIds: string[]) {
-    if (this.isLoadingState) {
-      return;
-    }
-
-    const newIdsSet = new Set(newIds);
-    this.people = this.people ? this.people.filter(p => newIdsSet.has(p.id)) : [];
-    const oldIdsSet = new Set(this.people ? this.people.map(p => p.id) : []);
-
-    const newToLoad = [];
-
-    for (const id of newIds) {
-      if (!oldIdsSet.has(id)) {
-        newToLoad.push(id);
-      }
-    }
-
-    if (newToLoad && newToLoad.length > 0) {
-      const provider = Providers.globalProvider;
-      if (!provider || provider.state !== ProviderState.SignedIn) {
-        return;
-      }
-
-      const graph = provider.graph.forComponent(this);
-
-      const newPeople = await getUsersForUserIds(graph, newToLoad);
-      this.people = (this.people || []).concat(newPeople);
-    }
-  }
 }

src/utils/Utils.ts

@@ -215,6 +215,44 @@ function equalsInternal(o1: any, o2: any, refs: Set<object>) {
   return o1 === o2;
 }
 
+/**
+ * Compares two arrays if the elements are equals
+ * Should be used for arrays of primitive types
+ *
+ * @export
+ * @template T the type of the elements in the array (should be primitive)
+ * @param {T[]} arr1
+ * @param {T[]} arr2
+ * @returns true if both arrays contain the same items or if both arrays are null or empty
+ */
+export function arraysAreEqual<T>(arr1: T[], arr2: T[]) {
+  if (arr1 === arr2) {
+    return true;
+  }
+
+  if (!arr1 || !arr2) {
+    return false;
+  }
+
+  if (arr1.length !== arr2.length) {
+    return false;
+  }
+
+  if (arr1.length === 0) {
+    return true;
+  }
+
+  const setArr1 = new Set(arr1);
+
+  for (const i of arr2) {
+    if (!setArr1.has(i)) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
 /**
  * converts a blob to base64 encoding
  *

stories/components/people.stories.js

@@ -23,3 +23,11 @@ export default {
 export const People = () => html`
   <mgt-people show-max="5"></mgt-people>
 `;
+
+export const GroupId = () => html`
+  <mgt-people group-id="02bd9fd6-8f93-4758-87c3-1fb73740a315"></mgt-people>
+`;
+
+export const UserIds = () => html`
+  <mgt-people user-ids="2804bc07-1e1f-4938-9085-ce6d756a32d2 ,e8a02cc7-df4d-4778-956d-784cc9506e5a,c8913c86-ceea-4d39-b1ea-f63a5b675166"></mgt-people>
+`;