From 582d5eca684d49c333a5d47bfa36eebc6d4604be Mon Sep 17 00:00:00 2001 From: Gavin Barron Date: Fri, 10 Nov 2023 16:34:54 +0000 Subject: [PATCH 1/2] feat!: split static config and permission methods out of mgt-personon-card splits out the logic for cacluating the scopes required for the mgt-person-card into a separate function and file splits the static config for MgtPersonCard into a separate class changes the schema of the config for MgtPersonCard fixes an issue where setting isSendMessageVisible to false did not stop the quick message ui from rendering adds tests to validate the permission sets BREAKING CHANGE: MgtPersonCard no longer has a static config property.This config has been moved to the MgtPersonCardConfig class to allow developers to import the config and associated getMgtPersonCardScopes function at the top level of their applicaiton without automatically adding the weight of the full mgt-person-card component and dependencies to the entry file for their applications. BREAKING CHANGE: The organization section for configuring MgtPersonCard may no longer be set to false, use undefined instead. --- .../src/components/components.ts | 2 + .../mgt-person-card/MgtPersonCardConfig.ts | 48 +++++++ .../getMgtPersonCardScopes.tests.ts | 123 ++++++++++++++++++ .../mgt-person-card/getMgtPersonCardScopes.ts | 57 ++++++++ .../mgt-person-card/mgt-person-card.graph.ts | 16 +-- .../mgt-person-card/mgt-person-card.ts | 110 +++------------- .../mgt-person-card/mgt-person-card.types.ts | 73 ----------- samples/react-contoso/src/index.tsx | 3 + 8 files changed, 258 insertions(+), 174 deletions(-) create mode 100644 packages/mgt-components/src/components/mgt-person-card/MgtPersonCardConfig.ts create mode 100644 packages/mgt-components/src/components/mgt-person-card/getMgtPersonCardScopes.tests.ts create mode 100644 packages/mgt-components/src/components/mgt-person-card/getMgtPersonCardScopes.ts diff --git a/packages/mgt-components/src/components/components.ts b/packages/mgt-components/src/components/components.ts index d4989026f6..c309fe5983 100644 --- a/packages/mgt-components/src/components/components.ts +++ b/packages/mgt-components/src/components/components.ts @@ -36,6 +36,8 @@ export * from './mgt-get/mgt-get'; export * from './mgt-login/mgt-login'; export * from './mgt-people-picker/mgt-people-picker'; export * from './mgt-people/mgt-people'; +export * from './mgt-person-card/MgtPersonCardConfig'; +export * from './mgt-person-card/getMgtPersonCardScopes'; export * from './mgt-person-card/mgt-person-card'; export * from './mgt-person/mgt-person'; export * from './mgt-person/mgt-person-types'; diff --git a/packages/mgt-components/src/components/mgt-person-card/MgtPersonCardConfig.ts b/packages/mgt-components/src/components/mgt-person-card/MgtPersonCardConfig.ts new file mode 100644 index 0000000000..8a0bb27bd7 --- /dev/null +++ b/packages/mgt-components/src/components/mgt-person-card/MgtPersonCardConfig.ts @@ -0,0 +1,48 @@ +import { IMgtPersonCardConfig } from './mgt-person-card.types'; + +interface SectionsConfig { + /** + * Gets or sets whether the organization section is shown + * + */ + organization?: { + /** + * Gets or sets whether the "Works with" section is shown + * + * @type {boolean} + */ + showWorksWith: boolean; + }; + + /** + * Gets or sets whether the messages section is shown + * + * @type {boolean} + */ + mailMessages: boolean; + + /** + * Gets or sets whether the files section is shown + * + * @type {boolean} + */ + files: boolean; + + /** + * Gets or sets whether the profile section is shown + * + * @type {boolean} + */ + profile: boolean; +} + +export class MgtPersonCardConfig { + public static sections = { + files: true, + mailMessages: true, + organization: { showWorksWith: true }, + profile: true + }; + public static useContactApis = true; + public static isSendMessageVisible = true; +} diff --git a/packages/mgt-components/src/components/mgt-person-card/getMgtPersonCardScopes.tests.ts b/packages/mgt-components/src/components/mgt-person-card/getMgtPersonCardScopes.tests.ts new file mode 100644 index 0000000000..0b1a5fdd38 --- /dev/null +++ b/packages/mgt-components/src/components/mgt-person-card/getMgtPersonCardScopes.tests.ts @@ -0,0 +1,123 @@ +import { MgtPersonCardConfig } from './MgtPersonCardConfig'; +import { expect } from '@open-wc/testing'; +import { getMgtPersonCardScopes } from './getMgtPersonCardScopes'; + +describe('getMgtPersonCardScopes() tests', () => { + let originalConfigMessaging: typeof MgtPersonCardConfig.isSendMessageVisible; + let originalConfigContactApis: typeof MgtPersonCardConfig.useContactApis; + let originalConfigOrgSection: typeof MgtPersonCardConfig.sections.organization; + let originalConfigSections: typeof MgtPersonCardConfig.sections; + before(() => { + originalConfigOrgSection = { ...MgtPersonCardConfig.sections.organization }; + originalConfigSections = { ...MgtPersonCardConfig.sections }; + originalConfigContactApis = MgtPersonCardConfig.useContactApis; + originalConfigMessaging = MgtPersonCardConfig.isSendMessageVisible; + }); + beforeEach(() => { + MgtPersonCardConfig.sections = { ...originalConfigSections }; + MgtPersonCardConfig.sections.organization = { ...originalConfigOrgSection }; + MgtPersonCardConfig.useContactApis = originalConfigContactApis; + MgtPersonCardConfig.isSendMessageVisible = originalConfigMessaging; + }); + it('should have a minimal permission set', () => { + const expectedScopes = [ + 'User.Read.All', + 'People.Read.All', + 'Sites.Read.All', + 'Mail.Read', + 'Mail.ReadBasic', + 'Contacts.Read', + 'Chat.ReadWrite' + ]; + expect(getMgtPersonCardScopes()).to.have.members(expectedScopes); + }); + + it('should have not have Sites.Read.All if files is configured off', () => { + MgtPersonCardConfig.sections.files = false; + + const expectedScopes = [ + 'User.Read.All', + 'People.Read.All', + 'Mail.Read', + 'Mail.ReadBasic', + 'Contacts.Read', + 'Chat.ReadWrite' + ]; + expect(getMgtPersonCardScopes()).to.have.members(expectedScopes); + }); + + it('should have not have Mail scopes if mail is configured off', () => { + MgtPersonCardConfig.sections.mailMessages = false; + + const expectedScopes = ['User.Read.All', 'People.Read.All', 'Sites.Read.All', 'Contacts.Read', 'Chat.ReadWrite']; + expect(getMgtPersonCardScopes()).to.have.members(expectedScopes); + }); + + it('should have People.Read but not People.Read.All if showWorksWith is false', () => { + MgtPersonCardConfig.sections.organization.showWorksWith = false; + const expectedScopes = [ + 'User.Read.All', + 'People.Read', + 'Sites.Read.All', + 'Mail.Read', + 'Mail.ReadBasic', + 'Contacts.Read', + 'Chat.ReadWrite' + ]; + expect(getMgtPersonCardScopes()).to.have.members(expectedScopes); + }); + + it('should have not have User.Read.All if profile and organization are false', () => { + MgtPersonCardConfig.sections.organization = undefined; + MgtPersonCardConfig.sections.profile = false; + + const expectedScopes = [ + 'User.Read', + 'User.ReadBasic.All', + 'People.Read', + 'Sites.Read.All', + 'Mail.Read', + 'Mail.ReadBasic', + 'Contacts.Read', + 'Chat.ReadWrite' + ]; + const actualScopes = getMgtPersonCardScopes(); + expect(actualScopes).to.have.members(expectedScopes); + + expect(actualScopes).to.not.include('User.Read.All'); + }); + + it('should have not have Chat.ReadWrite if isSendMessageVisible is false', () => { + MgtPersonCardConfig.isSendMessageVisible = false; + + const expectedScopes = [ + 'User.Read.All', + 'People.Read.All', + 'Sites.Read.All', + 'Mail.Read', + 'Mail.ReadBasic', + 'Contacts.Read' + ]; + const actualScopes = getMgtPersonCardScopes(); + expect(actualScopes).to.have.members(expectedScopes); + + expect(actualScopes).to.not.include('Chat.ReadWrite'); + }); + + it('should have not have Chat.ReadWrite if useContactApis is false', () => { + MgtPersonCardConfig.useContactApis = false; + + const expectedScopes = [ + 'User.Read.All', + 'People.Read.All', + 'Sites.Read.All', + 'Mail.Read', + 'Mail.ReadBasic', + 'Chat.ReadWrite' + ]; + const actualScopes = getMgtPersonCardScopes(); + expect(actualScopes).to.have.members(expectedScopes); + + expect(actualScopes).to.not.include('Contacts.Read'); + }); +}); diff --git a/packages/mgt-components/src/components/mgt-person-card/getMgtPersonCardScopes.ts b/packages/mgt-components/src/components/mgt-person-card/getMgtPersonCardScopes.ts new file mode 100644 index 0000000000..f30bf98e25 --- /dev/null +++ b/packages/mgt-components/src/components/mgt-person-card/getMgtPersonCardScopes.ts @@ -0,0 +1,57 @@ +import { MgtPersonCardConfig } from './MgtPersonCardConfig'; + +/** + * Scopes used to fetch data for the mgt-person-card component + * + * @static + * @return {*} {string[]} + * @memberof MgtPersonCard + */ + +export const getMgtPersonCardScopes = (): string[] => { + const scopes: string[] = []; + + if (MgtPersonCardConfig.sections.files) { + scopes.push('Sites.Read.All'); + } + + if (MgtPersonCardConfig.sections.mailMessages) { + scopes.push('Mail.Read'); + scopes.push('Mail.ReadBasic'); + } + + if (MgtPersonCardConfig.sections.organization) { + scopes.push('User.Read.All'); + + if (MgtPersonCardConfig.sections.organization.showWorksWith) { + scopes.push('People.Read.All'); + } + } + + if (MgtPersonCardConfig.sections.profile) { + scopes.push('User.Read.All'); + } + + if (MgtPersonCardConfig.useContactApis) { + scopes.push('Contacts.Read'); + } + + if (scopes.indexOf('User.Read.All') < 0) { + // at minimum, we need these scopes + scopes.push('User.ReadBasic.All'); + scopes.push('User.Read'); + } + + if (scopes.indexOf('People.Read.All') < 0) { + // at minimum, we need these scopes + scopes.push('People.Read'); + } + + if (MgtPersonCardConfig.isSendMessageVisible) { + // Chat.ReadWrite can create a chat and send a message, so just request one scope instead of two + scopes.push('Chat.ReadWrite'); + } + + // return unique + return [...new Set(scopes)]; +}; diff --git a/packages/mgt-components/src/components/mgt-person-card/mgt-person-card.graph.ts b/packages/mgt-components/src/components/mgt-person-card/mgt-person-card.graph.ts index c0ac5f8589..0fb63f6c79 100644 --- a/packages/mgt-components/src/components/mgt-person-card/mgt-person-card.graph.ts +++ b/packages/mgt-components/src/components/mgt-person-card/mgt-person-card.graph.ts @@ -11,7 +11,8 @@ import { Profile } from '@microsoft/microsoft-graph-types-beta'; import { getEmailFromGraphEntity } from '../../graph/graph.people'; import { IDynamicPerson } from '../../graph/types'; -import { MgtPersonCardConfig, MgtPersonCardState } from './mgt-person-card.types'; +import { MgtPersonCardState } from './mgt-person-card.types'; +import { MgtPersonCardConfig } from './MgtPersonCardConfig'; const userProperties = 'businessPhones,companyName,department,displayName,givenName,jobTitle,mail,mobilePhone,officeLocation,preferredLanguage,surname,userPrincipalName,id,accountEnabled'; @@ -37,8 +38,7 @@ const batchKeys = { export const getPersonCardGraphData = async ( graph: IGraph, personDetails: IDynamicPerson, - isMe: boolean, - config: MgtPersonCardConfig + isMe: boolean ): Promise => { const userId = personDetails.id; const email = getEmailFromGraphEntity(personDetails); @@ -51,20 +51,20 @@ export const getPersonCardGraphData = async ( const batch = graph.createBatch(); if (!isContactOrGroup) { - if (config.sections.organization) { + if (MgtPersonCardConfig.sections.organization) { buildOrgStructureRequest(batch, userId); - if (typeof config.sections.organization !== 'boolean' && config.sections.organization.showWorksWith) { + if (MgtPersonCardConfig.sections.organization.showWorksWith) { buildWorksWithRequest(batch, userId); } } } - if (config.sections.mailMessages && email) { + if (MgtPersonCardConfig.sections.mailMessages && email) { buildMessagesWithUserRequest(batch, email); } - if (config.sections.files) { + if (MgtPersonCardConfig.sections.files) { buildFilesRequest(batch, isMe ? null : email); } @@ -83,7 +83,7 @@ export const getPersonCardGraphData = async ( } } - if (!isContactOrGroup && config.sections.profile) { + if (!isContactOrGroup && MgtPersonCardConfig.sections.profile) { try { const profile = await getProfile(graph, userId); if (profile) { diff --git a/packages/mgt-components/src/components/mgt-person-card/mgt-person-card.ts b/packages/mgt-components/src/components/mgt-person-card/mgt-person-card.ts index 31c46ef6cc..e1a6381264 100644 --- a/packages/mgt-components/src/components/mgt-person-card/mgt-person-card.ts +++ b/packages/mgt-components/src/components/mgt-person-card/mgt-person-card.ts @@ -5,7 +5,7 @@ * ------------------------------------------------------------------------------------------- */ -import { html, TemplateResult } from 'lit'; +import { html, nothing, TemplateResult } from 'lit'; import { property, state } from 'lit/decorators.js'; import { classMap } from 'lit/directives/class-map.js'; import { @@ -33,7 +33,9 @@ import { MgtFileList, registerMgtFileListComponent } from '../mgt-file-list/mgt- import { MgtMessages, registerMgtMessagesComponent } from '../mgt-messages/mgt-messages'; import { MgtOrganization, registerMgtOrganizationComponent } from '../mgt-organization/mgt-organization'; import { MgtProfile, registerMgtProfileComponent } from '../mgt-profile/mgt-profile'; -import { MgtPersonCardConfig, MgtPersonCardState } from './mgt-person-card.types'; +import { MgtPersonCardState } from './mgt-person-card.types'; +import { MgtPersonCardConfig } from './MgtPersonCardConfig'; +import { getMgtPersonCardScopes } from './getMgtPersonCardScopes'; import { strings } from './strings'; import { isUser } from '../../graph/entityType'; @@ -63,7 +65,7 @@ interface MgtPersonCardStateHistory { export const registerMgtPersonCardComponent = () => { registerFluentComponents(fluentCard, fluentTabs, fluentTab, fluentTabPanel, fluentButton, fluentTextField); - // register self first to avoid infinte loop due to circular ref between person and person card and organization + // register self first to avoid infinite loop due to circular ref between person and person card and organization registerComponent('person-card', MgtPersonCard); registerMgtSpinnerComponent(); @@ -147,84 +149,9 @@ export class MgtPersonCard extends MgtTemplatedComponent implements IHistoryClea * @memberof MgtPersonCard */ public static get requiredScopes(): string[] { - return MgtPersonCard.getScopes(); + return getMgtPersonCardScopes(); } - /** - * Scopes used to fetch data for the component - * - * @static - * @return {*} {string[]} - * @memberof MgtPersonCard - */ - public static getScopes(): string[] { - const scopes: string[] = []; - - if (this.config.sections.files) { - scopes.push('Sites.Read.All'); - } - - if (this.config.sections.mailMessages) { - scopes.push('Mail.Read'); - scopes.push('Mail.ReadBasic'); - } - - if (this.config.sections.organization) { - scopes.push('User.Read.All'); - - if (typeof this.config.sections.organization !== 'boolean' && this.config.sections.organization.showWorksWith) { - scopes.push('People.Read.All'); - } - } - - if (this.config.sections.profile) { - scopes.push('User.Read.All'); - } - - if (this.config.useContactApis) { - scopes.push('Contacts.Read'); - } - - if (scopes.indexOf('User.Read.All') < 0) { - // at minimum, we need these scopes - scopes.push('User.ReadBasic.All'); - scopes.push('User.Read'); - } - - if (scopes.indexOf('People.Read.All') < 0) { - // at minimum, we need these scopes - scopes.push('People.Read'); - } - - scopes.push('Chat.Create', 'Chat.ReadWrite'); - - // return unique - return [...new Set(scopes)]; - } - - /** - * Global configuration object for - * all person card components - * - * @static - * @type {MgtPersonCardConfig} - * @memberof MgtPersonCard - */ - public static get config() { - return this._config; - } - - private static readonly _config: MgtPersonCardConfig = { - sections: { - files: true, - mailMessages: true, - organization: { showWorksWith: true }, - profile: true - }, - useContactApis: true, - isSendMessageVisible: true - }; - /** * Set the person details to render * @@ -954,14 +881,15 @@ export class MgtPersonCard extends MgtTemplatedComponent implements IHistoryClea * @returns {TemplateResult} * @memberof MgtPersonCard */ - protected renderMessagingSection(): TemplateResult { + protected renderMessagingSection() { const person = this.personDetails as User; const user = this._me.userPrincipalName; const chatInput = this._chatInput; if (person?.userPrincipalName === user) { return; } else { - return html` + return MgtPersonCardConfig.isSendMessageVisible + ? html`
- `; + ` + : nothing; } } @@ -1056,7 +985,7 @@ export class MgtPersonCard extends MgtTemplatedComponent implements IHistoryClea if (people?.length) { this.personDetails = people[0]; - await getPersonImage(graph, this.personDetails, MgtPersonCard.config.useContactApis).then(image => { + await getPersonImage(graph, this.personDetails, MgtPersonCardConfig.useContactApis).then(image => { if (image) { this.personDetails.personImage = image; this.personImage = image; @@ -1086,12 +1015,7 @@ export class MgtPersonCard extends MgtTemplatedComponent implements IHistoryClea // populate state if (this.personDetails?.id) { - this._cardState = await getPersonCardGraphData( - graph, - this.personDetails, - this._me === this.personDetails.id, - MgtPersonCard.config - ); + this._cardState = await getPersonCardGraphData(graph, this.personDetails, this._me === this.personDetails.id); } this.loadSections(); @@ -1281,19 +1205,19 @@ export class MgtPersonCard extends MgtTemplatedComponent implements IHistoryClea const { person, directReports, messages, files, profile } = this._cardState; - if (MgtPersonCard.config.sections.organization && (person?.manager || directReports?.length)) { + if (MgtPersonCardConfig.sections.organization && (person?.manager || directReports?.length)) { this.sections.push(new MgtOrganization(this._cardState, this._me)); } - if (MgtPersonCard.config.sections.mailMessages && messages?.length) { + if (MgtPersonCardConfig.sections.mailMessages && messages?.length) { this.sections.push(new MgtMessages(messages)); } - if (MgtPersonCard.config.sections.files && files?.length) { + if (MgtPersonCardConfig.sections.files && files?.length) { this.sections.push(new MgtFileList()); } - if (MgtPersonCard.config.sections.profile && profile) { + if (MgtPersonCardConfig.sections.profile && profile) { const profileSection = new MgtProfile(profile); if (profileSection.hasData) { this.sections.push(profileSection); diff --git a/packages/mgt-components/src/components/mgt-person-card/mgt-person-card.types.ts b/packages/mgt-components/src/components/mgt-person-card/mgt-person-card.types.ts index abb131b1e2..43108b67c7 100644 --- a/packages/mgt-components/src/components/mgt-person-card/mgt-person-card.types.ts +++ b/packages/mgt-components/src/components/mgt-person-card/mgt-person-card.types.ts @@ -8,79 +8,6 @@ import { Message, Person, SharedInsight, User } from '@microsoft/microsoft-graph-types'; import { Profile } from '@microsoft/microsoft-graph-types-beta'; -/** - * Person Card Global Configuration Type - * - * @export - * @interface MgtPersonCardConfig - */ -export interface MgtPersonCardConfig { - /** - * Sets or gets whether the person card component can use Contacts APIs to - * find contacts and their images - * - * @type {boolean} - */ - useContactApis: boolean; - - /** - * Sets whether the person card component can directly send messages - * - * @type {boolean} - */ - isSendMessageVisible: boolean; - - /** - * Gets or sets whether each subsection should be shown - * - * @type {{ - * contact: boolean; - * organization: boolean; - * mailMessages: boolean; - * files: boolean; - * profile: boolean; - * }} - * @memberof MgtPersonCardConfig - */ - sections: { - /** - * Gets or sets whether the organization section is shown - * - */ - organization: - | boolean - | { - /** - * Gets or sets whether the "Works with" section is shown - * - * @type {boolean} - */ - showWorksWith: boolean; - }; - - /** - * Gets or sets whether the messages section is shown - * - * @type {boolean} - */ - mailMessages: boolean; - - /** - * Gets or sets whether the files section is shown - * - * @type {boolean} - */ - files: boolean; - - /** - * Gets or sets whether the profile section is shown - * - * @type {boolean} - */ - profile: boolean; - }; -} - export type UserWithManager = User & { manager?: UserWithManager }; export interface MgtPersonCardState { diff --git a/samples/react-contoso/src/index.tsx b/samples/react-contoso/src/index.tsx index eeb2f8c302..d9c3da3f58 100644 --- a/samples/react-contoso/src/index.tsx +++ b/samples/react-contoso/src/index.tsx @@ -3,6 +3,9 @@ import { App } from './App'; import { mergeStyles } from '@fluentui/react'; import { Msal2Provider } from '@microsoft/mgt-msal2-provider/dist/es6/exports'; import { Providers, LoginType } from '@microsoft/mgt-element'; +import { MgtPersonCardConfig } from '@microsoft/mgt-components/dist/es6/exports'; + +MgtPersonCardConfig.isSendMessageVisible = false; // Inject some global styles mergeStyles({ From 7802228797efffccd34af565dbb734ccbe0c812e Mon Sep 17 00:00:00 2001 From: Gavin Barron Date: Fri, 10 Nov 2023 17:21:52 +0000 Subject: [PATCH 2/2] clean up dead import --- .../src/components/mgt-person-card/MgtPersonCardConfig.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/mgt-components/src/components/mgt-person-card/MgtPersonCardConfig.ts b/packages/mgt-components/src/components/mgt-person-card/MgtPersonCardConfig.ts index 8a0bb27bd7..ac7bf9c3b9 100644 --- a/packages/mgt-components/src/components/mgt-person-card/MgtPersonCardConfig.ts +++ b/packages/mgt-components/src/components/mgt-person-card/MgtPersonCardConfig.ts @@ -1,5 +1,3 @@ -import { IMgtPersonCardConfig } from './mgt-person-card.types'; - interface SectionsConfig { /** * Gets or sets whether the organization section is shown @@ -37,7 +35,7 @@ interface SectionsConfig { } export class MgtPersonCardConfig { - public static sections = { + public static sections: SectionsConfig = { files: true, mailMessages: true, organization: { showWorksWith: true },