baratgabor/mywarehouse

Clean Architecture and Domain Driven Design sample project based on C# 10 / .NET 6 / ASP.NET Core 6 / EF Core 6 & Angular 11 with Bootstrap.

mywarehouse's Issues

Procurement transaction creation displays confusing prices

Transactions aren't designed to be purchase and sales orders; they're more like modeling simple inventory transactions.

But while creating a procurement transaction, the form kind of looks like the products will be purchased for the given amount of money, which is nonsense, since what we're showing is based on the registered price of the products in our own system.

Adjust the form while keeping the following in mind:

• Would be nice if we didn't have to complicate it further and could still use the same form for sales and procurement.
• Transactions should be kept as inventory transactions, and we shouldn't overblow this sample app with PO and SO creation, separate purchase price, etc., because it would be too much trouble to model it nicely, and not doing that would make it look stupid.
• But still keep showing prices in some way, because that adds at least some visually appreciable complexity to the form.

Cannot browse all products when creating transactions

The typeahead search on transaction creation modal was hastily implemented, and it only shows a certain number of products, without ability to display a scrollable list. If too many products match a given search keyword, only the first X will be selectable.

Instead of hard-limiting the number of products to a low number, implement a scrolling result list and show significantly more products.

Optionally also implement an alternative product selector that lists all products without searching.

problem in update-datbase

when i use update-database ,I encounter the following error:
Update-Database
Build started...
Build succeeded.
An error occurred while accessing the Microsoft.Extensions.Hosting services. Continuing without the application service provider. Error: Exception has been thrown by the target of an invocation.
Unable to create an object of type 'ApplicationDbContext'. For the different patterns supported at design time, see https://go.microsoft.com/fwlink/?linkid=851728

Login payload change broke Swagger login

Recently the login payload was changed from the lame-ass OAuth2 access token response format to non-underscored property names (as part of generally extending the returned properties to include certain user data).

This broke the ability to log in via Swagger, because that actually expects on OAuth2 format.

Task:

• Reconfigure Swagger to look for the token via a different property name (if possible), or
• Create a separate token response for Swagger (it already uses a separate controller action), or
• Just change the name of the property back.

This bug doesn't affect "production", since Swagger is disabled in prod mode.

Add unit tests for WebApi

The backend entry point, the WebApi project, is currently not tested.

Since this is an extremely shallow layer that is almost solely responsible for mapping an URI to an execution path, simple happy path unit tests will suffice.

Tasks:

• Add a new unit testing project for WebApi.
• Create happy path unit tests for the controller actions.
• Include the new test project in the CI-CD pipelines and the coverage reporting.
• Make sure that the WebApi assembly coverage is 90%+.
• If needed, cover additional components too besides the controllers.

Swagger module incorrectly placed in Infrastructure layer

Swagger/SwaggerUI is clearly an API-concern that specifically relates to the presentation of the application core as a REST Web API.

I don't know why did I originally place it in the Infrastructure layer (I suppose I just wasn't thinking), but it should moved to the WebApi project.

One to many on ApplicationUser (TenantLike)

Hi,

Thanks for this faboulus project, looks very promising and i really like the architecture. I was wondering how to add a simple multitenant kind of approach to this project.
I would like to have an entity named "Company" that holds the ApplicationUsers in a one-to-many relationship. One company can have many users. And every company should have their own products etc... Im thinking of a BaseEntity with a public string CompanyId for every company-specific entity.

Though is the ApplicationUser defined in the Infrastructure-project which makes troubles if i want to add a IList of ApplicationUsers to the company-entity placed in the Domain-project because the Domain is not supposed to have references to other project, like Infrastructure.

Any suggestions or good practice about it?

Thanks!

Role base authorization not working ?

Hi,

I am working on implementing role-based authorization within your template.

In the Startup.cs of Infrastructure, I have already included the .AddRoles<IdentityRole>() configuration.

Furthermore, I have successfully added roles to the database, assigned roles to users, and incorporated roles into JWT claims.

However, I am encountering an issue where, even though I possess the required role, I consistently receive a 403 error when attempting to access methods protected by that role.

What am I missing here ?

Thank you in advance for your support.

Best regards,

Validation errors should be handled without exceptions

Currently all validations errors are handled by throwing exceptions, which is arguably not an elegant or clean solution (nor is it efficient from a request handling performance perspective). And handling validation via exceptions is a frequent matter of contention in the developer community. So it would be great to showcase a different solution.

The solution has to still keep API concerns out of the application layer, so setting or returning any sort of response is out of the question.

The best candidate is most likely implementing a response model that is used universally by all request handlers, and it would contain – besides the return value, if any – whether the operation was successful, and if not, the validation errors that has occurred.

The MediatR IPipelineBehavior responsible for request validation also should adapt to this mechanism.

Any plans to update to dot net 6?

Six is the most recent version with long-term support from Microsoft, it would seem a good match for a modern architecture example to use the core version offered?

Implement OpenID Connect authentication

While having traditional user management with registration, confirmation emails, etc. is probably not practical for this sample project, because hardly anyone would bother using it, implementing OIDC-based authentication with external authorities like Google or Facebook would provide a snappy way of letting people sign in without having to use some default user credentials.

ListQueryModel's PageIndex and PageSize is not validated

