Added mgt-people-picker support for filtering group members when using group-id (#741)

* Update azure-pipelines.yml for Azure Pipelines

* Adding support for group filtering

* Adding support for group filtering

* Adding new method documentation

* Fixing typo

* Bringing back the right port number

* Adding support for transitive search + group

* Adding the react type for transitiveSearch

* Merging getUserFromGroup and findUsersFromGroup

* Removing references to getPeopleFromGroup

* Removing commented code

* Update packages/mgt/src/components/mgt-people-picker/mgt-people-picker.ts

* Adding any and getting unique people from result

* Simplifying the unique people function

* Update packages/mgt/src/components/mgt-people-picker/mgt-people-picker.ts

Co-authored-by: Nikola Metulev <[email protected]>

* Refactoring for a findGroupMembers + simpler logic

* Apply suggestions from code review

Co-authored-by: Nikola Metulev <[email protected]>

Author: sebastienlevert

packages/mgt-react/src/generated/react.ts

@@ -54,6 +54,7 @@ export type PeoplePickerProps = {
 	groupId?: string;
 	type?: PersonType;
 	groupType?: GroupType;
+	transitiveSearch?: boolean;
 	people?: IDynamicPerson[];
 	defaultSelectedUserIds?: string[];
 	placeholder?: string;

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

@@ -9,9 +9,9 @@ import { User } from '@microsoft/microsoft-graph-types';
 import { customElement, html, internalProperty, property, TemplateResult } from 'lit-element';
 import { classMap } from 'lit-html/directives/class-map';
 import { repeat } from 'lit-html/directives/repeat';
-import { findGroups, GroupType } from '../../graph/graph.groups';
-import { findPeople, getPeople, getPeopleFromGroup, PersonType } from '../../graph/graph.people';
-import { findUsers, getUser, getUsersForUserIds } from '../../graph/graph.user';
+import { findGroups, findGroupsFromGroup, GroupType } from '../../graph/graph.groups';
+import { findPeople, getPeople, PersonType } from '../../graph/graph.people';
+import { findUsers, findGroupMembers, getUser, getUsersForUserIds } from '../../graph/graph.user';
 import { IDynamicPerson } from '../../graph/types';
 import { Providers, ProviderState, MgtTemplatedComponent } from '@microsoft/mgt-element';
 import '../../styles/style-helper';
@@ -182,6 +182,16 @@ export class MgtPeoplePicker extends MgtTemplatedComponent {
     this.requestStateUpdate(true);
   }
 
+  /**
+   * whether the return should contain a flat list of all nested members
+   * @type {boolean}
+   */
+  @property({
+    attribute: 'transitive-search',
+    type: Boolean
+  })
+  public transitiveSearch: boolean;
+
   /**
    * containing object of IDynamicPerson.
    * @type {IDynamicPerson[]}
@@ -644,7 +654,14 @@ export class MgtPeoplePicker extends MgtTemplatedComponent {
           if (this.groupId) {
             if (this._groupPeople === null) {
               try {
-                this._groupPeople = await getPeopleFromGroup(graph, this.groupId);
+                this._groupPeople = await findGroupMembers(
+                  graph,
+                  null,
+                  this.groupId,
+                  this.showMax,
+                  this.type,
+                  this.transitiveSearch
+                );
               } catch (_) {
                 this._groupPeople = [];
               }
@@ -671,42 +688,48 @@ export class MgtPeoplePicker extends MgtTemplatedComponent {
 
       if (input) {
         people = [];
-        if (this.type === PersonType.person || this.type === PersonType.any) {
-          try {
-            people = (await findPeople(graph, input, this.showMax)) || [];
-          } catch (e) {
-            // nop
-          }
 
-          if (people.length < this.showMax) {
+        if (this.groupId) {
+          people =
+            (await findGroupMembers(graph, input, this.groupId, this.showMax, this.type, this.transitiveSearch)) || [];
+        } else {
+          if (this.type === PersonType.person || this.type === PersonType.any) {
             try {
-              const users = (await findUsers(graph, input, this.showMax)) || [];
+              people = (await findPeople(graph, input, this.showMax)) || [];
+            } catch (e) {
+              // nop
+            }
 
-              // make sure only unique people
-              const peopleIds = new Set(people.map(p => p.id));
-              for (const user of users) {
-                if (!peopleIds.has(user.id)) {
-                  people.push(user);
+            if (people.length < this.showMax) {
+              try {
+                const users = (await findUsers(graph, input, this.showMax)) || [];
+
+                // make sure only unique people
+                const peopleIds = new Set(people.map(p => p.id));
+                for (const user of users) {
+                  if (!peopleIds.has(user.id)) {
+                    people.push(user);
+                  }
                 }
+              } catch (e) {
+                // nop
               }
+            }
+          }
+          if ((this.type === PersonType.group || this.type === PersonType.any) && people.length < this.showMax) {
+            let groups = [];
+            try {
+              groups = (await findGroups(graph, input, this.showMax, this.groupType)) || [];
+              people = people.concat(groups);
             } catch (e) {
               // nop
             }
           }
         }
-
-        if ((this.type === PersonType.group || this.type === PersonType.any) && people.length < this.showMax) {
-          people = [];
-          try {
-            const groups = (await findGroups(graph, input, this.showMax, this.groupType)) || [];
-            people = people.concat(groups);
-          } catch (e) {
-            // nop
-          }
-        }
       }
     }
 
+    //people = this.getUniquePeople(people);
     this._foundPeople = this.filterPeople(people);
   }
 

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

@@ -8,9 +8,9 @@
 import * as MicrosoftGraph from '@microsoft/microsoft-graph-types';
 import { customElement, html, property, TemplateResult } from 'lit-element';
 import { repeat } from 'lit-html/directives/repeat';
-import { getPeople, getPeopleFromGroup } from '../../graph/graph.people';
+import { getPeople, PersonType } from '../../graph/graph.people';
 import { getUsersPresenceByPeople } from '../../graph/graph.presence';
-import { getUsersForPeopleQueries, getUsersForUserIds } from '../../graph/graph.user';
+import { findGroupMembers, getUsersForPeopleQueries, getUsersForUserIds } from '../../graph/graph.user';
 import { IDynamicPerson } from '../../graph/types';
 import { Providers, ProviderState, MgtTemplatedComponent, arraysAreEqual } from '@microsoft/mgt-element';
 import '../../styles/style-helper';
@@ -315,7 +315,7 @@ export class MgtPeople extends MgtTemplatedComponent {
 
         // populate people
         if (this.groupId) {
-          this.people = await getPeopleFromGroup(graph, this.groupId);
+          this.people = await findGroupMembers(graph, null, this.groupId, this.showMax, PersonType.person);
         } else if (this.userIds) {
           this.people = await getUsersForUserIds(graph, this.userIds);
         } else if (this.peopleQueries) {

packages/mgt/src/graph/graph.groups.ts

@@ -160,3 +160,88 @@ export async function findGroups(
 
   return result ? result.value : null;
 }
+
+/**
+ * Searches the Graph for Groups
+ *
+ * @export
+ * @param {IGraph} graph
+ * @param {string} query - what to search for
+ * @param {string} groupId - what to search for
+ * @param {number} [top=10] - number of groups to return
+ * @param {boolean} [transitive=false] - whether the return should contain a flat list of all nested members
+ * @param {GroupType} [groupTypes=GroupType.any] - the type of group to search for
+ * @returns {Promise<Group[]>} An array of Groups
+ */
+export async function findGroupsFromGroup(
+  graph: IGraph,
+  query: string,
+  groupId: string,
+  top: number = 10,
+  transitive: boolean = false,
+  groupTypes: GroupType = GroupType.any
+): Promise<Group[]> {
+  const scopes = 'Group.Read.All';
+
+  let cache: CacheStore<CacheGroupQuery>;
+  const key = `${groupId}:${query || '*'}:${groupTypes}:${transitive}`;
+
+  if (groupsCacheEnabled()) {
+    cache = CacheService.getCache(cacheSchema, 'groupsQuery');
+    const cacheGroupQuery = await cache.getValue(key);
+    if (cacheGroupQuery && getGroupsInvalidationTime() > Date.now() - cacheGroupQuery.timeCached) {
+      if (cacheGroupQuery.top >= top) {
+        // if request is less than the cache's requests, return a slice of the results
+        return cacheGroupQuery.groups.map(x => JSON.parse(x)).slice(0, top + 1);
+      }
+      // if the new request needs more results than what's presently in the cache, graph must be called again
+    }
+  }
+
+  const apiUrl = `groups/${groupId}/${transitive ? 'transitiveMembers' : 'members'}/microsoft.graph.group`;
+  let filterQuery = '';
+  if (query !== '') {
+    filterQuery = `(startswith(displayName,'${query}') or startswith(mailNickname,'${query}') or startswith(mail,'${query}'))`;
+  }
+
+  if (groupTypes !== GroupType.any) {
+    const filterGroups = [];
+
+    // tslint:disable-next-line:no-bitwise
+    if (GroupType.unified === (groupTypes & GroupType.unified)) {
+      filterGroups.push("groupTypes/any(c:c+eq+'Unified')");
+    }
+
+    // tslint:disable-next-line:no-bitwise
+    if (GroupType.security === (groupTypes & GroupType.security)) {
+      filterGroups.push('(mailEnabled eq false and securityEnabled eq true)');
+    }
+
+    // tslint:disable-next-line:no-bitwise
+    if (GroupType.mailenabledsecurity === (groupTypes & GroupType.mailenabledsecurity)) {
+      filterGroups.push('(mailEnabled eq true and securityEnabled eq true)');
+    }
+
+    // tslint:disable-next-line:no-bitwise
+    if (GroupType.distribution === (groupTypes & GroupType.distribution)) {
+      filterGroups.push('(mailEnabled eq true and securityEnabled eq false)');
+    }
+
+    filterQuery += (query !== '' ? ' and ' : '') + filterGroups.join(' or ');
+  }
+
+  const result = await graph
+    .api(apiUrl)
+    .filter(filterQuery)
+    .count(true)
+    .top(top)
+    .header('ConsistencyLevel', 'eventual')
+    .middlewareOptions(prepScopes(scopes))
+    .get();
+
+  if (groupsCacheEnabled() && result) {
+    cache.putValue(key, { groups: result.value.map(x => JSON.stringify(x)), top: top });
+  }
+
+  return result ? result.value : null;
+}

packages/mgt/src/graph/graph.people.ts

@@ -191,36 +191,6 @@ export async function getPeople(graph: IGraph): Promise<Person[]> {
   return people ? people.value : null;
 }
 
-/**
- * async promise to the Graph for People, defined by a group id
- *
- * @param {string} groupId
- * @returns {(Promise<Person[]>)}
- * @memberof Graph
- */
-export async function getPeopleFromGroup(graph: IGraph, groupId: string): Promise<Person[]> {
-  const scopes = 'people.read';
-  let cache: CacheStore<CacheGroupPeople>;
-
-  if (peopleCacheEnabled()) {
-    cache = CacheService.getCache<CacheGroupPeople>(cacheSchema, groupStore);
-    const peopleItem = await cache.getValue(groupId);
-    if (peopleItem && getPeopleInvalidationTime() > Date.now() - peopleItem.timeCached) {
-      return peopleItem.people.map(peopleStr => JSON.parse(peopleStr));
-    }
-  }
-
-  const uri = `/groups/${groupId}/members`;
-  const people = await graph
-    .api(uri)
-    .middlewareOptions(prepScopes(scopes))
-    .get();
-  if (peopleCacheEnabled()) {
-    cache.putValue(groupId, { people: people.value.map(ppl => JSON.stringify(ppl)) });
-  }
-  return people ? people.value : null;
-}
-
 /**
  * returns a promise that resolves after specified time
  * @param time in milliseconds

packages/mgt/src/graph/graph.user.ts

@@ -9,7 +9,7 @@ import { IGraph } from '@microsoft/mgt-element';
 import { User } from '@microsoft/microsoft-graph-types';
 import { CacheItem, CacheSchema, CacheService, CacheStore } from '../utils/Cache';
 import { prepScopes } from '../utils/GraphHelpers';
-import { findPeople } from './graph.people';
+import { findPeople, PersonType } from './graph.people';
 import {
   getPhotoForResource,
   getPhotoFromCache,
@@ -420,3 +420,68 @@ export async function findUsers(graph: IGraph, query: string, top: number = 10):
   }
   return graphResult ? graphResult.value : null;
 }
+
+/**
+ * async promise, returns all matching Graph users who are member of the specified group
+ *
+ * @param {string} query
+ * @param {string} groupId - the group to query
+ * @param {number} [top=10] - number of people to return
+ * @param {PersonType} [personType=PersonType.person] - the type of person to search for
+ * @param {boolean} [transitive=false] - whether the return should contain a flat list of all nested members
+ * @returns {(Promise<User[]>)}
+ */
+export async function findGroupMembers(
+  graph: IGraph,
+  query: string,
+  groupId: string,
+  top: number = 10,
+  personType: PersonType = PersonType.person,
+  transitive: boolean = false
+): Promise<User[]> {
+  const scopes = ['user.read.all', 'people.read'];
+  const item = { maxResults: top, results: null };
+
+  let cache: CacheStore<CacheUserQuery>;
+  const key = `${groupId || '*'}:${query || '*'}:${personType}:${transitive}`;
+
+  if (usersCacheEnabled()) {
+    cache = CacheService.getCache<CacheUserQuery>(cacheSchema, queryStore);
+    const result: CacheUserQuery = await cache.getValue(key);
+
+    if (result && getUserInvalidationTime() > Date.now() - result.timeCached) {
+      return result.results.map(userStr => JSON.parse(userStr));
+    }
+  }
+
+  let filter: string = '';
+  if (query) {
+    filter = `startswith(displayName,'${query}') or startswith(givenName,'${query}') or startswith(surname,'${query}') or startswith(mail,'${query}') or startswith(userPrincipalName,'${query}')`;
+  }
+
+  let apiUrl: string = `/groups/${groupId}/${transitive ? 'transitiveMembers' : 'members'}`;
+  if (personType === PersonType.person) {
+    apiUrl += `/microsoft.graph.user`;
+  } else if (personType === PersonType.group) {
+    apiUrl += `/microsoft.graph.group`;
+    if (query) {
+      filter = `startswith(displayName,'${query}') or startswith(mail,'${query}')`;
+    }
+  }
+
+  const graphResult = await graph
+    .api(apiUrl)
+    .count(true)
+    .top(top)
+    .filter(filter)
+    .header('ConsistencyLevel', 'eventual')
+    .middlewareOptions(prepScopes(...scopes))
+    .get();
+
+  if (usersCacheEnabled() && graphResult) {
+    item.results = graphResult.value.map(userStr => JSON.stringify(userStr));
+    cache.putValue(key, item);
+  }
+
+  return graphResult ? graphResult.value : null;
+}

stories/components/peoplePicker.stories.js

@@ -74,6 +74,11 @@ export const pickPeopleAndGroups = () => html`
   <!-- type can be "any", "person", "group" -->
 `;
 
+export const pickPeopleAndGroupsNested = () => html`
+  <mgt-people-picker type="any" transitive-search="true"></mgt-people-picker>
+  <!-- type can be "any", "person", "group" -->
+`;
+
 export const pickGroups = () => html`
   <mgt-people-picker type="group"></mgt-people-picker>
   <!-- type can be "any", "person", "group" -->