Skip to content
This repository was archived by the owner on May 3, 2024. It is now read-only.

Update 2 1 call graph #174

Merged
merged 3 commits into from
Nov 14, 2022
Merged

Update 2 1 call graph #174

merged 3 commits into from
Nov 14, 2022

Conversation

salman90
Copy link
Contributor

Purpose

Minor updates to sample:

  1. Update sample sub path in the readme.
  2. Add contacts read permission to login request.
  3. Handle error case in contacts page.

Thanks.

Does this introduce a breaking change?

[ x ] Yes
[ ] No

Pull Request Type

What kind of change does this Pull Request introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ x ] Refactoring (no functional changes, no api changes)
[ x ] Documentation content changes
[ ] Other... Please describe:

How to Test

  • Get the code
git clone [repo-address]
cd [repo-name]
git checkout [branch-name]
npm install
  • Test the code

What to Check

Verify that the following are valid

  • ...

Other Information

@salman90 salman90 requested a review from derisen November 13, 2022 20:00
@@ -58,7 +58,7 @@ export const msalConfig = {
* https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-permissions-and-consent#openid-connect-scopes
*/
export const loginRequest = {
scopes: ['User.Read']
scopes: ['User.Read', 'Contacts.Read'],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@salman90 is dynamic consent not working in the sample?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@derisen The popup window is not stable when closing. I pumped the MSAL versions to include the fix that the MSAL team worked on, and when calling useMsalAuthentication hook with Popup, I redirect to an empty page.

It works fine for me now, but I am keeping the consent on login as a safeguard.

@salman90 salman90 requested a review from derisen November 14, 2022 15:58
Copy link
Contributor

@derisen derisen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@salman90 salman90 merged commit 27add75 into main Nov 14, 2022
@derisen derisen deleted the update-2-1-call-graph branch June 25, 2023 03:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants