Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a Financial Institutions feature to the Fin application, adding complete CRUD functionality for managing financial institutions with proper database schema and API endpoints.
- Added FinancialInstitution domain entity with related DTOs and enums
- Created database migrations and Entity Framework configuration
- Implemented service layer with validation and business logic
- Added REST API controller with admin authorization
Reviewed Changes
Copilot reviewed 14 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
Fin.Domain/FinancialInstitutions/ |
Domain models, DTOs, and enums for financial institutions |
Fin.Infrastructure/Database/ |
Entity configuration and database migrations |
Fin.Infrastructure/Seeders/ |
Updated menu seeder to include financial institutions menu |
Fin.Application/FinancialInstitutions/ |
Business logic service with CRUD operations |
Fin.Api/FinancialInstitutions/ |
REST API controller with authorization |
Fin.Api/appsettings.json |
Configuration file removed entirely |
Files not reviewed (2)
- Fin.Infrastructure/Migrations/20251001194905_AddFinancialInstitutions.Designer.cs: Language not supported
- Fin.Infrastructure/Migrations/20251003025422_RenameLogoUrlToIcon.Designer.cs: Language not supported
Comments suppressed due to low confidence (2)
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
RafaelKC
left a comment
RafaelKC
left a comment
There was a problem hiding this comment.
Tip: when push a new feature try use ust one migration
| } | ||
|
|
||
| [HttpPost("{id:guid}/activate")] | ||
| [Authorize(Roles = "Admin")] |
There was a problem hiding this comment.
Instead of "Admin" use AuthenticationRoles.Admin
| } | ||
|
|
||
| [HttpGet("{id:guid}")] | ||
| [Authorize(Roles = "Admin")] |
|
|
||
| [HttpPost("{id:guid}/deactivate")] | ||
| [Authorize(Roles = "Admin")] | ||
| public async Task<ActionResult> Deactivate([FromRoute] Guid id) |
There was a problem hiding this comment.
Instead of a active and deactive endpoint you can use a toggler endpoint to follow standards
| public async Task<PagedOutput<FinancialInstitutionOutput>> GetList(PagedFilteredAndSortedInput input) | ||
| { | ||
| return await repository.Query(false) | ||
| .OrderBy(f => f.Name) |
There was a problem hiding this comment.
Like in TitleCategory, first order by inactivated prop, than by name
| [Required] | ||
| public FinancialInstitutionType Type { get; set; } | ||
|
|
||
| [MaxLength(50)] |
There was a problem hiding this comment.
Like in front, Where color?
|
|
||
| namespace Fin.Domain.FinancialInstitutions.Entities; | ||
|
|
||
| public class FinancialInstitution : IAuditedEntity, ITenantEntity |
| builder.HasIndex(x => new { x.Name, x.TenantId }) | ||
| .IsUnique(); | ||
|
|
||
| builder.HasIndex(x => new { x.Code, x.TenantId }) |
|
|
||
| builder.Property(x => x.Type) | ||
| .IsRequired() | ||
| .HasConversion<string>(); |
There was a problem hiding this comment.
We gonna user enum type as int or smallint in db
|
|
||
| builder.Property(x => x.Code) | ||
| .HasMaxLength(3) | ||
| .IsRequired(); |
Gu financial institutions