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

Disable launching browser on Web API template #57682

Merged
merged 1 commit into from
Sep 10, 2024

Conversation

captainsafia
Copy link
Member

Description

Earlier in this release cycle, we removed the Swagger UI from the built-in templates and updated the launch URL on the Web API templates to point to the /weatherforecast endpoint.

Recently, we discovered that this default might be problematic for scenarios where the templates are instantiated with authentication configured. In this case, navigating to /weatherforecast on launch will result in 401 errors as the browser is unauthenticated.

This sparked conversation about whether it was meaningful to launch URL to this endpoint at all and we decided to remove it in favor of allowing editors/users to configure the launch behavior as desired by their launch configuration.

Resolve https://github.com/dotnet/AspNetCore-ManualTests/issues/3002.

Customer Impact

Prior to this PR, customers that are creating new templates with authentication enabled might see confusing behavior when launching their applications without authentication configured correctly.

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

This change only impacts the Web API templates in .NET 9 and beyond and doesn't affect the runtime behavior of the application.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Sep 3, 2024
Copy link
Contributor

@mikekistler mikekistler left a comment

Choose a reason for hiding this comment

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

Looks good! 👍

@sayedihashimi
Copy link
Member

LGTM

@halter73
Copy link
Member

halter73 commented Sep 4, 2024

We could remove these lines from the F# template as well for consistency, but that hasn't regressed since it never did have Swagger UI.

@gfoidl
Copy link
Member

gfoidl commented Sep 4, 2024

Maybe this also resolves #57507

@glen-84 is this what you aimed for?

@glen-84
Copy link
Contributor

glen-84 commented Sep 4, 2024

@gfoidl Not quite. I'm suggesting that the default for all templates is set to false (or omitted), since both Rider and VS Code open a new tab each time that the application is launched, which leads to a very poor UX.

@sayedihashimi
Copy link
Member

@gfoidl Not quite. I'm suggesting that the default for all templates is set to false (or omitted), since both Rider and VS Code open a new tab each time that the application is launched, which leads to a very poor UX.

I disagree. Having this enabled by default is important for new users as well as learners. Removing that will cause friction for those users. For the experienced developers, they can configure the setting to false if they prefer that.

Regardless of the default, some users will be dissatisfied, there is no setting here which every user will be happy with.

@captainsafia captainsafia added Servicing-consider Shiproom approval is required for the issue Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Sep 5, 2024
@captainsafia
Copy link
Member Author

Approved via email!

@captainsafia
Copy link
Member Author

@wtgodbe Can I get help merging this?

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 Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants