Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ProjectTemplates to Id Web 2.12.2 + MS Graph v5 support #48863

Merged
merged 4 commits into from
Jun 30, 2023

Conversation

jennyf19
Copy link
Contributor

Update Id Web ProjectTempates to latest version

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Summary of the changes (Less than 80 chars)

Description

Update to the Id Web Project Templates to latest version (2.12.2) which includes support for the new MS Graph SDK v5. Updates are in project references (from 2.10 to 2.12.2), and new package reference from Microsoft.Identity.Web.MicrosoftGraph to Microsoft.Identity.Web.GraphServiceClient and updates in the respective controllers in graphserviceclient section. More information here on the Graph updates. Thank you @surayya-MS and @halter73 for doing the bulk of the work in updating from 1.x to 2.10, much appreciated and unexpected! :)

cc: @captainsafia @Tratcher

Part of issue: AzureAD/microsoft-identity-web#2097

… SDK v5. Updates are in project references (from 2.10 to 2.12.2), and new package reference from Microsoft.Identity.Web.MicrosoftGraph to Microsoft.Identity.Web.GraphServiceClient and updates in the respective controllers in graphserviceclient section.
@jennyf19 jennyf19 requested review from a team and wtgodbe as code owners June 16, 2023 19:41
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jun 16, 2023
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Jun 16, 2023
@ghost
Copy link

ghost commented Jun 16, 2023

Thanks for your PR, @jennyf19. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@mkArtakMSFT
Copy link
Member

Thanks @jennyf19.
@halter73 can you please review this? Thanks!

@captainsafia
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@captainsafia
Copy link
Member

The template tests were failing because the new GraphService package was not in the mirrored feeds. I submitted a request to add it to the mirror and am kicking the build to see the results.

@mkArtakMSFT
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@captainsafia
Copy link
Member

@jennyf19 OK -- it looks like test failures related to NuGet packages not being in the mirrored feeds have been resolved. The current failures seem like issues with the template change:

2023-06-21T05:15:14.3581521Z [xUnit.net 00:02:02.01]     Templates.Mvc.Test.WebApiTemplateTest.WebApiTemplateCSharp_IdentityWeb_SingleOrg_BuildsAndPublishes(auth: "SingleOrg", args: ["--use-minimal-apis", "--calls-graph"]) [FAIL]
2023-06-21T05:15:14.3588716Z [xUnit.net 00:02:02.01]       Project new webapi  --auth SingleOrg --use-minimal-apis --calls-graph failed to publish. Exit code 1.
2023-06-21T05:15:14.3597815Z [xUnit.net 00:02:02.01]       dotnet publish --no-restore -c Release /bl \nStdErr: \nStdOut: MSBuild version 17.7.0-preview-23302-06+9604d20e7 for .NET
2023-06-21T05:15:14.3606111Z [xUnit.net 00:02:02.01]       C:\h\w\AD2F09A7\p\dotnet-cli\sdk\8.0.100-preview.6.23305.3\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.RuntimeIdentifierInference.targets(314,5): message NETSDK1057: You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy [C:\h\w\AD2F09A7\w\B21809B0\e\Templates\BaseFolder\AspNet.ndwbl0px51vv\AspNet.ndwbl0px51vv.csproj]
2023-06-21T05:15:14.3614138Z [xUnit.net 00:02:02.01]       C:\h\w\AD2F09A7\w\B21809B0\e\Templates\BaseFolder\AspNet.ndwbl0px51vv\Program.cs(43,44): error CS1061: 'MeRequestBuilder' does not contain a definition for 'Request' and no accessible extension method 'Request' accepting a first argument of type 'MeRequestBuilder' could be found (are you missing a using directive or an assembly reference?) [C:\h\w\AD2F09A7\w\B21809B0\e\Templates\BaseFolder\AspNet.ndw

It looks like there might be some other templates that need to be updated to use the correct API?

@jennyf19
Copy link
Contributor Author

Thanks @captainsafia , I thought I had all the Id Web ones. Will take a look. thanks for all your help.

@@ -0,0 +1,12 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

Is this change needed?

Copy link
Member

Choose a reason for hiding this comment

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

I was inclined to agree with this, and I went as far as to checkout the jennyf19:jennyf/idWebTemplatesUpdate branch and revert 331d122 before merging, only to notice that my VS had automatically created a different launchSettings.json file. I don't think it makes any sense for a Microsoft.NET.Sdk.BlazorWebAssembly project, but it's just going to keep cluttering git's untracked files until we check it in or ignore it.

I'm just going to check it in for now since it seems harmless and I don't want to rekick the build, but we should figure out why VS is generating this. @SteveSandersonMS @dotnet/razor-tooling

Choose a reason for hiding this comment

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

VS automatically creating a launchsettings file would be a question for @BillHiebert or @tmeschter I believe. I'm not sure why it necessarily doesn't make sense to have one for a Blazor WASM project though, the project system needs to know what to tell the debugger when people press F5, AFAIK. Maybe things can be changed to not write a default to disk though?

@@ -0,0 +1,12 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

I was inclined to agree with this, and I went as far as to checkout the jennyf19:jennyf/idWebTemplatesUpdate branch and revert 331d122 before merging, only to notice that my VS had automatically created a different launchSettings.json file. I don't think it makes any sense for a Microsoft.NET.Sdk.BlazorWebAssembly project, but it's just going to keep cluttering git's untracked files until we check it in or ignore it.

I'm just going to check it in for now since it seems harmless and I don't want to rekick the build, but we should figure out why VS is generating this. @SteveSandersonMS @dotnet/razor-tooling

@halter73 halter73 merged commit 9da6177 into dotnet:main Jun 30, 2023
@ghost ghost added this to the 8.0-preview7 milestone Jun 30, 2023
@halter73
Copy link
Member

Thanks for the PR @jennyf19, and thanks for mirroring the packages @captainsafia!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants