feat/sprint 9 communications module + buffer#191
Merged
Conversation
- Add detailed Communications module section (email, SMS, push) - Use Outbox Pattern for delivery guarantee - Define module architecture (full module like Providers/Users) - Map IntegrationEvent handlers (UserRegistered, ProviderVerification, etc.) - Add ModuleNames.Communications constant - Update roadmap status (Sprint 9E with Communications) This establishes infrastructure for post-MVP modules (Reviews, Payments, Bookings) that depend on communication capabilities.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdiciona o módulo Communications (email, SMS, push) com contratos/DTOs, outbox genérico, entidades/repositórios, processador + worker, handlers que enfileiram mensagens, API de templates/logs, integração no bootstrapper, EF context/migrations, helpers PII, template Razor e extensa suíte de testes. Changes
Sequence Diagram(s)sequenceDiagram
participant Event as IntegrationEvent
participant Handler as EventHandler
participant OutboxRepo as OutboxRepository
participant DB as Database
participant Worker as OutboxWorker
participant Sender as ProviderSender
participant LogRepo as CommunicationLogRepo
Event->>Handler: publica evento (ex: UserRegistered)
Handler->>OutboxRepo: ExistsByCorrelationIdAsync(correlationId)
OutboxRepo->>DB: consulta communication_logs
DB-->>OutboxRepo: resultado
alt já processado
OutboxRepo-->>Handler: true (skip)
else novo
Handler->>OutboxRepo: AddAsync(OutboxMessage)
OutboxRepo->>DB: INSERT + SaveChanges
end
rect rgba(200,150,255,0.5)
Worker->>OutboxRepo: GetPendingAsync(batch)
OutboxRepo->>DB: SELECT pending
DB-->>OutboxRepo: mensagens
loop cada mensagem
Worker->>DB: marcar Processing (transação)
Worker->>Sender: Dispatch(payload)
alt sucesso
Sender-->>Worker: ok
Worker->>DB: MarkAsSent
Worker->>LogRepo: AddAsync(CreateSuccess(...))
else falha
Sender-->>Worker: erro/falha
Worker->>DB: MarkAsFailed or ResetToPending
Worker->>LogRepo: AddAsync(CreateFailure(...)) when final
end
Worker->>DB: SaveChanges
end
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
frigini
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
docs/roadmap-current.md (3)
842-849: Considere adicionar linha em branco antes da tabela de mapeamento.Para melhor conformidade com padrões Markdown (MD058), adicione uma linha em branco antes da tabela de mapeamento de eventos.
📝 Sugestão de formatação
**Mapeamento de Integração com Eventos**: + | Evento Existing | Ação de Comunicação |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/roadmap-current.md` around lines 842 - 849, Add a blank line immediately before the events mapping table that begins with the header row "| Evento Existing | Ação de Comunicação |" so the Markdown has an empty line separating it from the preceding content (to satisfy MD058); simply insert one newline above that table block.
940-954: Estimativa de esforço realista.A estimativa de ~29h (~4 dias) parece adequada para a infraestrutura base. Note que o item 7 tem um pequeno erro de formatação ("7.Stub" deveria ser "7. Stub").
📝 Correção de formatação
-| 7.Stub SendGridEmailService | 4h | Interface channel | +| 7. Stub SendGridEmailService | 4h | Interface channel |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/roadmap-current.md` around lines 940 - 954, Fix the formatting typo in the roadmap table: change the entry "7.Stub SendGridEmailService" to include a space after the numeral so it reads "7. Stub SendGridEmailService" to match the numbering style used for other rows (refer to the table row text "7.Stub SendGridEmailService" and update it accordingly).
917-936: Blocos de código sem especificador de linguagem.Os blocos de código nas linhas 922 e 927 não possuem especificador de linguagem, o que pode afetar a renderização (syntax highlighting) no Markdown.
💡 Adicionar especificadores de linguagem
1. **Processo atual** (síncrono): - ``` + ```text User Registered → Save to DB → Send Email (pode falhar silenciosamente) ``` 2. **Processo com Outbox** (garantido): - ``` + ```text User Registered → Save to Outbox Table → Save to DB → Background Worker processa Outbox ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/roadmap-current.md` around lines 917 - 936, The two Markdown code fences containing the sequences "User Registered → Save to DB → Send Email (pode falhar silenciosamente)" and "User Registered → Save to Outbox Table → Save to DB → Background Worker processa Outbox" are missing language specifiers; update both triple-backtick fences to include a language (e.g., use ```text) so the blocks render with proper syntax highlighting and consistent styling in the docs/roadmap-current.md content.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Shared/Utilities/Constants/ModuleNames.cs`:
- Around line 47-49: Replace all references to the removed symbol
ModuleNames.Notifications with the new ModuleNames.Communications in the unit
test class ModuleNamesTests; specifically update the InlineData and assertion
usages that currently use ModuleNames.Notifications (e.g., the InlineData
attributes and parameterized test cases) so they reference
ModuleNames.Communications instead, ensuring compile-time references match the
renamed constant in ModuleNames (public const string Communications).
---
Nitpick comments:
In `@docs/roadmap-current.md`:
- Around line 842-849: Add a blank line immediately before the events mapping
table that begins with the header row "| Evento Existing | Ação de Comunicação
|" so the Markdown has an empty line separating it from the preceding content
(to satisfy MD058); simply insert one newline above that table block.
- Around line 940-954: Fix the formatting typo in the roadmap table: change the
entry "7.Stub SendGridEmailService" to include a space after the numeral so it
reads "7. Stub SendGridEmailService" to match the numbering style used for other
rows (refer to the table row text "7.Stub SendGridEmailService" and update it
accordingly).
- Around line 917-936: The two Markdown code fences containing the sequences
"User Registered → Save to DB → Send Email (pode falhar silenciosamente)" and
"User Registered → Save to Outbox Table → Save to DB → Background Worker
processa Outbox" are missing language specifiers; update both triple-backtick
fences to include a language (e.g., use ```text) so the blocks render with
proper syntax highlighting and consistent styling in the docs/roadmap-current.md
content.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d5cd4244-8e17-4d8a-8dbf-d64bcf1f16f7
📒 Files selected for processing (3)
docs/roadmap-current.mddocs/roadmap.mdsrc/Shared/Utilities/Constants/ModuleNames.cs
…ure, and authentication strategy
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/roadmap-current.md`:
- Around line 825-830: O documento refere-se a TemplateDto.cs mas a API usa
EmailTemplateDto; padronize o nome adotando EmailTemplateDto em todas as menções
do roadmap (substituir TemplateDto.cs por EmailTemplateDto) e atualize a segunda
ocorrência mencionada (as linhas referidas no comentário) para evitar drift
entre documentação e implementação; verifique também referências relacionadas
como TemplateDto/EmailTemplateDto nas listas DTOs e qualquer seção que descreva
o template DTO ou exemplos de payload.
- Around line 966-968: The fenced code block in docs/roadmap-current.md is
missing a language tag (triggering MD040); update the block that currently
starts with ``` and contains "Event Occurred → Save to Outbox Table (Mesma
Transação) → Background Worker processa Outbox" to use a language tag such as
```text so the block becomes ```text ... ```; ensure only the opening fence is
changed and the block content remains unchanged.
- Around line 875-883: The Markdown table block (starting at the header row "|
Evento Existing | Ação de Comunicação |" and including rows for
UserRegisteredIntegrationEvent, ProviderAwaitingVerificationIntegrationEvent,
ProviderVerificationStatusUpdatedIntegrationEvent,
DocumentVerifiedIntegrationEvent, DocumentRejectedIntegrationEvent) lacks
surrounding blank lines and triggers MD058; fix it by inserting a single blank
line immediately before the table header and a single blank line immediately
after the final table row so there is one empty line separating the table from
surrounding paragraphs.
- Around line 659-660: The roadmap contains a scope conflict: the Sprint 9
checklist includes "Módulo Comunicações: Implementar infraestrutura base (outbox
pattern, templates, event handlers)" while Sprint 9's risk/notes define the
sprint as a buffer with no new features; reconcile by either removing or
deferring the "Módulo Comunicações" checklist item from Sprint 9 or by updating
the Sprint 9 risk/notes to explicitly allow this feature; update the text for
"Módulo Comunicações", the Sprint 9 risk/notes, and any duplicated entries noted
around the also-applies section (lines referencing 788-793) so the sprint plan
and risks are consistent and unambiguous.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d2b8ee8b-cadb-4030-8816-b99fcb684e9f
📒 Files selected for processing (1)
docs/roadmap-current.md
added 2 commits
April 8, 2026 18:03
…stants, plus update project roadmap
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
♻️ Duplicate comments (2)
docs/roadmap-current.md (2)
966-969:⚠️ Potential issue | 🟡 MinorEspecifique a linguagem no bloco de código.
O bloco de código carece de identificador de linguagem, violando MD040. Adicionar
textmelhora a renderização.✏️ Correção proposta
- ``` + ```text Event Occurred → Save to Outbox Table (Mesma Transação) → Background Worker processa Outbox ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/roadmap-current.md` around lines 966 - 969, The fenced code block under the "Processo com Outbox" item is missing a language identifier (MD040); update the triple-backtick fence that contains "Event Occurred → Save to Outbox Table (Mesma Transação) → Background Worker processa Outbox" to include the language token `text` (i.e., change ``` to ```text) so the block is explicitly labeled and passes the lint rule.
875-884:⚠️ Potential issue | 🟡 MinorAdicione linhas em branco ao redor da tabela.
A tabela de mapeamento de eventos carece de linhas em branco antes/depois, violando MD058. Isso pode causar problemas de renderização em alguns parsers Markdown.
✏️ Correção proposta
|----------------|-------------------| | `UserRegisteredIntegrationEvent` | → Enviar email de boas-vindas | | `ProviderAwaitingVerificationIntegrationEvent` | → Notificar admin | | `ProviderVerificationStatusUpdatedIntegrationEvent` | → Notificar prestador | | `DocumentVerifiedIntegrationEvent` | → Notificar prestador | | `DocumentRejectedIntegrationEvent` | → Notificar prestador | +E adicione uma linha em branco antes da linha 877 (antes do cabeçalho da tabela).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/roadmap-current.md` around lines 875 - 884, The Markdown table under the "Mapeamento de Integração com Eventos" section needs blank lines surrounding it to satisfy MD058; add at least one empty line immediately before the table header (before the line with | Evento Existing | Ação de Comunicação |) and at least one empty line immediately after the final row (after the line with | `DocumentRejectedIntegrationEvent` | → Notificar prestador |) so the table is separated from surrounding text and paragraph flow.
🟡 Minor comments (7)
docs/roadmap-current.md-908-910 (1)
908-910:⚠️ Potential issue | 🟡 MinorInconsistência no tipo de retorno: PagedList vs PagedResult.
A documentação indica retorno
PagedList<CommunicationLogDto>mas a implementação emICommunicationsModuleApi.csusaPagedResult<CommunicationLogDto>. Padronize para evitar confusão.Com base nos snippets de código fornecidos, a implementação real usa
PagedResult. A documentação deve refletir isso.📝 Correção sugerida
// Logs (Idempotency check via CorrelationId) - Task<Result<PagedList<CommunicationLogDto>>> GetLogsAsync( + Task<Result<PagedResult<CommunicationLogDto>>> GetLogsAsync( CommunicationLogQuery query, CancellationToken ct = default);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/roadmap-current.md` around lines 908 - 910, The docs show GetLogsAsync returning PagedList<CommunicationLogDto> but the implementation (ICommunicationsModuleApi.GetLogsAsync) returns PagedResult<CommunicationLogDto>; update the documentation snippet to use PagedResult<CommunicationLogDto> to match the implementation (or, alternatively, change the implementation to PagedList if that is the intended canonical type), ensuring the signature in the docs and the code for GetLogsAsync and related type references are consistent.docs/roadmap-current.md-889-889 (1)
889-889:⚠️ Potential issue | 🟡 MinorCorrigir ordem do enum CommunicationPriority na documentação.
A documentação mostra
{ High, Normal, Low }, mas a implementação real emsrc/Contracts/Contracts/Modules/Communications/ICommunicationsModuleApi.csdefine comoLow = 0, Normal = 1, High = 2. A documentação deve refletir a ordem correta com os valores numéricos explícitos.Correção sugerida
-public enum CommunicationPriority { High, Normal, Low } +public enum CommunicationPriority { Low = 0, Normal = 1, High = 2 }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/roadmap-current.md` at line 889, The documentation's CommunicationPriority enum ordering is incorrect; update the docs entry for CommunicationPriority to match the implementation by listing the members as Low = 0, Normal = 1, High = 2 (in that order) and include the numeric values explicitly so the docs reflect the enum in ICommunicationsModuleApi (CommunicationPriority).src/Modules/Communications/Domain/Entities/CommunicationLog.cs-61-67 (1)
61-67:⚠️ Potential issue | 🟡 MinorValide
attemptCountpara impedir estados inválidos.
CreateSuccesseCreateFailureaceitamattemptCountnegativo, o que quebra a semântica de tentativas realizadas.🛠️ Sugestão de ajuste
public static CommunicationLog CreateSuccess( string correlationId, ECommunicationChannel channel, string recipient, int attemptCount, Guid? outboxMessageId = null, string? templateKey = null) { ArgumentException.ThrowIfNullOrWhiteSpace(correlationId); ArgumentException.ThrowIfNullOrWhiteSpace(recipient); + if (attemptCount < 0) + throw new ArgumentOutOfRangeException(nameof(attemptCount), "Attempt count cannot be negative."); return new CommunicationLog { @@ public static CommunicationLog CreateFailure( string correlationId, ECommunicationChannel channel, string recipient, string errorMessage, int attemptCount, Guid? outboxMessageId = null, string? templateKey = null) { ArgumentException.ThrowIfNullOrWhiteSpace(correlationId); ArgumentException.ThrowIfNullOrWhiteSpace(recipient); ArgumentException.ThrowIfNullOrWhiteSpace(errorMessage); + if (attemptCount < 0) + throw new ArgumentOutOfRangeException(nameof(attemptCount), "Attempt count cannot be negative."); return new CommunicationLog {Also applies to: 87-94
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Modules/Communications/Domain/Entities/CommunicationLog.cs` around lines 61 - 67, Valide o parâmetro attemptCount em CommunicationLog.CreateSuccess e CommunicationLog.CreateFailure para impedir valores negativos: dentro de cada método (CreateSuccess, CreateFailure) verifique if (attemptCount < 0) e lance ArgumentOutOfRangeException(nameof(attemptCount)) com mensagem clara; isso garante que tentativas negativas não sejam aceitas e mantém a semântica de tentativas realizadas consistente entre ambos os métodos.src/Modules/Communications/Domain/Entities/EmailTemplate.cs-66-88 (1)
66-88:⚠️ Potential issue | 🟡 MinorNormalize e valide
languageeoverrideKeyna criação.Hoje é possível criar template com
languageem branco; além disso,overrideKeycom espaços vira""em vez denull.🧩 Sugestão de ajuste
public static EmailTemplate Create( string templateKey, string subject, string htmlBody, string textBody, string language = "pt-BR", string? overrideKey = null, bool isSystemTemplate = false) { ArgumentException.ThrowIfNullOrWhiteSpace(templateKey); ArgumentException.ThrowIfNullOrWhiteSpace(subject); ArgumentException.ThrowIfNullOrWhiteSpace(htmlBody); ArgumentException.ThrowIfNullOrWhiteSpace(textBody); + ArgumentException.ThrowIfNullOrWhiteSpace(language); + + var normalizedOverrideKey = string.IsNullOrWhiteSpace(overrideKey) + ? null + : overrideKey.ToLowerInvariant().Trim(); return new EmailTemplate { TemplateKey = templateKey.ToLowerInvariant().Trim(), - OverrideKey = overrideKey?.ToLowerInvariant().Trim(), + OverrideKey = normalizedOverrideKey, Subject = subject, HtmlBody = htmlBody, TextBody = textBody, - Language = language, + Language = language.Trim(), IsActive = true, Version = 1, IsSystemTemplate = isSystemTemplate }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Modules/Communications/Domain/Entities/EmailTemplate.cs` around lines 66 - 88, In EmailTemplate.Create, validate and normalize the language and overrideKey: call ArgumentException.ThrowIfNullOrWhiteSpace(language) (or otherwise reject empty/whitespace) and then set Language = language.ToLowerInvariant().Trim(); for overrideKey, normalize and convert empty/whitespace to null by using OverrideKey = string.IsNullOrWhiteSpace(overrideKey) ? null : overrideKey.ToLowerInvariant().Trim(); update the Create method to perform these checks and assignments so language cannot be blank and overrideKey won't become an empty string.src/Modules/Communications/API/Extensions.cs-33-35 (1)
33-35:⚠️ Potential issue | 🟡 MinorPadronize os comentários deste grupo de endpoints para português.
Os placeholders daqui ficaram em inglês, diferente da convenção adotada para comentários/XML em arquivos C#.
Based on learnings, comentários e XML docs em arquivos C# devem ficar em português.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Modules/Communications/API/Extensions.cs` around lines 33 - 35, Padronize os comentários no grupo de endpoints para português: substitua as linhas de comentário que usam inglês (por exemplo as placeholders "// Endpoints will be mapped here" e "// group.MapGet("/templates", ...);", "// group.MapGet("/logs", ...);") por textos em português mantendo o mesmo contexto (ex.: "// Endpoints serão mapeados aqui" e "// group.MapGet("/templates", ...); // Exemplo: obter templates"), e ajuste qualquer XML doc no mesmo arquivo Extensions.cs para seguir a convenção em português; localize o bloco que referencia o objeto group e atualize apenas os comentários sem alterar a lógica do mapagem de endpoints.src/Contracts/Contracts/Modules/Communications/ICommunicationsModuleApi.cs-58-60 (1)
58-60:⚠️ Potential issue | 🟡 MinorPadronize o XML doc para PT-BR.
O trecho
Idempotency check via CorrelationIdficou misturado em inglês no meio da documentação pública.Based on learnings, comentários e XML docs em arquivos C# devem ficar em português.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Contracts/Contracts/Modules/Communications/ICommunicationsModuleApi.cs` around lines 58 - 60, Atualize o comentário XML público na interface ICommunicationsModuleApi para ficar totalmente em português: substitua o trecho misto "Idempotency check via CorrelationId" por uma frase em PT-BR, por exemplo "Verificação de idempotência via Identificador de Correlação (CorrelationId)" ou similar, garantindo que a tag <summary> da operação que "Obtém logs de comunicação paginados" esteja 100% em português sem termos soltos em inglês.src/Modules/Communications/Infrastructure/Persistence/Repositories/CommunicationLogRepository.cs-46-47 (1)
46-47:⚠️ Potential issue | 🟡 MinorNão ignore
channelinválido silenciosamente.Se o parse falhar, o filtro simplesmente some e a consulta passa a retornar logs de todos os canais. Isso mascara erro de input e entrega uma página muito mais ampla do que a pedida. Valide o valor antes de chegar aqui, ou retorne erro/resultado vazio quando ele for inválido.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Modules/Communications/Infrastructure/Persistence/Repositories/CommunicationLogRepository.cs` around lines 46 - 47, O filtro de canal em CommunicationLogRepository atualmente ignora silently valores inválidos porque o resultado de Enum.TryParse<ECommunicationChannel> é descartado; modifique o trecho que usa channel e Enum.TryParse para validar o input: se TryParse falhar, ou lance um ArgumentException/BadRequest com mensagem clara sobre o canal inválido, ou aplique um filtro que garanta resultado vazio (por exemplo atribuir query = query.Where(x => false)) e retorne imediatamente; preserve o comportamento atual quando TryParse succeeds (usar ch no Where).
🧹 Nitpick comments (1)
src/Modules/Communications/Infrastructure/Persistence/Configurations/OutboxMessageConfiguration.cs (1)
37-38: Ajuste de índice pode melhorar o throughput do polling do Outbox.Nas consultas de pendentes (filtro por status + agendamento e ordenação por prioridade/criação), o desenho atual pode gerar mais custo de plano em volume alto. Vale considerar um índice composto alinhado a esse padrão.
💡 Proposta de ajuste
- builder.HasIndex(x => new { x.Status, x.Priority, x.CreatedAt }); - builder.HasIndex(x => x.ScheduledAt); + builder.HasIndex(x => new { x.Status, x.ScheduledAt, x.Priority, x.CreatedAt });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Modules/Communications/Infrastructure/Persistence/Configurations/OutboxMessageConfiguration.cs` around lines 37 - 38, Substitua os índices atuais por um índice composto alinhado ao padrão de consulta de polling do Outbox: remova/evite o índice separado em ScheduledAt e crie um único índice composto usando builder.HasIndex(x => new { x.Status, x.ScheduledAt, x.Priority, x.CreatedAt }) para que o engine possa filtrar por Status+ScheduledAt e já aproveitar a ordenação por Priority/CreatedAt, melhorando throughput em alto volume.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/Modules/Communications/Application/Handlers/ProviderActivatedIntegrationEventHandler.cs`:
- Around line 28-34: O código no ProviderActivatedIntegrationEventHandler usa
logRepository.ExistsByCorrelationIdAsync seguido de logRepository.AddAsync
(também em outra ocorrência nas linhas 53-54), o que causa TOCTOU e permite
enfileiramento duplicado sob concorrência; altere a lógica para uma operação
atômica (por exemplo implementar e usar
logRepository.AddIfNotExistsAsync/CaptureUniqueInsertAsync ou executar AddAsync
dentro de uma transação com constraint única no campo CorrelationId) e, como
alternativa/backup, trate a exceção de chave duplicada ao chamar AddAsync para
ignorar o segundo inserto; aplique a mudança nas duas ocorrências referenciadas
no ProviderActivatedIntegrationEventHandler para garantir idempotência.
In
`@src/Modules/Communications/Application/Handlers/UserRegisteredIntegrationEventHandler.cs`:
- Around line 26-49: Ocorre que o correlationId só é usado no pré-check
(logRepository.ExistsByCorrelationIdAsync) mas não é gravado com a mensagem de
Outbox, permitindo duplicados; ajuste a criação de OutboxMessage
(OutboxMessage.Create) para incluir o correlationId — por exemplo adicionando-o
aos metadados/headers do OutboxMessage e também no payload serializado para que
o processor e o CommunicationLog possam reconstruir/associar o id; garanta que o
mesmo correlationId gerado (`var correlationId =
$"{TemplateKey}:{integrationEvent.UserId}";`) seja passado tanto em payload
quanto nos campos/metadados de OutboxMessage antes de enfileirar.
In `@src/Modules/Communications/Application/ModuleApi/CommunicationsModuleApi.cs`:
- Around line 47-52: O método GetTemplatesAsync está retornando sempre sucesso
com lista vazia, escondendo falhas; atualize a implementação de
GetTemplatesAsync para consultar o repositório real (ex.: usar
IEmailTemplateRepository.GetAll() ou um novo método GetByKeys(...) se GetAll não
existir), mapear os resultados para EmailTemplateDto e retornar
Result.Success(comLista) ou, se a operação não puder ser feita ainda,
lançar/retornar um Result.Failure claro (ou NotImplementedException) em vez de
sempre retornar sucesso vazio; localize a lógica em GetTemplatesAsync na classe
CommunicationsModuleApi e ajuste para usar o repositório e o mapper existentes
para obter templates reais.
- Around line 32-36: Add fail-fast guard clauses at the start of SendEmailAsync
and the other public API methods in CommunicationsModuleApi (the async public
methods around the mentioned ranges) to validate inputs: throw
ArgumentNullException when DTO/query parameters like email are null, throw
ArgumentOutOfRangeException or ArgumentException for invalid pagination or
numeric arguments, and/or return an appropriate failed Result when your codebase
prefers Result-based errors; place these checks at the top of SendEmailAsync
(and the other public methods) before any processing or enqueuing to prevent
null/invalid payloads from being enqueued.
In `@src/Modules/Communications/Application/Services/Email/StubEmailService.cs`:
- Around line 18-20: The info log in StubEmailService is exposing PII
(message.To) and possibly sensitive message.Subject; update the logging in the
StubEmailService (where logger.LogInformation is called) to stop including
message.To and message.Subject—log only non-sensitive metadata such as
message.Body.Length or a masked/aggregate indicator (e.g., recipient count or
"recipient masked") and keep the existing descriptive text, ensuring no raw
email addresses or subject text are written to logs.
In `@src/Modules/Communications/Application/Services/OutboxProcessorService.cs`:
- Around line 27-35: O serviço OutboxProcessorService não está persistindo
CommunicationLog após cada tentativa de entrega; para corrigir, após cada
send/attempt path in OutboxProcessorService (both the success and failure
branches around where OutboxMessage is updated — see the send logic between the
earlier constructor and the 53-82 region) create a CommunicationLog instance
populated with correlationId, outboxMessageId, status (Succeeded/Failed),
provider details, timestamp and error info when present, and persist it via the
repository (use _logRepository.AddAsync / CreateAsync / SaveAsync as
appropriate) and commit/save changes; ensure the existing idempotency check
(ExistsByCorrelationIdAsync) and GetLogsAsync will reflect these persisted logs
so delivery state is accurate.
In `@src/Modules/Communications/Domain/Entities/OutboxMessage.cs`:
- Around line 100-124: Proteja as transições de estado em MarkAsSent e
MarkAsFailed impedindo mutações quando a entidade já está num estado terminal:
verifique Status contra EOutboxMessageStatus.Sent e EOutboxMessageStatus.Failed
no início de MarkAsSent e MarkAsFailed, e em caso de estado terminal lance uma
InvalidOperationException (ou retorne sem alterar) para evitar incrementar
RetryCount, alterar SentAt, ErrorMessage ou chamar MarkAsUpdated em estados
finais; aplique essa verificação antes de qualquer modificação das propriedades
Status, RetryCount, SentAt, ErrorMessage ou chamada a MarkAsUpdated para
garantir integridade concorrente.
In
`@src/Modules/Communications/Infrastructure/MeAjudaAi.Modules.Communications.Infrastructure.csproj`:
- Around line 32-34: Crie uma factory de DbContext em tempo de design chamada
CommunicationsDbContextFactory dentro de Persistence
(src/Modules/Communications/Infrastructure/Persistence/) que herde de
BaseDesignTimeDbContextFactory<CommunicationsDbContext>; a factory deve expor a
classe pública CommunicationsDbContextFactory :
BaseDesignTimeDbContextFactory<CommunicationsDbContext> para permitir que o
tooling do EF descubra e crie uma instância de CommunicationsDbContext em tempo
de design (siga o mesmo padrão usado pelos módulos
Documents/Locations/Providers/SearchProviders/ServiceCatalogs/Users), assegure o
namespace correto e que o projeto de Infrastructure compile com essa nova classe
para que dotnet ef migrations add funcione.
In
`@src/Modules/Communications/Infrastructure/Persistence/CommunicationsDbContext.cs`:
- Around line 33-40: Os overrides GetDomainEventsAsync e ClearDomainEvents estão
vazios, portanto BaseDbContext.SaveChangesAsync perde/descarta eventos de
domínio; implemente aqui a varredura do ChangeTracker para coletar eventos das
entidades (ex.: procurar entidades que implementam IHasDomainEvents / têm uma
coleção DomainEvents) e retornar/remover esses eventos em
GetDomainEventsAsync/ClearDomainEvents respectivamente, ou se o módulo realmente
não usa eventos adicione um comentário/XML doc em português explicando
explicitamente que não há eventos a processar para evitar silent no-op.
In
`@src/Modules/Communications/Infrastructure/Persistence/Configurations/EmailTemplateConfiguration.cs`:
- Line 37: The current unique index defined via builder.HasIndex(x => new {
x.TemplateKey, x.Language, x.OverrideKey }).IsUnique() doesn't enforce
uniqueness for default templates because OverrideKey is nullable; update the
configuration in EmailTemplateConfiguration to handle NULLs explicitly: if
targeting PostgreSQL 15+ call AreNullsDistinct(false) on that index to treat
NULLs as equal (use builder.HasIndex(...).IsUnique().AreNullsDistinct(false));
if you need older PostgreSQL compatibility replace the single index with two
filtered unique indexes—one unique index on (TemplateKey, Language) filtered
where OverrideKey IS NULL and another unique index on (TemplateKey, Language,
OverrideKey) filtered where OverrideKey IS NOT NULL—so TemplateKey/Language have
at most one NULL OverrideKey and non-null overrides remain uniquely constrained.
In
`@src/Modules/Communications/Infrastructure/Persistence/Repositories/OutboxMessageRepository.cs`:
- Around line 20-25: GetPendingAsync currently reads messages with Status ==
EOutboxMessageStatus.Pending but does not atomically claim them, so concurrent
processors can pick the same rows; modify GetPendingAsync to claim rows inside a
DB transaction (e.g., UPDATE ... OUTPUT/RETURNING or SELECT ... FOR UPDATE SKIP
LOCKED depending on the DB) so the method atomically sets Status to a
"Processing" state (or writes ClaimOwner/ClaimedAt) for the selected messages
before returning them. Locate GetPendingAsync in OutboxMessageRepository,
perform the selection ordered by Priority/CreatedAt and ScheduledAt filter
within a transaction, use an UPDATE that marks those rows as claimed and returns
the updated entities (or use row locking with SKIP LOCKED) to ensure no two
workers can fetch the same messages concurrently.
In `@src/Modules/Communications/Infrastructure/Services/EmailSenderStub.cs`:
- Line 14: The stub is logging potential PII (message.To and message.Subject) in
EmailSenderStub via logger.LogInformation; remove or redact those fields:
replace the detailed template with a non-PII message (e.g., "Stub email sent"
plus a non-identifying id or masked/hashed recipient if you must correlate), or
log only metadata like message length or a boolean flag; update the
LogInformation call in EmailSenderStub to avoid including message.To and
message.Subject directly.
In `@src/Modules/Communications/Infrastructure/Services/PushSenderStub.cs`:
- Line 14: O log atual em PushSenderStub (logger.LogInformation) inclui o campo
sensível notification.DeviceToken; remova o DeviceToken do template de log ou
substitua-o por uma versão não recuperável (por exemplo, um hash/mascara) e
preserve apenas metadados não sensíveis como notification.Title; atualize a
chamada logger.LogInformation para não persistir notification.DeviceToken
diretamente, garantindo que qualquer identificação do token seja anonimizada
antes de qualquer logging.
In `@src/Modules/Communications/Infrastructure/Services/SmsSenderStub.cs`:
- Line 14: O log atual em SmsSenderStub (logger.LogInformation("[STUB SMS
SENDER] To: {To} | Message: {Msg}", message.PhoneNumber, message.Body)) está
expondo PII e conteúdo sensível; altere a chamada de log para não gravar o corpo
do SMS e substituir/anonimizar o número (por exemplo, mascarar mostrando só
últimos 2-4 dígitos ou usar um hash irreversível como SHA‑256 do PhoneNumber) e
mantenha apenas metadados não sensíveis (p.ex. "redacted" ou o hash/mascara) no
texto de log para indicar envio sem expor dados brutos na classe SmsSenderStub e
em qualquer método que use logger.LogInformation.
---
Minor comments:
In `@docs/roadmap-current.md`:
- Around line 908-910: The docs show GetLogsAsync returning
PagedList<CommunicationLogDto> but the implementation
(ICommunicationsModuleApi.GetLogsAsync) returns
PagedResult<CommunicationLogDto>; update the documentation snippet to use
PagedResult<CommunicationLogDto> to match the implementation (or, alternatively,
change the implementation to PagedList if that is the intended canonical type),
ensuring the signature in the docs and the code for GetLogsAsync and related
type references are consistent.
- Line 889: The documentation's CommunicationPriority enum ordering is
incorrect; update the docs entry for CommunicationPriority to match the
implementation by listing the members as Low = 0, Normal = 1, High = 2 (in that
order) and include the numeric values explicitly so the docs reflect the enum in
ICommunicationsModuleApi (CommunicationPriority).
In `@src/Contracts/Contracts/Modules/Communications/ICommunicationsModuleApi.cs`:
- Around line 58-60: Atualize o comentário XML público na interface
ICommunicationsModuleApi para ficar totalmente em português: substitua o trecho
misto "Idempotency check via CorrelationId" por uma frase em PT-BR, por exemplo
"Verificação de idempotência via Identificador de Correlação (CorrelationId)" ou
similar, garantindo que a tag <summary> da operação que "Obtém logs de
comunicação paginados" esteja 100% em português sem termos soltos em inglês.
In `@src/Modules/Communications/API/Extensions.cs`:
- Around line 33-35: Padronize os comentários no grupo de endpoints para
português: substitua as linhas de comentário que usam inglês (por exemplo as
placeholders "// Endpoints will be mapped here" e "// group.MapGet("/templates",
...);", "// group.MapGet("/logs", ...);") por textos em português mantendo o
mesmo contexto (ex.: "// Endpoints serão mapeados aqui" e "//
group.MapGet("/templates", ...); // Exemplo: obter templates"), e ajuste
qualquer XML doc no mesmo arquivo Extensions.cs para seguir a convenção em
português; localize o bloco que referencia o objeto group e atualize apenas os
comentários sem alterar a lógica do mapagem de endpoints.
In `@src/Modules/Communications/Domain/Entities/CommunicationLog.cs`:
- Around line 61-67: Valide o parâmetro attemptCount em
CommunicationLog.CreateSuccess e CommunicationLog.CreateFailure para impedir
valores negativos: dentro de cada método (CreateSuccess, CreateFailure)
verifique if (attemptCount < 0) e lance
ArgumentOutOfRangeException(nameof(attemptCount)) com mensagem clara; isso
garante que tentativas negativas não sejam aceitas e mantém a semântica de
tentativas realizadas consistente entre ambos os métodos.
In `@src/Modules/Communications/Domain/Entities/EmailTemplate.cs`:
- Around line 66-88: In EmailTemplate.Create, validate and normalize the
language and overrideKey: call
ArgumentException.ThrowIfNullOrWhiteSpace(language) (or otherwise reject
empty/whitespace) and then set Language = language.ToLowerInvariant().Trim();
for overrideKey, normalize and convert empty/whitespace to null by using
OverrideKey = string.IsNullOrWhiteSpace(overrideKey) ? null :
overrideKey.ToLowerInvariant().Trim(); update the Create method to perform these
checks and assignments so language cannot be blank and overrideKey won't become
an empty string.
In
`@src/Modules/Communications/Infrastructure/Persistence/Repositories/CommunicationLogRepository.cs`:
- Around line 46-47: O filtro de canal em CommunicationLogRepository atualmente
ignora silently valores inválidos porque o resultado de
Enum.TryParse<ECommunicationChannel> é descartado; modifique o trecho que usa
channel e Enum.TryParse para validar o input: se TryParse falhar, ou lance um
ArgumentException/BadRequest com mensagem clara sobre o canal inválido, ou
aplique um filtro que garanta resultado vazio (por exemplo atribuir query =
query.Where(x => false)) e retorne imediatamente; preserve o comportamento atual
quando TryParse succeeds (usar ch no Where).
---
Duplicate comments:
In `@docs/roadmap-current.md`:
- Around line 966-969: The fenced code block under the "Processo com Outbox"
item is missing a language identifier (MD040); update the triple-backtick fence
that contains "Event Occurred → Save to Outbox Table (Mesma Transação) →
Background Worker processa Outbox" to include the language token `text` (i.e.,
change ``` to ```text) so the block is explicitly labeled and passes the lint
rule.
- Around line 875-884: The Markdown table under the "Mapeamento de Integração
com Eventos" section needs blank lines surrounding it to satisfy MD058; add at
least one empty line immediately before the table header (before the line with |
Evento Existing | Ação de Comunicação |) and at least one empty line immediately
after the final row (after the line with | `DocumentRejectedIntegrationEvent` |
→ Notificar prestador |) so the table is separated from surrounding text and
paragraph flow.
---
Nitpick comments:
In
`@src/Modules/Communications/Infrastructure/Persistence/Configurations/OutboxMessageConfiguration.cs`:
- Around line 37-38: Substitua os índices atuais por um índice composto alinhado
ao padrão de consulta de polling do Outbox: remova/evite o índice separado em
ScheduledAt e crie um único índice composto usando builder.HasIndex(x => new {
x.Status, x.ScheduledAt, x.Priority, x.CreatedAt }) para que o engine possa
filtrar por Status+ScheduledAt e já aproveitar a ordenação por
Priority/CreatedAt, melhorando throughput em alto volume.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 924e8f0e-1f8a-466f-8468-7ea2e6959192
⛔ Files ignored due to path filters (20)
src/Aspire/MeAjudaAi.AppHost/packages.lock.jsonis excluded by!**/packages.lock.jsonsrc/Bootstrapper/MeAjudaAi.ApiService/packages.lock.jsonis excluded by!**/packages.lock.jsonsrc/Modules/Communications/API/packages.lock.jsonis excluded by!**/packages.lock.jsonsrc/Modules/Communications/Application/packages.lock.jsonis excluded by!**/packages.lock.jsonsrc/Modules/Communications/Domain/packages.lock.jsonis excluded by!**/packages.lock.jsonsrc/Modules/Communications/Infrastructure/Persistence/Migrations/20260409000512_InitialCommunications.Designer.csis excluded by!**/Migrations/**,!**/*.Designer.cssrc/Modules/Communications/Infrastructure/Persistence/Migrations/20260409000512_InitialCommunications.csis excluded by!**/Migrations/**src/Modules/Communications/Infrastructure/Persistence/Migrations/CommunicationsDbContextModelSnapshot.csis excluded by!**/Migrations/**src/Modules/Communications/Infrastructure/packages.lock.jsonis excluded by!**/packages.lock.jsonsrc/Modules/Documents/Tests/packages.lock.jsonis excluded by!**/packages.lock.jsonsrc/Modules/Locations/Tests/packages.lock.jsonis excluded by!**/packages.lock.jsonsrc/Modules/Providers/Tests/packages.lock.jsonis excluded by!**/packages.lock.jsonsrc/Modules/SearchProviders/Tests/packages.lock.jsonis excluded by!**/packages.lock.jsonsrc/Modules/ServiceCatalogs/Tests/packages.lock.jsonis excluded by!**/packages.lock.jsonsrc/Modules/Users/Tests/packages.lock.jsonis excluded by!**/packages.lock.jsontests/MeAjudaAi.ApiService.Tests/packages.lock.jsonis excluded by!**/packages.lock.jsontests/MeAjudaAi.Architecture.Tests/packages.lock.jsonis excluded by!**/packages.lock.jsontests/MeAjudaAi.E2E.Tests/packages.lock.jsonis excluded by!**/packages.lock.jsontests/MeAjudaAi.Integration.Tests/packages.lock.jsonis excluded by!**/packages.lock.jsontests/MeAjudaAi.Shared.Tests/packages.lock.jsonis excluded by!**/packages.lock.json
📒 Files selected for processing (56)
MeAjudaAi.slnxdocs/roadmap-current.mdsrc/Bootstrapper/MeAjudaAi.ApiService/MeAjudaAi.ApiService.csprojsrc/Bootstrapper/MeAjudaAi.ApiService/MigrationExtensions.cssrc/Bootstrapper/MeAjudaAi.ApiService/Program.cssrc/Contracts/Contracts/Modules/Communications/Channels/IEmailChannel.cssrc/Contracts/Contracts/Modules/Communications/Channels/IPushChannel.cssrc/Contracts/Contracts/Modules/Communications/Channels/ISmsChannel.cssrc/Contracts/Contracts/Modules/Communications/DTOs/CommunicationLogDto.cssrc/Contracts/Contracts/Modules/Communications/DTOs/EmailMessageDto.cssrc/Contracts/Contracts/Modules/Communications/DTOs/EmailTemplateDto.cssrc/Contracts/Contracts/Modules/Communications/DTOs/PushMessageDto.cssrc/Contracts/Contracts/Modules/Communications/DTOs/SmsMessageDto.cssrc/Contracts/Contracts/Modules/Communications/ICommunicationsModuleApi.cssrc/Contracts/Contracts/Modules/Communications/Queries/CommunicationLogQuery.cssrc/Modules/Communications/API/Extensions.cssrc/Modules/Communications/API/MeAjudaAi.Modules.Communications.API.csprojsrc/Modules/Communications/Application/Extensions.cssrc/Modules/Communications/Application/Handlers/ProviderActivatedIntegrationEventHandler.cssrc/Modules/Communications/Application/Handlers/UserRegisteredIntegrationEventHandler.cssrc/Modules/Communications/Application/MeAjudaAi.Modules.Communications.Application.csprojsrc/Modules/Communications/Application/ModuleApi/CommunicationsModuleApi.cssrc/Modules/Communications/Application/Services/Email/IEmailService.cssrc/Modules/Communications/Application/Services/Email/StubEmailService.cssrc/Modules/Communications/Application/Services/OutboxProcessorService.cssrc/Modules/Communications/Domain/Entities/CommunicationLog.cssrc/Modules/Communications/Domain/Entities/EmailTemplate.cssrc/Modules/Communications/Domain/Entities/OutboxMessage.cssrc/Modules/Communications/Domain/Enums/ECommunicationChannel.cssrc/Modules/Communications/Domain/Enums/ECommunicationPriority.cssrc/Modules/Communications/Domain/Enums/EOutboxMessageStatus.cssrc/Modules/Communications/Domain/MeAjudaAi.Modules.Communications.Domain.csprojsrc/Modules/Communications/Domain/Repositories/ICommunicationLogRepository.cssrc/Modules/Communications/Domain/Repositories/IEmailTemplateRepository.cssrc/Modules/Communications/Domain/Repositories/IOutboxMessageRepository.cssrc/Modules/Communications/Domain/Services/IEmailSender.cssrc/Modules/Communications/Domain/Services/IPushSender.cssrc/Modules/Communications/Domain/Services/ISmsSender.cssrc/Modules/Communications/Infrastructure/Extensions.cssrc/Modules/Communications/Infrastructure/MeAjudaAi.Modules.Communications.Infrastructure.csprojsrc/Modules/Communications/Infrastructure/Persistence/CommunicationsDbContext.cssrc/Modules/Communications/Infrastructure/Persistence/Configurations/CommunicationLogConfiguration.cssrc/Modules/Communications/Infrastructure/Persistence/Configurations/EmailTemplateConfiguration.cssrc/Modules/Communications/Infrastructure/Persistence/Configurations/OutboxMessageConfiguration.cssrc/Modules/Communications/Infrastructure/Persistence/Repositories/CommunicationLogRepository.cssrc/Modules/Communications/Infrastructure/Persistence/Repositories/EmailTemplateRepository.cssrc/Modules/Communications/Infrastructure/Persistence/Repositories/OutboxMessageRepository.cssrc/Modules/Communications/Infrastructure/Services/EmailSenderStub.cssrc/Modules/Communications/Infrastructure/Services/PushSenderStub.cssrc/Modules/Communications/Infrastructure/Services/SmsSenderStub.cssrc/Shared/Communications/Templates/WelcomeEmail.cshtmltests/MeAjudaAi.Integration.Tests/Modules/Documents/DocumentsEndpointsTests.cstests/MeAjudaAi.Integration.Tests/Modules/Locations/GeographicRestrictionTests.cstests/MeAjudaAi.Integration.Tests/Modules/ServiceCatalogs/ServiceCatalogsEndpointsTests.cstests/MeAjudaAi.Integration.Tests/Modules/Users/UsersEndpointsTests.cstests/MeAjudaAi.Shared.Tests/Unit/Constants/ModuleNamesTests.cs
✅ Files skipped from review due to trivial changes (11)
- src/Modules/Communications/Domain/Enums/ECommunicationPriority.cs
- src/Modules/Communications/API/MeAjudaAi.Modules.Communications.API.csproj
- src/Contracts/Contracts/Modules/Communications/DTOs/PushMessageDto.cs
- src/Contracts/Contracts/Modules/Communications/DTOs/EmailMessageDto.cs
- src/Contracts/Contracts/Modules/Communications/Queries/CommunicationLogQuery.cs
- src/Modules/Communications/Domain/Enums/ECommunicationChannel.cs
- src/Modules/Communications/Domain/Enums/EOutboxMessageStatus.cs
- src/Shared/Communications/Templates/WelcomeEmail.cshtml
- src/Contracts/Contracts/Modules/Communications/DTOs/CommunicationLogDto.cs
- src/Bootstrapper/MeAjudaAi.ApiService/MeAjudaAi.ApiService.csproj
- src/Contracts/Contracts/Modules/Communications/DTOs/EmailTemplateDto.cs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/Modules/Communications/Application/ModuleApi/CommunicationsModuleApi.cs (2)
32-36:⚠️ Potential issue | 🟠 MajorAdicione validações fail-fast nas entradas públicas
Os métodos aceitam parâmetros sem validação, permitindo enfileirar payload inválido ou consultar logs com paginação inconsistente.
🔧 Patch objetivo
public async Task<Result<Guid>> SendEmailAsync( EmailMessageDto email, CommunicationPriority priority = CommunicationPriority.Normal, CancellationToken ct = default) { + ArgumentNullException.ThrowIfNull(email); var payload = JsonSerializer.Serialize(email); @@ public async Task<Result<Guid>> SendSmsAsync(SmsMessageDto sms, CancellationToken ct = default) { + ArgumentNullException.ThrowIfNull(sms); var payload = JsonSerializer.Serialize(sms); @@ public async Task<Result<Guid>> SendPushAsync(PushMessageDto push, CancellationToken ct = default) { + ArgumentNullException.ThrowIfNull(push); var payload = JsonSerializer.Serialize(push); @@ public async Task<Result<PagedResult<CommunicationLogDto>>> GetLogsAsync( CommunicationLogQuery query, CancellationToken ct = default) { + ArgumentNullException.ThrowIfNull(query); + if (query.PageNumber < 1) throw new ArgumentOutOfRangeException(nameof(query.PageNumber)); + if (query.PageSize < 1) throw new ArgumentOutOfRangeException(nameof(query.PageSize)); + var (items, totalCount) = await logRepository.SearchAsync(Also applies to: 56-57, 70-71, 84-86
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Modules/Communications/Application/ModuleApi/CommunicationsModuleApi.cs` around lines 32 - 36, Adicione validações fail-fast em todos os métodos públicos do módulo (por exemplo SendEmailAsync) para validar imediatamente parâmetros de entrada: verifique email != null e campos obrigatórios dentro de EmailMessageDto (destinatários, subject, body), valide que priority é um valor válido do enum CommunicationPriority, e para métodos de paginação valide page/pageSize > 0 e limites máximos; ao detectar entrada inválida retorne de imediato um Result de falha com mensagem clara em vez de enfileirar ou prosseguir. Implemente essas checagens no início de SendEmailAsync e nas demais APIs públicas listadas no arquivo (aplique mesma lógica para os métodos aos quais o comentário se refere) usando os símbolos existentes (SendEmailAsync, EmailMessageDto, CommunicationPriority) e mantenha comportamento assíncrono/assinatura (Task<Result<...>>) retornando Result.Failure apropriado.
49-54:⚠️ Potential issue | 🟠 Major
GetTemplatesAsyncnão deve retornar sucesso vazio de forma permanenteRetornar sucesso com lista vazia fixa esconde lacuna funcional para consumidores que dependem de templates reais. Ou implemente a leitura no repositório, ou falhe explicitamente até concluir a feature.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Modules/Communications/Application/ModuleApi/CommunicationsModuleApi.cs` around lines 49 - 54, O método GetTemplatesAsync atualmente devolve sempre um Success com lista vazia; substitua esse placeholder por uma implementação real: invoque o repositório de templates (ex.: _emailTemplateRepository.GetAllAsync(ct) ou adicione um método GetAll/FindByKeys em IEmailTemplateRepository) e mapeie os resultados para EmailTemplateDto retornando Result.Success(list) ou, enquanto a persistência não existir, retorne explicitamente Result.Failure com uma mensagem clara (por exemplo "Email templates not implemented") para que consumidores não ocultem a falta de funcionalidade; ajuste o método GetTemplatesAsync para depender do repositório e remover o retorno estático.
🧹 Nitpick comments (3)
src/Modules/Communications/Application/Handlers/ProviderAwaitingVerificationIntegrationEventHandler.cs (1)
24-27: Evite destinatário administrativo hardcodedExtrair
admin@meajudaai.com.brpara configuração (IOptions) evita envio acidental entre ambientes e facilita manutenção operacional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Modules/Communications/Application/Handlers/ProviderAwaitingVerificationIntegrationEventHandler.cs` around lines 24 - 27, The handler currently hardcodes the admin recipient ("admin@meajudaai.com.br"); inject a configuration-backed option (e.g., IOptions<AdminEmailsOptions> or IConfiguration) into ProviderAwaitingVerificationIntegrationEventHandler, add a property like AdminEmail or ProviderVerificationRecipient to that options class, and replace the hardcoded To value with the configured value (e.g., _options.Value.ProviderVerificationRecipient) when constructing the email (the EmailMessage/To assignment in the handler). Update constructor signature to accept the options, store it in a private readonly field, and use that field instead of the literal string so environments can override the admin recipient via configuration.tests/MeAjudaAi.Integration.Tests/Modules/Communications/CommunicationsModuleApiTests.cs (1)
45-53: Considere adicionar o cenário 401 para/templatesComo o grupo inteiro exige autenticação, vale incluir um teste equivalente sem autenticação para
/api/v1/communications/templatespara manter cobertura simétrica.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/MeAjudaAi.Integration.Tests/Modules/Communications/CommunicationsModuleApiTests.cs` around lines 45 - 53, Add a symmetric unauthenticated test for the templates endpoint: in the CommunicationsModuleApiTests class, create a new [Fact] async test named GetTemplates_WithoutAuthentication_ShouldReturnUnauthorized that mirrors GetLogs_WithoutAuthentication_ShouldReturnUnauthorized by calling Client.GetAsync("/api/v1/communications/templates") and asserting response.StatusCode.Should().Be(HttpStatusCode.Unauthorized); keep the same test structure and naming convention as the existing GetLogs_WithoutAuthentication_ShouldReturnUnauthorized.tests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cs (1)
432-433: Padronize oCommunicationsDbContextno setup de testeAgora que o fluxo resolve e migra
CommunicationsDbContext, recomendo registrá-lo explicitamente noConfigureServicesde teste (como os demais contextos) para reduzir drift de configuração entre ambiente real e integração.Also applies to: 480-485
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cs` around lines 432 - 433, Registre explicitamente o CommunicationsDbContext no método ConfigureServices de teste (na classe BaseApiTest) para padronizar com os outros DbContexts: adicione a mesma chamada de AddDbContext/AddDbContextPool usada para os demais contextos, garantindo que o provider resolva CommunicationsDbContext (referenciado em modules.HasFlag(TestModule.Communications) e serviceProvider.GetRequiredService<CommunicationsDbContext>), e repita a mesma mudança onde houver setup similar mencionado nas linhas 480-485 para evitar drift entre ambiente real e integração.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/Modules/Communications/Application/Handlers/ProviderVerificationStatusUpdatedIntegrationEventHandler.cs`:
- Around line 21-23: O handler
ProviderVerificationStatusUpdatedIntegrationEventHandler está usando um
placeholder ("provider-email-lookup-pending@meajudaai.com.br") em
recipientEmail; em vez disso, resolva o e-mail real do prestador usando o
IUsersModuleApi com integrationEvent.UserId antes de enfileirar a notificação,
tratando erros/nulos (ex.: lançar/registrar ou abortar o envio se não houver
e-mail) e substituir a atribuição do campo recipientEmail pela chamada ao método
do IUsersModuleApi que retorna o e-mail do usuário.
In `@src/Modules/Communications/Application/Services/OutboxProcessorService.cs`:
- Line 184: O comentário em inglês "// Payload records (serialized into Outbox)"
no arquivo OutboxProcessorService.cs deve ser padronizado para português;
substitua esse comentário por uma versão em português consistente com o estilo
do código (por exemplo "Registros de payload (serializados no Outbox)" ou
semelhante) no local onde aparece na classe OutboxProcessorService para manter a
uniformidade dos comentários na base.
- Around line 57-120: The code currently dispatches external messages via
DispatchMessageAsync and only persists message state/logs at the end
(outboxRepository.SaveChangesAsync/_logRepository.SaveChangesAsync), which can
cause duplicate deliveries if persistence fails; change the flow so that after a
successful DispatchMessageAsync you immediately call message.MarkAsSent(...)
then persist that state and the CommunicationLog by calling
_logRepository.AddAsync(...) and saving both repositories (or, preferably,
enlisting both operations in the same transaction) before proceeding to the next
message, and ensure that exceptions thrown by _logRepository.AddAsync or
SaveChangesAsync do not call message.MarkAsFailed or revert the already-sent
external effect—handle persistence failures separately (retry/save to
dead-letter/store) instead of marking messages failed in the catch blocks that
also wrap dispatch.
---
Duplicate comments:
In `@src/Modules/Communications/Application/ModuleApi/CommunicationsModuleApi.cs`:
- Around line 32-36: Adicione validações fail-fast em todos os métodos públicos
do módulo (por exemplo SendEmailAsync) para validar imediatamente parâmetros de
entrada: verifique email != null e campos obrigatórios dentro de EmailMessageDto
(destinatários, subject, body), valide que priority é um valor válido do enum
CommunicationPriority, e para métodos de paginação valide page/pageSize > 0 e
limites máximos; ao detectar entrada inválida retorne de imediato um Result de
falha com mensagem clara em vez de enfileirar ou prosseguir. Implemente essas
checagens no início de SendEmailAsync e nas demais APIs públicas listadas no
arquivo (aplique mesma lógica para os métodos aos quais o comentário se refere)
usando os símbolos existentes (SendEmailAsync, EmailMessageDto,
CommunicationPriority) e mantenha comportamento assíncrono/assinatura
(Task<Result<...>>) retornando Result.Failure apropriado.
- Around line 49-54: O método GetTemplatesAsync atualmente devolve sempre um
Success com lista vazia; substitua esse placeholder por uma implementação real:
invoque o repositório de templates (ex.:
_emailTemplateRepository.GetAllAsync(ct) ou adicione um método GetAll/FindByKeys
em IEmailTemplateRepository) e mapeie os resultados para EmailTemplateDto
retornando Result.Success(list) ou, enquanto a persistência não existir, retorne
explicitamente Result.Failure com uma mensagem clara (por exemplo "Email
templates not implemented") para que consumidores não ocultem a falta de
funcionalidade; ajuste o método GetTemplatesAsync para depender do repositório e
remover o retorno estático.
---
Nitpick comments:
In
`@src/Modules/Communications/Application/Handlers/ProviderAwaitingVerificationIntegrationEventHandler.cs`:
- Around line 24-27: The handler currently hardcodes the admin recipient
("admin@meajudaai.com.br"); inject a configuration-backed option (e.g.,
IOptions<AdminEmailsOptions> or IConfiguration) into
ProviderAwaitingVerificationIntegrationEventHandler, add a property like
AdminEmail or ProviderVerificationRecipient to that options class, and replace
the hardcoded To value with the configured value (e.g.,
_options.Value.ProviderVerificationRecipient) when constructing the email (the
EmailMessage/To assignment in the handler). Update constructor signature to
accept the options, store it in a private readonly field, and use that field
instead of the literal string so environments can override the admin recipient
via configuration.
In `@tests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cs`:
- Around line 432-433: Registre explicitamente o CommunicationsDbContext no
método ConfigureServices de teste (na classe BaseApiTest) para padronizar com os
outros DbContexts: adicione a mesma chamada de AddDbContext/AddDbContextPool
usada para os demais contextos, garantindo que o provider resolva
CommunicationsDbContext (referenciado em
modules.HasFlag(TestModule.Communications) e
serviceProvider.GetRequiredService<CommunicationsDbContext>), e repita a mesma
mudança onde houver setup similar mencionado nas linhas 480-485 para evitar
drift entre ambiente real e integração.
In
`@tests/MeAjudaAi.Integration.Tests/Modules/Communications/CommunicationsModuleApiTests.cs`:
- Around line 45-53: Add a symmetric unauthenticated test for the templates
endpoint: in the CommunicationsModuleApiTests class, create a new [Fact] async
test named GetTemplates_WithoutAuthentication_ShouldReturnUnauthorized that
mirrors GetLogs_WithoutAuthentication_ShouldReturnUnauthorized by calling
Client.GetAsync("/api/v1/communications/templates") and asserting
response.StatusCode.Should().Be(HttpStatusCode.Unauthorized); keep the same test
structure and naming convention as the existing
GetLogs_WithoutAuthentication_ShouldReturnUnauthorized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f2a310a3-3e37-4881-892e-a452b38ffef6
📒 Files selected for processing (16)
src/Modules/Communications/API/Endpoints/CommunicationsModuleEndpoints.cssrc/Modules/Communications/API/Extensions.cssrc/Modules/Communications/Application/Extensions.cssrc/Modules/Communications/Application/Handlers/ProviderAwaitingVerificationIntegrationEventHandler.cssrc/Modules/Communications/Application/Handlers/ProviderVerificationStatusUpdatedIntegrationEventHandler.cssrc/Modules/Communications/Application/ModuleApi/CommunicationsModuleApi.cssrc/Modules/Communications/Application/Services/OutboxProcessorService.cssrc/Modules/Communications/Domain/Repositories/ICommunicationLogRepository.cssrc/Modules/Communications/Domain/Repositories/IOutboxMessageRepository.cssrc/Modules/Communications/Infrastructure/Extensions.cssrc/Modules/Communications/Infrastructure/Persistence/Repositories/CommunicationLogRepository.cssrc/Modules/Communications/Infrastructure/Persistence/Repositories/OutboxMessageRepository.cssrc/Modules/Communications/Infrastructure/Services/CommunicationsOutboxWorker.cstests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cstests/MeAjudaAi.Integration.Tests/Modules/Communications/CommunicationsModuleApiTests.cstests/MeAjudaAi.Integration.Tests/Modules/Communications/OutboxPatternTests.cs
🚧 Files skipped from review as they are similar to previous changes (6)
- src/Modules/Communications/Infrastructure/Extensions.cs
- src/Modules/Communications/Application/Extensions.cs
- src/Modules/Communications/API/Extensions.cs
- src/Modules/Communications/Infrastructure/Persistence/Repositories/OutboxMessageRepository.cs
- src/Modules/Communications/Domain/Repositories/ICommunicationLogRepository.cs
- src/Modules/Communications/Infrastructure/Persistence/Repositories/CommunicationLogRepository.cs
…x idempotency, pii masking and doc sync
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/Modules/Communications/Application/Services/OutboxProcessorService.cs (1)
200-200:⚠️ Potential issue | 🟡 MinorPadronizar comentário para português.
O comentário na linha 200 está em inglês, mas a política do repositório exige que comentários e documentação XML sejam em português.
🔧 Correção proposta
- // Payload records (serialized into Outbox) + // Registros de payload (serializados no Outbox) private sealed record EmailOutboxPayload(string To, string Subject, string HtmlBody, string TextBody, string? From = null, string? TemplateKey = null);Based on learnings, comentários e documentação XML em C# devem ser escritos em português em todo o codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Modules/Communications/Application/Services/OutboxProcessorService.cs` at line 200, Padronize o comentário em src/Modules/Communications/Application/Services/OutboxProcessorService.cs alterando o comentário em inglês "Payload records (serialized into Outbox)" para português; localize o comentário dentro da classe OutboxProcessorService e substitua por uma versão em português, por exemplo "Registros de payload (serializados no Outbox)" garantindo que o estilo e tom estejam consistentes com os demais comentários do repositório.src/Modules/Communications/Application/ModuleApi/CommunicationsModuleApi.cs (1)
107-138:⚠️ Potential issue | 🟡 MinorFalta validação de entrada em
GetLogsAsync.O método
GetLogsAsyncnão valida o parâmetroquery. Se viernullou com paginação inválida, pode causar comportamento inesperado.🛡️ Correção proposta
public async Task<Result<PagedResult<CommunicationLogDto>>> GetLogsAsync( CommunicationLogQuery query, CancellationToken ct = default) { + ArgumentNullException.ThrowIfNull(query); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(query.PageNumber); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(query.PageSize); + var (items, totalCount) = await logRepository.SearchAsync(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Modules/Communications/Application/ModuleApi/CommunicationsModuleApi.cs` around lines 107 - 138, O método GetLogsAsync não valida o parâmetro query nem os campos de paginação; adicione uma validação no início de GetLogsAsync que verifique se query é nulo e retorne Result<PagedResult<CommunicationLogDto>>.Failure com uma mensagem de erro apropriada, e valide query.PageNumber e query.PageSize (por exemplo PageNumber >= 1 e PageSize entre 1 e um limite razoável como 100) antes de chamar logRepository.SearchAsync; se os valores de paginação forem inválidos, retorne Failure indicando o problema. Garanta que as referências usadas na validação correspondam a CommunicationLogQuery, GetLogsAsync, logRepository.SearchAsync e ao tipo de retorno Result<PagedResult<CommunicationLogDto>>.
🧹 Nitpick comments (1)
src/Modules/Communications/Infrastructure/Persistence/Repositories/OutboxMessageRepository.cs (1)
15-50: Implementação de claim atômico está correta, mas considere recuperação de mensagens "travadas".A transação em
GetPendingAsyncresolve o problema de processamento duplicado por workers concorrentes. Porém, se o worker falhar após o commit da transação (linha 42) mas antes de persistir o status final (Sent/Failed), as mensagens ficarão permanentemente no estadoProcessinge nunca serão reprocessadas.Recomendo implementar um mecanismo de timeout para recuperar mensagens travadas:
- Um job periódico que busca mensagens
ProcessingcomUpdatedAt < DateTime.UtcNow.AddMinutes(-X)- Reverter essas mensagens para
Pendingpara reprocessamento💡 Exemplo de query para mensagens travadas
public async Task<int> ResetStuckMessagesAsync(TimeSpan timeout, CancellationToken ct = default) { var threshold = DateTime.UtcNow - timeout; var stuckMessages = await context.OutboxMessages .Where(x => x.Status == EOutboxMessageStatus.Processing && x.UpdatedAt < threshold) .ToListAsync(ct); foreach (var msg in stuckMessages) msg.ResetToPending(); // Novo método na entidade return await context.SaveChangesAsync(ct); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Modules/Communications/Infrastructure/Persistence/Repositories/OutboxMessageRepository.cs` around lines 15 - 50, GetPendingAsync correctly claims messages but can leave them stuck in Processing if a worker crashes; add a recovery mechanism by implementing a repository method ResetStuckMessagesAsync(TimeSpan timeout, CancellationToken ct = default) that computes a threshold = DateTime.UtcNow - timeout, queries context.OutboxMessages for Status == EOutboxMessageStatus.Processing && UpdatedAt < threshold, calls a new entity method ResetToPending() on each result, and saves changes; wire this into a periodic background job (cron/hosted service) to run the recovery job at your desired interval so stuck messages are reverted to Pending for reprocessing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Modules/Communications/Application/Services/OutboxProcessorService.cs`:
- Around line 128-139: Change the ExtractTemplateKey method signature in
OutboxProcessorService to return a nullable string (string?) instead of string
to eliminate CS8603 warnings; update the method declaration for
ExtractTemplateKey(OutboxMessage message) to string? and keep the existing logic
that returns null on non-email channels or deserialization exceptions, and then
review any callers of ExtractTemplateKey to handle the nullable return (use null
checks or the null-coalescing operator) so no new nullable warnings are
introduced; references: ExtractTemplateKey, OutboxProcessorService,
OutboxMessage, EmailOutboxPayload.
---
Duplicate comments:
In `@src/Modules/Communications/Application/ModuleApi/CommunicationsModuleApi.cs`:
- Around line 107-138: O método GetLogsAsync não valida o parâmetro query nem os
campos de paginação; adicione uma validação no início de GetLogsAsync que
verifique se query é nulo e retorne
Result<PagedResult<CommunicationLogDto>>.Failure com uma mensagem de erro
apropriada, e valide query.PageNumber e query.PageSize (por exemplo PageNumber
>= 1 e PageSize entre 1 e um limite razoável como 100) antes de chamar
logRepository.SearchAsync; se os valores de paginação forem inválidos, retorne
Failure indicando o problema. Garanta que as referências usadas na validação
correspondam a CommunicationLogQuery, GetLogsAsync, logRepository.SearchAsync e
ao tipo de retorno Result<PagedResult<CommunicationLogDto>>.
In `@src/Modules/Communications/Application/Services/OutboxProcessorService.cs`:
- Line 200: Padronize o comentário em
src/Modules/Communications/Application/Services/OutboxProcessorService.cs
alterando o comentário em inglês "Payload records (serialized into Outbox)" para
português; localize o comentário dentro da classe OutboxProcessorService e
substitua por uma versão em português, por exemplo "Registros de payload
(serializados no Outbox)" garantindo que o estilo e tom estejam consistentes com
os demais comentários do repositório.
---
Nitpick comments:
In
`@src/Modules/Communications/Infrastructure/Persistence/Repositories/OutboxMessageRepository.cs`:
- Around line 15-50: GetPendingAsync correctly claims messages but can leave
them stuck in Processing if a worker crashes; add a recovery mechanism by
implementing a repository method ResetStuckMessagesAsync(TimeSpan timeout,
CancellationToken ct = default) that computes a threshold = DateTime.UtcNow -
timeout, queries context.OutboxMessages for Status ==
EOutboxMessageStatus.Processing && UpdatedAt < threshold, calls a new entity
method ResetToPending() on each result, and saves changes; wire this into a
periodic background job (cron/hosted service) to run the recovery job at your
desired interval so stuck messages are reverted to Pending for reprocessing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8b4df4bd-263e-4ee1-bf35-f89eea47829d
📒 Files selected for processing (22)
docs/roadmap-current.mdsrc/Contracts/Contracts/Modules/Communications/ICommunicationsModuleApi.cssrc/Modules/Communications/Application/Handlers/ProviderActivatedIntegrationEventHandler.cssrc/Modules/Communications/Application/Handlers/UserRegisteredIntegrationEventHandler.cssrc/Modules/Communications/Application/ModuleApi/CommunicationsModuleApi.cssrc/Modules/Communications/Application/Services/Email/StubEmailService.cssrc/Modules/Communications/Application/Services/OutboxProcessorService.cssrc/Modules/Communications/Domain/Entities/CommunicationLog.cssrc/Modules/Communications/Domain/Entities/EmailTemplate.cssrc/Modules/Communications/Domain/Entities/OutboxMessage.cssrc/Modules/Communications/Domain/Enums/EOutboxMessageStatus.cssrc/Modules/Communications/Domain/Repositories/IEmailTemplateRepository.cssrc/Modules/Communications/Infrastructure/Persistence/CommunicationsDbContext.cssrc/Modules/Communications/Infrastructure/Persistence/CommunicationsDbContextFactory.cssrc/Modules/Communications/Infrastructure/Persistence/Configurations/EmailTemplateConfiguration.cssrc/Modules/Communications/Infrastructure/Persistence/Configurations/OutboxMessageConfiguration.cssrc/Modules/Communications/Infrastructure/Persistence/Repositories/CommunicationLogRepository.cssrc/Modules/Communications/Infrastructure/Persistence/Repositories/EmailTemplateRepository.cssrc/Modules/Communications/Infrastructure/Persistence/Repositories/OutboxMessageRepository.cssrc/Modules/Communications/Infrastructure/Services/EmailSenderStub.cssrc/Modules/Communications/Infrastructure/Services/PushSenderStub.cssrc/Modules/Communications/Infrastructure/Services/SmsSenderStub.cs
✅ Files skipped from review due to trivial changes (2)
- src/Modules/Communications/Infrastructure/Services/PushSenderStub.cs
- src/Contracts/Contracts/Modules/Communications/ICommunicationsModuleApi.cs
🚧 Files skipped from review as they are similar to previous changes (12)
- src/Modules/Communications/Infrastructure/Services/SmsSenderStub.cs
- src/Modules/Communications/Domain/Enums/EOutboxMessageStatus.cs
- src/Modules/Communications/Infrastructure/Services/EmailSenderStub.cs
- src/Modules/Communications/Application/Services/Email/StubEmailService.cs
- src/Modules/Communications/Infrastructure/Persistence/Configurations/EmailTemplateConfiguration.cs
- src/Modules/Communications/Infrastructure/Persistence/Configurations/OutboxMessageConfiguration.cs
- src/Modules/Communications/Application/Handlers/ProviderActivatedIntegrationEventHandler.cs
- src/Modules/Communications/Application/Handlers/UserRegisteredIntegrationEventHandler.cs
- src/Modules/Communications/Domain/Repositories/IEmailTemplateRepository.cs
- src/Modules/Communications/Infrastructure/Persistence/Repositories/EmailTemplateRepository.cs
- src/Modules/Communications/Infrastructure/Persistence/Repositories/CommunicationLogRepository.cs
- src/Modules/Communications/Domain/Entities/OutboxMessage.cs
added 3 commits
April 8, 2026 21:47
…ers and correlation ID support
…ement search E2E tests
…age recovery, fail-fast validation and robust persistence
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
docs/roadmap-current.md (1)
659-659:⚠️ Potential issue | 🟡 MinorPadronize terminologia PT-BR e corrija pequenos pontos de texto para reduzir ruído no roadmap.
Há mistura de termos (
templates,Follow-ups,Evento Existing) e um acento faltando (facil), o que reduz consistência editorial.✏️ Sugestão objetiva
- - [ ] **Módulo Comunicações**: Implementar infraestrutura base (outbox pattern, templates, event handlers) + - [ ] **Módulo Comunicações**: Implementar infraestrutura base (outbox pattern, modelos, event handlers) - | **Unitários** | Lógica de templates, cálculo de retries, mapeamento de DTOs | xUnit + FluentAssertions | + | **Unitários** | Lógica de modelos, cálculo de retries, mapeamento de DTOs | xUnit + FluentAssertions | - > **📝 Decisão Técnica** (8 Abr 2026): ... permitindo troca facil. + > **📝 Decisão Técnica** (8 Abr 2026): ... permitindo troca fácil. - **Localização de Templates**: + **Localização de Modelos**: - | Evento Existing | Ação de Comunicação | + | Evento existente | Ação de Comunicação | - | 8. Criar templates básicos (.cshtml) | 3h | - | + | 8. Criar modelos básicos (.cshtml) | 3h | - | - ✅ Sistema de templates híbrido funcional + ✅ Sistema de modelos híbrido funcional - **Follow-ups Pendentes**: + **Acompanhamentos Pendentes**: - - [ ] Admin UI para gestão dinâmica de templates + - [ ] Admin UI para gestão dinâmica de modelosAlso applies to: 798-798, 815-815, 868-871, 877-877, 991-991, 1002-1002, 1009-1011
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/roadmap-current.md` at line 659, Resumo: padronizar terminologia PT-BR e corrigir acentuação no roadmap. In the docs line containing "Módulo Comunicações" replace English terms with Portuguese equivalents (e.g., change "templates" → "modelos", "Follow-ups" → "acompanhamentos" or "seguimentos" where context fits, and "Evento Existing" → "Evento existente"), and fix the missing accent "facil" → "fácil"; apply the same translations/orthography fixes to the other referenced occurrences ("templates", "Follow-ups", "Evento Existing", "facil") across the document so terminology is consistent.
🧹 Nitpick comments (8)
src/Shared/Utilities/PiiMaskingHelper.cs (1)
54-59: Resumo XML deMaskSensitiveDatanão condiz com a implementação.As Lines 54-55 indicam preservação de tamanho/prefixo, mas a função (Line 59) sempre retorna
"[REDACTED]". Recomendo alinhar a documentação ao comportamento atual para evitar uso incorreto por outros módulos.📝 Ajuste sugerido de documentação
/// <summary> -/// Mascara um dado sensível genérico mantendo apenas o tamanho ou prefixo. +/// Mascara um dado sensível genérico com redação total. /// </summary>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Shared/Utilities/PiiMaskingHelper.cs` around lines 54 - 59, The XML summary for MaskSensitiveData is incorrect: it claims the method preserves size/prefix but the implementation always returns "[REDACTED]" for non-empty input and "[EMPTY]" for null/whitespace. Fix by updating the method's XML doc comment (the /// summary for MaskSensitiveData) to accurately describe the current behavior—state that it returns "[EMPTY]" when input is null/whitespace and otherwise returns a constant "[REDACTED]"; alternatively, if you prefer the original behavior, modify the MaskSensitiveData implementation to preserve length/prefix as documented—choose one consistent change so doc and implementation match.docs/roadmap-current.md (1)
887-912: Reforce explicitamente a fonte canônica dos contratos para evitar drift.No bloco da
ICommunicationsModuleApi, vale anotar que enums/DTOs devem vir deMeAjudaAi.Shared.Contracts(ou contrato compartilhado equivalente), para manter Web/backend sincronizados.Based on learnings: prioritize reuse of Shared.Contracts for enums/constants to keep Web aligned with backend/shared code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/roadmap-current.md` around lines 887 - 912, Atualize a documentação da interface ICommunicationsModuleApi para explicitar que todos os tipos e enums usados (por exemplo CommunicationPriority, EmailMessageDto, SmsMessageDto, PushMessageDto, CommunicationLogDto, CommunicationLogQuery, PagedResult) devem ser importados do contrato canônico MeAjudaAi.Shared.Contracts (ou do namespace de contrato compartilhado equivalente); adicione uma frase clara junto à declaração da interface indicando "Usar tipos de MeAjudaAi.Shared.Contracts" e, quando pertinente, troque menções locais de enums/DTOs por referências ao contrato compartilhado para evitar drift entre backend e frontend.tests/MeAjudaAi.Integration.Tests/Modules/SearchProviders/SearchProvidersE2ETests.cs (2)
17-19: Padronize comentários inline para português neste arquivo C#.Os comentários nesses trechos estão em inglês; vale alinhar ao padrão do repositório para manter consistência.
Based on learnings: em C#, comentários/XML devem estar em português, mantendo logs em inglês.
Also applies to: 46-46, 69-69
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/MeAjudaAi.Integration.Tests/Modules/SearchProviders/SearchProvidersE2ETests.cs` around lines 17 - 19, Padronize os comentários neste arquivo SearchProvidersE2ETests.cs para português: substitua os comentários inline em inglês (ex.: o bloco de comentários na área de "Arrange: Seed providers at specific coordinates" nas proximidades de SearchProvidersE2ETests, e os comentários nas posições referenciadas por 46 e 69) por equivalentes em português mantendo o mesmo sentido; preserve mensagens de log em inglês conforme a orientação e verifique também comentários XML/documentação (se houver) no mesmo arquivo para tradução consistente.
62-63: GarantaDistanceInKmpreenchido antes de validar ordenação.A ordenação atual usa chave nullable. Se vier
null, o teste pode não detectar regressão no cálculo de distância.💡 Ajuste sugerido
- var result = await response.Content.ReadFromJsonAsync<PagedResult<SearchableProviderDto>>(); - result!.Items.Should().BeInAscendingOrder(x => x.DistanceInKm); + var result = await response.Content.ReadFromJsonAsync<PagedResult<SearchableProviderDto>>(); + result.Should().NotBeNull(); + result!.Items.Should().OnlyContain(x => x.DistanceInKm.HasValue); + result.Items.Should().BeInAscendingOrder(x => x.DistanceInKm!.Value);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/MeAjudaAi.Integration.Tests/Modules/SearchProviders/SearchProvidersE2ETests.cs` around lines 62 - 63, Before asserting ordering on result!.Items by DistanceInKm, ensure DistanceInKm is populated for every SearchableProviderDto: verify no nulls (e.g., assert Items.All(x => x.DistanceInKm.HasValue)) or fail the test if any DistanceInKm is null so the sort assertion on result!.Items.Should().BeInAscendingOrder(x => x.DistanceInKm) is meaningful; reference the PagedResult<SearchableProviderDto> result and the Items collection and add a precondition check for DistanceInKm to guarantee the test detects regressions in distance calculation.src/Modules/Communications/Application/Services/OutboxProcessorService.cs (1)
35-35: Campo_logRepositoryredundante.O primary constructor já captura
logRepositorycomo parâmetro. A reatribuição para_logRepositoryé desnecessária — as outras dependências (outboxRepository,emailSender, etc.) são usadas diretamente sem reatribuição.♻️ Correção proposta
internal sealed class OutboxProcessorService( IOutboxMessageRepository outboxRepository, ICommunicationLogRepository logRepository, IEmailSender emailSender, ISmsSender smsSender, IPushSender pushSender, ILogger<OutboxProcessorService> logger) : IOutboxProcessorService { - private readonly ICommunicationLogRepository _logRepository = logRepository; - public async Task<int> ProcessPendingMessagesAsync(E substituir
_logRepositoryporlogRepositorynas linhas 95, 120 e 121.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Modules/Communications/Application/Services/OutboxProcessorService.cs` at line 35, Remove the redundant re-assignment field `_logRepository = logRepository`; instead use the injected constructor parameter `logRepository` directly. Delete the unnecessary field declaration `_logRepository` and update all usages of `_log_repository` (e.g., in methods referencing `_logRepository`) to `logRepository` (the constructor parameter) so the service relies on the injected dependency without reassigning it.src/Contracts/Contracts/Modules/Communications/ICommunicationsModuleApi.cs (2)
42-56: Considere adicionar parâmetropriorityaos métodos SMS e Push para consistência.
SendEmailAsyncaceitaCommunicationPriority, masSendSmsAsynceSendPushAsyncnão. Isso pode limitar a flexibilidade de priorização para outros canais.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Contracts/Contracts/Modules/Communications/ICommunicationsModuleApi.cs` around lines 42 - 56, Add an optional CommunicationPriority parameter to SendSmsAsync and SendPushAsync to match SendEmailAsync: update the method signatures for SendSmsAsync(SmsMessageDto sms, CommunicationPriority priority = CommunicationPriority.Normal, CancellationToken ct = default) and SendPushAsync(PushMessageDto push, CommunicationPriority priority = CommunicationPriority.Normal, CancellationToken ct = default), update their XML docs to document the new priority parameter and default, and ensure any implementing interfaces/classes that implement these methods (and any callers) are updated to accept/forward the new priority value; reference SendEmailAsync, CommunicationPriority, SmsMessageDto, PushMessageDto, SendSmsAsync, and SendPushAsync when locating the change.
11-16: CentralizeCommunicationPriorityemShared.Contracts.Existe duplicação de enums:
CommunicationPriority(Contracts) eECommunicationPriority(Domain) com valores idênticos (Low=0, Normal=1, High=2). Atualmente há mapeamento explícito (MapPriority()emCommunicationsModuleApi.cs) que converte entre eles. Mova o enum paraShared.Contractsconforme padrão do repositório, eliminando a conversão desnecessária e mantendo API contracts alinhados com domínio.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Contracts/Contracts/Modules/Communications/ICommunicationsModuleApi.cs` around lines 11 - 16, Há duplicação do enum (CommunicationPriority em Contracts e ECommunicationPriority em Domain); mova a definição única para Shared.Contracts, remova a outra e elimine a conversão em MapPriority. Concretamente: cutar/remover the enum CommunicationPriority from ICommunicationsModuleApi.cs and the enum ECommunicationPriority from the domain, add a single enum CommunicationPriority in the Shared.Contracts project/namespace, update all references to use Shared.Contracts.CommunicationPriority (including usages in CommunicationsModuleApi.cs and domain code) and delete the MapPriority() method in CommunicationsModuleApi.cs (or replace its callers to use the shared enum directly). Certifique-se de ajustar namespaces/imports and run build to resolve any remaining references.src/Modules/Communications/Application/ModuleApi/CommunicationsModuleApi.cs (1)
45-54: Reduzir duplicação no fluxo de enfileiramento do OutboxOs três métodos de envio repetem o mesmo padrão (serializar payload → criar
OutboxMessage→AddAsync→SaveChangesAsync). Vale extrair um helper privado para diminuir duplicação e risco de divergência futura.Also applies to: 79-88, 98-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Modules/Communications/Application/ModuleApi/CommunicationsModuleApi.cs` around lines 45 - 54, Resumo: reduzir duplicação extraindo o padrão de enfileiramento do Outbox para um helper privado. Crie um método privado como EnqueueOutboxAsync<TPayload>(ECommunicationChannel channel, TPayload payloadObj, PriorityType priority, CancellationToken ct) que faça JsonSerializer.Serialize(payloadObj), chame OutboxMessage.Create(channel, serializedPayload, MapPriority(priority)), invoque outboxRepository.AddAsync(...) e outboxRepository.SaveChangesAsync(...), e retorne Result<Guid>.Substitua os três blocos de serialização/criação/AddAsync/SaveChangesAsync nas operações de envio para delegarem a esse helper, mantendo MapPriority, OutboxMessage.Create, outboxRepository.AddAsync e SaveChangesAsync como os pontos de integração.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Modules/Communications/Application/ModuleApi/CommunicationsModuleApi.cs`:
- Around line 37-40: Padronize as mensagens de validação para português trocando
as strings passadas para Error.BadRequest(...) na classe CommunicationsModuleApi
(os chamados onde se valida email == null, email.To, email.Subject, email.Body e
os outros Error.BadRequest apontados nas ranges mencionadas) por mensagens em
pt-BR concisas e consistentes (ex.: "Mensagem de email não pode ser nula.",
"Email do destinatário é obrigatório.", "Assunto do email é obrigatório.",
"Corpo do email é obrigatório."), aplicando a mesma padronização nos demais
Error.BadRequest encontrados nas outras verificações da mesma classe (as
ocorrências em torno das linhas 43, 75, 93 e 114).
In `@src/Modules/Communications/Application/Services/OutboxProcessorService.cs`:
- Around line 211-214: Os handlers
ProviderAwaitingVerificationIntegrationEventHandler e
ProviderVerificationStatusUpdatedIntegrationEventHandler estão serializando o
email payload com um campo Body enquanto o record EmailOutboxPayload define
HtmlBody e TextBody, causando desserialização nula; para corrigir, alinhe os
formatos: ou atualize EmailOutboxPayload (o record EmailOutboxPayload) para
aceitar um único Body opcional (por exemplo Body) e manter compatibilidade com
From/TemplateKey, ou altere os dois handlers para serializar HtmlBody/TextBody
em vez de Body; ajuste também qualquer código que desserialize/consuma o payload
(onde o payload é desserializado antes do envio de email) para usar os nomes de
campo atualizados para evitar valores nulos no envio de email.
In `@src/Modules/Communications/Domain/Entities/OutboxMessage.cs`:
- Around line 145-161: MarkAsFailed currently leaves Status as Processing when
retries remain, preventing requeue; update the method (MarkAsFailed) to set
Status = EOutboxMessageStatus.Pending whenever RetryCount is incremented and
RetryCount < MaxRetries (while still setting ErrorMessage, calling MarkAsUpdated
and only setting Status = EOutboxMessageStatus.Failed when RetryCount >=
MaxRetries), referencing the RetryCount, MaxRetries, Status and
EOutboxMessageStatus symbols so the message becomes eligible for
GetPendingAsync.
In `@src/Shared/Utilities/PiiMaskingHelper.cs`:
- Around line 45-50: MaskPhoneNumber currently concatenates a 5-char prefix and
a 4-char suffix for short inputs which can overlap and leak PII; update
MaskPhoneNumber to handle short lengths explicitly: keep the existing
null/whitespace and very-short (<=4) handling, add a branch for
phoneNumber.Length between 5 and 8 that returns a fully masked prefix with only
the last 2 characters shown, and only use the current
$"{phoneNumber[..5]}****{phoneNumber[^4..]}" form when phoneNumber.Length is
sufficiently long (e.g., >8) to avoid overlap; implement these checks inside the
MaskPhoneNumber method.
In
`@tests/MeAjudaAi.Integration.Tests/Modules/SearchProviders/SearchProvidersE2ETests.cs`:
- Around line 54-64: Add explicit HTTP status assertions before calling
ReadFromJsonAsync in the tests: in Search_ShouldBeOrderedByDistanceAscending
(and the other test around lines 83-95), assert response.IsSuccessStatusCode or
check response.StatusCode == HttpStatusCode.OK immediately after awaiting
Client.GetAsync, then proceed to ReadFromJsonAsync and the existing
FluentAssertions on result.Items; update test method names
Search_ShouldBeOrderedByDistanceAscending (and the corresponding second test) to
include this pre-deserialization status check so failures show clear HTTP error
codes.
- Around line 17-32: In SearchProvidersE2ETests, the test currently accepts any
non-null payload and omits verifying actual nearby results; restore a strict
assertion by asserting result.Items is not empty (uncomment or add
result.Items.Should().NotBeEmpty()) and additionally validate each
SearchableProviderDto in result.Items has a location within the requested radius
(use the DTO's latitude/longitude fields and a simple haversine or distance
check) after calling Client.GetAsync; reference
PagedResult<SearchableProviderDto>, result.Items and Client.GetAsync to find
where to add these assertions.
---
Duplicate comments:
In `@docs/roadmap-current.md`:
- Line 659: Resumo: padronizar terminologia PT-BR e corrigir acentuação no
roadmap. In the docs line containing "Módulo Comunicações" replace English terms
with Portuguese equivalents (e.g., change "templates" → "modelos", "Follow-ups"
→ "acompanhamentos" or "seguimentos" where context fits, and "Evento Existing" →
"Evento existente"), and fix the missing accent "facil" → "fácil"; apply the
same translations/orthography fixes to the other referenced occurrences
("templates", "Follow-ups", "Evento Existing", "facil") across the document so
terminology is consistent.
---
Nitpick comments:
In `@docs/roadmap-current.md`:
- Around line 887-912: Atualize a documentação da interface
ICommunicationsModuleApi para explicitar que todos os tipos e enums usados (por
exemplo CommunicationPriority, EmailMessageDto, SmsMessageDto, PushMessageDto,
CommunicationLogDto, CommunicationLogQuery, PagedResult) devem ser importados do
contrato canônico MeAjudaAi.Shared.Contracts (ou do namespace de contrato
compartilhado equivalente); adicione uma frase clara junto à declaração da
interface indicando "Usar tipos de MeAjudaAi.Shared.Contracts" e, quando
pertinente, troque menções locais de enums/DTOs por referências ao contrato
compartilhado para evitar drift entre backend e frontend.
In `@src/Contracts/Contracts/Modules/Communications/ICommunicationsModuleApi.cs`:
- Around line 42-56: Add an optional CommunicationPriority parameter to
SendSmsAsync and SendPushAsync to match SendEmailAsync: update the method
signatures for SendSmsAsync(SmsMessageDto sms, CommunicationPriority priority =
CommunicationPriority.Normal, CancellationToken ct = default) and
SendPushAsync(PushMessageDto push, CommunicationPriority priority =
CommunicationPriority.Normal, CancellationToken ct = default), update their XML
docs to document the new priority parameter and default, and ensure any
implementing interfaces/classes that implement these methods (and any callers)
are updated to accept/forward the new priority value; reference SendEmailAsync,
CommunicationPriority, SmsMessageDto, PushMessageDto, SendSmsAsync, and
SendPushAsync when locating the change.
- Around line 11-16: Há duplicação do enum (CommunicationPriority em Contracts e
ECommunicationPriority em Domain); mova a definição única para Shared.Contracts,
remova a outra e elimine a conversão em MapPriority. Concretamente:
cutar/remover the enum CommunicationPriority from ICommunicationsModuleApi.cs
and the enum ECommunicationPriority from the domain, add a single enum
CommunicationPriority in the Shared.Contracts project/namespace, update all
references to use Shared.Contracts.CommunicationPriority (including usages in
CommunicationsModuleApi.cs and domain code) and delete the MapPriority() method
in CommunicationsModuleApi.cs (or replace its callers to use the shared enum
directly). Certifique-se de ajustar namespaces/imports and run build to resolve
any remaining references.
In `@src/Modules/Communications/Application/ModuleApi/CommunicationsModuleApi.cs`:
- Around line 45-54: Resumo: reduzir duplicação extraindo o padrão de
enfileiramento do Outbox para um helper privado. Crie um método privado como
EnqueueOutboxAsync<TPayload>(ECommunicationChannel channel, TPayload payloadObj,
PriorityType priority, CancellationToken ct) que faça
JsonSerializer.Serialize(payloadObj), chame OutboxMessage.Create(channel,
serializedPayload, MapPriority(priority)), invoque
outboxRepository.AddAsync(...) e outboxRepository.SaveChangesAsync(...), e
retorne Result<Guid>.Substitua os três blocos de
serialização/criação/AddAsync/SaveChangesAsync nas operações de envio para
delegarem a esse helper, mantendo MapPriority, OutboxMessage.Create,
outboxRepository.AddAsync e SaveChangesAsync como os pontos de integração.
In `@src/Modules/Communications/Application/Services/OutboxProcessorService.cs`:
- Line 35: Remove the redundant re-assignment field `_logRepository =
logRepository`; instead use the injected constructor parameter `logRepository`
directly. Delete the unnecessary field declaration `_logRepository` and update
all usages of `_log_repository` (e.g., in methods referencing `_logRepository`)
to `logRepository` (the constructor parameter) so the service relies on the
injected dependency without reassigning it.
In `@src/Shared/Utilities/PiiMaskingHelper.cs`:
- Around line 54-59: The XML summary for MaskSensitiveData is incorrect: it
claims the method preserves size/prefix but the implementation always returns
"[REDACTED]" for non-empty input and "[EMPTY]" for null/whitespace. Fix by
updating the method's XML doc comment (the /// summary for MaskSensitiveData) to
accurately describe the current behavior—state that it returns "[EMPTY]" when
input is null/whitespace and otherwise returns a constant "[REDACTED]";
alternatively, if you prefer the original behavior, modify the MaskSensitiveData
implementation to preserve length/prefix as documented—choose one consistent
change so doc and implementation match.
In
`@tests/MeAjudaAi.Integration.Tests/Modules/SearchProviders/SearchProvidersE2ETests.cs`:
- Around line 17-19: Padronize os comentários neste arquivo
SearchProvidersE2ETests.cs para português: substitua os comentários inline em
inglês (ex.: o bloco de comentários na área de "Arrange: Seed providers at
specific coordinates" nas proximidades de SearchProvidersE2ETests, e os
comentários nas posições referenciadas por 46 e 69) por equivalentes em
português mantendo o mesmo sentido; preserve mensagens de log em inglês conforme
a orientação e verifique também comentários XML/documentação (se houver) no
mesmo arquivo para tradução consistente.
- Around line 62-63: Before asserting ordering on result!.Items by DistanceInKm,
ensure DistanceInKm is populated for every SearchableProviderDto: verify no
nulls (e.g., assert Items.All(x => x.DistanceInKm.HasValue)) or fail the test if
any DistanceInKm is null so the sort assertion on
result!.Items.Should().BeInAscendingOrder(x => x.DistanceInKm) is meaningful;
reference the PagedResult<SearchableProviderDto> result and the Items collection
and add a precondition check for DistanceInKm to guarantee the test detects
regressions in distance calculation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3b5319b9-fc8c-4e2f-942a-650e77d3bfff
⛔ Files ignored due to path filters (3)
src/Modules/Communications/Infrastructure/Persistence/Migrations/20260409004631_AddCorrelationIdToOutbox.Designer.csis excluded by!**/Migrations/**,!**/*.Designer.cssrc/Modules/Communications/Infrastructure/Persistence/Migrations/20260409004631_AddCorrelationIdToOutbox.csis excluded by!**/Migrations/**src/Modules/Communications/Infrastructure/Persistence/Migrations/CommunicationsDbContextModelSnapshot.csis excluded by!**/Migrations/**
📒 Files selected for processing (31)
docs/roadmap-current.mddocs/technical-debt.mdsrc/Contracts/Contracts/Modules/Communications/ICommunicationsModuleApi.cssrc/Modules/Communications/Application/Handlers/ProviderActivatedIntegrationEventHandler.cssrc/Modules/Communications/Application/Handlers/ProviderAwaitingVerificationIntegrationEventHandler.cssrc/Modules/Communications/Application/Handlers/ProviderVerificationStatusUpdatedIntegrationEventHandler.cssrc/Modules/Communications/Application/Handlers/UserRegisteredIntegrationEventHandler.cssrc/Modules/Communications/Application/ModuleApi/CommunicationsModuleApi.cssrc/Modules/Communications/Application/Services/Email/StubEmailService.cssrc/Modules/Communications/Application/Services/OutboxProcessorService.cssrc/Modules/Communications/Domain/Entities/CommunicationLog.cssrc/Modules/Communications/Domain/Entities/EmailTemplate.cssrc/Modules/Communications/Domain/Entities/OutboxMessage.cssrc/Modules/Communications/Domain/Enums/EOutboxMessageStatus.cssrc/Modules/Communications/Domain/Repositories/IEmailTemplateRepository.cssrc/Modules/Communications/Domain/Repositories/IOutboxMessageRepository.cssrc/Modules/Communications/Infrastructure/Persistence/CommunicationsDbContext.cssrc/Modules/Communications/Infrastructure/Persistence/CommunicationsDbContextFactory.cssrc/Modules/Communications/Infrastructure/Persistence/Configurations/EmailTemplateConfiguration.cssrc/Modules/Communications/Infrastructure/Persistence/Configurations/OutboxMessageConfiguration.cssrc/Modules/Communications/Infrastructure/Persistence/Repositories/CommunicationLogRepository.cssrc/Modules/Communications/Infrastructure/Persistence/Repositories/EmailTemplateRepository.cssrc/Modules/Communications/Infrastructure/Persistence/Repositories/OutboxMessageRepository.cssrc/Modules/Communications/Infrastructure/Services/CommunicationsOutboxWorker.cssrc/Modules/Communications/Infrastructure/Services/EmailSenderStub.cssrc/Modules/Communications/Infrastructure/Services/PushSenderStub.cssrc/Modules/Communications/Infrastructure/Services/SmsSenderStub.cssrc/Shared/Utilities/PiiMaskingHelper.cstests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cstests/MeAjudaAi.Integration.Tests/Modules/Communications/CommunicationsModuleApiTests.cstests/MeAjudaAi.Integration.Tests/Modules/SearchProviders/SearchProvidersE2ETests.cs
✅ Files skipped from review due to trivial changes (9)
- src/Modules/Communications/Domain/Enums/EOutboxMessageStatus.cs
- src/Modules/Communications/Infrastructure/Services/PushSenderStub.cs
- src/Modules/Communications/Application/Services/Email/StubEmailService.cs
- docs/technical-debt.md
- src/Modules/Communications/Infrastructure/Persistence/Configurations/OutboxMessageConfiguration.cs
- src/Modules/Communications/Domain/Repositories/IEmailTemplateRepository.cs
- src/Modules/Communications/Infrastructure/Persistence/Configurations/EmailTemplateConfiguration.cs
- src/Modules/Communications/Infrastructure/Services/EmailSenderStub.cs
- src/Modules/Communications/Domain/Entities/CommunicationLog.cs
🚧 Files skipped from review as they are similar to previous changes (10)
- src/Modules/Communications/Infrastructure/Persistence/CommunicationsDbContextFactory.cs
- src/Modules/Communications/Application/Handlers/ProviderAwaitingVerificationIntegrationEventHandler.cs
- tests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cs
- src/Modules/Communications/Application/Handlers/ProviderActivatedIntegrationEventHandler.cs
- src/Modules/Communications/Application/Handlers/ProviderVerificationStatusUpdatedIntegrationEventHandler.cs
- tests/MeAjudaAi.Integration.Tests/Modules/Communications/CommunicationsModuleApiTests.cs
- src/Modules/Communications/Domain/Repositories/IOutboxMessageRepository.cs
- src/Modules/Communications/Infrastructure/Services/CommunicationsOutboxWorker.cs
- src/Modules/Communications/Infrastructure/Persistence/Repositories/CommunicationLogRepository.cs
- src/Modules/Communications/Infrastructure/Persistence/Repositories/EmailTemplateRepository.cs
added 2 commits
April 8, 2026 22:31
…view findings - stuck message recovery
…ly review findings
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
docs/roadmap-current.md (1)
777-777:⚠️ Potential issue | 🟡 MinorAtualize o item de observabilidade para a stack atual.
Na Line 777, o texto ainda aponta para “Blazor WASM”, mas o próprio roadmap já consolidou frontend em React + Next.js. Isso pode desalinhar backlog e execução.
✏️ Ajuste sugerido
-- [ ] **Registro do Frontend**: Integração com Application Insights (Blazor WASM) +- [ ] **Registro do Frontend**: Integração com Application Insights (React + Next.js)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/roadmap-current.md` at line 777, Atualize o item de observabilidade que ainda menciona “Blazor WASM”: localize a linha contendo the text "Registro do Frontend: Integração com Application Insights (Blazor WASM)" e altere para refletir a stack atual, por exemplo "Registro do Frontend: Integração com Application Insights (React + Next.js)", garantindo consistência do roadmap com o frontend consolidado.
🧹 Nitpick comments (2)
src/Modules/Communications/Domain/Entities/OutboxMessage.cs (1)
146-168: Chamada duplicada aMarkAsUpdated().O método
MarkAsUpdated()é chamado duas vezes: na linha 155 (antes da verificação de retries) e na linha 167 (após a atribuição de status). Isso é redundante e pode ser simplificado.♻️ Sugestão de refatoração
public void MarkAsFailed(string errorMessage) { if (Status is EOutboxMessageStatus.Sent or EOutboxMessageStatus.Failed) return; ArgumentException.ThrowIfNullOrWhiteSpace(errorMessage); RetryCount++; ErrorMessage = errorMessage; - MarkAsUpdated(); if (RetryCount >= MaxRetries) { Status = EOutboxMessageStatus.Failed; } else { // Reseta para Pending para permitir que seja capturada novamente pelo polling Status = EOutboxMessageStatus.Pending; } - + MarkAsUpdated(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Modules/Communications/Domain/Entities/OutboxMessage.cs` around lines 146 - 168, O método MarkAsFailed tem uma chamada duplicada a MarkAsUpdated(); remova a chamada anterior à verificação de RetryCount (a que ocorre logo após atribuir ErrorMessage e incrementar RetryCount) e deixe apenas uma chamada única a MarkAsUpdated() depois de definir o Status (uso das propriedades RetryCount, MaxRetries, ErrorMessage e Status dentro de MarkAsFailed), garantindo que ArgumentException.ThrowIfNullOrWhiteSpace(errorMessage) permaneça antes do incremento de RetryCount.docs/roadmap-current.md (1)
1302-1340: Exemplo técnico está acoplado a Blazor em uma trilha React.As Lines 1317+ usam sintaxe/componentes de Blazor (
@inject,MudTimeline). Recomendo trocar por pseudocódigo agnóstico ou exemplo React para evitar drift arquitetural no roadmap.♻️ Ajuste sugerido (pseudocódigo agnóstico)
-// Componente Frontend -@inject HubConnection HubConnection - -<MudTimeline> - `@foreach` (var activity in RecentActivities) - { - <MudTimelineItem Color="@GetActivityColor(activity.Type)"> - <MudText>@activity.Description</MudText> - <MudText Typo="Typo.caption">@activity.Timestamp.ToRelativeTime()</MudText> - </MudTimelineItem> - } -</MudTimeline> - -@code { - protected override async Task OnInitializedAsync() - { - HubConnection.On<ActivityDto>("ReceiveActivity", activity => - { - RecentActivities.Insert(0, activity); - StateHasChanged(); - }); - await HubConnection.StartAsync(); - } -} +// Componente Frontend (React - exemplo conceitual) +// useEffect(() => subscribeToActivityFeed(...), []) +// renderTimeline(activities)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/roadmap-current.md` around lines 1302 - 1340, The example in the docs mixes Blazor-specific syntax/components (e.g., `@inject` HubConnection, MudTimeline, MudTimelineItem, OnInitializedAsync) while the roadmap track is React; replace the Blazor fragment with either framework-agnostic pseudocode or a small React example using the same domain events and SignalR hub (ActivityHub and its BroadcastActivity method) so readers in the React track see a consistent example; update the client snippet to show subscribing to "ReceiveActivity" on a HubConnection (or equivalent pseudocode) and updating a RecentActivities array/state so the mapping between ProviderRegisteredEvent/DocumentUploadedEvent/VerificationStatusChangedEvent → ActivityDto is clear without Blazor-specific markup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/Modules/Communications/Application/Handlers/ProviderVerificationStatusUpdatedIntegrationEventHandler.cs`:
- Around line 60-61: Replace the plain recipientEmail in the LogInformation call
inside ProviderVerificationStatusUpdatedIntegrationEventHandler with a masked
value: add "using MeAjudaAi.Shared.Utilities;", compute a maskedEmail (e.g., var
maskedEmail = EmailMasker.Mask(recipientEmail) or the repository's email-masking
helper) and pass maskedEmail to logger.LogInformation instead of recipientEmail;
keep the rest of the template (integrationEvent.UserId, correlationId) and
ensure the log message text remains in English.
In `@tests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cs`:
- Around line 187-191: The TestModule flag handling for SearchProviders misses
implicitly adding Locations, causing LocationsDbContext (used in the seeding at
the block that opens LocationsDbContext around the seeding lines) to run without
migrations; update the conditional that checks
modules.HasFlag(TestModule.SearchProviders) to also set modules |=
TestModule.Locations if not already set (in the same style as Providers and
ServiceCatalogs) so SearchProviders brings Locations as a dependency.
- Around line 276-282: The retry loop in ApplyMigrationForContextAsync swallows
failures when all 3 attempts fail; modify the method so that after the for loop
completes it throws an exception (preserving the last caught exception or
wrapping it) to surface migration failure; specifically update
ApplyMigrationForContextAsync to capture the last Npgsql.PostgresException from
the catch around context.Database.MigrateAsync and, if retries are exhausted,
rethrow or throw a new informative exception that includes the original
exception details so test startup fails instead of silently continuing.
---
Duplicate comments:
In `@docs/roadmap-current.md`:
- Line 777: Atualize o item de observabilidade que ainda menciona “Blazor WASM”:
localize a linha contendo the text "Registro do Frontend: Integração com
Application Insights (Blazor WASM)" e altere para refletir a stack atual, por
exemplo "Registro do Frontend: Integração com Application Insights (React +
Next.js)", garantindo consistência do roadmap com o frontend consolidado.
---
Nitpick comments:
In `@docs/roadmap-current.md`:
- Around line 1302-1340: The example in the docs mixes Blazor-specific
syntax/components (e.g., `@inject` HubConnection, MudTimeline, MudTimelineItem,
OnInitializedAsync) while the roadmap track is React; replace the Blazor
fragment with either framework-agnostic pseudocode or a small React example
using the same domain events and SignalR hub (ActivityHub and its
BroadcastActivity method) so readers in the React track see a consistent
example; update the client snippet to show subscribing to "ReceiveActivity" on a
HubConnection (or equivalent pseudocode) and updating a RecentActivities
array/state so the mapping between
ProviderRegisteredEvent/DocumentUploadedEvent/VerificationStatusChangedEvent →
ActivityDto is clear without Blazor-specific markup.
In `@src/Modules/Communications/Domain/Entities/OutboxMessage.cs`:
- Around line 146-168: O método MarkAsFailed tem uma chamada duplicada a
MarkAsUpdated(); remova a chamada anterior à verificação de RetryCount (a que
ocorre logo após atribuir ErrorMessage e incrementar RetryCount) e deixe apenas
uma chamada única a MarkAsUpdated() depois de definir o Status (uso das
propriedades RetryCount, MaxRetries, ErrorMessage e Status dentro de
MarkAsFailed), garantindo que
ArgumentException.ThrowIfNullOrWhiteSpace(errorMessage) permaneça antes do
incremento de RetryCount.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d819f846-a1cb-4e6f-853b-81eafedf38fb
📒 Files selected for processing (14)
docs/roadmap-current.mdsrc/Contracts/Contracts/Modules/Communications/ICommunicationsModuleApi.cssrc/Contracts/Shared/ECommunicationPriority.cssrc/Modules/Communications/Application/Handlers/ProviderActivatedIntegrationEventHandler.cssrc/Modules/Communications/Application/Handlers/ProviderAwaitingVerificationIntegrationEventHandler.cssrc/Modules/Communications/Application/Handlers/ProviderVerificationStatusUpdatedIntegrationEventHandler.cssrc/Modules/Communications/Application/Handlers/UserRegisteredIntegrationEventHandler.cssrc/Modules/Communications/Application/ModuleApi/CommunicationsModuleApi.cssrc/Modules/Communications/Application/Services/OutboxProcessorService.cssrc/Modules/Communications/Domain/Entities/OutboxMessage.cssrc/Shared/Utilities/PiiMaskingHelper.cstests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cstests/MeAjudaAi.Integration.Tests/Modules/Communications/OutboxPatternTests.cstests/MeAjudaAi.Integration.Tests/Modules/SearchProviders/SearchProvidersE2ETests.cs
✅ Files skipped from review due to trivial changes (2)
- src/Contracts/Shared/ECommunicationPriority.cs
- tests/MeAjudaAi.Integration.Tests/Modules/Communications/OutboxPatternTests.cs
🚧 Files skipped from review as they are similar to previous changes (4)
- src/Modules/Communications/Application/Handlers/ProviderActivatedIntegrationEventHandler.cs
- src/Modules/Communications/Application/Handlers/UserRegisteredIntegrationEventHandler.cs
- tests/MeAjudaAi.Integration.Tests/Modules/SearchProviders/SearchProvidersE2ETests.cs
- src/Modules/Communications/Application/ModuleApi/CommunicationsModuleApi.cs
… serialization and improve test stability
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/Modules/SearchProviders/Domain/Enums/ESubscriptionTier.cs (1)
9-9: RemovaJsonStringEnumConverterdo enum de DomínioA linha 9 acopla desnecessariamente a camada Domain ao
System.Text.Json. O Domain deve manter a política de serialização separada — apenas o Contract (src/Contracts/Contracts/Modules/SearchProviders/Enums/ESubscriptionTier.cs) serializa como número, e a conversão entre os dois tipos ocorre explicitamente emSearchProvidersModuleApi.cs(linhas 250–269).Remova o atributo desnecessário
- [JsonConverter(typeof(JsonStringEnumConverter))] public enum ESubscriptionTierIsto alinha-se com a prática de reutilizar
Shared.Contractspara enums/constantes, mantendo a Web sincronizada com o código do backend.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Modules/SearchProviders/Domain/Enums/ESubscriptionTier.cs` at line 9, Remove the System.Text.Json attribute from the domain enum: delete the [JsonConverter(typeof(JsonStringEnumConverter))] annotation on ESubscriptionTier in the Domain enum file so the Domain layer is not coupled to System.Text.Json; keep serialization policy in the Contracts enum (src/Contracts/.../ESubscriptionTier.cs) and rely on the existing explicit mapping in SearchProvidersModuleApi (methods around SearchProvidersModuleApi.cs lines ~250–269) to convert between the Contract and Domain enum types.tests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cs (1)
277-301: O tratamento delastExceptionresolve a questão da revisão anterior. Considere restringir retentativas a erros transientes.A correção que captura e propaga a exceção após esgotar as tentativas está correta. No entanto, o bloco
catch (Exception ex)nas linhas 293-297 faz retry para qualquer exceção, incluindo erros não-transientes como conflitos de schema ou migrações inválidas, que não serão resolvidos com retentativas.♻️ Sugestão: limitar retries a exceções transientes
catch (Npgsql.PostgresException ex) when (ex.SqlState == "57P01") { lastException = ex; await Task.Delay(1000 * attempt); } - catch (Exception ex) + catch (Npgsql.NpgsqlException ex) when (ex.IsTransient) { lastException = ex; - logger?.LogError(ex, "❌ Failed to apply {Module} migrations on attempt {Attempt}", moduleName, attempt); + logger?.LogWarning(ex, "⚠️ Transient error applying {Module} migrations on attempt {Attempt}, retrying...", moduleName, attempt); + await Task.Delay(1000 * attempt); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cs` around lines 277 - 301, The retry loop in ApplyMigrationForContextAsync currently retries on any Exception; change it so only transient errors trigger retries: keep the existing catch (Npgsql.PostgresException ex) when (ex.SqlState == "57P01") for that transient case, add explicit transient checks (e.g., Postgres transient SqlState codes like "57P01","53300","08006" or host/timeout-related exceptions, and transient exception types such as TimeoutException or transient DbUpdateException) and only await Task.Delay/continue the loop for those; for all other exceptions (caught by catch (Exception ex)) immediately log via logger?.LogError(ex, ...) and rethrow (or set lastException and break) so non-transient errors do not cause retries, preserving the final throw of InvalidOperationException with lastException if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/roadmap-current.md`:
- Around line 649-653: The Sprint 9 header "### ⏳ Sprint 9 - BUFFER & Mitigação
de Risco (25 Mar - 11 Mai 2026)" shows a date range that does not match the
shown "Duração: 12 dias de buffer"; update the document so the two are
consistent by either changing the "Duração" value to the actual number of days
covered by 25 Mar–11 Mai 2026 (replace "Duração: 12 dias de buffer" with the
correct total days) or by adjusting the parenthetical date range in the "### ⏳
Sprint 9 - BUFFER & Mitigação de Risco (...)" header to a 12-day interval;
ensure you edit the exact strings "### ⏳ Sprint 9 - BUFFER & Mitigação de Risco
(25 Mar - 11 Mai 2026)" and "Duração: 12 dias de buffer" so they match.
- Around line 867-871: The docs claim IsSystemTemplate on the EmailTemplate
entity prevents deletion but the domain class EmailTemplate only defines the
flag without enforcement; either update the roadmap text to mark this as a
"planned" rule or implement enforcement in the application layer by checking
EmailTemplate.IsSystemTemplate before deleting—e.g., add a guard in
EmailTemplateService.DeleteTemplateAsync (or in the repository method
DeleteAsync) to throw a DomainException/ValidationException or return a 403 when
IsSystemTemplate==true, and update the docs to reference the enforcement
location (EmailTemplate class + EmailTemplateService.DeleteTemplateAsync /
IEmailTemplateRepository.DeleteAsync) so behavior and documentation stay in
sync.
In
`@src/Modules/Communications/Application/Handlers/ProviderVerificationStatusUpdatedIntegrationEventHandler.cs`:
- Around line 64-69: A detecção de duplicidade por texto na exceção dentro de
ProviderVerificationStatusUpdatedIntegrationEventHandler é frágil; replace o
filtro "duplicate key" usando o utilitário PostgreSqlExceptionProcessor que
identifica violações pelo SQLSTATE 23505. Altere o bloco try/catch para capturar
Exception ex (sem filtrar por mensagem) e dentro do catch chamar
PostgreSqlExceptionProcessor.IsUniqueViolation(ex) (ou o método equivalente
existente) — se retornar true, executar o mesmo logger.LogInformation(...)
existente para pular o processamento; caso contrário, rethrow ou tratar a
exceção conforme o padrão do projeto. Certifique-se de usar a mesma assinatura
de logger e de importar/receber PostgreSqlExceptionProcessor conforme usado em
RegisterProviderCommandHandler e GlobalExceptionHandler.
- Around line 36-40: integrationEvent.NewStatus is coming from an enum (e.g.,
"Verified", "Rejected", "Pending") so the current check NewStatus.ToLower() ==
"approved" is incorrect and causes every non-"approved" value to be treated as
rejection; update the logic in the handler to normalize NewStatus using
ToLowerInvariant() (and Trim()) and explicitly map the expected enum string
values ("verified" -> use templateKey "provider-verification-approved",
"rejected" -> "provider-verification-rejected", "pending" -> e.g.
"provider-verification-pending" or a safe default) instead of a simple equality
to "approved", and use the same normalizedStatus value when building
correlationId (e.g.,
$"verification_status_update:{integrationEvent.ProviderId}:{normalizedStatus}")
so correlationId and template selection remain consistent.
In `@tests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cs`:
- Line 214: O bloco try/catch que envolve
context.Database.ExecuteSqlRawAsync("CREATE EXTENSION IF NOT EXISTS postgis;")
está com um catch vazio que oculta falhas; altere-o para capturar a exceção
(catch (Exception ex)) e registrar ou propagar o erro para diagnóstico — por
exemplo, usar o logger de teste ou Console/Error (ou rethrow como throw;) dentro
do catch para incluir ex.Message/stacktrace, garantindo que falhas na criação da
extensão PostGIS sejam visíveis.
---
Nitpick comments:
In `@src/Modules/SearchProviders/Domain/Enums/ESubscriptionTier.cs`:
- Line 9: Remove the System.Text.Json attribute from the domain enum: delete the
[JsonConverter(typeof(JsonStringEnumConverter))] annotation on ESubscriptionTier
in the Domain enum file so the Domain layer is not coupled to System.Text.Json;
keep serialization policy in the Contracts enum
(src/Contracts/.../ESubscriptionTier.cs) and rely on the existing explicit
mapping in SearchProvidersModuleApi (methods around SearchProvidersModuleApi.cs
lines ~250–269) to convert between the Contract and Domain enum types.
In `@tests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cs`:
- Around line 277-301: The retry loop in ApplyMigrationForContextAsync currently
retries on any Exception; change it so only transient errors trigger retries:
keep the existing catch (Npgsql.PostgresException ex) when (ex.SqlState ==
"57P01") for that transient case, add explicit transient checks (e.g., Postgres
transient SqlState codes like "57P01","53300","08006" or host/timeout-related
exceptions, and transient exception types such as TimeoutException or transient
DbUpdateException) and only await Task.Delay/continue the loop for those; for
all other exceptions (caught by catch (Exception ex)) immediately log via
logger?.LogError(ex, ...) and rethrow (or set lastException and break) so
non-transient errors do not cause retries, preserving the final throw of
InvalidOperationException with lastException if needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f9d21847-de3a-4a5e-ba13-62b9521bc5d3
📒 Files selected for processing (7)
docs/roadmap-current.mdsrc/Contracts/Shared/ECommunicationPriority.cssrc/Modules/Communications/Application/Handlers/ProviderVerificationStatusUpdatedIntegrationEventHandler.cssrc/Modules/Communications/Domain/Entities/OutboxMessage.cssrc/Modules/SearchProviders/Domain/Enums/ESubscriptionTier.cstests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cstests/MeAjudaAi.Integration.Tests/Modules/SearchProviders/SearchProvidersE2ETests.cs
🚧 Files skipped from review as they are similar to previous changes (3)
- src/Contracts/Shared/ECommunicationPriority.cs
- tests/MeAjudaAi.Integration.Tests/Modules/SearchProviders/SearchProvidersE2ETests.cs
- src/Modules/Communications/Domain/Entities/OutboxMessage.cs
…y headers and fix middleware tests
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/Bootstrapper/MeAjudaAi.ApiService/Middlewares/SecurityHeadersMiddleware.cs (1)
18-19: Validação de argumentos deve estar no construtor, não em cada requisição.
nexteenvironmentsão parâmetros do construtor e devem ser validados uma única vez na construção, não em cada invocação do middleware. Validar a cada requisição adiciona overhead desnecessário.♻️ Mover validação para o construtor
public sealed class SecurityHeadersMiddleware( RequestDelegate next, IWebHostEnvironment environment, ILogger<SecurityHeadersMiddleware>? logger = null) { + private readonly RequestDelegate _next = next ?? throw new ArgumentNullException(nameof(next)); + private readonly IWebHostEnvironment _environment = environment ?? throw new ArgumentNullException(nameof(environment)); + public async Task InvokeAsync(HttpContext context) { - ArgumentNullException.ThrowIfNull(next); - ArgumentNullException.ThrowIfNull(environment); - logger?.LogTrace("Adding security headers to response."); // ... resto do código - await next(context); + await _next(context); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Bootstrapper/MeAjudaAi.ApiService/Middlewares/SecurityHeadersMiddleware.cs` around lines 18 - 19, Move the null-checks out of the per-request path: remove ArgumentNullException.ThrowIfNull(next) and ArgumentNullException.ThrowIfNull(environment) from the middleware invocation method (Invoke or InvokeAsync) and add them once in the SecurityHeadersMiddleware constructor (validate constructor parameters next and environment there). Keep the constructor parameter names next and environment and ensure the Invoke/InvokeAsync method no longer performs those checks to avoid per-request overhead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/Bootstrapper/MeAjudaAi.ApiService/Middlewares/SecurityHeadersMiddleware.cs`:
- Around line 22-29: The SecurityHeadersMiddleware currently appends
X-Frame-Options, X-Content-Type-Options and Referrer-Policy which are also added
by EnvironmentSpecificExtensions.UseProductionMiddlewares, causing duplicate
headers and inconsistent Referrer-Policy values; update
SecurityHeadersMiddleware to check for existing headers before adding (e.g., use
TryAdd-like behavior or if (!context.Response.Headers.ContainsKey(...)) ), align
the Referrer-Policy value with the production middleware (or centralize the
chosen policy) and consider moving all security header setup into a single
helper/extension used by both SecurityHeadersMiddleware and
UseProductionMiddlewares to avoid future divergence.
- Around line 36-44: The hardcoded CSP in SecurityHeadersMiddleware (the call to
context.Response.Headers.Append that sets Content-Security-Policy with
"https://*.Betenbough.com") is incorrect and duplicates the dynamic
ContentSecurityPolicyMiddleware; remove the static CSP header from
SecurityHeadersMiddleware so the existing ContentSecurityPolicyMiddleware is the
single source of truth, or if you must keep it, externalize the connect-src
value into configuration and read it (rather than the hardcoded Betenbough
domain) before appending the header; update SecurityHeadersMiddleware to stop
emitting conflicting CSP headers and reference the dynamic config used by
ContentSecurityPolicyMiddleware.
---
Nitpick comments:
In
`@src/Bootstrapper/MeAjudaAi.ApiService/Middlewares/SecurityHeadersMiddleware.cs`:
- Around line 18-19: Move the null-checks out of the per-request path: remove
ArgumentNullException.ThrowIfNull(next) and
ArgumentNullException.ThrowIfNull(environment) from the middleware invocation
method (Invoke or InvokeAsync) and add them once in the
SecurityHeadersMiddleware constructor (validate constructor parameters next and
environment there). Keep the constructor parameter names next and environment
and ensure the Invoke/InvokeAsync method no longer performs those checks to
avoid per-request overhead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 47bbd978-dfa2-4995-b7af-7a0315dea89f
📒 Files selected for processing (3)
src/Bootstrapper/MeAjudaAi.ApiService/Extensions/SecurityExtensions.cssrc/Bootstrapper/MeAjudaAi.ApiService/Middlewares/SecurityHeadersMiddleware.cssrc/Bootstrapper/MeAjudaAi.ApiService/Program.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Bootstrapper/MeAjudaAi.ApiService/Program.cs
…domain decoupling, robust idempotency and test resilience
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
docs/roadmap-current.md (3)
874-883:⚠️ Potential issue | 🟡 MinorFormatação Markdown: Adicionar linhas em branco ao redor da tabela.
A tabela de mapeamento de eventos (linhas 876-882) viola a regra MD058 do markdownlint, que exige uma linha em branco antes e depois de cada tabela.
🔧 Correção sugerida
**Mapeamento de Integração com Eventos**: + | Evento existente | Ação de Comunicação | |----------------|-------------------| | `UserRegisteredIntegrationEvent` | ✅ Enviar e-mail de boas-vindas | | `ProviderAwaitingVerificationIntegrationEvent` | ✅ Notificar administrador | | `ProviderVerificationStatusUpdatedIntegrationEvent` | ✅ Notificar prestador | | `DocumentVerifiedIntegrationEvent` | → Notificar prestador | | `DocumentRejectedIntegrationEvent` | → Notificar prestador | + ---🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/roadmap-current.md` around lines 874 - 883, The Markdown table under the heading "**Mapeamento de Integração com Eventos**" currently lacks blank lines before and after it (contains rows with events like `UserRegisteredIntegrationEvent`, `ProviderAwaitingVerificationIntegrationEvent`, etc.); update the document so there is an empty line immediately before the table header line and an empty line immediately after the final table row to satisfy MD058 (ensure the blank line placement surrounds the entire table block).
976-977:⚠️ Potential issue | 🟡 MinorBloco de código sem especificação de linguagem (MD040).
O bloco de código na linha 976 não possui um identificador de linguagem, violando a regra MD040 do markdownlint.
🔧 Correção sugerida
- ``` + ```text Event Occurred → Save to Outbox Table (Mesma Transação) → Background Worker processa Outbox ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/roadmap-current.md` around lines 976 - 977, The fenced code block containing "Evento Ocorrido → Salvar na Tabela de Outbox (Mesma Transação) → Trabalhador de Background processa Outbox" lacks a language identifier (MD040); update that block by adding a language tag such as text (i.e., change ``` to ```text) so markdownlint recognizes it and the block renders correctly, locate the fenced block by its content and modify the opening fence only.
659-659:⚠️ Potential issue | 🟡 MinorInconsistência parcialmente resolvida: Exceção de escopo precisa ser mais explícita.
A linha 659 lista a implementação do Módulo de Comunicações como tarefa da Sprint 9, que é descrita como "BUFFER" sem novas funcionalidades. Embora a linha 668 declare explicitamente uma exceção para este módulo, seria mais claro reforçar essa exceção diretamente na lista de tarefas para evitar ambiguidade no planejamento.
📝 Sugestão de clarificação
-- [ ] **Módulo de Comunicações**: Implementar infraestrutura base (outbox pattern, modelos, handlers de evento) +- [ ] **Módulo de Comunicações** *(Exceção planejada à regra de buffer)*: Implementar infraestrutura base (outbox pattern, modelos, handlers de evento)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/roadmap-current.md` at line 659, Atualize a checklist do item "Módulo de Comunicações" para incluir explicitamente a exceção inline (ex.: "exceção: BUFFER — sem implementação nesta sprint") em vez de depender apenas da declaração posterior; localize a linha contendo "**Módulo de Comunicações**: Implementar infraestrutura base (outbox pattern, modelos, handlers de evento)" e altere o texto para deixar claro que o trabalho está adiado/é uma exceção nesta sprint, correspondendo à declaração de exceção já presente mais adiante no documento.
🧹 Nitpick comments (1)
src/Modules/Communications/Infrastructure/Persistence/Repositories/EmailTemplateRepository.cs (1)
51-54: Mensagem de exceção deve estar em português conforme política de linguagem.A mensagem de exceção na linha 53 está em inglês. Se essa exceção propagar até a camada de API sem tratamento, o usuário final receberá uma mensagem em inglês. Conforme a política do repositório, mensagens voltadas ao usuário devem estar em português.
♻️ Proposta de tradução
if (template.IsSystemTemplate) { - throw new InvalidOperationException($"Cannot delete system template with ID {id}."); + throw new InvalidOperationException($"Não é possível excluir o template de sistema com ID {id}."); }Based on learnings: "User-facing validation messages and API responses shown to the frontend must be in Portuguese."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Modules/Communications/Infrastructure/Persistence/Repositories/EmailTemplateRepository.cs` around lines 51 - 54, The exception message thrown in the EmailTemplateRepository (throw new InvalidOperationException($"Cannot delete system template with ID {id}.")) is in English; replace it with a Portuguese user-facing message (for example: $"Não é possível excluir o template do sistema com ID {id}.") and ensure the change is applied to the InvalidOperationException thrown when template.IsSystemTemplate is true; also scan for any other user-facing exception messages in the same class (e.g., other InvalidOperationException uses) and localize them to Portuguese as well.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/roadmap-current.md`:
- Line 870: Docs claim IsSystemTemplate protects templates from deletion but the
domain model only defines the property without enforcement; either update the
docs to mark this as a planned rule or implement the check. To implement, add a
validation in the deletion flow (e.g., EmailTemplateRepository.DeleteAsync or
EmailTemplateService.DeleteAsync) to fetch the EmailTemplate by id, return a
not-found result if null, and return a forbidden/failure result when
template.IsSystemTemplate is true before performing the delete; alternatively,
change the roadmap text around IsSystemTemplate to state it is planned and not
yet enforced.
In
`@src/Modules/Communications/Infrastructure/Persistence/Repositories/EmailTemplateRepository.cs`:
- Around line 9-12: O método AddAsync está apenas chamando
context.EmailTemplates.AddAsync(template, cancellationToken) sem persistir
(SaveChangesAsync), enquanto DeleteAsync usa SaveChangesAsync; para tornar o
padrão consistente, faça AddAsync também chamar await
context.SaveChangesAsync(cancellationToken) após AddAsync (ou, alternativamente,
documente e aplique um padrão de UnitOfWork em todos os repositórios),
garantindo que o método AddAsync(EmailTemplate template, CancellationToken
cancellationToken) persista como DeleteAsync faz.
---
Duplicate comments:
In `@docs/roadmap-current.md`:
- Around line 874-883: The Markdown table under the heading "**Mapeamento de
Integração com Eventos**" currently lacks blank lines before and after it
(contains rows with events like `UserRegisteredIntegrationEvent`,
`ProviderAwaitingVerificationIntegrationEvent`, etc.); update the document so
there is an empty line immediately before the table header line and an empty
line immediately after the final table row to satisfy MD058 (ensure the blank
line placement surrounds the entire table block).
- Around line 976-977: The fenced code block containing "Evento Ocorrido →
Salvar na Tabela de Outbox (Mesma Transação) → Trabalhador de Background
processa Outbox" lacks a language identifier (MD040); update that block by
adding a language tag such as text (i.e., change ``` to ```text) so markdownlint
recognizes it and the block renders correctly, locate the fenced block by its
content and modify the opening fence only.
- Line 659: Atualize a checklist do item "Módulo de Comunicações" para incluir
explicitamente a exceção inline (ex.: "exceção: BUFFER — sem implementação nesta
sprint") em vez de depender apenas da declaração posterior; localize a linha
contendo "**Módulo de Comunicações**: Implementar infraestrutura base (outbox
pattern, modelos, handlers de evento)" e altere o texto para deixar claro que o
trabalho está adiado/é uma exceção nesta sprint, correspondendo à declaração de
exceção já presente mais adiante no documento.
---
Nitpick comments:
In
`@src/Modules/Communications/Infrastructure/Persistence/Repositories/EmailTemplateRepository.cs`:
- Around line 51-54: The exception message thrown in the EmailTemplateRepository
(throw new InvalidOperationException($"Cannot delete system template with ID
{id}.")) is in English; replace it with a Portuguese user-facing message (for
example: $"Não é possível excluir o template do sistema com ID {id}.") and
ensure the change is applied to the InvalidOperationException thrown when
template.IsSystemTemplate is true; also scan for any other user-facing exception
messages in the same class (e.g., other InvalidOperationException uses) and
localize them to Portuguese as well.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9875a98d-8ed6-44d2-be1b-1cb6717e7581
📒 Files selected for processing (8)
docs/roadmap-current.mdsrc/Modules/Communications/Application/Handlers/ProviderActivatedIntegrationEventHandler.cssrc/Modules/Communications/Application/Handlers/ProviderAwaitingVerificationIntegrationEventHandler.cssrc/Modules/Communications/Application/Handlers/ProviderVerificationStatusUpdatedIntegrationEventHandler.cssrc/Modules/Communications/Application/Handlers/UserRegisteredIntegrationEventHandler.cssrc/Modules/Communications/Domain/Repositories/IEmailTemplateRepository.cssrc/Modules/Communications/Infrastructure/Persistence/Repositories/EmailTemplateRepository.cstests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cs
✅ Files skipped from review due to trivial changes (2)
- src/Modules/Communications/Application/Handlers/ProviderAwaitingVerificationIntegrationEventHandler.cs
- tests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cs
🚧 Files skipped from review as they are similar to previous changes (3)
- src/Modules/Communications/Application/Handlers/ProviderActivatedIntegrationEventHandler.cs
- src/Modules/Communications/Application/Handlers/ProviderVerificationStatusUpdatedIntegrationEventHandler.cs
- src/Modules/Communications/Application/Handlers/UserRegisteredIntegrationEventHandler.cs
added 2 commits
April 9, 2026 11:27
…ix enum serialization and improve test stability
…on flow and ratelimit fallback
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
docs/roadmap-current.md (1)
867-871:⚠️ Potential issue | 🟠 MajorValide o enforcement real de
IsSystemTemplateantes de afirmar “Garantido”.O texto afirma bloqueio de deleção em
EmailTemplateRepository.DeleteAsync, mas este diff não mostra a regra. Se a validação não estiver implementada, isso vira promessa incorreta de comportamento.#!/bin/bash # Verificar se existe validação de IsSystemTemplate no fluxo de deleção de templates. set -euo pipefail echo "## Localizando repositório/serviço de template" rg -n --type=cs -C3 'class\s+.*EmailTemplate.*Repository|interface\s+IEmailTemplateRepository|DeleteAsync\s*\(' echo echo "## Procurando guard clause com IsSystemTemplate em deleção" rg -n --type=cs -C4 'DeleteAsync\s*\(|IsSystemTemplate|Forbidden|ValidationException|DomainException' echo echo "## Procurando endpoint/serviço que chama deleção de template" rg -n --type=cs -C3 'Delete.*Template|Template.*Delete|ICommunicationsModuleApi|EmailTemplate'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/roadmap-current.md` around lines 867 - 871, Update EmailTemplateRepository.DeleteAsync to actually enforce the IsSystemTemplate flag: locate the DeleteAsync method in EmailTemplateRepository (and any higher-level service methods that call it) and add a guard that checks the EmailTemplate.IsSystemTemplate property, throwing a clear domain/validation/forbidden exception (e.g., ValidationException/DomainException/ForbiddenException used elsewhere) when true; also ensure any API/endpoint that delegates deletion respects and surfaces that exception so the deletion cannot proceed and logs/returns an appropriate error.
🧹 Nitpick comments (3)
docs/roadmap-current.md (1)
887-891: Evite redeclararECommunicationPriorityno exemplo da interface.A nota diz para usar tipos compartilhados, mas o snippet declara enum localmente. Prefira referenciar o enum de
MeAjudaAi.Shared.Contractspara reduzir drift documental.✏️ Ajuste sugerido
- public enum ECommunicationPriority { Low = 0, Normal = 1, High = 2 } - public interface ICommunicationsModuleApi : IModuleApi { // E-mail Task<Result<Guid>> SendEmailAsync( EmailMessageDto email, - ECommunicationPriority priority = ECommunicationPriority.Normal, + ECommunicationPriority priority = ECommunicationPriority.Normal, CancellationToken ct = default);// Nota no texto (fora do bloco): // ECommunicationPriority vem de MeAjudaAi.Shared.Contracts.EnumsBased on learnings, prioritize reuse of Shared.Contracts for enums/constants to keep Web aligned with backend/shared code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/roadmap-current.md` around lines 887 - 891, Remova a declaração local do enum ECommunicationPriority do bloco de exemplo e substitua por uma nota textual apontando que o enum é proveniente de MeAjudaAi.Shared.Contracts (ou MeAjudaAi.Shared.Contracts.Enums); ou seja, elimine o snippet "public enum ECommunicationPriority { ... }" e adicione uma linha comentada fora do bloco de código indicando "ECommunicationPriority vem de MeAjudaAi.Shared.Contracts.Enums" para garantir que o exemplo referencia o tipo compartilhado em vez de redeclará‑lo.src/Bootstrapper/MeAjudaAi.ApiService/Middlewares/SecurityHeadersMiddleware.cs (1)
13-19: DependênciaIWebHostEnvironmentnão utilizada.O parâmetro
environmenté injetado e armazenado em_environment, mas nunca é utilizado no métodoInvokeAsyncou em qualquer outro lugar da classe. Isso parece ser código residual de uma implementação anterior.♻️ Remover dependência não utilizada
public sealed class SecurityHeadersMiddleware( RequestDelegate next, - IWebHostEnvironment environment, ILogger<SecurityHeadersMiddleware>? logger = null) { private readonly RequestDelegate _next = next ?? throw new ArgumentNullException(nameof(next)); - private readonly IWebHostEnvironment _environment = environment ?? throw new ArgumentNullException(nameof(environment));E remover os usings não utilizados:
-using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Bootstrapper/MeAjudaAi.ApiService/Middlewares/SecurityHeadersMiddleware.cs` around lines 13 - 19, O construtor da classe SecurityHeadersMiddleware injeta e armazena IWebHostEnvironment em _environment, mas esse campo não é usado (nem em InvokeAsync), então remova o parâmetro environment do construtor e a inicialização/declaração do campo _environment, atualize a assinatura de SecurityHeadersMiddleware para só receber RequestDelegate next e ILogger<SecurityHeadersMiddleware>? logger = null, e remova quaisquer using relacionados a IWebHostEnvironment/usings não utilizados; verifique também chamadas/registro de middleware que instanciam SecurityHeadersMiddleware para ajustar a nova assinatura.tests/MeAjudaAi.Integration.Tests/Modules/SearchProviders/SearchProvidersApiTests.cs (1)
130-130: Use enum de contrato compartilhado para valores de tier, não literals.Linhas 130 e 149 usam abordagens inconsistentes (
"Gold"vs1). Prefira importar doShared.Contractse usarnameof()ou o enum diretamente para garantir segurança em refactors.♻️ Proposta
+using MeAjudaAi.Contracts.Modules.SearchProviders.Enums; @@ - var subscriptionTier = "Gold"; + var subscriptionTier = nameof(ESubscriptionTier.Gold);Também na linha 149:
- var subscriptionTier = 1; // Standard + var subscriptionTier = nameof(ESubscriptionTier.Standard);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/MeAjudaAi.Integration.Tests/Modules/SearchProviders/SearchProvidersApiTests.cs` at line 130, Replace the hard-coded tier literals with the shared contract enum: in SearchProvidersApiTests replace the string assignment to subscriptionTier ("Gold") and the numeric literal (1) with the Shared.Contracts SubscriptionTier enum (e.g., SubscriptionTier.Gold) or use nameof(SubscriptionTier.Gold) if a string name is required; update the using/import to include Shared.Contracts and change references to use SubscriptionTier.Gold to ensure refactor-safety and consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/Bootstrapper/MeAjudaAi.ApiService/Middlewares/SecurityHeadersMiddleware.cs`:
- Around line 46-50: O header Permissions-Policy definido em
SecurityHeadersMiddleware é código inativo porque
ContentSecurityPolicyMiddleware (registrado via UseApiServices) já adiciona
Permissions-Policy primeiro; remova as linhas que checam/appendam
Permissions-Policy em SecurityHeadersMiddleware (as linhas contendo
context.Response.Headers.ContainsKey(...) e
context.Response.Headers.Append(...)) ou, alternativamente, consolide o
comportamento movendo toda a lógica de gestão de Permissions-Policy para um
único middleware (preferencialmente ContentSecurityPolicyMiddleware) e elimine a
redundância em SecurityHeadersMiddleware para evitar valores conflitantes.
In `@src/Modules/SearchProviders/Domain/Enums/ESubscriptionTier.cs`:
- Line 1: Remova a dependência de serialização do enum de domínio
ESubscriptionTier: eliminar o using System.Text.Json.Serialization e qualquer
atributo [JsonConverter] do enum em
src/Modules/SearchProviders/Domain/Enums/ESubscriptionTier.cs; em seguida,
atualize a DTO SearchableProviderDto para importar e referenciar o enum do
contrato (using MeAjudaAi.Contracts.Modules.SearchProviders.Enums e usar esse
ESubscriptionTier contratual) em vez do enum de domínio; mantenha o
[JsonConverter] apenas no enum presente em
src/Contracts/.../ESubscriptionTier.cs para centralizar a configuração de
serialização na camada de contrato/API.
In `@tests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cs`:
- Around line 187-193: Ajuste a resolução de dependências para que
TestModule.Providers inclua implicitamente TestModule.ServiceCatalogs (além do
atual comportamento onde apenas SearchProviders expande dependências): ao
calcular módulos requisitados (onde atualmente você tem a lógica que adiciona
ServiceCatalogs/Locations quando modules.HasFlag(TestModule.SearchProviders)),
adicione uma condição para que se modules.HasFlag(TestModule.Providers) então
modules |= TestModule.ServiceCatalogs; isso garante que quando RequiredModules =
TestModule.Providers a migração de Providers (por exemplo na rotina que executa
migrations na linha referente a Providers) também eleve ServiceCatalogs antes de
prosseguir.
- Around line 295-303: No trate toda DbUpdateException como transitória: dentro
del bloque catch que asigna lastException y calcula isTransient (la comprobación
que actualmente incluye "ex is DbUpdateException"), elimine esa inclusión y
limite isTransient a TimeoutException y a Npgsql.PostgresException con los
SQLSTATEs transitórios ("57P01", "53300", "08006"); ajuste la expresión que
calcula isTransient (referencia: la variable isTransient en el catch junto a
lastException y attempt) para que solo considere TimeoutException y las
PostgresException con esos SqlState, de modo que errores determinísticos de
migración no se reintenten.
---
Duplicate comments:
In `@docs/roadmap-current.md`:
- Around line 867-871: Update EmailTemplateRepository.DeleteAsync to actually
enforce the IsSystemTemplate flag: locate the DeleteAsync method in
EmailTemplateRepository (and any higher-level service methods that call it) and
add a guard that checks the EmailTemplate.IsSystemTemplate property, throwing a
clear domain/validation/forbidden exception (e.g.,
ValidationException/DomainException/ForbiddenException used elsewhere) when
true; also ensure any API/endpoint that delegates deletion respects and surfaces
that exception so the deletion cannot proceed and logs/returns an appropriate
error.
---
Nitpick comments:
In `@docs/roadmap-current.md`:
- Around line 887-891: Remova a declaração local do enum ECommunicationPriority
do bloco de exemplo e substitua por uma nota textual apontando que o enum é
proveniente de MeAjudaAi.Shared.Contracts (ou MeAjudaAi.Shared.Contracts.Enums);
ou seja, elimine o snippet "public enum ECommunicationPriority { ... }" e
adicione uma linha comentada fora do bloco de código indicando
"ECommunicationPriority vem de MeAjudaAi.Shared.Contracts.Enums" para garantir
que o exemplo referencia o tipo compartilhado em vez de redeclará‑lo.
In
`@src/Bootstrapper/MeAjudaAi.ApiService/Middlewares/SecurityHeadersMiddleware.cs`:
- Around line 13-19: O construtor da classe SecurityHeadersMiddleware injeta e
armazena IWebHostEnvironment em _environment, mas esse campo não é usado (nem em
InvokeAsync), então remova o parâmetro environment do construtor e a
inicialização/declaração do campo _environment, atualize a assinatura de
SecurityHeadersMiddleware para só receber RequestDelegate next e
ILogger<SecurityHeadersMiddleware>? logger = null, e remova quaisquer using
relacionados a IWebHostEnvironment/usings não utilizados; verifique também
chamadas/registro de middleware que instanciam SecurityHeadersMiddleware para
ajustar a nova assinatura.
In
`@tests/MeAjudaAi.Integration.Tests/Modules/SearchProviders/SearchProvidersApiTests.cs`:
- Line 130: Replace the hard-coded tier literals with the shared contract enum:
in SearchProvidersApiTests replace the string assignment to subscriptionTier
("Gold") and the numeric literal (1) with the Shared.Contracts SubscriptionTier
enum (e.g., SubscriptionTier.Gold) or use nameof(SubscriptionTier.Gold) if a
string name is required; update the using/import to include Shared.Contracts and
change references to use SubscriptionTier.Gold to ensure refactor-safety and
consistency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3dc9dbd1-2cf2-4ac3-93af-6414184cd587
📒 Files selected for processing (8)
docs/roadmap-current.mdsrc/Bootstrapper/MeAjudaAi.ApiService/Extensions/SecurityExtensions.cssrc/Bootstrapper/MeAjudaAi.ApiService/Middlewares/SecurityHeadersMiddleware.cssrc/Contracts/Contracts/Modules/SearchProviders/Enums/ESubscriptionTier.cssrc/Modules/SearchProviders/Domain/Enums/ESubscriptionTier.cstests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cstests/MeAjudaAi.Integration.Tests/Modules/SearchProviders/SearchProvidersApiTests.cstests/MeAjudaAi.Integration.Tests/Modules/SearchProviders/SearchProvidersE2ETests.cs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Bootstrapper/MeAjudaAi.ApiService/Extensions/SecurityExtensions.cs
- tests/MeAjudaAi.Integration.Tests/Modules/SearchProviders/SearchProvidersE2ETests.cs
…epository, subscription tier enum, and CI/CD pipeline configuration
… prevent startup failure
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/roadmap-current.md (2)
8-8:⚠️ Potential issue | 🟡 MinorAtualize o status global no topo para refletir o encerramento da Sprint 9.
A linha ainda diz “Sprint 9 (Estabilização) em andamento (26 Mar 2026)”, mas a própria seção da Sprint 9 já está como encerrada em 11 Abr 2026. Isso gera leitura conflitante no cabeçalho do documento.
✏️ Sugestão objetiva
-> **📅 Status Atual**: Sprint 9 (Estabilização) em andamento (26 Mar 2026) +> **📅 Status Atual**: Sprint 9 (Estabilização) encerrada (11 Abr 2026)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/roadmap-current.md` at line 8, Atualize o cabeçalho que atualmente contém "**📅 Status Atual**: Sprint 9 (Estabilização) em andamento (26 Mar 2026)" para refletir que a Sprint 9 foi encerrada em 11 Abr 2026; localize e substitua esse texto no topo do documento (docs/roadmap-current.md) por uma versão como "**📅 Status Atual**: Sprint 9 (Estabilização) encerrada (11 Abr 2026)" ou outra formulação consistente com a seção "Sprint 9" para evitar conflito de leitura.
454-454:⚠️ Potential issue | 🟡 MinorHá inconsistência de status da Sprint 5.5 no próprio documento.
Em um ponto está “✅ CONCLUÍDO”, e na seção detalhada está “🔄 EM ANDAMENTO - Aguardando CI/CD”. Defina explicitamente se é snapshot histórico ou status atual para não confundir leitura executiva.
Also applies to: 512-513
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/roadmap-current.md` at line 454, Há uma inconsistência entre a linha que mostra "Status: ✅ CONCLUÍDO (Sprint 5.5 - 19 Dez 2025)" e a seção detalhada que ainda mostra "🔄 EM ANDAMENTO - Aguardando CI/CD"; escolha e aplique uma única fonte de verdade: ou mantenha como snapshot histórico (ex.: "Snapshot — CONCLUÍDO em 19 Dez 2025") ou como status atual, e atualize ambas ocorrências correspondentes (a linha "Status: ✅ CONCLUÍDO (Sprint 5.5 - 19 Dez 2025)" e a seção detalhada que contém "🔄 EM ANDAMENTO - Aguardando CI/CD") para usar a mesma texto/ícone; se optar por snapshot, inclua uma nota de data/contexto visível; se optar por status atual, remova/altere a marcação de snapshot para evitar ambiguidade.
♻️ Duplicate comments (2)
docs/roadmap-current.md (1)
867-867:⚠️ Potential issue | 🟠 MajorEvite afirmar “garantido” sem evidência explícita de enforcement no texto técnico.
Aqui o roadmap afirma que a proteção de deleção por
IsSystemTemplateé garantida emEmailTemplateRepository.DeleteAsync; se essa validação não estiver claramente implementada, o documento pode prometer comportamento inexistente.✏️ Ajuste seguro de documentação
-- **IsSystemTemplate**: Flag para proteger modelos críticos de deleção (Garantido em `EmailTemplateRepository.DeleteAsync`). +- **IsSystemTemplate**: Flag para proteger modelos críticos de deleção (regra aplicada na camada de aplicação/repositório; manter este trecho alinhado com a implementação vigente).Based on learnings, foi priorizada a consistência entre contratos compartilhados/implementação para evitar drift documental.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/roadmap-current.md` at line 867, The doc line claims deletion protection is "guaranteed" by IsSystemTemplate in EmailTemplateRepository.DeleteAsync; change the wording to avoid asserting enforcement unless the repository method explicitly implements the check—either remove "Garantido" and state "intended to prevent deletion when IsSystemTemplate is true" or update the doc to "enforced in EmailTemplateRepository.DeleteAsync" only after verifying that DeleteAsync contains a concrete IsSystemTemplate validation and throws/returns appropriately; if enforcement exists, add a brief note referencing EmailTemplateRepository.DeleteAsync as the enforcement point, otherwise note it as a design intention and file a follow-up task to implement the check.src/Modules/Communications/Application/Services/OutboxProcessorService.cs (1)
74-75:⚠️ Potential issue | 🟡 MinorPadronize estes comentários para português.
Os comentários inline das Lines 74 e 97 ainda estão em inglês. Mantém o arquivo consistente com o padrão do repositório.
Based on learnings, comments and XML documentation in C# should be written in Portuguese across the codebase.
Also applies to: 97-97
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Modules/Communications/Application/Services/OutboxProcessorService.cs` around lines 74 - 75, Padronize os comentários em inglês para português no arquivo, substituindo os comentários inline atuais nas proximidades de OutboxProcessorService (comentário acima de SaveChanges na classe OutboxProcessorService) e o comentário na linha referida como 97 por versões em português coerentes com o padrão do repositório; atualize a redação mantendo o mesmo significado técnico e estilo de comentário usado no restante do projeto (por exemplo, traduzir "Remove immediate SaveChanges to allow OutboxProcessorBase to commit everything together" para uma versão em português clara e equivalente).
🧹 Nitpick comments (1)
src/Bootstrapper/MeAjudaAi.ApiService/Extensions/SecurityExtensions.cs (1)
597-603: Evite registrarSecurityHeadersMiddlewareduas vezes.
UseSecurityHardening()adiciona o mesmo middleware que já entra viaapp.UseApiMiddlewares()emsrc/Bootstrapper/MeAjudaAi.ApiService/Extensions/ServiceCollectionExtensions.cs:164. Hoje isso tende a ser idempotente, mas ainda duplica callbacksOnStartinge deixa a ordem de aplicação dos headers mais difícil de raciocinar. Centralize esse middleware em um único ponto do pipeline.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Bootstrapper/MeAjudaAi.ApiService/Extensions/SecurityExtensions.cs` around lines 597 - 603, The UseSecurityHardening method is registering SecurityHeadersMiddleware redundantly; remove the app.UseMiddleware<SecurityHeadersMiddleware>() call from UseSecurityHardening and rely on the single registration already performed by UseApiMiddlewares (the method that currently wires SecurityHeadersMiddleware into the pipeline), so the middleware is only added once and header ordering/OnStarting callbacks remain deterministic; keep UseSecurityHardening for other hardening steps if present but do not re-register SecurityHeadersMiddleware.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/roadmap-current.md`:
- Line 1460: O sumário executivo mostra "⏳ **Sprint 9: BUFFER E MITIGAÇÃO DE
RISCO**" enquanto o corpo registra "✅ ENCERRADA (11 Abr 2026)"; alinhe-os
atualizando o bloco do sumário para refletir o encerramento — substitua o ícone
e texto na entrada do sumário (a string contendo "⏳ **Sprint 9: BUFFER E
MITIGAÇÃO DE RISCO**") para usar "✅" e indicar "ENCERRADA (11 Abr 2026)" igual
ao corpo.
In
`@src/Bootstrapper/MeAjudaAi.ApiService/Extensions/ServiceCollectionExtensions.cs`:
- Around line 172-174: A chamada a app.UseAntiforgery() está antes da
autenticação/autorization; atualize a ordem no método de configuração em
ServiceCollectionExtensions.cs movendo app.UseAntiforgery() para depois de
app.UseAuthentication() e, especificamente, após app.UseAuthorization() (que já
existe no arquivo) para garantir que antiforgery execute somente após o usuário
ser autenticado/autoriza-do.
In `@src/Modules/Communications/Application/Services/OutboxProcessorService.cs`:
- Around line 65-75: The CommunicationLog creation/adding
(CommunicationLog.CreateSuccess and logRepository.AddAsync) must not be allowed
to throw and break the OutboxProcessorBase commit flow called in
OutboxProcessorBase.ProcessPendingMessagesAsync (which waits until
OnSuccessAsync/OnFailureAsync finish), so wrap the log creation + await
logRepository.AddAsync(...) in a try/catch that catches and records the logging
failure (e.g., processLogger.Error with correlationId/message.Id and the
exception) but does not rethrow; apply the same pattern to the failure-path code
that calls CommunicationLog.CreateFailure and logRepository.AddAsync (and ensure
MarkAsFailed is still executed/persisted by not letting log errors bubble up).
---
Outside diff comments:
In `@docs/roadmap-current.md`:
- Line 8: Atualize o cabeçalho que atualmente contém "**📅 Status Atual**:
Sprint 9 (Estabilização) em andamento (26 Mar 2026)" para refletir que a Sprint
9 foi encerrada em 11 Abr 2026; localize e substitua esse texto no topo do
documento (docs/roadmap-current.md) por uma versão como "**📅 Status Atual**:
Sprint 9 (Estabilização) encerrada (11 Abr 2026)" ou outra formulação
consistente com a seção "Sprint 9" para evitar conflito de leitura.
- Line 454: Há uma inconsistência entre a linha que mostra "Status: ✅ CONCLUÍDO
(Sprint 5.5 - 19 Dez 2025)" e a seção detalhada que ainda mostra "🔄 EM
ANDAMENTO - Aguardando CI/CD"; escolha e aplique uma única fonte de verdade: ou
mantenha como snapshot histórico (ex.: "Snapshot — CONCLUÍDO em 19 Dez 2025") ou
como status atual, e atualize ambas ocorrências correspondentes (a linha
"Status: ✅ CONCLUÍDO (Sprint 5.5 - 19 Dez 2025)" e a seção detalhada que contém
"🔄 EM ANDAMENTO - Aguardando CI/CD") para usar a mesma texto/ícone; se optar
por snapshot, inclua uma nota de data/contexto visível; se optar por status
atual, remova/altere a marcação de snapshot para evitar ambiguidade.
---
Duplicate comments:
In `@docs/roadmap-current.md`:
- Line 867: The doc line claims deletion protection is "guaranteed" by
IsSystemTemplate in EmailTemplateRepository.DeleteAsync; change the wording to
avoid asserting enforcement unless the repository method explicitly implements
the check—either remove "Garantido" and state "intended to prevent deletion when
IsSystemTemplate is true" or update the doc to "enforced in
EmailTemplateRepository.DeleteAsync" only after verifying that DeleteAsync
contains a concrete IsSystemTemplate validation and throws/returns
appropriately; if enforcement exists, add a brief note referencing
EmailTemplateRepository.DeleteAsync as the enforcement point, otherwise note it
as a design intention and file a follow-up task to implement the check.
In `@src/Modules/Communications/Application/Services/OutboxProcessorService.cs`:
- Around line 74-75: Padronize os comentários em inglês para português no
arquivo, substituindo os comentários inline atuais nas proximidades de
OutboxProcessorService (comentário acima de SaveChanges na classe
OutboxProcessorService) e o comentário na linha referida como 97 por versões em
português coerentes com o padrão do repositório; atualize a redação mantendo o
mesmo significado técnico e estilo de comentário usado no restante do projeto
(por exemplo, traduzir "Remove immediate SaveChanges to allow
OutboxProcessorBase to commit everything together" para uma versão em português
clara e equivalente).
---
Nitpick comments:
In `@src/Bootstrapper/MeAjudaAi.ApiService/Extensions/SecurityExtensions.cs`:
- Around line 597-603: The UseSecurityHardening method is registering
SecurityHeadersMiddleware redundantly; remove the
app.UseMiddleware<SecurityHeadersMiddleware>() call from UseSecurityHardening
and rely on the single registration already performed by UseApiMiddlewares (the
method that currently wires SecurityHeadersMiddleware into the pipeline), so the
middleware is only added once and header ordering/OnStarting callbacks remain
deterministic; keep UseSecurityHardening for other hardening steps if present
but do not re-register SecurityHeadersMiddleware.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 25dd146a-ea3a-4f7f-ae44-bd4e3eb5738c
📒 Files selected for processing (16)
docs/roadmap-current.mdsrc/Bootstrapper/MeAjudaAi.ApiService/Extensions/SecurityExtensions.cssrc/Bootstrapper/MeAjudaAi.ApiService/Extensions/ServiceCollectionExtensions.cssrc/Modules/Communications/Application/Handlers/DocumentRejectedIntegrationEventHandler.cssrc/Modules/Communications/Application/Handlers/DocumentVerifiedIntegrationEventHandler.cssrc/Modules/Communications/Application/Services/OutboxProcessorService.cssrc/Modules/Communications/Infrastructure/Extensions.cssrc/Modules/Communications/Tests/Architecture/CommunicationsArchitectureTests.cssrc/Modules/Communications/Tests/Unit/Application/Handlers/DocumentRejectedIntegrationEventHandlerTests.cssrc/Modules/Communications/Tests/Unit/Application/Handlers/DocumentVerifiedIntegrationEventHandlerTests.cssrc/Modules/SearchProviders/Infrastructure/Events/Handlers/ServiceDeactivatedIntegrationEventHandler.cssrc/Modules/ServiceCatalogs/Infrastructure/Events/Handlers/ServiceActivatedDomainEventHandler.cssrc/Modules/ServiceCatalogs/Infrastructure/Events/Handlers/ServiceDeactivatedDomainEventHandler.cssrc/Shared/Database/Outbox/OutboxProcessorBase.cstests/MeAjudaAi.Integration.Tests/Middleware/SecurityHeadersMiddlewareTests.cstests/MeAjudaAi.Integration.Tests/Modules/SearchProviders/SearchProvidersE2ETests.cs
✅ Files skipped from review due to trivial changes (1)
- src/Modules/Communications/Tests/Unit/Application/Handlers/DocumentRejectedIntegrationEventHandlerTests.cs
🚧 Files skipped from review as they are similar to previous changes (8)
- src/Modules/ServiceCatalogs/Infrastructure/Events/Handlers/ServiceDeactivatedDomainEventHandler.cs
- src/Modules/ServiceCatalogs/Infrastructure/Events/Handlers/ServiceActivatedDomainEventHandler.cs
- src/Modules/SearchProviders/Infrastructure/Events/Handlers/ServiceDeactivatedIntegrationEventHandler.cs
- src/Modules/Communications/Tests/Architecture/CommunicationsArchitectureTests.cs
- src/Modules/Communications/Application/Handlers/DocumentRejectedIntegrationEventHandler.cs
- src/Modules/Communications/Application/Handlers/DocumentVerifiedIntegrationEventHandler.cs
- tests/MeAjudaAi.Integration.Tests/Middleware/SecurityHeadersMiddlewareTests.cs
- src/Modules/Communications/Tests/Unit/Application/Handlers/DocumentVerifiedIntegrationEventHandlerTests.cs
added 3 commits
April 12, 2026 17:55
- Align Sprint 9 and 5.5 status in roadmap. - Move UseAntiforgery after UseAuthorization in middleware pipeline. - Remove redundant SecurityHeadersMiddleware registration. - Improve logging resilience in OutboxProcessorService and translate comments. - Refactor idempotency test to use typed UniqueConstraintException for better reliability.
…cessorBase to fix unit tests - Fix ProcessPendingMessagesAsync_WhenSuccessfulDispatch_ShouldMarkAsSent test failure. - Ensure message is explicitly marked as processing and persisted before dispatch.
- Implement AntiforgeryCookieMiddleware to ensure CSRF tokens are issued on GET requests (Hardening). - Configure https_port in WebApplicationFactory to enable HttpsRedirection testing. - Ensure deterministic middleware ordering for security headers and antiforgery.
Owner
Author
|
@corerabbitai review |
added 2 commits
April 12, 2026 18:34
- Unified UseAuthorization() and UseAntiforgery() in the correct pipeline order. - Enhanced AntiforgeryCookieMiddleware with try-catch to prevent 500 errors in test environments. - Confirmed all 161 E2E tests are passing.
…p status - Updated BusinessMetrics to accept custom meter names for test isolation. - Refactored BusinessMetricsTests to use unique meter names per instance. - Finalized Sprint 9 status in roadmap header. - Guaranteed all tests (Unit, Integration, E2E) are passing after security and outbox changes.
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
tests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cs (1)
215-215:⚠️ Potential issue | 🟠 MajorNão mascare
PendingModelChangesWarningem todos osDbContexts.Esse
Ignore(...)global tira do harness o fail-fast para drift real entre modelo e migrations. Se existe falso positivo, limite a supressão ao contexto que realmente precisa dela ou a uma condição explícita de debug.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cs` at line 215, Remova a supressão global do aviso PendingModelChangesWarning aplicada via options.ConfigureWarnings(w => w.Ignore(Microsoft.EntityFrameworkCore.Diagnostics.RelationalEventId.PendingModelChangesWarning)) e, em vez disso, aplique-a apenas no DbContext específico que precisa (por ex. no OnConfiguring/DbContextOptionsBuilder do YourSpecialDbContext) ou envolva-a numa condição explícita de debug/flag de teste; localize a chamada ConfigureWarnings e mova/condicione essa Ignore para o construtor/opções do contexto alvo (ou remova-a completamente) para restaurar o comportamento fail-fast nos demais DbContext.
🧹 Nitpick comments (3)
src/Modules/Communications/Tests/Unit/Application/Handlers/DocumentRejectedIntegrationEventHandlerTests.cs (1)
34-82: Amplie os testes para cobrir ramos críticos do handlerOs testes atuais não cobrem falha de provider (
IsSuccess=false), provider sem email e deduplicação por unique constraint. Vale adicionar esses cenários para blindar comportamento de erro e idempotência.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Modules/Communications/Tests/Unit/Application/Handlers/DocumentRejectedIntegrationEventHandlerTests.cs` around lines 34 - 82, Add three new unit tests for DocumentRejectedIntegrationEventHandler: (1) HandleAsync_WhenProviderFetchFails_ShouldSkipAndNotEnqueue — mock _providersModuleApiMock.GetProviderByIdAsync to return Result.Failure(...) and assert _outboxRepositoryMock.AddAsync/SaveChangesAsync are never called; (2) HandleAsync_WhenProviderHasNoEmail_ShouldSkipEnqueue — return a ModuleProviderDto with null/empty Email and assert no outbox operations; (3) HandleAsync_WhenOutboxDuplicateExists_ShouldNotCreateDuplicate — simulate deduplication by having the outbox repository indicate an existing OutboxMessage (e.g., mock a query or make AddAsync throw a unique constraint exception) and assert handler does not create a second message (verify AddAsync not called or SaveChangesAsync not called, depending on how duplicates are detected). Use the existing _handler.HandleAsync(integrationEvent), _providersModuleApiMock, and _outboxRepositoryMock symbols to set up expectations and verifications.src/Shared/Monitoring/BusinessMetrics.cs (1)
24-26: Extraia o nome padrão do meter para uma constante compartilhada.O default ficou correto e mantém o DI estável, mas o literal
"MeAjudaAi.Business"agora está duplicado aqui e também em testes que dependem do nome canônico. Centralizar isso no próprioBusinessMetricsreduz drift silencioso entre produção e suíte.♻️ Refactor sugerido
public class BusinessMetrics : IDisposable { + public const string DefaultMeterName = "MeAjudaAi.Business"; + private readonly Meter _meter; private readonly Counter<long> _userRegistrations; private readonly Counter<long> _userLogins; @@ - public BusinessMetrics(string meterName = "MeAjudaAi.Business") + public BusinessMetrics(string meterName = DefaultMeterName) { _meter = new Meter(meterName, "1.0.0");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Shared/Monitoring/BusinessMetrics.cs` around lines 24 - 26, Extraia o literal do nome do meter para uma constante pública dentro da classe BusinessMetrics e use essa constante como valor padrão do parâmetro do construtor; por exemplo, adicionar um symbol BusinessMetrics.DefaultMeterName = "MeAjudaAi.Business" e alterar o construtor BusinessMetrics(string meterName = DefaultMeterName) que cria _meter = new Meter(meterName, "1.0.0"); e atualize os testes para referenciar BusinessMetrics.DefaultMeterName em vez do string literal para evitar duplicação e drift.src/Modules/Communications/Application/Handlers/DocumentVerifiedIntegrationEventHandler.cs (1)
23-23: Considere substituir payload anônimo por contrato tipado compartilhado.Aqui,
TemplateKey+ campos do payload estão “soltos” no handler. Um DTO/contrato compartilhado reduz drift entre módulos e facilita evolução segura do schema da mensagem.Based on learnings: prioritize reuse of Shared.Contracts for enums/constants to keep Web aligned with backend/shared code.
Also applies to: 45-52
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Modules/Communications/Application/Handlers/DocumentVerifiedIntegrationEventHandler.cs` at line 23, O handler DocumentVerifiedIntegrationEventHandler está usando TemplateKey = "document-verified" e um payload anônimo — substitua esse uso por um compartilhado contrato tipado em Shared.Contracts (por exemplo DocumentVerifiedEventDto ou ISharedDocumentVerifiedContract) e mova constantes/enums reutilizáveis para Shared.Contracts; atualize o handler para consumir esse DTO (em vez de campos soltos nas linhas 45-52), referenciar a constante TemplateKey a partir do contrato/enum compartilhado e ajustar qualquer serialização/deserialização para usar o novo tipo.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Bootstrapper/MeAjudaAi.ApiService/Extensions/SecurityExtensions.cs`:
- Around line 565-567: The GlobalLimiter's permit calculation uses the wrong
config keys (it reads
RateLimit:AuthenticatedRequestsPerMinute/RateLimit:DefaultRequestsPerMinute)
while ValidateSecurityConfiguration() validates AdvancedRateLimit; update the
permit calculation in SecurityExtensions (the var permitLimit assignment inside
GlobalLimiter) to read AdvancedRateLimit:AuthenticatedRequestsPerMinute and
AdvancedRateLimit:AnonymousRequestsPerMinute (or otherwise align keys with
ValidateSecurityConfiguration()), so the limiter uses the same configured values
as the validation routine.
In
`@src/Bootstrapper/MeAjudaAi.ApiService/Extensions/ServiceCollectionExtensions.cs`:
- Around line 174-178: Add the antiforgery response header to the CORS exposed
headers so cross-origin clients can read the token: update the CORS
DefaultPolicy configuration (the builder where DefaultPolicy is created in
ServiceCollectionExtensions) to call WithExposedHeaders("X-XSRF-TOKEN"); this
ensures the header written by AntiforgeryCookieMiddleware ("X-XSRF-TOKEN") is
available to browser clients across origins while leaving other CORS settings
intact.
In
`@src/Bootstrapper/MeAjudaAi.ApiService/Middlewares/AntiforgeryCookieMiddleware.cs`:
- Around line 20-34: The empty catch in AntiforgeryCookieMiddleware (around
_antiforgery.GetAndStoreTokens(context)) hides DataProtection/antiforgery
configuration errors; change the catch to capture the Exception (e.g., catch
(Exception ex)) and log it in English via an injected
ILogger<AntiforgeryCookieMiddleware> (include context info and ex) and only
swallow the exception for explicit test/bypass environments (use an injected
IHostEnvironment or environment flag such as
_env.IsEnvironment("Test")/_env.IsDevelopment()); in non-test/production
environments log at warning/error level and do not silently drop the error
(optionally rethrow or let middleware pipeline handle it) so observability is
preserved.
In
`@src/Modules/Communications/Application/Handlers/DocumentRejectedIntegrationEventHandler.cs`:
- Around line 74-80: The catch currently treats any
MeAjudaAi.Shared.Database.Exceptions.UniqueConstraintException as "already
enqueued" which can hide real integrity errors; in
DocumentRejectedIntegrationEventHandler adjust the catch handling for
processedException so you only swallow the exception when it specifically
matches the outbox/correlation unique constraint (e.g., check a ConstraintName
property, error code or message on processedException for the outbox correlation
constraint), and for other UniqueConstraintException instances rethrow or let
them bubble (or log as an error) instead; update the conditional around
processedException to inspect that identifier before returning.
In
`@src/Modules/Communications/Application/Handlers/DocumentVerifiedIntegrationEventHandler.cs`:
- Around line 74-76: Centralize the outbox constraint name: define a shared
constant (e.g., OutboxMessageConstraints.CorrelationIdIndexName) and use it both
in the EF configuration class (add .HasName("...") in
OutboxMessageConfiguration) and in the DocumentVerifiedIntegrationEventHandler
when checking UniqueConstraintException.ConstraintName instead of the hardcoded
"ix_outbox_messages_correlation_id"; update the OutboxMessageConfiguration to
call HasName using that constant and change the handler's comparison to
reference the same constant to guarantee a single source of truth for the
constraint name.
In `@src/Modules/Communications/Application/Services/OutboxProcessorService.cs`:
- Around line 80-85: The log messages are in Portuguese; change them to English
to standardize runtime logs: update the logger.LogError call that logs "Falha ao
criar log de sucesso..." to an English message including the same contextual
placeholders (message.Id, message.CorrelationId), and update the
logger.LogInformation call that logs "Mensagem outbox ..." (and the other
logging block later in the file) to English messages preserving the placeholders
(message.Id, message.Channel, recipientMasked); ensure formatting and exception
parameterization remain unchanged.
In
`@src/Modules/ServiceCatalogs/Infrastructure/Events/Handlers/ServiceActivatedDomainEventHandler.cs`:
- Around line 21-25: The handler currently swallows missing-service cases by
logging and returning in the ServiceActivatedDomainEventHandler (check the
service == null branch and the logger.LogWarning call), which prevents retries;
replace the early return with throwing an exception (e.g.,
InvalidOperationException or a domain-specific exception) that includes
domainEvent.ServiceId and a clear message so the failure is propagated and the
event can be retried/handled by the retry/fault system.
In `@tests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cs`:
- Around line 248-255: Reordene as chamadas de ApplyMigrationForContextAsync em
BaseApiTest.cs para espelhar a prioridade do bootstrap de produção: chame nos
módulos na ordem Users -> ServiceCatalogs -> Locations -> Documents -> Providers
-> Communications (e adicione SearchProviders se existir no enum
TestModule/infra), usando os mesmos TestModule flags (TestModule.Users,
TestModule.ServiceCatalogs, TestModule.Locations, TestModule.Documents,
TestModule.Providers, TestModule.Communications, TestModule.SearchProviders)
para decidir quais migrations aplicar; ajuste a sequência de invocações que
atualmente usa
ApplyMigrationForContextAsync(serviceProvider.GetRequiredService<...DbContext>(),
"...", logger) para seguir exatamente essa ordem.
---
Duplicate comments:
In `@tests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cs`:
- Line 215: Remova a supressão global do aviso PendingModelChangesWarning
aplicada via options.ConfigureWarnings(w =>
w.Ignore(Microsoft.EntityFrameworkCore.Diagnostics.RelationalEventId.PendingModelChangesWarning))
e, em vez disso, aplique-a apenas no DbContext específico que precisa (por ex.
no OnConfiguring/DbContextOptionsBuilder do YourSpecialDbContext) ou envolva-a
numa condição explícita de debug/flag de teste; localize a chamada
ConfigureWarnings e mova/condicione essa Ignore para o construtor/opções do
contexto alvo (ou remova-a completamente) para restaurar o comportamento
fail-fast nos demais DbContext.
---
Nitpick comments:
In
`@src/Modules/Communications/Application/Handlers/DocumentVerifiedIntegrationEventHandler.cs`:
- Line 23: O handler DocumentVerifiedIntegrationEventHandler está usando
TemplateKey = "document-verified" e um payload anônimo — substitua esse uso por
um compartilhado contrato tipado em Shared.Contracts (por exemplo
DocumentVerifiedEventDto ou ISharedDocumentVerifiedContract) e mova
constantes/enums reutilizáveis para Shared.Contracts; atualize o handler para
consumir esse DTO (em vez de campos soltos nas linhas 45-52), referenciar a
constante TemplateKey a partir do contrato/enum compartilhado e ajustar qualquer
serialização/deserialização para usar o novo tipo.
In
`@src/Modules/Communications/Tests/Unit/Application/Handlers/DocumentRejectedIntegrationEventHandlerTests.cs`:
- Around line 34-82: Add three new unit tests for
DocumentRejectedIntegrationEventHandler: (1)
HandleAsync_WhenProviderFetchFails_ShouldSkipAndNotEnqueue — mock
_providersModuleApiMock.GetProviderByIdAsync to return Result.Failure(...) and
assert _outboxRepositoryMock.AddAsync/SaveChangesAsync are never called; (2)
HandleAsync_WhenProviderHasNoEmail_ShouldSkipEnqueue — return a
ModuleProviderDto with null/empty Email and assert no outbox operations; (3)
HandleAsync_WhenOutboxDuplicateExists_ShouldNotCreateDuplicate — simulate
deduplication by having the outbox repository indicate an existing OutboxMessage
(e.g., mock a query or make AddAsync throw a unique constraint exception) and
assert handler does not create a second message (verify AddAsync not called or
SaveChangesAsync not called, depending on how duplicates are detected). Use the
existing _handler.HandleAsync(integrationEvent), _providersModuleApiMock, and
_outboxRepositoryMock symbols to set up expectations and verifications.
In `@src/Shared/Monitoring/BusinessMetrics.cs`:
- Around line 24-26: Extraia o literal do nome do meter para uma constante
pública dentro da classe BusinessMetrics e use essa constante como valor padrão
do parâmetro do construtor; por exemplo, adicionar um symbol
BusinessMetrics.DefaultMeterName = "MeAjudaAi.Business" e alterar o construtor
BusinessMetrics(string meterName = DefaultMeterName) que cria _meter = new
Meter(meterName, "1.0.0"); e atualize os testes para referenciar
BusinessMetrics.DefaultMeterName em vez do string literal para evitar duplicação
e drift.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5af23134-d466-4874-a686-e2747010c298
📒 Files selected for processing (21)
docs/roadmap-current.mdsrc/Bootstrapper/MeAjudaAi.ApiService/Extensions/SecurityExtensions.cssrc/Bootstrapper/MeAjudaAi.ApiService/Extensions/ServiceCollectionExtensions.cssrc/Bootstrapper/MeAjudaAi.ApiService/Middlewares/AntiforgeryCookieMiddleware.cssrc/Bootstrapper/MeAjudaAi.ApiService/appsettings.jsonsrc/Modules/Communications/Application/Handlers/DocumentRejectedIntegrationEventHandler.cssrc/Modules/Communications/Application/Handlers/DocumentVerifiedIntegrationEventHandler.cssrc/Modules/Communications/Application/Services/OutboxProcessorService.cssrc/Modules/Communications/Infrastructure/Extensions.cssrc/Modules/Communications/Tests/Architecture/CommunicationsArchitectureTests.cssrc/Modules/Communications/Tests/Unit/Application/Handlers/DocumentRejectedIntegrationEventHandlerTests.cssrc/Modules/Communications/Tests/Unit/Application/Handlers/DocumentVerifiedIntegrationEventHandlerTests.cssrc/Modules/SearchProviders/Infrastructure/Events/Handlers/ServiceDeactivatedIntegrationEventHandler.cssrc/Modules/ServiceCatalogs/Infrastructure/Events/Handlers/ServiceActivatedDomainEventHandler.cssrc/Modules/ServiceCatalogs/Infrastructure/Events/Handlers/ServiceDeactivatedDomainEventHandler.cssrc/Shared/Database/Outbox/OutboxProcessorBase.cssrc/Shared/Monitoring/BusinessMetrics.cstests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cstests/MeAjudaAi.Integration.Tests/Middleware/SecurityHeadersMiddlewareTests.cstests/MeAjudaAi.Integration.Tests/Modules/SearchProviders/SearchProvidersE2ETests.cstests/MeAjudaAi.Shared.Tests/Unit/Monitoring/BusinessMetricsTests.cs
✅ Files skipped from review due to trivial changes (3)
- src/Bootstrapper/MeAjudaAi.ApiService/appsettings.json
- src/Modules/Communications/Tests/Architecture/CommunicationsArchitectureTests.cs
- src/Shared/Database/Outbox/OutboxProcessorBase.cs
🚧 Files skipped from review as they are similar to previous changes (4)
- src/Modules/ServiceCatalogs/Infrastructure/Events/Handlers/ServiceDeactivatedDomainEventHandler.cs
- tests/MeAjudaAi.Integration.Tests/Middleware/SecurityHeadersMiddlewareTests.cs
- src/Modules/Communications/Infrastructure/Extensions.cs
- src/Modules/Communications/Tests/Unit/Application/Handlers/DocumentVerifiedIntegrationEventHandlerTests.cs
added 4 commits
April 12, 2026 21:41
…view - Refactored CompressionSecurityMiddleware to be more robust and avoid 'response has already started' error. - Simplified integration tests by using /health endpoint to verify security policy without redirect side-effects. - Configured BaseAddress to HTTPS in WebApplicationFactory to prevent header loss during redirects. - Resolved ambiguous OutboxMessage references by using type aliases. - Fully addressed all remaining items from the latest code review.
- Injected ILogger mock into middleware constructor. - Updated assertions to match 'identity' encoding instead of removal.
- Added localized PendingModelChangesWarning suppression to BaseDatabaseTest and UserRepositoryIntegrationTests to handle naming convention drift during tests. - Finalized translation of remaining comments and assertion messages in integration and business metrics files. - Verified system stability with 161 E2E tests passing.
… failures - Fixed AntiforgeryCookieMiddleware to always call _next(context) even when token generation fails in Test environment. - Added /api/logout to sensitive paths in CompressionSecurityMiddleware. - Verified all 207 Unit, 342 Integration, and 161 E2E tests are passing.
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/Modules/Communications/Application/Handlers/DocumentRejectedIntegrationEventHandler.cs (1)
44-44: Inconsistência no formato do correlationId entre handlers.Este handler usa
$"{templateKey}:..."enquantoDocumentVerifiedIntegrationEventHandlerusa$"document_verified:..."hardcoded. Ambas abordagens funcionam para idempotência, mas padronizar facilitaria manutenção futura.💡 Sugestão de padronização
Opção 1: Usar template key em ambos (como este handler):
var correlationId = $"{templateKey}:{integrationEvent.DocumentId}:{integrationEvent.ProviderId}";Opção 2: Usar prefixo descritivo em ambos (como DocumentVerifiedIntegrationEventHandler):
var correlationId = $"document_rejected:{integrationEvent.DocumentId}:{integrationEvent.ProviderId}";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Modules/Communications/Application/Handlers/DocumentRejectedIntegrationEventHandler.cs` at line 44, Padronize o formato de correlationId entre DocumentRejectedIntegrationEventHandler e DocumentVerifiedIntegrationEventHandler; escolha uma das abordagens sugeridas e atualize a criação do correlationId dentro do método que constrói o evento (em DocumentRejectedIntegrationEventHandler) para usar o mesmo padrão que o outro handler (por exemplo, substituir var correlationId = $"{templateKey}:..."; por um prefixo descritivo $"document_rejected:{integrationEvent.DocumentId}:{integrationEvent.ProviderId}" ou, alternativamente, alterar DocumentVerifiedIntegrationEventHandler para usar templateKey em vez do literal), garantindo que ambos handlers usem o mesmo formato para idempotência e atualize quaisquer testes ou consumidores que dependam do formato antigo.src/Bootstrapper/MeAjudaAi.ApiService/Middlewares/CompressionSecurityMiddleware.cs (2)
49-77: Considere manterIsSafeForCompressioncomostatic.O método
IsSafeForCompressionnão utiliza nenhum campo de instância (_loggerou_next). Poderia permanecerstaticpara clareza, a menos que haja planos de usar o logger dentro dele futuramente.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Bootstrapper/MeAjudaAi.ApiService/Middlewares/CompressionSecurityMiddleware.cs` around lines 49 - 77, The IsSafeForCompression method in CompressionSecurityMiddleware does not access any instance fields and should be declared static for clarity; change its signature to static bool IsSafeForCompression(HttpContext context) and update any internal calls (e.g., from InvokeAsync or other methods in CompressionSecurityMiddleware) to call the static method directly, ensuring there are no references to instance state inside it and updating any unit tests or callers accordingly.
16-21: Logger injetado mas não utilizado.O
_loggeré injetado no construtor mas não é utilizado em nenhum lugar do middleware. Considere:
- Adicionar logs para auditoria quando compressão é desabilitada (especialmente útil para debugging em produção)
- Ou remover a dependência se não for necessária
💡 Sugestão opcional para uso do logger
if (!isSafe) { + _logger.LogDebug("Compression disabled for request to {Path} due to security policy", context.Request.Path); context.Request.Headers["Accept-Encoding"] = "identity";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Bootstrapper/MeAjudaAi.ApiService/Middlewares/CompressionSecurityMiddleware.cs` around lines 16 - 21, O campo _logger é injetado na classe CompressionSecurityMiddleware (construtor CompressionSecurityMiddleware) mas não está a ser usado; ou remova a dependência (_logger e o parâmetro ILogger no construtor) ou passe a usá‑la no método InvokeAsync/Invoke para auditoria: registe mensagens informativas/aviso quando a compressão for desabilitada (por exemplo quando checa Accept-Encoding ou adiciona/retira headers de Content-Encoding), inclua contexto relevante (Request.Path, razão da desabilitação) e use níveis apropriados (Information/Warning) para facilitar debugging em produção.tests/MeAjudaAi.Integration.Tests/Middleware/CompressionSecurityMiddlewareTests.cs (1)
47-66: Considere verificar ausência do headerX-Compression-Disabled.Para usuários anônimos, seria útil também verificar que o header de debug
X-Compression-Disablednão está presente, garantindo que o middleware não interferiu.💡 Sugestão de assertion adicional
// Assert response.StatusCode.Should().Be(HttpStatusCode.OK); + // Header de debug não deve estar presente para usuários anônimos + response.Headers.Should().NotContainKey("X-Compression-Disabled", + "Security middleware should not add debug header for anonymous users"); + // For anonymous users, compression may be enabled🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/MeAjudaAi.Integration.Tests/Middleware/CompressionSecurityMiddlewareTests.cs` around lines 47 - 66, In the test CompressionSecurity_AnonymousUser_ShouldAllowCompression, after asserting StatusCode, add an assertion that the response does NOT contain the debug header "X-Compression-Disabled" to ensure middleware didn't disable compression for anonymous users; use the HttpResponseMessage (response) headers APIs (e.g., response.Headers.Contains("X-Compression-Disabled") or TryGetValues) to assert false/empty. Keep the existing header cleanup (HttpClient.DefaultRequestHeaders.Remove("Accept-Encoding")) as-is.src/Bootstrapper/MeAjudaAi.ApiService/Extensions/SecurityExtensions.cs (1)
600-607: Evite um extension method vazio com nome operacional.
UseSecurityHardening()não aplica nenhum middleware e hoje só devolveapp. Isso passa a impressão de que o hardening está centralizado aqui, quando a ordem real continua espalhada em outro ponto do pipeline. Ou mova o registro efetivo para este método, ou renomeie-o para deixar claro que ele é apenas um placeholder.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Bootstrapper/MeAjudaAi.ApiService/Extensions/SecurityExtensions.cs` around lines 600 - 607, The extension method UseSecurityHardening currently does nothing and misleads readers about where security hardening is applied; either move the actual middleware registrations into this method (e.g., register SecurityHeadersMiddleware and any other hardening middlewares here so the pipeline order is centralized) or rename the method to an explicit placeholder name (e.g., UseSecurityHardeningPlaceholder or UseNoOpSecurityHardening) and update the XML comment to reflect that it intentionally does not register middleware; adjust any callers (e.g., places that currently rely on UseApiMiddlewares) to call the new implementation or keep behavior consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Bootstrapper/MeAjudaAi.ApiService/Extensions/SecurityExtensions.cs`:
- Around line 568-570: Ajuste o fallback usado no cálculo de permitLimit para
alinhar com os patamares reais de AdvancedRateLimit (120 para autenticados, 30
para anônimos) em vez de 1000/100; localize o trecho que usa isAuthenticated e
configuration.GetValue("AdvancedRateLimit:Authenticated:RequestsPerMinute",
1000) / configuration.GetValue("AdvancedRateLimit:Anonymous:RequestsPerMinute",
100) (usado pelo GlobalLimiter) e altere os valores padrão para 120 e 30
respectivamente para manter comportamento consistente quando a seção
AdvancedRateLimit não estiver presente.
- Around line 298-299: A configuração atual apenas expõe o header antiforgery
via policy.WithExposedHeaders("X-XSRF-TOKEN") but não o inclui nos headers
permitidos do preflight; altere o trecho onde o CORS policy é construído
(referência: policy.WithExposedHeaders("X-XSRF-TOKEN") e à configuração
AllowedHeaders) para também adicionar "X-XSRF-TOKEN" aos headers permitidos
quando AllowedHeaders não for "*" — ou seja, ao aplicar a configuração de
AllowedHeaders, garanta que "X-XSRF-TOKEN" seja incluído (por exemplo
adicionando/mesclando com policy.WithHeaders(...) ou equivalente) para que o
preflight permita o header do SPA.
In `@tests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cs`:
- Around line 414-443: The detection of Result/Response types in
ReadJsonAsync<T> is fragile; replace the current isResultType logic (which uses
GetGenericTypeDefinition().Name.StartsWith) with a robust check: if
typeof(T).IsGenericType compare GetGenericTypeDefinition().Name exactly to
"Result`1" or "Response`1" (or compare the definition to your concrete generic
type if available), and also add a branch to treat non-generic response wrapper
types by checking typeof(T).Name.EndsWith("Response") or a known namespace
(e.g., Contracts.Functional) so plain types like UploadDocumentResponse are
unwrapped; keep using the existing JSON inspection and keep error handling via
BuildSafeResponsePreview and SerializationDefaults.Api unchanged.
---
Nitpick comments:
In `@src/Bootstrapper/MeAjudaAi.ApiService/Extensions/SecurityExtensions.cs`:
- Around line 600-607: The extension method UseSecurityHardening currently does
nothing and misleads readers about where security hardening is applied; either
move the actual middleware registrations into this method (e.g., register
SecurityHeadersMiddleware and any other hardening middlewares here so the
pipeline order is centralized) or rename the method to an explicit placeholder
name (e.g., UseSecurityHardeningPlaceholder or UseNoOpSecurityHardening) and
update the XML comment to reflect that it intentionally does not register
middleware; adjust any callers (e.g., places that currently rely on
UseApiMiddlewares) to call the new implementation or keep behavior consistent.
In
`@src/Bootstrapper/MeAjudaAi.ApiService/Middlewares/CompressionSecurityMiddleware.cs`:
- Around line 49-77: The IsSafeForCompression method in
CompressionSecurityMiddleware does not access any instance fields and should be
declared static for clarity; change its signature to static bool
IsSafeForCompression(HttpContext context) and update any internal calls (e.g.,
from InvokeAsync or other methods in CompressionSecurityMiddleware) to call the
static method directly, ensuring there are no references to instance state
inside it and updating any unit tests or callers accordingly.
- Around line 16-21: O campo _logger é injetado na classe
CompressionSecurityMiddleware (construtor CompressionSecurityMiddleware) mas não
está a ser usado; ou remova a dependência (_logger e o parâmetro ILogger no
construtor) ou passe a usá‑la no método InvokeAsync/Invoke para auditoria:
registe mensagens informativas/aviso quando a compressão for desabilitada (por
exemplo quando checa Accept-Encoding ou adiciona/retira headers de
Content-Encoding), inclua contexto relevante (Request.Path, razão da
desabilitação) e use níveis apropriados (Information/Warning) para facilitar
debugging em produção.
In
`@src/Modules/Communications/Application/Handlers/DocumentRejectedIntegrationEventHandler.cs`:
- Line 44: Padronize o formato de correlationId entre
DocumentRejectedIntegrationEventHandler e
DocumentVerifiedIntegrationEventHandler; escolha uma das abordagens sugeridas e
atualize a criação do correlationId dentro do método que constrói o evento (em
DocumentRejectedIntegrationEventHandler) para usar o mesmo padrão que o outro
handler (por exemplo, substituir var correlationId = $"{templateKey}:..."; por
um prefixo descritivo
$"document_rejected:{integrationEvent.DocumentId}:{integrationEvent.ProviderId}"
ou, alternativamente, alterar DocumentVerifiedIntegrationEventHandler para usar
templateKey em vez do literal), garantindo que ambos handlers usem o mesmo
formato para idempotência e atualize quaisquer testes ou consumidores que
dependam do formato antigo.
In
`@tests/MeAjudaAi.Integration.Tests/Middleware/CompressionSecurityMiddlewareTests.cs`:
- Around line 47-66: In the test
CompressionSecurity_AnonymousUser_ShouldAllowCompression, after asserting
StatusCode, add an assertion that the response does NOT contain the debug header
"X-Compression-Disabled" to ensure middleware didn't disable compression for
anonymous users; use the HttpResponseMessage (response) headers APIs (e.g.,
response.Headers.Contains("X-Compression-Disabled") or TryGetValues) to assert
false/empty. Keep the existing header cleanup
(HttpClient.DefaultRequestHeaders.Remove("Accept-Encoding")) as-is.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: afb2776a-f4c4-4b43-99d0-62ece6a24ccf
⛔ Files ignored due to path filters (1)
src/Modules/Users/Infrastructure/Persistence/Migrations/UsersDbContextModelSnapshot.csis excluded by!**/Migrations/**
📒 Files selected for processing (20)
src/Bootstrapper/MeAjudaAi.ApiService/Extensions/SecurityExtensions.cssrc/Bootstrapper/MeAjudaAi.ApiService/Middlewares/AntiforgeryCookieMiddleware.cssrc/Bootstrapper/MeAjudaAi.ApiService/Middlewares/CompressionSecurityMiddleware.cssrc/Contracts/Contracts/Modules/Communications/DTOs/OutboxPayloads.cssrc/Contracts/Shared/CommunicationTemplateKeys.cssrc/Modules/Communications/Application/Handlers/DocumentRejectedIntegrationEventHandler.cssrc/Modules/Communications/Application/Handlers/DocumentVerifiedIntegrationEventHandler.cssrc/Modules/Communications/Application/Services/OutboxProcessorService.cssrc/Modules/Communications/Infrastructure/Persistence/Configurations/OutboxMessageConfiguration.cssrc/Modules/Communications/Tests/Unit/Application/Handlers/DocumentRejectedIntegrationEventHandlerTests.cssrc/Modules/Communications/Tests/Unit/Application/Handlers/DocumentVerifiedIntegrationEventHandlerTests.cssrc/Modules/ServiceCatalogs/Infrastructure/Events/Handlers/ServiceActivatedDomainEventHandler.cssrc/Modules/Users/Tests/Integration/UserRepositoryIntegrationTests.cssrc/Shared/Database/Outbox/OutboxMessageConstraints.cssrc/Shared/Monitoring/BusinessMetrics.cstests/MeAjudaAi.ApiService.Tests/Unit/Middlewares/CompressionSecurityMiddlewareTests.cstests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cstests/MeAjudaAi.Integration.Tests/Middleware/CompressionSecurityMiddlewareTests.cstests/MeAjudaAi.Shared.Tests/TestInfrastructure/Base/BaseDatabaseTest.cstests/MeAjudaAi.Shared.Tests/Unit/Monitoring/BusinessMetricsTests.cs
✅ Files skipped from review due to trivial changes (3)
- src/Shared/Database/Outbox/OutboxMessageConstraints.cs
- src/Contracts/Shared/CommunicationTemplateKeys.cs
- src/Modules/Communications/Tests/Unit/Application/Handlers/DocumentVerifiedIntegrationEventHandlerTests.cs
🚧 Files skipped from review as they are similar to previous changes (4)
- src/Shared/Monitoring/BusinessMetrics.cs
- src/Modules/ServiceCatalogs/Infrastructure/Events/Handlers/ServiceActivatedDomainEventHandler.cs
- src/Bootstrapper/MeAjudaAi.ApiService/Middlewares/AntiforgeryCookieMiddleware.cs
- src/Modules/Communications/Tests/Unit/Application/Handlers/DocumentRejectedIntegrationEventHandlerTests.cs
- Robust type detection in BaseApiTest.ReadJsonAsync. - Standardized correlationId format across integration handlers. - Fixed CORS preflight to permit X-XSRF-TOKEN header. - Standardized all logs to English. - Cleaned up dead security hardening placeholder. - Verified system stability with all 710+ tests passing (Unit, Integration, E2E).
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Bootstrapper/MeAjudaAi.ApiService/Middlewares/CompressionSecurityMiddleware.cs (1)
55-61:⚠️ Potential issue | 🟠 MajorCobertura de headers sensíveis incompleta no critério de segurança.
O middleware verifica apenas
AuthorizationeX-API-Key(linhas 57-58). Embora a API não implemente autenticação por sessão/cookie, ela utiliza XSRF-TOKEN como proteção antiforgery. Se a requisição incluir o headerX-XSRF-TOKEN(ou o cookieXSRF-TOKEN), ela contém dados sensíveis que devem evitar compressão para proteção BREACH/CRIME.🔧 Ajuste sugerido
- if (request.Headers.ContainsKey("Authorization") || - request.Headers.ContainsKey("X-API-Key")) + if (request.Headers.ContainsKey("Authorization") || + request.Headers.ContainsKey("X-API-Key") || + request.Headers.ContainsKey("X-XSRF-TOKEN")) { return false; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Bootstrapper/MeAjudaAi.ApiService/Middlewares/CompressionSecurityMiddleware.cs` around lines 55 - 61, The compression security check in CompressionSecurityMiddleware currently only inspects Authorization and X-API-Key headers; update the check in the method that evaluates request compression eligibility to also detect the anti-forgery token by checking for the header "X-XSRF-TOKEN" and the cookie "XSRF-TOKEN" (i.e., treat requests containing either request.Headers.ContainsKey("X-XSRF-TOKEN") or request.Cookies.ContainsKey("XSRF-TOKEN") as authenticated/sensitive and return false), ensuring the same early-return logic used for Authorization/X-API-Key applies to these antiforgery indicators.
🧹 Nitpick comments (1)
src/Modules/Communications/Application/Handlers/DocumentRejectedIntegrationEventHandler.cs (1)
53-59: CentralizeSubject/Bodyno template para evitar drift de conteúdo.Como
TemplateKeyeTemplateDatajá estão presentes, manterSubjecteBodyhardcoded aqui aumenta risco de divergência entre handler e template versionado. Vale centralizar esses textos na camada de template e deixar o handler só com dados dinâmicos.Based on learnings: "Consider consolidating translations via a centralized logger or localization keys to maintain consistency and ease future localization."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Modules/Communications/Application/Handlers/DocumentRejectedIntegrationEventHandler.cs` around lines 53 - 59, The handler DocumentRejectedIntegrationEventHandler currently sets Subject and Body inline when constructing EmailOutboxPayload, which risks drift from the versioned template; remove the hardcoded Subject and Body from the EmailOutboxPayload construction and instead rely solely on TemplateKey and TemplateData (ensure templateData contains the dynamic values like provider name, document type, and reason). Update the EmailOutboxPayload invocation in the handler to omit Subject and Body, populate TemplateData keys (e.g., ProviderName, DocumentType, Reason) expected by the centralized template, and ensure the email template (referenced by templateKey) contains the localized/templated subject and body. Ensure any downstream consumer of EmailOutboxPayload handles missing Subject/Body or falls back to template rendering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@src/Bootstrapper/MeAjudaAi.ApiService/Middlewares/CompressionSecurityMiddleware.cs`:
- Around line 55-61: The compression security check in
CompressionSecurityMiddleware currently only inspects Authorization and
X-API-Key headers; update the check in the method that evaluates request
compression eligibility to also detect the anti-forgery token by checking for
the header "X-XSRF-TOKEN" and the cookie "XSRF-TOKEN" (i.e., treat requests
containing either request.Headers.ContainsKey("X-XSRF-TOKEN") or
request.Cookies.ContainsKey("XSRF-TOKEN") as authenticated/sensitive and return
false), ensuring the same early-return logic used for Authorization/X-API-Key
applies to these antiforgery indicators.
---
Nitpick comments:
In
`@src/Modules/Communications/Application/Handlers/DocumentRejectedIntegrationEventHandler.cs`:
- Around line 53-59: The handler DocumentRejectedIntegrationEventHandler
currently sets Subject and Body inline when constructing EmailOutboxPayload,
which risks drift from the versioned template; remove the hardcoded Subject and
Body from the EmailOutboxPayload construction and instead rely solely on
TemplateKey and TemplateData (ensure templateData contains the dynamic values
like provider name, document type, and reason). Update the EmailOutboxPayload
invocation in the handler to omit Subject and Body, populate TemplateData keys
(e.g., ProviderName, DocumentType, Reason) expected by the centralized template,
and ensure the email template (referenced by templateKey) contains the
localized/templated subject and body. Ensure any downstream consumer of
EmailOutboxPayload handles missing Subject/Body or falls back to template
rendering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5a9f27ba-bd13-44f7-8cd3-2c05df1846d3
📒 Files selected for processing (6)
src/Bootstrapper/MeAjudaAi.ApiService/Extensions/SecurityExtensions.cssrc/Bootstrapper/MeAjudaAi.ApiService/Middlewares/CompressionSecurityMiddleware.cssrc/Bootstrapper/MeAjudaAi.ApiService/Program.cssrc/Modules/Communications/Application/Handlers/DocumentRejectedIntegrationEventHandler.cstests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cstests/MeAjudaAi.Integration.Tests/Middleware/CompressionSecurityMiddlewareTests.cs
✅ Files skipped from review due to trivial changes (1)
- tests/MeAjudaAi.Integration.Tests/Base/BaseApiTest.cs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Bootstrapper/MeAjudaAi.ApiService/Program.cs
- tests/MeAjudaAi.Integration.Tests/Middleware/CompressionSecurityMiddlewareTests.cs
…rationTests to avoid constraint violations
Code Coverage Report
|
Minimum allowed line rate is |
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Context
This establishes infrastructure for post-MVP modules (Reviews, Payments, Bookings) that depend on communication capabilities.
Changes
docs/roadmap-current.md: +207 lines (Communications module section)docs/roadmap.md: +3 lines (status update)src/Shared/Utilities/Constants/ModuleNames.cs: +6 lines (Communications constant)Type
Summary by CodeRabbit