feat: Allow inviting people to projects / orgs who are not signed up on Swetrix#484
Merged
feat: Allow inviting people to projects / orgs who are not signed up on Swetrix#484
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds a pending-invitations system: new entity/service/module and DB migration, email templates, backend endpoints and redemption logic, integration into project/org invite flows and auth, frontend invitation signup route/page, and a user flag Changes
Sequence Diagram(s)sequenceDiagram
actor Inviter as Inviter (User)
participant Ctrl as Project/Organisation Controller
participant PIS as PendingInvitation Service
participant DB as Database
participant Mailer as Mailer Service
participant Email as Email Provider
Inviter->>Ctrl: invite(email)
Ctrl->>PIS: create pending invitation
PIS->>DB: insert pending_invitation
Ctrl->>Mailer: send unregistered-invite(email, url)
Mailer->>Email: send template
Email-->>Inviter: deliver invite email
actor Invitee as Invitee (New User)
participant Frontend as Frontend
participant AuthAPI as Auth Controller
participant AuthService as Auth Service
Invitee->>Frontend: click signup link (id)
Frontend->>AuthAPI: GET /invitation/:id
AuthAPI->>DB: read pending_invitation
AuthAPI-->>Frontend: return invitation metadata
Invitee->>Frontend: submit registration (email,password)
Frontend->>AuthAPI: POST /register/invitation
AuthAPI->>AuthService: create user
AuthService->>PIS: findByEmail & redeem invitations
AuthService->>DB: create ProjectShare / Org member as applicable
AuthService->>PIS: delete pending invitations
AuthAPI-->>Frontend: return JWT + user (registeredViaInvitation=true)
Frontend->>Invitee: redirect to dashboard
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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.
Actionable comments posted: 19
🧹 Nitpick comments (5)
backend/migrations/mysql/2026_03_09_pending_invitations.sql (1)
2-15: Missing FOREIGN KEY constraints - diverges from recent migration patterns.Per recent migrations (
2025_12_07_ai_chat.sql,2025_12_08_feature_flags.sql,2025_12_14_pinned_projects.sql), the established pattern includes FOREIGN KEY constraints withON DELETE CASCADEforprojectIdanduserIdreferences.Without FK constraints:
- Orphaned rows will remain if referenced projects/organisations/users are deleted
- No referential integrity enforcement
If intentional (e.g., invitations should persist after inviter deletion), please add a comment explaining this design decision. Otherwise, consider adding FK constraints after fixing the column length mismatch.
Example FK constraints (after fixing column lengths)
FOREIGN KEY (`projectId`) REFERENCES `project` (`id`) ON DELETE CASCADE, FOREIGN KEY (`organisationId`) REFERENCES `organisation` (`id`) ON DELETE CASCADE, FOREIGN KEY (`inviterId`) REFERENCES `user` (`id`) ON DELETE CASCADENote:
ON DELETE CASCADEmay not be appropriate for all cases. If invitations should persist after project deletion but be cleaned up when the inviter is deleted, useON DELETE SET NULLforprojectId/organisationId(requires nullable columns) andON DELETE CASCADEforinviterId.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/migrations/mysql/2026_03_09_pending_invitations.sql` around lines 2 - 15, The pending_issue is missing FOREIGN KEY constraints on pending_invitation which breaks referential integrity; update the pending_invitation table definition (table name: pending_invitation) to match referenced id column types (fix column lengths for projectId, organisationId, inviterId to the same type/length as project.id/organisation.id/user.id—likely varchar(36)), then add FOREIGN KEY clauses referencing project(id), organisation(id) and user(id) with appropriate ON DELETE behavior (e.g., ON DELETE CASCADE for inviterId, and either ON DELETE CASCADE or ON DELETE SET NULL for projectId/organisationId—make those columns NULLABLE if using SET NULL); alternatively, if persistence after deletion is intentional, add an inline comment in the migration explaining that design choice.backend/apps/cloud/src/pending-invitation/pending-invitation.entity.ts (1)
27-31: Consider adding constraint: projectId XOR organisationId based on type.For
PROJECT_SHAREinvitations,projectIdshould be non-null andorganisationIdshould be null (and vice versa forORGANISATION_MEMBER). Currently, both are nullable with no database-level constraint enforcing this relationship.While application logic may enforce this, a CHECK constraint would provide defense-in-depth against data inconsistencies.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/cloud/src/pending-invitation/pending-invitation.entity.ts` around lines 27 - 31, Add a DB-level CHECK constraint to the PendingInvitation entity to enforce that projectId and organisationId are mutually exclusive based on the invitation type: update the entity (class PendingInvitation) to include a CHECK (via TypeORM's `@Check` decorator or equivalent) that requires (type = 'PROJECT_SHARE' AND projectId IS NOT NULL AND organisationId IS NULL) OR (type = 'ORGANISATION_MEMBER' AND organisationId IS NOT NULL AND projectId IS NULL); if your ORM/version doesn't support `@Check` on the entity, create a migration that adds the equivalent SQL CHECK constraint referencing the projectId and organisationId columns to enforce the XOR invariant.backend/apps/cloud/src/auth/auth.controller.ts (2)
576-578: Note: Rate limit is shared between regular and invitation registration.Both
/registerand/register/invitationuse the same rate limit key ('register'), meaning 5 total attempts per IP across both endpoints in 24 hours. This prevents bypassing limits but could block legitimate invitation users if their IP has exhausted attempts via regular registration.Consider using a distinct key like
'register-invitation'if invitation-based registration should have independent limits.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/cloud/src/auth/auth.controller.ts` around lines 576 - 578, The rate limit call uses the shared key 'register' which makes /register and /register/invitation share the same allowance; update the call in the invitation registration flow to use a distinct key (e.g., 'register-invitation') so it has its own quota. Locate the invocation of checkRateLimit (function name) where ip is computed via getIPFromHeaders and change the second argument from 'register' to a separate string like 'register-invitation'; ensure any tests or documentation that reference the rate-limit key are updated accordingly.
154-167: Consider adding a null check forupdatedUser.While
findUserByIdshould succeed for a just-created user, the code directly accessesupdatedUser.sharedProjectswithout a null check. If the database query fails for any reason, this would throw a runtime error.🛡️ Defensive fix
const updatedUser = redeemedCount > 0 ? await this.userService.findUserById(newUser.id) : newUser - if (redeemedCount > 0) { + if (redeemedCount > 0 && updatedUser) { const [sharedProjects, organisationMemberships] = await Promise.all([🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/cloud/src/auth/auth.controller.ts` around lines 154 - 167, The code assumes updatedUser from userService.findUserById is non-null before assigning sharedProjects and organisationMemberships; add a null check after the await (in auth.controller.ts around the updatedUser assignment) and handle the null case (either set updatedUser = newUser as a safe fallback or throw/log and return an error response) before calling this.authService.getSharedProjectsForUser and this.userService.getOrganisationsForUser and before assigning updatedUser.sharedProjects/updatedUser.organisationMemberships to avoid runtime exceptions.web/app/routes/signup_.invitation.$id.tsx (1)
119-121: Basic email validation could allow invalid formats.The check
!email.includes('@')permits invalid emails like"@"or"a@b". While the backend likely performs stricter validation, improving frontend validation provides better UX with immediate feedback.♻️ More robust email check
- if (!email || !email.includes('@')) { + const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/ + if (!email || !emailRegex.test(email)) { fieldErrors.email = 'Please enter a valid email address' }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/routes/signup_.invitation`.$id.tsx around lines 119 - 121, The current email check using `!email || !email.includes('@')` (in the signup invitation handler where `email` is validated and `fieldErrors.email` is set) is too permissive; replace it with a stronger validation (e.g., test `email` against a simple robust regex such as one that requires non-space local part, an '@', and a domain with a dot like /^[^\s@]+@[^\s@]+\.[^\s@]+$/ or use a validation helper/library) and update the conditional that sets `fieldErrors.email` so only truly invalid formats trigger the error message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/apps/cloud/src/auth/auth.controller.ts`:
- Around line 663-672: The code calls userService.findUserById and then
unconditionally assigns updatedUser.sharedProjects and
updatedUser.organisationMemberships, but findUserById may return null; guard
against a null updatedUser by checking its existence before assigning (e.g., if
(!updatedUser) { handle/error/throw }): locate the updatedUser variable and the
call to userService.findUserById, and ensure you either throw or return an
appropriate error/response when updatedUser is null before using
authService.getSharedProjectsForUser and userService.getOrganisationsForUser
results or move those calls after the null check so you only assign properties
on a non-null updatedUser.
In `@backend/apps/cloud/src/auth/auth.service.ts`:
- Around line 208-225: The duplicate-share check fails because
projectService.findOne only loads relations: ['share'] but not the nested
share.user; update the findOne call used here (where pending.projectId is
loaded) to include the nested relation for users (e.g., relations: ['share',
'share.user']) so that the check using project.share?.some(s => s.user?.id ===
user.id) can correctly detect existing ProjectShare entries; ensure ProjectShare
and methods like this.projectService.findOne and this.projectService.createShare
remain unchanged except for adding the nested relation.
In `@backend/apps/cloud/src/auth/dtos/register-invitation.dto.ts`:
- Around line 28-35: The ApiProperty for the password declares maxLength: 72 but
the validator uses `@MaxLength`(50), causing a mismatch; update the `@MaxLength`
decorator on the password property (and its validation message) to 72 to match
ApiProperty (or alternatively change ApiProperty.maxLength to 50) so the Swagger
contract and runtime validation align, ensuring you modify the `@MaxLength`(...)
value and its message string where the decorator is applied to the password
property in register-invitation DTO.
In
`@backend/apps/cloud/src/common/templates/en/organisation-invitation-unregistered.html`:
- Around line 11-14: The fallback plain-text URL in the
organisation-invitation-unregistered.html template should be made clickable:
replace the raw {{url}} text with an anchor using the same {{url}} for href
(e.g., <a href="{{url}}">...</a>), and include safe attributes like
target="_blank" and rel="noopener noreferrer" so the fallback link remains
usable when the main CTA is stripped; update the block containing the "If you
are having trouble..." copy to use that anchor around {{url}}.
In `@backend/apps/cloud/src/organisation/organisation.controller.ts`:
- Around line 218-254: The current flow creates a PendingInvitation via
pendingInvitationService.create then does mailerService.sendEmail inside the
same try/catch, which can leave a dangling DB record if send fails and also can
mis-report success; change it to first create the pendingInvitation, then wrap
only the mail send in a try/catch: call mailerService.sendEmail(...) and on
failure call pendingInvitationService.delete(pendingInvitation.id) (or
pendingInvitationService.remove/purge equivalent) to rollback the created
invitation, log the error with reason and email/orgId, then throw the
BadRequestException; only after a successful send call
organisationService.findOne(...) and return that result. Ensure you reference
pendingInvitation.id when deleting and keep logging using this.logger.error with
{ orgId: organisation?.id, email: inviteDTO.email, reason }.
In `@backend/apps/cloud/src/pending-invitation/pending-invitation.entity.ts`:
- Around line 33-34: The role column in PendingInvitation (role: string in
pending-invitation.entity.ts) is untyped and allows invalid values; update the
code to either (A) change the entity to use a DB enum column mapped to the
appropriate union of enums (or separate columns) so only valid Role /
OrganisationRole values can be persisted, or (B) add explicit validation inside
auth.service.ts (redeemPendingInvitations) before casting: check pending.role
against Object.values(Role) for project invitations and
Object.values(OrganisationRole) for organisation invitations, log/warn and skip
or throw on invalid values, then safely cast to Role or OrganisationRole for
assignment to ProjectShare.role / OrganisationMember.role; reference Role
(project-share.entity) and OrganisationRole (organisation-member.entity) when
implementing validation or changing the entity mapping.
In `@backend/apps/cloud/src/pending-invitation/pending-invitation.service.ts`:
- Around line 14-16: Add DB-level UNIQUE constraints in the migration for
pending_invitation: UNIQUE(email, projectId, type) and UNIQUE(email,
organisationId, type) to prevent race-condition duplicates, and update the
PendingInvitationService.create method (the async create(data:
Partial<PendingInvitation>): Promise<PendingInvitation> that currently calls
this.repository.save(data)) to handle constraint violations: catch DB errors
from repository.save, detect duplicate-key errors (e.g., MySQL ER_DUP_ENTRY or
Postgres 23505), and convert them into a graceful response (return the existing
pending invitation, noop, or throw a dedicated DuplicatePendingInvitationError)
instead of allowing duplicate inserts; alternatively implement an atomic upsert
via the ORM/query builder (INSERT ... ON DUPLICATE KEY UPDATE / ON CONFLICT DO
NOTHING) to avoid the race entirely.
In `@backend/apps/cloud/src/project/project.controller.ts`:
- Around line 1060-1094: The code creates a pending invite via
pendingInvitationService.create and then calls mailerService.sendEmail, but on
send failure it returns BadRequestException(reason) (leaking internal details)
and leaves the pending invite in DB; update the try/catch around the send flow
so that if mailerService.sendEmail throws you delete the created invite (use
pendingInvitation.id via pendingInvitationService.delete or remove method) and
log the internal error locally (console.error or processLogger.error) but throw
a generic BadRequestException('Could not send invitation') (or similar fixed
message) instead of BadRequestException(reason); also defensively check
pendingInvitation exists before attempting to delete and continue to return
processProjectUser(updatedProject) on success by keeping projectService.findOne
and processProjectUser usage as-is.
In `@backend/migrations/mysql/2026_03_09_pending_invitations.sql`:
- Around line 2-15: The pending_invitation table's FK target lengths don't match
referenced tables: change column definitions in pending_invitation — set
projectId to varchar(12), organisationId to varchar(36), and inviterId to
varchar(36) (preserve NULL/default behavior for projectId/organisationId and NOT
NULL for inviterId as appropriate), then add FOREIGN KEY constraints referencing
project(id), organisation(id), and user(id) respectively and keep the existing
indexes (IDX_pending_invitation_project / IDX_pending_invitation_organisation /
email index) to enable referential integrity and prevent invalid IDs from being
stored.
In `@docs/content/docs/sitesettings/how-to-invite-users-to-your-website.mdx`:
- Line 43: The sentence "If an invited user wants to create their own personal
projects later on, they will need to start a free trial and subscribe to a
plan." overstates requirements; update that sentence (the invited-user personal
project line) to state that starting a free trial is sufficient because the
backend only blocks PlanCode.none — e.g., change wording to indicate a free
trial (or any non-none plan) allows creating personal projects rather than
requiring an immediate subscription.
In `@web/app/api/api.server.ts`:
- Around line 506-540: The invitation registration path (registerViaInvitation)
currently forces persistent cookies by calling createAuthCookies(..., true);
change it to respect the caller's session policy like registerUser does — either
accept a remember/rememberSession boolean on the data payload and pass that
through to createAuthCookies, or explicitly use false if invitations must always
be session-scoped; update the registerViaInvitation signature and the call-site
to use that boolean (referencing registerViaInvitation, createAuthCookies and
registerUser to mirror behavior).
In `@web/app/pages/Dashboard/AddProject.tsx`:
- Around line 38-42: Tailwind dark/group-hover variants are reversed in the
className strings inside AddProject.tsx: replace occurrences of
"group-hover:dark:text-gray-300" with "dark:group-hover:text-gray-300" (both in
the icon's class list that contains "dark:text-gray-200
group-hover:dark:text-gray-300" and in the span with className 'mt-2 block
text-sm font-semibold text-gray-900 dark:text-gray-50
group-hover:dark:text-gray-300') so the selectors compile as .dark .group:hover
… and work with root-level dark mode.
In `@web/app/pages/Dashboard/Dashboard.tsx`:
- Around line 248-255: The subscription gate in Dashboard.tsx currently blocks
users with planCode === 'none' even if they were invited; update the if
condition that checks isSelfhosted, user?.planCode === 'none', and
!newProjectOrganisationId to also allow users where user.registeredViaInvitation
is true (i.e. add && !user?.registeredViaInvitation or otherwise exclude invited
users from the check), so invited signups are exempt and won’t trigger
toast.error(t('project.settings.subscriptionRequired')) or return.
In `@web/app/routes/signup_.invitation`.$id.tsx:
- Around line 69-91: claimInvitation failures are currently ignored which lets
users be redirected to dashboard even if claiming the invite failed; update the
block that calls claimInvitation to await and capture the result or catch thrown
errors from claimInvitation (referencing claimInvitation and authResult in the
existing branch), and if it fails log the error and surface it to the user
before redirecting (for example set a flash message or redirect to an
error/onboarding flow instead of immediately redirecting to '/dashboard'),
making sure to preserve cookies via createHeadersWithCookies when redirecting
and avoid proceeding to redirect('/dashboard') on claim failure.
- Around line 139-144: The code uses a non-null assertion on id when calling
registerViaInvitation (registerViaInvitation(request, { pendingInvitationId:
id!, ... })), which can throw at runtime; instead validate params.id (the id
variable derived from params) before calling registerViaInvitation and handle
the missing case explicitly (e.g., return a 400/throw a controlled error).
Update the signup_.invitation.$id route handler to check that id is a defined
non-empty string and only pass it to registerViaInvitation if valid, otherwise
short-circuit with an appropriate response or error; ensure any callers or error
messages reference pendingInvitationId and registerViaInvitation for clarity.
In `@web/app/routes/subscribe.tsx`:
- Around line 58-61: The conditional in subscribe.tsx is using
user.registeredViaInvitation as a permanent entitlement check; change it so the
redirect depends on current access state (e.g., check user.planCode and actual
memberships/access such as user.sharedProjects.length, user.orgs.length, or an
explicit user.hasActiveAccess flag) instead of the signup-origin flag, or
alternatively clear user.registeredViaInvitation at the end of the invitation
onboarding (e.g., in the invite completion handler/completeOnboarding function)
so it no longer causes future redirects; update the conditional that references
user.registeredViaInvitation accordingly and ensure any onboarding flow updates
the user record to remove registeredViaInvitation once finished.
In `@web/public/locales/de.json`:
- Around line 729-736: The German translations reuse the single {{type}} token
across phrases with different grammatical cases, causing awkward forms like
"Konto erstellen & Projekt beitreten"; update the locale to provide separate,
case-appropriate strings instead of a shared {{type}} token by replacing usages
of "invitedToJoin", "invitedByAs" and "createAndJoin" to reference specific keys
(e.g., "project" and "organisation") or new keyed variants (e.g.,
"projectAccusative"/"organisationAccusative") and adjust the templates to
interpolate the correct key for each context so each phrase uses the
grammatically correct form.
In `@web/public/locales/en.json`:
- Around line 729-736: The page metadata in organisation.invite.$id.tsx
currently uses the project-specific titles.invitation ("Shared project
invitation"); update the metadata generation in the route (the exported
loader/metadata or default export that sets page title — look for where
titles.invitation is referenced) to use the neutral auth.invitation keys or
compute the title from the invitation.type (project vs organisation) so the
title matches auth.invitation entries; replace the hardcoded titles.invitation
usage with a dynamic lookup that selects auth.invitation.* or a single neutral
key like auth.invitation.invitedToJoin depending on the invitation payload.
In `@web/public/locales/uk.json`:
- Around line 729-736: The Ukrainian invitation strings use a shared {{type}}
token which causes incorrect case/inflection for different entities; update the
keys so each grammatical form is explicit: replace the combined "invitedToJoin"
/ "invitedByAs" usages that rely on {{type}} with separate keys such as
"invitedToJoinProject", "invitedToJoinOrganisation", "invitedByAsProject",
"invitedByAsOrganisation" (or other clear names), and update "createAndJoin"
similarly (e.g., "createAndJoinProject" / "createAndJoinOrganisation"), keeping
or removing the generic "project" and "organisation" tokens; then update any
code that references the old keys to use the new entity-specific keys so the
correct Ukrainian inflections are used.
---
Nitpick comments:
In `@backend/apps/cloud/src/auth/auth.controller.ts`:
- Around line 576-578: The rate limit call uses the shared key 'register' which
makes /register and /register/invitation share the same allowance; update the
call in the invitation registration flow to use a distinct key (e.g.,
'register-invitation') so it has its own quota. Locate the invocation of
checkRateLimit (function name) where ip is computed via getIPFromHeaders and
change the second argument from 'register' to a separate string like
'register-invitation'; ensure any tests or documentation that reference the
rate-limit key are updated accordingly.
- Around line 154-167: The code assumes updatedUser from
userService.findUserById is non-null before assigning sharedProjects and
organisationMemberships; add a null check after the await (in auth.controller.ts
around the updatedUser assignment) and handle the null case (either set
updatedUser = newUser as a safe fallback or throw/log and return an error
response) before calling this.authService.getSharedProjectsForUser and
this.userService.getOrganisationsForUser and before assigning
updatedUser.sharedProjects/updatedUser.organisationMemberships to avoid runtime
exceptions.
In `@backend/apps/cloud/src/pending-invitation/pending-invitation.entity.ts`:
- Around line 27-31: Add a DB-level CHECK constraint to the PendingInvitation
entity to enforce that projectId and organisationId are mutually exclusive based
on the invitation type: update the entity (class PendingInvitation) to include a
CHECK (via TypeORM's `@Check` decorator or equivalent) that requires (type =
'PROJECT_SHARE' AND projectId IS NOT NULL AND organisationId IS NULL) OR (type =
'ORGANISATION_MEMBER' AND organisationId IS NOT NULL AND projectId IS NULL); if
your ORM/version doesn't support `@Check` on the entity, create a migration that
adds the equivalent SQL CHECK constraint referencing the projectId and
organisationId columns to enforce the XOR invariant.
In `@backend/migrations/mysql/2026_03_09_pending_invitations.sql`:
- Around line 2-15: The pending_issue is missing FOREIGN KEY constraints on
pending_invitation which breaks referential integrity; update the
pending_invitation table definition (table name: pending_invitation) to match
referenced id column types (fix column lengths for projectId, organisationId,
inviterId to the same type/length as project.id/organisation.id/user.id—likely
varchar(36)), then add FOREIGN KEY clauses referencing project(id),
organisation(id) and user(id) with appropriate ON DELETE behavior (e.g., ON
DELETE CASCADE for inviterId, and either ON DELETE CASCADE or ON DELETE SET NULL
for projectId/organisationId—make those columns NULLABLE if using SET NULL);
alternatively, if persistence after deletion is intentional, add an inline
comment in the migration explaining that design choice.
In `@web/app/routes/signup_.invitation`.$id.tsx:
- Around line 119-121: The current email check using `!email ||
!email.includes('@')` (in the signup invitation handler where `email` is
validated and `fieldErrors.email` is set) is too permissive; replace it with a
stronger validation (e.g., test `email` against a simple robust regex such as
one that requires non-space local part, an '@', and a domain with a dot like
/^[^\s@]+@[^\s@]+\.[^\s@]+$/ or use a validation helper/library) and update the
conditional that sets `fieldErrors.email` so only truly invalid formats trigger
the error message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 924e5b1f-8bc4-4858-910f-69cc5fde08df
📒 Files selected for processing (38)
backend/apps/cloud/src/app.module.tsbackend/apps/cloud/src/auth/auth.controller.tsbackend/apps/cloud/src/auth/auth.module.tsbackend/apps/cloud/src/auth/auth.service.tsbackend/apps/cloud/src/auth/dtos/index.tsbackend/apps/cloud/src/auth/dtos/register-invitation.dto.tsbackend/apps/cloud/src/common/templates/en/organisation-invitation-unregistered.htmlbackend/apps/cloud/src/common/templates/en/project-invitation-unregistered.htmlbackend/apps/cloud/src/mailer/letter.tsbackend/apps/cloud/src/mailer/mailer.service.tsbackend/apps/cloud/src/organisation/organisation.controller.tsbackend/apps/cloud/src/organisation/organisation.module.tsbackend/apps/cloud/src/pending-invitation/pending-invitation.entity.tsbackend/apps/cloud/src/pending-invitation/pending-invitation.module.tsbackend/apps/cloud/src/pending-invitation/pending-invitation.service.tsbackend/apps/cloud/src/project/project.controller.tsbackend/apps/cloud/src/project/project.module.tsbackend/apps/cloud/src/user/entities/user.entity.tsbackend/migrations/mysql/2026_03_09_pending_invitations.sqldocs/content/docs/sitesettings/how-to-invite-users-to-your-website.mdxweb/app/api/api.server.tsweb/app/lib/models/User.tsweb/app/pages/Auth/Signup/InvitationSignup.tsxweb/app/pages/Dashboard/AddProject.tsxweb/app/pages/Dashboard/Dashboard.tsxweb/app/routes/dashboard.tsxweb/app/routes/login.tsxweb/app/routes/signup_.invitation.$id.tsxweb/app/routes/subscribe.tsxweb/app/ui/Input.tsxweb/app/ui/Text.tsxweb/app/utils/auth.tsweb/app/utils/routes.tsweb/public/locales/de.jsonweb/public/locales/en.jsonweb/public/locales/fr.jsonweb/public/locales/pl.jsonweb/public/locales/uk.json
Comment on lines
+663
to
+672
| const updatedUser = await this.userService.findUserById(newUser.id) | ||
|
|
||
| const [sharedProjects, organisationMemberships] = await Promise.all([ | ||
| this.authService.getSharedProjectsForUser(newUser.id), | ||
| this.userService.getOrganisationsForUser(newUser.id), | ||
| ]) | ||
|
|
||
| updatedUser.sharedProjects = sharedProjects | ||
| updatedUser.organisationMemberships = organisationMemberships | ||
|
|
There was a problem hiding this comment.
Potential null reference: updatedUser could be null.
findUserById may return null (e.g., in case of a database issue), but lines 670-671 unconditionally assign properties to updatedUser. This would throw a runtime error.
🛡️ Proposed fix
const updatedUser = await this.userService.findUserById(newUser.id)
+ if (!updatedUser) {
+ throw new ConflictException('Failed to retrieve user after registration')
+ }
+
const [sharedProjects, organisationMemberships] = await Promise.all([
this.authService.getSharedProjectsForUser(newUser.id),
this.userService.getOrganisationsForUser(newUser.id),
])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const updatedUser = await this.userService.findUserById(newUser.id) | |
| const [sharedProjects, organisationMemberships] = await Promise.all([ | |
| this.authService.getSharedProjectsForUser(newUser.id), | |
| this.userService.getOrganisationsForUser(newUser.id), | |
| ]) | |
| updatedUser.sharedProjects = sharedProjects | |
| updatedUser.organisationMemberships = organisationMemberships | |
| const updatedUser = await this.userService.findUserById(newUser.id) | |
| if (!updatedUser) { | |
| throw new ConflictException('Failed to retrieve user after registration') | |
| } | |
| const [sharedProjects, organisationMemberships] = await Promise.all([ | |
| this.authService.getSharedProjectsForUser(newUser.id), | |
| this.userService.getOrganisationsForUser(newUser.id), | |
| ]) | |
| updatedUser.sharedProjects = sharedProjects | |
| updatedUser.organisationMemberships = organisationMemberships |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/apps/cloud/src/auth/auth.controller.ts` around lines 663 - 672, The
code calls userService.findUserById and then unconditionally assigns
updatedUser.sharedProjects and updatedUser.organisationMemberships, but
findUserById may return null; guard against a null updatedUser by checking its
existence before assigning (e.g., if (!updatedUser) { handle/error/throw }):
locate the updatedUser variable and the call to userService.findUserById, and
ensure you either throw or return an appropriate error/response when updatedUser
is null before using authService.getSharedProjectsForUser and
userService.getOrganisationsForUser results or move those calls after the null
check so you only assign properties on a non-null updatedUser.
Comment on lines
+208
to
+225
| const project = await this.projectService.findOne({ | ||
| where: { id: pending.projectId }, | ||
| relations: ['share'], | ||
| }) | ||
|
|
||
| if (project) { | ||
| const alreadyShared = project.share?.some( | ||
| (s) => s.user?.id === user.id, | ||
| ) | ||
|
|
||
| if (!alreadyShared) { | ||
| const share = new ProjectShare() | ||
| share.role = pending.role as any | ||
| share.user = user | ||
| share.project = project | ||
| share.confirmed = true | ||
|
|
||
| await this.projectService.createShare(share) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ProjectShare entity relation config =="
fd 'project-share.entity.ts$' | while read -r file; do
echo "-- $file"
rg -n -C3 'ManyToOne|OneToMany|eager|user' "$file"
done
echo
echo "== Call sites that rely on share.user or explicit share relations =="
rg -n -C3 "share\\.user\\?\\.id|relations: \\[[^]]*share|share\\.user" backend/apps/cloud/srcRepository: Swetrix/swetrix
Length of output: 12168
Load share.user when checking for duplicate shares.
The ProjectShare.user relation is not eager-loaded. The query at line 210 loads relations: ['share'] but the code at line 214-217 accesses s.user?.id without loading the nested user. This check will always fail to detect existing shares, allowing duplicates.
All other call sites in the codebase explicitly load share.user (e.g., project.controller.ts:1085, project.service.ts:247). This code must do the same.
🛠️ Proposed fix
const project = await this.projectService.findOne({
where: { id: pending.projectId },
- relations: ['share'],
+ relations: ['share', 'share.user'],
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const project = await this.projectService.findOne({ | |
| where: { id: pending.projectId }, | |
| relations: ['share'], | |
| }) | |
| if (project) { | |
| const alreadyShared = project.share?.some( | |
| (s) => s.user?.id === user.id, | |
| ) | |
| if (!alreadyShared) { | |
| const share = new ProjectShare() | |
| share.role = pending.role as any | |
| share.user = user | |
| share.project = project | |
| share.confirmed = true | |
| await this.projectService.createShare(share) | |
| const project = await this.projectService.findOne({ | |
| where: { id: pending.projectId }, | |
| relations: ['share', 'share.user'], | |
| }) | |
| if (project) { | |
| const alreadyShared = project.share?.some( | |
| (s) => s.user?.id === user.id, | |
| ) | |
| if (!alreadyShared) { | |
| const share = new ProjectShare() | |
| share.role = pending.role as any | |
| share.user = user | |
| share.project = project | |
| share.confirmed = true | |
| await this.projectService.createShare(share) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/apps/cloud/src/auth/auth.service.ts` around lines 208 - 225, The
duplicate-share check fails because projectService.findOne only loads relations:
['share'] but not the nested share.user; update the findOne call used here
(where pending.projectId is loaded) to include the nested relation for users
(e.g., relations: ['share', 'share.user']) so that the check using
project.share?.some(s => s.user?.id === user.id) can correctly detect existing
ProjectShare entries; ensure ProjectShare and methods like
this.projectService.findOne and this.projectService.createShare remain unchanged
except for adding the nested relation.
backend/apps/cloud/src/common/templates/en/organisation-invitation-unregistered.html
Show resolved
Hide resolved
Comment on lines
+218
to
+254
| try { | ||
| const pendingInvitation = await this.pendingInvitationService.create({ | ||
| email: inviteDTO.email, | ||
| type: PendingInvitationType.ORGANISATION_MEMBER, | ||
| organisationId: orgId, | ||
| role: inviteDTO.role, | ||
| inviterId: userId, | ||
| }) | ||
|
|
||
| const origin = | ||
| isDevelopment && typeof headers?.origin === 'string' | ||
| ? headers.origin | ||
| : PRODUCTION_ORIGIN | ||
| const url = `${origin}/signup/invitation/${pendingInvitation.id}?email=${encodeURIComponent(inviteDTO.email)}` | ||
|
|
||
| await this.mailerService.sendEmail( | ||
| inviteDTO.email, | ||
| LetterTemplate.OrganisationInvitationUnregistered, | ||
| { | ||
| url, | ||
| email: user.email, | ||
| name: organisation.name, | ||
| role: inviteDTO.role, | ||
| }, | ||
| ) | ||
|
|
||
| return await this.organisationService.findOne({ | ||
| where: { id: orgId }, | ||
| relations: ['members', 'members.user'], | ||
| }) | ||
| } catch (reason) { | ||
| this.logger.error( | ||
| { orgId: organisation?.id, email: inviteDTO.email, reason }, | ||
| 'Could not create pending invitation for organisation', | ||
| ) | ||
| throw new BadRequestException('Failed to invite member to organisation') | ||
| } |
There was a problem hiding this comment.
Rollback only pre-send failures in the unregistered invite flow.
create() persists immediately, so a mailer failure leaves a dangling pending invitation that blocks every retry via existingPending. Keeping the post-send return path in the same try also means a successfully sent email can still surface as a failed invite.
🛠️ Proposed fix
- try {
- const pendingInvitation = await this.pendingInvitationService.create({
+ let pendingInvitation
+ try {
+ pendingInvitation = await this.pendingInvitationService.create({
email: inviteDTO.email,
type: PendingInvitationType.ORGANISATION_MEMBER,
organisationId: orgId,
role: inviteDTO.role,
inviterId: userId,
@@
await this.mailerService.sendEmail(
inviteDTO.email,
LetterTemplate.OrganisationInvitationUnregistered,
{
url,
@@
role: inviteDTO.role,
},
)
-
- return await this.organisationService.findOne({
- where: { id: orgId },
- relations: ['members', 'members.user'],
- })
} catch (reason) {
+ if (pendingInvitation?.id) {
+ await this.pendingInvitationService
+ .delete(pendingInvitation.id)
+ .catch(() => undefined)
+ }
this.logger.error(
{ orgId: organisation?.id, email: inviteDTO.email, reason },
'Could not create pending invitation for organisation',
)
throw new BadRequestException('Failed to invite member to organisation')
}
+
+ return organisation🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/apps/cloud/src/organisation/organisation.controller.ts` around lines
218 - 254, The current flow creates a PendingInvitation via
pendingInvitationService.create then does mailerService.sendEmail inside the
same try/catch, which can leave a dangling DB record if send fails and also can
mis-report success; change it to first create the pendingInvitation, then wrap
only the mail send in a try/catch: call mailerService.sendEmail(...) and on
failure call pendingInvitationService.delete(pendingInvitation.id) (or
pendingInvitationService.remove/purge equivalent) to rollback the created
invitation, log the error with reason and email/orgId, then throw the
BadRequestException; only after a successful send call
organisationService.findOne(...) and return that result. Ensure you reference
pendingInvitation.id when deleting and keep logging using this.logger.error with
{ orgId: organisation?.id, email: inviteDTO.email, reason }.
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.
Changes
If applicable, please describe what changes were made in this pull request.
Community Edition support
Database migrations
Documentation
Summary by CodeRabbit
New Features
Documentation