Skip to content

Make ControllerBase.User virtual for unit testability. #4893

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

Closed
2 of 3 tasks
mkArtakMSFT opened this issue May 23, 2018 · 4 comments
Closed
2 of 3 tasks

Make ControllerBase.User virtual for unit testability. #4893

mkArtakMSFT opened this issue May 23, 2018 · 4 comments
Assignees
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates breaking-change This issue / pr will introduce a breaking change, when resolved / merged. enhancement This issue represents an ask for new feature or an enhancement to an existing one ✔️ Resolution: Won't Fix Resolved because we decided not to change the behavior reported in this issue.

Comments

@mkArtakMSFT
Copy link
Contributor

mkArtakMSFT commented May 23, 2018

This issue is to track a repo wide impacting changes:

Remove the compatibility switches

  • Remove compat switch for Enum DataAnnotation localization. In Update package branding to 3.0.0-preview4 #7748 we decided to fix the fact that Enum localization didn't use DataAnnotationLocalizerProvider behind a compat switch because it was a breaking change. For 3.0.0 we should remove that compat switch and only support the new behavior (usage of DataAnnotationLocalizerProvider).

Remove all the obsolete methods

Unit testing cleanup

  • Make ControllerBase.User virtual for unit testability.
@dougbu
Copy link
Contributor

dougbu commented Nov 29, 2018

This looks like the uber-issue for 3.0 😈 But, do we have an issue covering removal of [Obsolete] types and members?

I see #7382 but nothing that covers all of the ~38 [Obsolete] members e.g. the many old constructor overloads in input formatters and model binders or the ActionResult.ExecuteResult(...) overrides.

@aspnet-hello aspnet-hello transferred this issue from aspnet/Mvc Dec 14, 2018
@aspnet-hello aspnet-hello added this to the 3.0.0-preview2 milestone Dec 14, 2018
@aspnet-hello aspnet-hello added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates breaking-change This issue / pr will introduce a breaking change, when resolved / merged. enhancement This issue represents an ask for new feature or an enhancement to an existing one labels Dec 14, 2018
@mkArtakMSFT
Copy link
Contributor Author

@pranavkm, what is left here? Are some of these listed items done already?

@pranavkm
Copy link
Contributor

pranavkm commented Mar 7, 2019

Make ControllerBase.User virtual for unit testability.

I think that's the only one. I'll update the title

@pranavkm pranavkm changed the title Things to do in 3.0 Make ControllerBase.User virtual for unit testability. Mar 7, 2019
@pranavkm
Copy link
Contributor

Looking at "Make ControllerBase.User virtual for unit testability.", it's fairly trivial to assign the User property using ControllerContext. In addition, none of the properties on the ControllerBase type are virtual. I'm going to close this until we get more feedback here.

@pranavkm pranavkm added the ✔️ Resolution: Won't Fix Resolved because we decided not to change the behavior reported in this issue. label Mar 27, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates breaking-change This issue / pr will introduce a breaking change, when resolved / merged. enhancement This issue represents an ask for new feature or an enhancement to an existing one ✔️ Resolution: Won't Fix Resolved because we decided not to change the behavior reported in this issue.
Projects
None yet
Development

No branches or pull requests

4 participants