fix(invite): fix duplicate invite insert on db#17
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughController and use-case refactor: the controller now passes an invite token to the send-invite use case; the use case fetches the invite by token, throws when missing, and sends the email using invite-derived fields. Changes
Sequence DiagramsequenceDiagram
participant Controller
participant SendInviteUseCase
participant AdminRepository
participant EmailService
Controller->>SendInviteUseCase: execute({ invite_token })
SendInviteUseCase->>AdminRepository: getInviteByToken(invite_token)
AdminRepository-->>SendInviteUseCase: invite / null
alt invite == null
SendInviteUseCase-->>Controller: throw InviteNotFound (404)
else invite found
SendInviteUseCase->>EmailService: sendEmail({ to: invite.email, organization_id: invite.organization_id, expires_at: invite.expires_at })
EmailService-->>SendInviteUseCase: send result
SendInviteUseCase-->>Controller: { invite_id, email, expires_at }
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/core/use-cases/email/send-invite/index.ts (1)
15-33:⚠️ Potential issue | 🟠 MajorInconsistent
expires_atusage may mislead users.Line 15 calculates a new
expires_at(72 hours from now) and passes it tosendInviteEmail(line 27), but the return value usesinvite.expires_atfrom the database (line 33). This inconsistency means:
- The email tells the user the invite expires 72 hours from the email send time
- The actual invite expiration is whatever was stored when the invite was created
If the invite was created earlier (or has a different expiration policy), the email will contain incorrect expiration information.
🛠️ Suggested fix: use the stored expiration consistently
async execute({ invite_token, }: SendInviteUseCasePayload): Promise<SendInviteUseCaseReturn> { - const expires_at = new Date(Date.now() + 1000 * 60 * 60 * 72) - const invite = await this.adminRepository.getInviteByToken(invite_token) if (!invite) { throw new InviteNotFound() } await this.resendRepository.sendInviteEmail({ to: invite.email, token: invite.token, organization_id: invite.organization_id, - expires_at + expires_at: invite.expires_at }) return { invite_id: invite.id, email: invite.email, expires_at: invite.expires_at } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/use-cases/email/send-invite/index.ts` around lines 15 - 33, The code computes a new expires_at and sends it in sendInviteEmail but returns invite.expires_at, causing inconsistent expiration info; fix by using the stored invite.expires_at from the invite returned by getInviteByToken for both the email send and the return value (i.e., remove or stop using the newly computed expires_at and pass invite.expires_at into resendRepository.sendInviteEmail and the returned object), keeping references to invite, invite.expires_at, sendInviteEmail, and getInviteByToken to locate the change.
🧹 Nitpick comments (1)
src/core/domain/exceptions/invites.ts (1)
1-7: Consider relocatingControllerErrorto the domain layer as part of a broader refactoring.The domain exception
InviteNotFoundimportsControllerErrorfrom the adapter layer. While this follows the established pattern in the codebase (6+ other exception files use the same import), it does violate clean architecture principles where domain should not depend on adapters.Moving
ControllerErrorto@/core/domain/exceptions/baseand updating all exception imports would be a valuable refactoring to align the architecture properly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/domain/exceptions/invites.ts` around lines 1 - 7, The domain exception InviteNotFound currently imports ControllerError from the adapter layer; move the ControllerError class into the domain exceptions module (create a new base file, e.g., a ControllerError or BaseDomainError in `@/core/domain/exceptions/base`) and update InviteNotFound (and the other domain exception files that import ControllerError) to import from that new domain location; ensure the moved class preserves its API (constructor signature and exports) so classes like InviteNotFound (and the 6+ other exception classes) keep functioning unchanged while removing the dependency on the adapter layer.
🤖 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/core/use-cases/email/send-invite/index.ts`:
- Around line 15-33: The code computes a new expires_at and sends it in
sendInviteEmail but returns invite.expires_at, causing inconsistent expiration
info; fix by using the stored invite.expires_at from the invite returned by
getInviteByToken for both the email send and the return value (i.e., remove or
stop using the newly computed expires_at and pass invite.expires_at into
resendRepository.sendInviteEmail and the returned object), keeping references to
invite, invite.expires_at, sendInviteEmail, and getInviteByToken to locate the
change.
---
Nitpick comments:
In `@src/core/domain/exceptions/invites.ts`:
- Around line 1-7: The domain exception InviteNotFound currently imports
ControllerError from the adapter layer; move the ControllerError class into the
domain exceptions module (create a new base file, e.g., a ControllerError or
BaseDomainError in `@/core/domain/exceptions/base`) and update InviteNotFound (and
the other domain exception files that import ControllerError) to import from
that new domain location; ensure the moved class preserves its API (constructor
signature and exports) so classes like InviteNotFound (and the 6+ other
exception classes) keep functioning unchanged while removing the dependency on
the adapter layer.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 654be0c7-ec83-45f5-a70d-80051a9dd456
📒 Files selected for processing (4)
src/adapters/inbound/http/controllers/admin/create-and-send-invite/index.tssrc/core/domain/exceptions/invites.tssrc/core/use-cases/email/send-invite/index.tssrc/core/use-cases/email/send-invite/types.ts
2 participants
Summary by CodeRabbit
Bug Fixes
New Features