-
-
Notifications
You must be signed in to change notification settings - Fork 12
feat: invite user to team/org #358
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…epted notification
WalkthroughIntroduces an invitations feature end-to-end: backend models, storage, service, controller, routes, email templates, and public auth bypass for accept endpoint. Adds API base URL config. Frontend switches team management to invitation flow, updates hooks, components, Redux endpoints/types, and translations. Includes DB migrations and version timestamp update. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User (Owner/Admin)
participant V as Frontend (View)
participant A as API Controller<br/>(Invitations)
participant S as Service<br/>(Invitations)
participant ST as Store<br/>(Invitations)
participant E as Email Manager
participant DB as DB
U->>V: Fill name, email, role
V->>A: POST /api/v1/invitations {email,name,role}
A->>S: CreateInvite(inviterID, req)
S->>DB: Lookup/Create user, role, org
S->>ST: GetInvitationByUserAndOrg
alt existing invite
S->>ST: UpdateInvitationForReinvite
else new invite
S->>ST: CreateInvitation
end
S->>E: Send invitation email (AcceptURL, temp password?)
E->>DB: Load SMTP/template
E-->>S: ok/error
S-->>A: Invitation + token (masked)
A-->>V: 200 {data: invitation}
V-->>U: Toast: Invitation sent
sequenceDiagram
autonumber
actor I as Invitee
participant V as Browser
participant A as API Controller<br/>(Public Accept)
participant S as Service<br/>(Invitations)
participant ST as Store
participant DB as DB
participant N as Notification Manager
I->>V: Click AcceptURL?token=...
V->>A: GET /api/v1/invitations/accept?token=...
note over A: Public endpoint (auth bypass)
A->>S: AcceptInvite(token)
S->>ST: GetInvitationByToken
S->>DB: Validate expiry, resolve role
S->>ST: AddUserToOrganization
S->>ST: MarkAccepted
S->>N: Notify inviter (accepted)
S-->>A: {status:"accepted"}
A-->>V: 200 {data:{status}}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
@coderabbitai: Add title and description |
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed:
Length of output: 313530 @zhravan I'll help you create a comprehensive title and description for this PR. Suggested PR Title:
Suggested PR Description:## Summary
This PR implements a complete invitation system that allows organization administrators to invite new users to their teams/organizations via email. Users receive an invitation email with credentials and can accept the invitation to join the organization with the specified role.
## Features Added
- 🎫 **Invitation Management**: Create, send, and track user invitations
- 📧 **Email Notifications**: Automated invitation emails with temporary credentials
- 🔐 **Secure Token-Based Acceptance**: Time-limited invitation tokens (72 hours)
- 👥 **Role-Based Invitations**: Assign roles to users during invitation
- 🔄 **Re-invitation Support**: Update existing invitations with new tokens
- 🎨 **UI Components**: Frontend integration for team member management
## Implementation Details
### Backend Changes
- **New API Endpoints**:
- `POST /api/v1/invitations` - Create new invitation
- `GET /api/v1/invitations/accept` - Accept invitation via token
- `GET /api/v1/invitations/organization-users` - Get users with invitation status
- **Database Schema**:
- Added `invitations` table with fields for tracking invitation lifecycle
- Stores inviter, invitee, organization, role, token, and expiration
- **Services**:
- Invitation service handles creation, validation, and acceptance logic
- Integration with email service for sending notifications
- Automatic user creation with temporary passwords
### Frontend Changes
- Updated team settings UI to display invitation status
- Added member management components
- Integrated invitation API calls in Redux store
### Email Templates
- `invitation_email.html` - Sent to invitees with credentials
- `invitation_accepted.html` - Notification to inviter upon acceptance
## Security Considerations
- Temporary passwords are randomly generated and hashed
- Invitation tokens expire after 72 hours
- One active invitation per user-organization pair
- Email verification upon invitation acceptance
## Migration Required
Run migration `029_create_invitations_up.sql` to create the invitations table.
## Testing Recommendations
- Test invitation creation with various roles
- Verify email delivery and content
- Test token expiration scenarios
- Validate re-invitation functionality
- Check organization membership after acceptance
## Configuration
Ensure `API_URL` environment variable is set for generating invitation acceptance links.
Closes #[issue_number] (if applicable) This provides a clear overview of what the PR accomplishes, making it easier for reviewers to understand the scope and impact of the changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 26
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
view/lib/i18n/locales/kn.json (1)
554-585
: Remove invite dialog password field and update button label
- In
view/lib/i18n/locales/kn.json
underteams.addMember.dialog.fields
, remove the entirepassword
entry.- Change
teams.addMember.dialog.buttons.add
from"ಬಳಕೆದಾರರನ್ನು ಸೇರಿಸಿ"
to"ಆಹ್ವಾನಿಸಿ"
.No code references to
teams.addMember.dialog.fields.password
remain.view/lib/i18n/locales/fr.json (1)
554-586
: Remove password field and update “add” button label in addMember dialog
The invite dialog no longer collects a password—drop thepassword
field—and change the add button from “Ajouter l’Utilisateur” to “Envoyer l’invitation”."role": { "label": "Rôle", "placeholder": "Sélectionnez un rôle", "options": { "admin": "Administrateur", "member": "Membre", "viewer": "Observateur" } - }, - "password": { - "label": "Mot de passe", - "placeholder": "Entrez le mot de passe de l'utilisateur" - } + } }, "buttons": { "cancel": "Annuler", - "add": "Ajouter l'Utilisateur" + "add": "Envoyer l'invitation" }view/lib/i18n/locales/en.json (3)
590-591
: Align CTA with invite flow.Change “Add User” to “Send Invite”.
- "add": "Add User" + "add": "Send Invite"
651-656
: Add missing i18n key for Status header used in TeamMembers."headers": { "user": "User", "role": "Role", "permissions": "Permissions", - "actions": "Actions" + "status": "Status", + "actions": "Actions" }
670-690
: Provide localized labels for member invite statuses.Add under settings.teams.members:
"members": { "title": "Team Members", "description": "Manage users and their roles in your team", + "status": { + "pending": "Pending", + "expired": "Expired", + "none": "—" + }, "table": {api/internal/features/notification/helpers/email/email_helper.go (2)
121-131
: Bug: emails sent to FromEmail instead of the user’s email.Envelope and “To” header target smtpConfig.FromEmail; should be user.Email.
- from := smtpConfig.Username - to := []string{smtpConfig.FromEmail} + from := smtpConfig.FromEmail + to := []string{user.Email} @@ - "%s", data.Subject, from, smtpConfig.FromEmail, data.ContentType, body.String())) + "%s", data.Subject, from, user.Email, data.ContentType, body.String()))
320-323
: Missing Content-Type and data payload for “remove user” email.Without Content-Type, some clients render raw HTML/plain text incorrectly.
emailData := EmailData{ - Subject: "User Removed from Organization", - Template: "remove_user_from_organization.html", + Subject: "User Removed from Organization", + Template: "remove_user_from_organization.html", + Data: data, + ContentType: "text/html; charset=UTF-8", }
🧹 Nitpick comments (18)
api/api/versions.json (1)
6-6
: Timestamp format consistencyConsider using UTC "Z" (e.g., 2025-08-29T11:40:07Z) to align with end_of_life and avoid TZ parsing drift. If consumers already accept offsets, ignore.
api/internal/features/notification/templates/invitation_email.html (1)
19-19
: Nit: stray leading space before closing tag.- </html> +</html>api/migrations/users/029_create_invitations_up.sql (2)
3-3
: Consider case-insensitive emails (CITEXT) to prevent duplicate variants.Switch email to CITEXT or enforce lowercasing at write-time.
- email TEXT NOT NULL, + email CITEXT NOT NULL,Add (if not already present in earlier migrations):
CREATE EXTENSION IF NOT EXISTS citext;
5-5
: Optional: reference roles by ID to ensure integrity.If roles are managed in a roles table, prefer role_id UUID FK over free-form TEXT.
- role TEXT NOT NULL, + role_id UUID NOT NULL REFERENCES roles(id),(Adjust service/storage accordingly.)
api/internal/features/invitations/types/userinvite.go (1)
12-18
: Consider omitting null invite fields from JSON.Adding ,omitempty reduces payload noise when fields are absent.
- ExpiresAt *time.Time `json:"expires_at"` - AcceptedAt *time.Time `json:"accepted_at"` - InvitedBy *uuid.UUID `json:"invited_by"` - InviteEmail *string `json:"invite_email"` - InviteName *string `json:"invite_name"` - InviteRole *string `json:"invite_role"` + ExpiresAt *time.Time `json:"expires_at,omitempty"` + AcceptedAt *time.Time `json:"accepted_at,omitempty"` + InvitedBy *uuid.UUID `json:"invited_by,omitempty"` + InviteEmail *string `json:"invite_email,omitempty"` + InviteName *string `json:"invite_name,omitempty"` + InviteRole *string `json:"invite_role,omitempty"`api/internal/middleware/auth.go (1)
221-226
: Avoid shadowing the user_storage package with a local varShadowing confuses readers and tools. Use a distinct local name.
- user_storage := user_storage.UserStorage{ + us := user_storage.UserStorage{ DB: db, Ctx: ctx, } - user, err := user_storage.FindUserByEmail(email) + user, err := us.FindUserByEmail(email)Also applies to: 225-225
view/app/settings/teams/components/TeamMembers.tsx (1)
139-143
: Use a stable key for permission badges.Avoid index keys to prevent unnecessary remounts when toggling “show more/less”.
- {visiblePermissions.map((permission, index) => ( - <Badge key={index} variant="outline" className="bg-primary/10 text-primary rounded-full"> + {visiblePermissions.map((permission) => ( + <Badge key={permission} variant="outline" className="bg-primary/10 text-primary rounded-full"> {permission} </Badge> ))}api/internal/features/notification/templates/invitation_accepted.html (1)
4-6
: Minor wording: use “invitation” consistently.- <p>Your invite has been accepted.</p> + <p>Your invitation has been accepted.</p>view/lib/i18n/locales/en.json (1)
583-587
: Remove obsolete password field strings if no longer used.Avoid stale translations to reduce maintenance.
api/internal/features/notification/helpers/email/email_helper.go (2)
196-203
: Ensure Content-Type is set for all flows.Password reset correctly sets Content-Type; keep consistent elsewhere.
106-115
: DRY the template loading path.Factor a small helper to resolve/parse templates once; reduces duplication and path drift.
Also applies to: 157-161
api/internal/routes.go (1)
151-156
: RBAC resource for invitations group.Using "organization" is okay if the permissions model aligns, but consider a dedicated "invitations" resource for finer control.
view/redux/services/users/userApi.ts (1)
113-127
: Prefer explicit type import and query params over inline type/URL concatCleaner types and safer encoding with fetchBaseQuery params.
-import { - UserOrganization, - CreateInviteRequest -} from '@/redux/types/orgs'; +import { + UserOrganization, + CreateInviteRequest, + OrganizationUserWithInvite +} from '@/redux/types/orgs'; @@ - getInvitedOrganizationUsers: builder.query< - import('@/redux/types/orgs').OrganizationUserWithInvite[], - string - >({ + getInvitedOrganizationUsers: builder.query<OrganizationUserWithInvite[], string>({ query(organizationId) { return { - url: `${INVITATIONURLS.ORGANIZATION_USERS}?id=${organizationId}`, - method: 'GET' + url: INVITATIONURLS.ORGANIZATION_USERS, + method: 'GET', + params: { id: organizationId } }; }, providesTags: [{ type: 'User', id: 'LIST' }], - transformResponse: (response: { - data: import('@/redux/types/orgs').OrganizationUserWithInvite[]; - }) => response.data + transformResponse: (response: { data: OrganizationUserWithInvite[] }) => response.data }),api/internal/features/invitations/controller/controller.go (2)
24-26
: Drop unused Store param and import from constructorRemoves dead param and import; cleans API.
-import "github.com/raghavyuva/nixopus-api/internal/storage" @@ -func NewController(s *storage.Store, svc *inv_service.Service, orgs *org_service.OrganizationService, l logger.Logger, n *notification.NotificationManager) *Controller { +func NewController(svc *inv_service.Service, orgs *org_service.OrganizationService, l logger.Logger, n *notification.NotificationManager) *Controller { return &Controller{svc: svc, orgs: orgs, logger: l, notifications: n} }Also applies to: 12-12
80-83
: Return a descriptive 400 error when id is missingImproves client debuggability.
- if id == "" { - return nil, fuego.HTTPError{Status: http.StatusBadRequest} - } + if id == "" { + return nil, fuego.HTTPError{Status: http.StatusBadRequest, Detail: "missing id query param"} + }api/internal/features/invitations/service/service.go (2)
112-123
: Race conditions on reinvite/upsert and acceptanceConsider DB uniqueness on (user_id, organization_id) and transactionally upserting invites and acceptance + org add.
- Add a unique index on (user_id, organization_id).
- Wrap CreateInvite and AcceptInvite critical sections in a DB transaction; handle unique violations with retry.
Also applies to: 229-307
56-66
: Validate email format and role name earlyAdd basic email validation and a clearer error for invalid roles.
- Validate email with a simple parser or dedicated util.
- Return 400-class domain errors from controller for bad input.
api/internal/features/invitations/storage/store.go (1)
103-124
: Add DB uniqueness to prevent duplicates and simplify logicEnforce at schema level:
- Unique index on invitations (user_id, organization_id).
- Unique index on organization_users (user_id, organization_id).
- Optional: index on invitations (updated_at DESC, organization_id) to accelerate latest-by-user queries.
Also applies to: 39-50, 81-92
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (26)
api/.env.sample
(1 hunks)api/api/versions.json
(1 hunks)api/internal/config/config.go
(1 hunks)api/internal/features/invitations/controller/controller.go
(1 hunks)api/internal/features/invitations/service/service.go
(1 hunks)api/internal/features/invitations/storage/store.go
(1 hunks)api/internal/features/invitations/types/userinvite.go
(1 hunks)api/internal/features/notification/helpers/email/email_helper.go
(9 hunks)api/internal/features/notification/templates/invitation_accepted.html
(1 hunks)api/internal/features/notification/templates/invitation_email.html
(1 hunks)api/internal/middleware/auth.go
(2 hunks)api/internal/routes.go
(3 hunks)api/internal/types/invitation.go
(1 hunks)api/internal/types/types.go
(1 hunks)api/migrations/users/029_create_invitations_down.sql
(1 hunks)api/migrations/users/029_create_invitations_up.sql
(1 hunks)view/app/settings/hooks/use-team-settings.ts
(5 hunks)view/app/settings/teams/components/AddMember.tsx
(0 hunks)view/app/settings/teams/components/TeamMembers.tsx
(3 hunks)view/lib/i18n/locales/en.json
(2 hunks)view/lib/i18n/locales/es.json
(2 hunks)view/lib/i18n/locales/fr.json
(2 hunks)view/lib/i18n/locales/kn.json
(3 hunks)view/redux/api-conf.ts
(1 hunks)view/redux/services/users/userApi.ts
(4 hunks)view/redux/types/orgs.ts
(2 hunks)
💤 Files with no reviewable changes (1)
- view/app/settings/teams/components/AddMember.tsx
🧰 Additional context used
🧬 Code graph analysis (8)
view/app/settings/teams/components/TeamMembers.tsx (2)
view/components/ui/table.tsx (2)
TableHead
(92-92)TableCell
(92-92)view/components/ui/badge.tsx (1)
Badge
(39-39)
view/redux/types/orgs.ts (1)
api/internal/types/organization.go (1)
OrganizationUsers
(66-79)
api/internal/features/invitations/controller/controller.go (6)
api/internal/features/invitations/service/service.go (2)
Service
(27-35)CreateInviteRequest
(37-42)api/internal/features/organization/service/init.go (1)
OrganizationService
(15-23)api/internal/features/notification/types.go (4)
NotificationManager
(82-93)NotificationPayloadTypeUpdateOrganization
(124-124)NotificationCategoryOrganization
(131-131)api/internal/types/response.go (1)
Response
(5-20)api/internal/utils/get_user.go (2)
GetUser
(25-35)GetOrganizationID
(37-54)api/internal/features/notification/init.go (1)
NewNotificationPayload
(17-25)
api/internal/features/invitations/service/service.go (9)
api/internal/features/invitations/storage/store.go (1)
InvitationStore
(13-17)api/internal/features/auth/storage/user.go (1)
AuthRepository
(23-40)api/internal/features/role/service/init.go (1)
RoleService
(12-17)api/internal/features/organization/service/init.go (1)
OrganizationService
(15-23)api/internal/features/notification/helpers/email/email_helper.go (2)
EmailManager
(21-25)EmailData
(35-42)api/internal/types/invitation.go (1)
Invitation
(11-25)api/internal/features/auth/utils/hash_password.go (1)
HashPassword
(14-24)api/internal/types/user.go (1)
NewUser
(61-74)api/internal/features/invitations/types/userinvite.go (1)
UserWithInvite
(10-18)
api/internal/features/notification/helpers/email/email_helper.go (1)
api/internal/features/notification/types.go (3)
Category
(216-216)SecurityCategory
(220-220)ActivityCategory
(219-219)
view/redux/services/users/userApi.ts (1)
view/redux/types/orgs.ts (1)
CreateInviteRequest
(114-119)
api/internal/routes.go (7)
api/internal/storage/store.go (2)
App
(17-21)Store
(12-15)api/internal/middleware/auth.go (1)
AuthMiddleware
(22-112)api/internal/features/invitations/storage/store.go (1)
InvitationStore
(13-17)api/internal/features/invitations/service/service.go (1)
Service
(27-35)api/internal/features/notification/helpers/email/email_helper.go (1)
NewEmailManager
(27-33)api/internal/features/invitations/controller/controller.go (1)
NewController
(24-26)api/internal/middleware/rbac.go (1)
RBACMiddleware
(12-56)
api/internal/features/invitations/storage/store.go (1)
api/internal/types/invitation.go (1)
Invitation
(11-25)
🔇 Additional comments (14)
api/migrations/users/029_create_invitations_down.sql (1)
1-1
: Verify foreign-key dependencies and up-migration for invitations
No FK references or CREATE TABLE forinvitations
were found in the repo; please confirm that the up-migration exists and that no tables referenceinvitations
. If dependencies may exist, change to:DROP TABLE IF EXISTS invitations CASCADE;view/redux/api-conf.ts (1)
18-21
: Confirmed: backend exposes GET /api/v1/invitations/organization-users — enum is correctOpenAPI (api/doc/openapi.json) defines GET /api/v1/invitations/organization-users (controller: GetOrganizationUsersWithInviteStatus); frontend view/redux/api-conf.ts (line 19) value 'v1/invitations/organization-users' matches — no change required. Optional: rename ORGANIZATION_USERS → ORGANIZATION_USERS_WITH_INVITES for clarity.
api/internal/features/notification/templates/invitation_email.html (2)
4-18
: Invitation email uses html/template for rendering
Verified thatinvitation_email.html
is passed intoSendEmailToAddress
, andemail_helper.go
imports and leverages Go’shtml/template
—no use oftext/template
or manual string concatenation.
16-17
: Require invitation acceptance via POST
Couldn’t locate the GET handler for/api/v1/invitations/accept
; verify that GET is view-only/idempotent and move state changes behind a POST (with CSRF protection).api/migrations/users/029_create_invitations_up.sql (1)
2-2
: Extensionuuid-ossp
already created
Theuuid-ossp
extension is enabled in earlier migrations (e.g. auth/001_create_users_up.sql), souuid_generate_v4()
is available. No additional guard needed.api/internal/features/invitations/types/userinvite.go (1)
6-8
: Import path is correct: the module declared in api/go.mod is github.com/raghavyuva/nixopus-api, so importing shared_types from"github.com/raghavyuva/nixopus-api/internal/types"
is valid and no change is needed.Likely an incorrect or invalid review comment.
view/redux/types/orgs.ts (1)
83-90
: Shape matches backend and is forward-compatible.Optional invite metadata aligns with Go struct (nullable fields). Looks good.
view/lib/i18n/locales/kn.json (2)
673-675
: LGTM: message keys switched to invitation semanticsKeys and strings match the new flow.
784-785
: LGTM: header title tweakNo functional impact; consistent with other locales.
view/lib/i18n/locales/fr.json (1)
673-675
: LGTM: userInvited/userInviteFailedMatches backend behavior.
view/lib/i18n/locales/es.json (2)
672-674
: LGTM: messages updated to invitation flowConsistent with other locales.
553-585
: Use invitation verbs and remove password field
Apply the following diff to es.json:"role": { "label": "Rol", "placeholder": "Selecciona un rol", "options": { "admin": "Administrador", "member": "Miembro", "viewer": "Espectador" } - }, - "password": { - "label": "Contraseña", - "placeholder": "Ingresa la contraseña del usuario" - } + } }, "buttons": { "cancel": "Cancelar", - "add": "Agregar Usuario" + "add": "Enviar Invitación" }No stale references found for the removed password key or updated add button key; verify that deleting the password translation and renaming the button won’t break any UI rendering or code lookups.
view/redux/services/users/userApi.ts (1)
128-137
: X-Organization-Id header is already injected via baseQueryWithReauth
TheprepareHeaders
implementation in view/redux/base-query.ts (lines 37–39) setsX-Organization-Id
whenorganizationId
is present, and all API services (includinguserApi.createInvite
) usebaseQueryWithReauth
.view/app/settings/hooks/use-team-settings.ts (1)
92-103
: Sanity-check permissions heuristicAdmin likely needs superset permissions vs Member. Confirm intended behavior.
If inverted, swap the arrays accordingly.
# backend base URL used for generating absolute links | ||
# API_URL=http://localhost:8080/ or API_URL=https://api.example.com/ | ||
API_URL= | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Align example and default with config; no trailing slash, include '/api' if used
Inconsistency with config.go can lead to malformed links. Recommend explicit, normalized example.
-# backend base URL used for generating absolute links
-# API_URL=http://localhost:8080/ or API_URL=https://api.example.com/
-API_URL=
+# Public API base used to generate absolute links (no trailing slash).
+# Include '/api' if the server mounts routes under that prefix.
+# Examples: http://localhost:8080/api or https://api.example.com/api
+API_URL=http://localhost:8080/api
📝 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.
# backend base URL used for generating absolute links | |
# API_URL=http://localhost:8080/ or API_URL=https://api.example.com/ | |
API_URL= | |
# Public API base used to generate absolute links (no trailing slash). | |
# Include '/api' if the server mounts routes under that prefix. | |
# Examples: http://localhost:8080/api or https://api.example.com/api | |
API_URL=http://localhost:8080/api |
🤖 Prompt for AI Agents
In api/.env.sample around lines 42-45, the API_URL example and blank default are
inconsistent with config.go and may produce malformed links; update the sample
so the commented example and the default value are normalized (no trailing
slash) and explicitly include "/api" if the service expects that path — e.g.,
change the example comment and the API_URL default to a normalized host form
(http://localhost:8080 or https://api.example.com/api) so they match config.go’s
expectations.
api/internal/config/config.go
Outdated
// public API base (e.g., https://api.example.com/api) | ||
viper.BindEnv("app.api_url", "API_URL") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Define and normalize API_URL semantics to avoid broken invite links
Docs/sample and this comment disagree on whether API_URL includes '/api' and a trailing slash. Normalize at startup and document “no trailing slash”. Also log the configured base.
Change comment for clarity:
-// public API base (e.g., https://api.example.com/api)
+// Public API base WITHOUT trailing slash (e.g., https://api.example.com/api)
viper.BindEnv("app.api_url", "API_URL")
Add normalization and logging (outside this hunk):
// after viper.Unmarshal(&AppConfig)
AppConfig.App.APIURL = strings.TrimRight(AppConfig.App.APIURL, "/")
log.Printf("Public API base URL: %s", AppConfig.App.APIURL)
Optional validation (outside this hunk):
u, perr := url.Parse(AppConfig.App.APIURL)
if AppConfig.App.APIURL != "" && (perr != nil || u.Scheme == "" || u.Host == "") {
return fmt.Errorf("invalid app.api_url: %q", AppConfig.App.APIURL)
}
🤖 Prompt for AI Agents
In api/internal/config/config.go around lines 189-191, the handling and
documentation of app.api_url/API_URL is inconsistent about including "/api" and
trailing slashes which breaks invite links; after viper.Unmarshal(&AppConfig)
normalize AppConfig.App.APIURL by trimming any trailing slashes, log the
normalized public API base (e.g., using log.Printf) and update the comment/doc
to state "provide API_URL without a trailing slash" (and whether it should
include the "/api" path); optionally validate the resulting URL (parse and check
scheme/host) and return an error if invalid.
func (c *Controller) CreateInvite(ctx fuego.ContextWithBody[CreateInviteRequest]) (*shared_types.Response, error) { | ||
w, r := ctx.Response(), ctx.Request() | ||
inviter := utils.GetUser(w, r) | ||
if inviter == nil { | ||
return nil, fuego.HTTPError{Status: http.StatusUnauthorized} | ||
} | ||
// Require organization from context (set by auth middleware) | ||
orgID := utils.GetOrganizationID(r) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Avoid double-write and status mismatch when user is missing
utils.GetUser writes a 500 response on failure; returning 401 here causes inconsistency. Align status and avoid implying auth failure.
- inviter := utils.GetUser(w, r)
- if inviter == nil {
- return nil, fuego.HTTPError{Status: http.StatusUnauthorized}
- }
+ inviter := utils.GetUser(w, r)
+ if inviter == nil {
+ // Align with util behavior to prevent double-write/status skew.
+ return nil, fuego.HTTPError{Status: http.StatusInternalServerError, Detail: "failed to get user from context"}
+ }
If feasible, refactor utils.GetUser to not write to the response and return an error instead; then use 401 here. Do you want a patch for that?
🏁 Script executed:
#!/usr/bin/env bash
# Locate the definition of utils.GetUser to verify its behavior
rg -nP --type=go 'func GetUser' -C3 .
Length of output: 1461
🏁 Script executed:
#!/usr/bin/env bash
# Display the contents of the GetUser utility to inspect its behavior
sed -n '1,200p' api/internal/utils/get_user.go
Length of output: 1440
Align error handling to avoid double‐write and status mismatch
utils.GetUser writes a 500 response on failure via SendErrorResponse before returning nil; the subsequent 401 return in CreateInvite triggers a second write with a conflicting status. In the nil case, return a 500 HTTPError (e.g.
return nil, fuego.HTTPError{Status: http.StatusInternalServerError, Detail: "failed to retrieve user from context"}
) or refactor GetUser to return an error (not write to w) so you can return 401 here.
🤖 Prompt for AI Agents
In api/internal/features/invitations/controller/controller.go around lines 36 to
43, the current nil-check for inviter can cause a double HTTP write because
utils.GetUser already writes a 500 response before returning nil; update the
handler so it returns a matching 500 fuego.HTTPError instead of a 401 (or
alternatively change utils.GetUser to return (user, error) without writing to
the response and handle a 401 here). Concretely, either return nil,
fuego.HTTPError{Status: http.StatusInternalServerError, Detail: "failed to
retrieve user from context"} when inviter == nil, or refactor GetUser to return
an error and change this branch to return a 401 HTTPError when the error
indicates missing/unauthenticated user.
// notify inviter that invite has been sent (optional) | ||
c.notifications.SendNotification(notification.NewNotificationPayload(notification.NotificationPayloadTypeUpdateOrganization, inviter.ID.String(), map[string]string{"message": "Invite sent"}, notification.NotificationCategoryOrganization)) | ||
return &shared_types.Response{Status: "success", Message: "Invitation created", Data: inv}, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Guard notification call against nil manager
Avoid NPE if notifications are not wired.
- c.notifications.SendNotification(notification.NewNotificationPayload(notification.NotificationPayloadTypeUpdateOrganization, inviter.ID.String(), map[string]string{"message": "Invite sent"}, notification.NotificationCategoryOrganization))
+ if c.notifications != nil {
+ c.notifications.SendNotification(
+ notification.NewNotificationPayload(
+ notification.NotificationPayloadTypeUpdateOrganization,
+ inviter.ID.String(),
+ map[string]string{"message": "Invite sent"},
+ notification.NotificationCategoryOrganization,
+ ),
+ )
+ }
📝 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.
// notify inviter that invite has been sent (optional) | |
c.notifications.SendNotification(notification.NewNotificationPayload(notification.NotificationPayloadTypeUpdateOrganization, inviter.ID.String(), map[string]string{"message": "Invite sent"}, notification.NotificationCategoryOrganization)) | |
return &shared_types.Response{Status: "success", Message: "Invitation created", Data: inv}, nil | |
// notify inviter that invite has been sent (optional) | |
if c.notifications != nil { | |
c.notifications.SendNotification( | |
notification.NewNotificationPayload( | |
notification.NotificationPayloadTypeUpdateOrganization, | |
inviter.ID.String(), | |
map[string]string{"message": "Invite sent"}, | |
notification.NotificationCategoryOrganization, | |
), | |
) | |
} | |
return &shared_types.Response{Status: "success", Message: "Invitation created", Data: inv}, nil |
🤖 Prompt for AI Agents
In api/internal/features/invitations/controller/controller.go around lines 61 to
63, the call c.notifications.SendNotification can panic if c.notifications is
nil; guard this call by checking if c.notifications != nil before invoking
SendNotification (and optionally log a debug/warn if notifications are not
configured) so the function returns normally even when the notification manager
is not wired.
gen, perr := randPassword() | ||
if perr != nil { | ||
s.Logger.Log(logger.Error, "password generation failed", perr.Error()) | ||
return nil, "", fmt.Errorf("failed to generate password: %w", perr) | ||
} | ||
generatedPassword = gen | ||
hashed, err := auth_utils.HashPassword(generatedPassword) | ||
if err != nil { | ||
return nil, "", err | ||
} | ||
user.Password = hashed | ||
user.UpdatedAt = time.Now() | ||
if err := s.Users.UpdateUser(&user); err != nil { | ||
return nil, "", err | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not reset existing users' passwords during reinvite
This is a critical security and UX issue (silent credential rotation). Existing users should authenticate as usual; only new users may receive a bootstrap flow.
} else {
s.Logger.Log(logger.Info, "invitation user branch", fmt.Sprintf("email=%s action=existing-user", req.Email))
user = *dbUser
- // reinvite flow: rotate a new password and update the user so email carries a fresh password
- gen, perr := randPassword()
- if perr != nil {
- s.Logger.Log(logger.Error, "password generation failed", perr.Error())
- return nil, "", fmt.Errorf("failed to generate password: %w", perr)
- }
- generatedPassword = gen
- hashed, err := auth_utils.HashPassword(generatedPassword)
- if err != nil {
- return nil, "", err
- }
- user.Password = hashed
- user.UpdatedAt = time.Now()
- if err := s.Users.UpdateUser(&user); err != nil {
- return nil, "", err
- }
+ // Do NOT rotate password for existing users.
+ generatedPassword = ""
}
📝 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.
gen, perr := randPassword() | |
if perr != nil { | |
s.Logger.Log(logger.Error, "password generation failed", perr.Error()) | |
return nil, "", fmt.Errorf("failed to generate password: %w", perr) | |
} | |
generatedPassword = gen | |
hashed, err := auth_utils.HashPassword(generatedPassword) | |
if err != nil { | |
return nil, "", err | |
} | |
user.Password = hashed | |
user.UpdatedAt = time.Now() | |
if err := s.Users.UpdateUser(&user); err != nil { | |
return nil, "", err | |
} | |
} | |
} else { | |
s.Logger.Log(logger.Info, "invitation user branch", fmt.Sprintf("email=%s action=existing-user", req.Email)) | |
user = *dbUser | |
// Do NOT rotate password for existing users. | |
generatedPassword = "" | |
} |
updated_at TIMESTAMP NOT NULL DEFAULT now(), | ||
inviter_user_id UUID NOT NULL REFERENCES users(id) ON DELETE CASCADE, | ||
organization_id UUID NOT NULL REFERENCES organizations(id) ON DELETE CASCADE, | ||
user_id UUID NOT NULL REFERENCES users(id) ON DELETE CASCADE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make user_id nullable until acceptance.
Requiring user_id at invite creation prevents inviting non-registered emails. Defer binding to a user until the invite is accepted.
- user_id UUID NOT NULL REFERENCES users(id) ON DELETE CASCADE
+ user_id UUID NULL REFERENCES users(id) ON DELETE SET NULL
🤖 Prompt for AI Agents
In api/migrations/users/029_create_invitations_up.sql around line 13, the
invitations table currently defines user_id as NOT NULL which prevents creating
invites for emails not yet registered; change the column to allow NULL by
removing the NOT NULL constraint (i.e., user_id UUID REFERENCES users(id) ON
DELETE CASCADE) so the FK remains but can be unset until acceptance, and if this
migration already ran in environments provide an accompanying ALTER TABLE ...
ALTER COLUMN user_id DROP NOT NULL migration step instead of a destructive
change.
try { | ||
const user = await createUser(tempUser as any); | ||
await createInvite({ | ||
email: newUser.email || '', | ||
name: newUser.name || '', | ||
role: newUser.role | ||
}).unwrap(); | ||
await refetchUsers(); | ||
toast.success(t('settings.teams.messages.userAdded')); | ||
toast.success(t('settings.teams.messages.userInvited')); | ||
} catch (error) { | ||
toast.error(t('settings.teams.messages.userAddFailed')); | ||
toast.error(t('settings.teams.messages.userInviteFailed')); | ||
} | ||
setNewUser({ name: '', email: '', role: 'Member', password: '' }); | ||
setNewUser({ name: '', email: '', role: 'Member' }); | ||
setIsAddUserDialogOpen(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Rollback optimistic row on invite failure
Avoid ghost entries when createInvite fails.
try {
@@
await refetchUsers();
toast.success(t('settings.teams.messages.userInvited'));
} catch (error) {
+ setUsers((prev: any[]) => prev.filter((u) => u.id !== newId));
toast.error(t('settings.teams.messages.userInviteFailed'));
}
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In view/app/settings/hooks/use-team-settings.ts around lines 104 to 116, the
invite handler applies an optimistic UI update but does not roll it back if
createInvite fails, causing ghost entries; update the catch block to revert the
optimistic change by calling refetchUsers (or removing the optimistic user from
local state) before showing the error toast, ensure refetchUsers is awaited or
its promise handled so the UI is consistent, and move the setNewUser and
setIsAddUserDialogOpen cleanup into a finally block (or after the rollback) so
cleanup always runs.
const r = normalizeRole(role); | ||
switch (r) { | ||
case 'Owner': | ||
return 'default'; | ||
case 'Admin': | ||
case 'admin': | ||
return 'destructive'; | ||
case 'Member': | ||
case 'member': | ||
return 'default'; | ||
case 'Viewer': | ||
case 'viewer': | ||
return 'secondary'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix case mismatch: 'owner' never matched after normalizeRole()
normalizeRole lowercases input; switch must use 'owner'.
- switch (r) {
- case 'Owner':
+ switch (r) {
+ case 'owner':
return 'default';
case 'admin':
return 'destructive';
case 'member':
return 'default';
case 'viewer':
📝 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.
const r = normalizeRole(role); | |
switch (r) { | |
case 'Owner': | |
return 'default'; | |
case 'Admin': | |
case 'admin': | |
return 'destructive'; | |
case 'Member': | |
case 'member': | |
return 'default'; | |
case 'Viewer': | |
case 'viewer': | |
return 'secondary'; | |
const r = normalizeRole(role); | |
switch (r) { | |
case 'owner': | |
return 'default'; | |
case 'admin': | |
return 'destructive'; | |
case 'member': | |
return 'default'; | |
case 'viewer': | |
return 'secondary'; |
🤖 Prompt for AI Agents
In view/app/settings/hooks/use-team-settings.ts around lines 147 to 156, the
switch uses 'Owner' which will never match because normalizeRole() lowercases
the role; change the case label from 'Owner' to 'owner' (and ensure all other
case labels match the normalized lowercase values: 'admin', 'member', 'viewer')
so the switch correctly matches normalized roles.
status?: 'Pending' | 'Expired' | '-'; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Localize “Status” and avoid the '-' sentinel; default to em dash when status is undefined.
- Hardcoded strings ("Status", "Pending", "Expired") bypass i18n.
- Using '-' as a sentinel complicates the type and logic; the field is optional already.
Apply:
type EditUser = {
id: string;
name: string;
email: string;
avatar: string;
role: 'Owner' | 'Admin' | 'Member' | 'Viewer';
permissions: string[];
- status?: 'Pending' | 'Expired' | '-';
+ status?: 'Pending' | 'Expired';
};
@@
- <TableHead>Status</TableHead>
+ <TableHead>{t('settings.teams.members.table.headers.status')}</TableHead>
@@
- <TableCell>
- {user.status === 'Pending' && (
- <Badge variant="outline" className="w-max">Pending</Badge>
- )}
- {user.status === 'Expired' && (
- <Badge variant="destructive" className="w-max">Expired</Badge>
- )}
- {user.status === '-' && <span className="text-muted-foreground">—</span>}
- </TableCell>
+ <TableCell>
+ {user.status === 'Pending' && (
+ <Badge variant="outline" className="w-max">
+ {t('settings.teams.members.status.pending')}
+ </Badge>
+ )}
+ {user.status === 'Expired' && (
+ <Badge variant="destructive" className="w-max">
+ {t('settings.teams.members.status.expired')}
+ </Badge>
+ )}
+ {!user.status && <span className="text-muted-foreground">—</span>}
+ </TableCell>
Also applies to: 190-191, 222-230
view/redux/types/orgs.ts
Outdated
export interface CreateInviteRequest { | ||
email: string; | ||
name: string; | ||
role: UserTypes | string; | ||
organization_id?: string; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Tighten role typing to prevent invalid values at compile-time.
Unless the API accepts arbitrary roles, prefer the union type only.
-export interface CreateInviteRequest {
+export interface CreateInviteRequest {
email: string;
name: string;
- role: UserTypes | string;
+ role: UserTypes;
organization_id?: string;
}
If arbitrary roles are needed, add a runtime validator instead of widening the type.
📝 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.
export interface CreateInviteRequest { | |
email: string; | |
name: string; | |
role: UserTypes | string; | |
organization_id?: string; | |
} | |
export interface CreateInviteRequest { | |
email: string; | |
name: string; | |
role: UserTypes; | |
organization_id?: string; | |
} |
🤖 Prompt for AI Agents
In view/redux/types/orgs.ts around lines 114 to 119, the CreateInviteRequest
interface currently types role as UserTypes | string which allows arbitrary
strings; change the role property to only UserTypes to enforce valid
compile-time values (role: UserTypes). If the API truly accepts arbitrary role
strings, keep the narrower UserTypes type and add a runtime validator where
CreateInviteRequest objects are constructed/received to allow extra values
explicitly instead of widening the type.
Summary by CodeRabbit
New Features
UI
Localization
Configuration