Skip to content

Comments

fix(user invitation): failed but no warning.#8731

Open
Bo-Onyx wants to merge 9 commits intomainfrom
bo/user_invitation_flow_bug_fix
Open

fix(user invitation): failed but no warning.#8731
Bo-Onyx wants to merge 9 commits intomainfrom
bo/user_invitation_flow_bug_fix

Conversation

@Bo-Onyx
Copy link

@Bo-Onyx Bo-Onyx commented Feb 24, 2026

Description

Ticket Link: https://linear.app/onyx-app/issue/ENG-3505

Problem

When an admin invites users, email notifications could silently fail to send in two scenarios:

  • ENABLE_EMAIL_INVITES=true but no SMTP/SendGrid is configured
  • ENABLE_EMAIL_INVITES=true, email is configured, but sending fails (e.g. bad credentials)

In both cases the invite was saved but the admin always saw "Users invited!" with no indication that the invited user never received a notification.

Changes

Backend

  • Added EmailInviteStatus enum and BulkInviteResponse model to models.py
  • Changed bulk_invite_users return type from int to BulkInviteResponse, explicitly tracking the email delivery outcome across all code paths
  • Fixed a pre-existing bug where EMAIL_CONFIGURED was never checked before attempting to send — placeholder default values for SMTP_USER/SMTP_PASS
    caused EMAIL_CONFIGURED to always evaluate as True

Frontend

  • BulkAdd.tsx parses the response and passes email_invite_status to onSuccess
  • page.tsx shows a warning toast alongside "Users invited!" when status is not_configured or send_failed; disabled and sent show no warning
Screenshot 2026-02-24 at 4 07 17 PM

How Has This Been Tested?

Added unit tests

  • Fixed pre-existing broken test (test_trial_tenant_can_invite_within_limit was missing enforce_seat_limit patch and asserting against the old int
    return type)
  • Added 4 new unit tests covering all EmailInviteStatus outcomes

Add local test on UI before change:
Add bulk invitations show no warning.
After changes
Screenshot 2026-02-24 at 4 08 41 PM

Additional Options

  • [Optional] Please cherry-pick this PR to the latest release version.
  • [Optional] Override Linear Check

Summary by cubic

Shows clear warnings when invite emails don’t send, and returns delivery status in the bulk invite API. Addresses Linear ENG-3505.

  • Bug Fixes

    • Backend: added EmailInviteStatus and BulkInviteResponse; bulk_invite_users now always returns { invited_count, email_invite_status } with DISABLED/NOT_CONFIGURED/SENT/SEND_FAILED and checks EMAIL_CONFIGURED.
    • Config: removed default SMTP server/user/pass so EMAIL_CONFIGURED is accurate.
    • Frontend: BulkAdd parses JSON and passes email_invite_status to onSuccess; AddUserButton shows warning toasts for NOT_CONFIGURED and SEND_FAILED, otherwise success.
    • Tests: added coverage for all status outcomes; fixed trial invite limit test by patching enforce_seat_limit.
  • Migration

    • bulk_invite_users now returns JSON { invited_count, email_invite_status } instead of an int; update callers accordingly.

Written for commit 59a0a31. Summary will update on new commits.

@Bo-Onyx Bo-Onyx requested a review from a team as a code owner February 24, 2026 20:28
@greptile-apps

This comment was marked as duplicate.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@Bo-Onyx Bo-Onyx force-pushed the bo/user_invitation_flow_bug_fix branch from 44c6bcf to bf7fee2 Compare February 24, 2026 20:31
@github-actions
Copy link
Contributor

github-actions bot commented Feb 24, 2026

Preview Deployment

Status Preview Commit Updated
https://onyx-preview-458i2khts-danswer.vercel.app 59a0a31 2026-02-25 02:19:20 UTC

@Bo-Onyx Bo-Onyx changed the title Fix User Invitation no warning. Fix: User Invitation no warning. Feb 24, 2026
@Bo-Onyx Bo-Onyx changed the title Fix: User Invitation no warning. Fix: User invitation failed but no warning. Feb 24, 2026
cubic-dev-ai[bot]

This comment was marked as duplicate.

@Bo-Onyx Bo-Onyx changed the title Fix: User invitation failed but no warning. fix(user invitation): failed but no warning. Feb 24, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Feb 24, 2026

🖼️ Visual Regression Report

Project Changed Added Removed Unchanged Report
admin 1 0 0 108 View Report
exclusive 0 0 0 8 ✅ No changes

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="web/src/app/admin/users/page.tsx">

<violation number="1" location="web/src/app/admin/users/page.tsx:281">
P3: The SEND_FAILED toast message claims the failure was transient, but the backend marks SEND_FAILED for any email-sending exception (including permanent configuration errors). This message is misleading; suggest wording that covers both transient and configuration failures or instructs checking SMTP/SendGrid settings.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Copy link
Member

@wenxi-onyx wenxi-onyx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments, approving as they're not 100% critical

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants