Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds Docker Compose support for PostgreSQL and Redis in development mode, along with refactoring user credential creation and notification system enhancements.
- Adds Docker infrastructure with PostgreSQL 16 and Redis 7 Alpine for development environments
- Refactors UserCredential entity to use a factory pattern for better maintainability
- Enhances notification system with severity levels, links, and improved WebSocket handling
Reviewed Changes
Copilot reviewed 35 out of 47 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| docker-compose.yml | Adds PostgreSQL and Redis containers for development |
| docker.md | Documentation for Docker setup and usage |
| UserCredentialFactory.cs | New factory pattern for creating user credentials |
| UserCredential.cs | Refactored entity with public setters and removed constructors |
| NotificationDeliveryService.cs | Enhanced notification handling with better error handling and Firebase improvements |
| UnitOfWork.cs | Improved transaction management with proper disposal pattern |
Files not reviewed (1)
- Fin.Infrastructure/Migrations/20250922223535_adding_severity_link_on_notification.Designer.cs: Language not supported
Comments suppressed due to low confidence (1)
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| | allowedWaysToSend.Contains(NotificationWay.Message) | ||
| | allowedWaysToSend.Contains(NotificationWay.Push) |
There was a problem hiding this comment.
Using bitwise OR operator '|' instead of logical OR '||' for boolean conditions. This should use '||' for proper short-circuit evaluation.
| | allowedWaysToSend.Contains(NotificationWay.Message) | |
| | allowedWaysToSend.Contains(NotificationWay.Push) | |
| || allowedWaysToSend.Contains(NotificationWay.Message) | |
| || allowedWaysToSend.Contains(NotificationWay.Push) |
| public string EncryptedEmail { get; set; } | ||
| public string EncryptedPassword { get; set; } | ||
|
|
||
| public string GoogleId { get; set; } | ||
|
|
||
| public string ResetToken { get; set; } = ""; | ||
| public int FailLoginAttempts { get; private set; } | ||
| public int FailLoginAttempts { get; set; } | ||
|
|
||
| public Guid UserId { get; private set; } | ||
| public Guid UserId { get; set; } |
There was a problem hiding this comment.
Making security-sensitive properties like EncryptedEmail, EncryptedPassword, and UserId publicly settable reduces encapsulation and could lead to unintended modifications. Consider using private setters with factory methods or init-only setters.
| public string EncryptedEmail { get; set; } | ||
| public string EncryptedPassword { get; set; } | ||
|
|
||
| public string GoogleId { get; set; } | ||
|
|
||
| public string ResetToken { get; set; } = ""; | ||
| public int FailLoginAttempts { get; private set; } | ||
| public int FailLoginAttempts { get; set; } | ||
|
|
||
| public Guid UserId { get; private set; } | ||
| public Guid UserId { get; set; } |
There was a problem hiding this comment.
Making security-sensitive properties like EncryptedEmail, EncryptedPassword, and UserId publicly settable reduces encapsulation and could lead to unintended modifications. Consider using private setters with factory methods or init-only setters.
| public int FailLoginAttempts { get; set; } | ||
|
|
||
| public Guid UserId { get; private set; } | ||
| public Guid UserId { get; set; } |
There was a problem hiding this comment.
Making security-sensitive properties like EncryptedEmail, EncryptedPassword, and UserId publicly settable reduces encapsulation and could lead to unintended modifications. Consider using private setters with factory methods or init-only setters.
| public Guid UserId { get; set; } | |
| public Guid UserId { get; private set; } |
RafaelKC
left a comment
RafaelKC
left a comment
There was a problem hiding this comment.
Aproved with reviewer changes.
dei merge da FIN-26 na minha gu/docker
por isso a quantidade de alterações no code
validar para seguir