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

Separate DbContexts, separate all views/controllers from user-defined configuration #169

Closed
skoruba opened this issue Feb 15, 2019 · 3 comments
Assignees
Labels
help wanted Extra attention is needed priority: high task Task

Comments

@skoruba
Copy link
Owner

skoruba commented Feb 15, 2019

Related to amazing feedback from @Brice-xCIT - I've decided to rework this solution - especially these areas:

  • Completely separate all code from AdminDbContext. Any method that has AdminDbContext as a generic parameter is a design issue. It means “I assume you will use one context”. In the current code, most such methods do not offer a variant for separated contexts. All these methods should either offer such a variant, or be rewritten to use the interfaces atomically. It’s okay to offer variants to support the use case where people want AdminDbContext, but it shouldn’t be a requirement. (* This is the main contribution of the fork, and in the process, the below problems emerged)
  • Completely separate all controllers from user-defined configuration. This means no generic type in any dependency of any controller, or if generics are unavoidable, use Mvc mechanisms to generate generic controller instances at runtime. (* This was done in the fork)
  • Completely separate all views from user-defined configuration. This means no more Dtos in views, or make Dtos inherit non-generic types that expose invariant representations on their data. More likely, compute view models in controllers by calling on user-customizable services that do provide the configuration or conversions (e.g. defining key type and converting it to string). (* not done in the fork because it seemed like a massive refactor)
  • Provide implementations of separated data contexts. I had to write new adapter classes to bind to vanilla IS data contexts. That’s weird for such a basic use case. (* This was done in the fork)
  • Provide more helpful startup helpers, or a builder class to help configurate. The ones that exist are very tied to AdminDbContext. I had to write many variants to support my use case. A builder with nice config methods would also go a long way.
  • Extract base lib project and provide NuGet package. Last but not least: most of the .Admin code should be extracted into a library project, so that it could be used easily from an app. And then there would be a .Admin.App that would use this extracted lib as well. This way the code could be usable right away in any ASP.Net Core app. (Issue encapsulate project to injectable parts #28 ftw!)

The fork with changes from @Brice-xCIT is available - here.

Another awesome feedback is from @TheEvilPenguin - which provides solution for - separate all views from user-defined configuration - here.

I will use these solutions and rework this repo according the suggestions above. :)

If you will have a time for creating the PR's - it would be great!

Thank you guys.

@skoruba skoruba added help wanted Extra attention is needed task Task priority: high labels Feb 15, 2019
@skoruba skoruba self-assigned this Feb 15, 2019
@skoruba
Copy link
Owner Author

skoruba commented Feb 17, 2019

@Brice-xCIT @TheEvilPenguin - I have used your changes and put them into dev branch.
Please take a look. :)
Thanks!

@Brice-xCIT
Copy link
Contributor

Hi @skoruba! I have used the latest changes from the dev branch, and it was much easier to adapt it to my use case, and to refactor for the PR #225. Congrats! As far as I'm concerned, this issue is now solved.

@skoruba skoruba mentioned this issue Apr 4, 2019
@skoruba
Copy link
Owner Author

skoruba commented Apr 4, 2019

Done on master. Please check new release.
Thanks!

@skoruba skoruba closed this as completed Apr 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed priority: high task Task
Projects
None yet
Development

No branches or pull requests

2 participants