Currently sending a ListQueryModel (to retrieve a paged list of any entities) with PageIndex = 0 leads to error 500, instead of 400. Furthermore, PageSize is not capped.

Reason is that validation attributes in ListQueryModel were commented out during switching to FluentValidation. But FluentValidation actually doesn't have a great support for automatic validation of open generic base types, which ListQueryModel is.

Task:

• Restore validation attributes on ListQueryModel to:
  • Return 400 with helpful message in case PageIndex is set to e.g. 0.
  • Return 400 with helpful message if PageSize is greater than predefined maximum value.

(Using FluentValidation and built-in validation together is a supported scenario.)

API is not versioned

Add API-versioning to WebAPI, ensure version selection support in Swagger UI, and designate current controllers as V1.

Create Blazor WASM UI

Would be cool to create an alternative, Blazor web assembly based UI for the Web API.

Changing page size doesn't reset current page index

Should have been fixed already previously, but apparently not.

It can cause empty result pages when the result set updates, if a larger page size is selected.

Area: Check it at transaction index, product index, partner index; all should consistently reset set to reasonably expected page index (i.e. if displaying 21-60 with page size 20, switching to size 5 should display 21-25).

Note that using row index in the query model instead of page index would be actually better with respect to this, because we could always continue paging from the same position. Currently if we're showing e.g. 16-20 with page size 5, switching to page size 20 will only support resetting the view to 1-20.

Infrastructure.Persistence module does not utilize IEntityTypeConfiguration

The Clean Architecture Template utilizes IEntityTypeConfiguration to separate EF Core configuration on a per-entity basis.

This sample project does not utilize this technique, which makes this aspect of the design of the Persistence module arguably inferior. (The reason for this peculiar omission is that originally I didn't actually use the template, but started from an empty default solution, to force myself to engage more with the design instead of relying on a predefined structure.)

Task:

• Spread out entity configurations from ApplicationDbContext into classes implementing IEntityTypeConfiguration.
• Try to keep owned type configuration centralized in some way (since it seems currently all references to these types, e.g. Money, have to be configured individually, including their value conversion, even though this configuration is basically the same in all cases).

Strongly typed settings are not validated during registration

The readme mentions this as a feature, but registration-time validation wasn't implemented in code; there is todo tag instead.

Implement this validation step when registering settings types, so it could be actually true that missing configuration values throw at application startup.

OIDC signin exceptions aren't mapped to proper HTTP responses

When implementing Open ID Connect signin, I forgot to map the exceptions potentially thrown. 😨 This means if anything goes wrong, the backend will produce a 500 HTTP response.

Tasks:

• Map Authentication.External.Exceptions thrown in Infrastructure into proper HTTP responses in WebApi.
• In particular, ExternalAuthenticationPreventedException should lead to 503.

Database concurrency conflicts are not handled

Even though this is just a sample application, it doesn't seem prudent that currently it makes no effort to handle database concurrency conflicts.

If two new Transactions (referring to the domain entity Transaction, which involves changing product stocks) were to be created in an overlapping manner, it would be currently possible for the stock count to be corrupted, because the last Transaction write to complete would overwrite the modifications of the first.

Task:

• Implement optimistic or pessimistic concurrency handling.
• At the very least cover new Transactions.
• Consider if simple entity updates (e.g. Product, Partner) would also need to be covered.
• Even if optimistic concurrency handling is chosen, it would be probably prudent to still expose transaction begin/commit methods in the UOW component, with an appropriately selected isolation level (to be able to start a transaction directly when we select an entity from the database).

Some entity modifications don't update LastModifiedBy/LastModifiedAt

When certain properties of the domain entities (e.g. Product) are modified, it would be expected for the backend to update the LastModifiedAt and LastModifiedBy audit fields on the given entity.

But this doesn't happen, because certain properties are value objects, configured as EF Core owned type navigation properties. For example updating the price of a product fails to update the two aforementioned fields on the given product entity; even though the modification involves assigning a completely new Money instance to the property, since they are immutable.

I'm actually not sure if this is a glitch due to EF Core's slightly sketchy support for owned types (or perhaps navigation properties that have value conversion configured), OR actually normal expected behavior.

But in either case, it would be probably better to move the responsibility of setting these properties right into the domain entities, because arguably it's a domain concern that entities should have properly set last modified fields.

Tasks:

• Make sure all modifications of domain entities lead to updated LastModified fields.
• Consider moving this responsibility from DbContext right into the domain entities (possibly with the help of MyEntity abstract base class).
• Also check and possibly expand the integration tests... it seems these things aren't covered.

Logging should be moved to Application layer

Currently logging is done in the web API project, but logging the executed operations is an Application layer concern that shouldn't depend on whether the business logic is running via a web API (even if it's currently exposed only in that fashion).

Coverage badge misrepresenting the actual coverage

The coverage is currently only calculated for the application core, i.e. Application and Domain layers.

This is still highly meaningful, since that covers the actual business operations, but the WebApi and the Infrastructure assembly is excluded, as well as the Angular frontend assembly (WebUI).

Edit: Added unit tests with high coverage for WebApi.

Would be good to communicate this somehow, because currently it can potentially mislead people about the quality of the project, while the frontend in particular has zero testing.

(Sadly Coveralls badges don't provide customizability.)