feat: add onboarding form for cloud first timers

[tristantr]

No description provided.

[cloudflare-workers-and-pages]

Deploying windmill with    Cloudflare Pages

Latest commit:	9d73761
Status:	✅  Deploy successful!
Preview URL:	https://80d47b8b.windmill.pages.dev
Branch Preview URL:	https://tl-first-timers-onboarding2.windmill.pages.dev

View logs

[claude]

Claude finished @tristantr's task —— View job

---

Code Review: Add onboarding form for first timers ✅

I've completed a comprehensive review of this PR. Overall, this is a solid implementation of a first-time user onboarding feature for cloud-hosted instances. Here's my detailed feedback:

---

✅ Strengths

1. Clean separation of concerns - OSS stub implementation in users_oss.rs properly isolates enterprise features
2. Proper conditional logic - Onboarding only triggers for cloud-hosted instances via isCloudHosted() checks
3. Good UX flow - Two-step onboarding process with clear navigation and progress indicators
4. Database query optimization - SQLx queries properly include the new first_time_user field

---

🔍 Issues & Recommendations

Critical Issues

None found. The code is functional and safe.

High Priority Issues

1. Missing Error Handling in Onboarding Component (frontend/src/routes/(root)/(logged)/user/(user)/onboarding/+page.svelte:78-84)

The error handling gracefully continues to workspaces even on failure, but the user experience could be improved:

} catch (error) {
    console.error('Error submitting onboarding data:', error)
    sendUserToast('Failed to save information: ' + (error?.body || error?.message || error), true)
} finally {
    isSubmitting = false
    goto('/user/workspaces')  // ⚠️ Navigates away even on error
}

Recommendation: Consider whether failed onboarding submission should block navigation or just log silently. Current behavior might confuse users who see an error toast but are immediately redirected.

2. Unused Variable in Backend (backend/windmill-api/src/users.rs:1880)

if let Some((email, hash, super_admin, _first_time_user)) = email_w_h {

The _first_time_user is fetched from the database but prefixed with _ (unused). This was previously used for cookie-based onboarding detection which has been removed.

Recommendation: Either remove it from the query entirely or document why it's kept (possibly for future use).

---

Medium Priority Issues

3. Svelte 5 Compliance - Missing Runes Best Practices

The onboarding component uses Svelte 5 runes ($state, $derived) but could follow best practices more closely:

let currentStep = $state(1)
let useCaseText = $state('')
let selectedSource = $state<string | null>(null)
let isSubmitting = $state(false)

According to frontend/svelte5-best-practices.mdc, this is correct, but the component doesn't use $derived where it could be beneficial.

Potential improvement: Consider using $derived for computed states like button disabled state:

const canContinue = $derived(selectedSource !== null && !isSubmitting)

4. API Type Safety

The onboarding component constructs the request body manually:

await UserService.submitOnboardingData({
    requestBody: {
        email,
        customer_id: null,
        is_ee_trial: null,
        touch_point: touchPoint,
        use_case: useCaseText
    }
})

Recommendation: Verify that the TypeScript types generated from the OpenAPI spec are being used. If UserService.submitOnboardingData isn't auto-generated yet, ensure the frontend build regenerates types from the updated openapi.yaml.

5. Performance - Multiple globalWhoami() Calls

In Login.svelte (line 123) and login_callback/+page.svelte (line 51), the code calls UserService.globalWhoami() to check first_time_user:

const globalUserInfo = await UserService.globalWhoami()
if (globalUserInfo.first_time_user) {
    goto('/user/onboarding')
    return
}

Observation: This adds an extra API call on every login. Consider whether this check could be included in the login response itself to reduce round trips.

6. Missing Input Validation

The textarea in step 2 has no length validation:

<textarea
    bind:value={useCaseText}
   
    class="input mt-1"
    rows="8"
></textarea>

Recommendation: Add maxlength validation to prevent excessively long submissions:

<textarea
    bind:value={useCaseText}
    maxlength="1000"
    ...
/>

---

Low Priority / Style Issues

7. Console Logging in Production Code

onboarding/+page.svelte:61-62:

if (!email) {
    console.error('No email found in global user info:', globalUserInfo)
    throw new Error('User email not found')
}

Recommendation: This is fine for debugging, but consider using a proper logging service in production.

8. Hardcoded Success Message

onboarding/+page.svelte:77:

sendUserToast('Information saved successfully')

Recommendation: Consider making toast messages consistent across the app or using i18n if internationalization is planned.

9. Button Disabled State Could Be More Explicit

onboarding/+page.svelte:151-152:

disabled={isSubmitting}
loading={isSubmitting}

Recommendation: The "Continue" button should also check if useCaseText is provided (though the skip option makes this optional, clarify if use case is truly optional).

---

🔒 Security Review

✅ No security issues found:

• API endpoint properly uses ApiAuthed for authentication
• No SQL injection risks (using SQLx parameterized queries)
• No XSS risks (Svelte auto-escapes template values)
• OSS version properly stubs out the feature
• Input data is properly typed and validated on backend (in EE version)

---

⚡ Performance Considerations

Positive:

• Efficient database queries with proper indexing
• Minimal bundle size impact (single new route)
• Conditional loading (only for cloud instances)

Minor concern:

• Extra globalWhoami() API call on every login (see issue chore: lint the frontend and the python-client codebase #5 above)

---

📋 Code Quality & Best Practices

Backend (Rust):

• ✅ Follows project structure (OSS stubs in users_oss.rs)
• ✅ Proper error handling with Result<T, Error>
• ✅ OpenAPI spec updated correctly
• ✅ SQLx query cache files committed
• ⚠️ Unused variable in login function (see issue Typo on site homepage #2)

Frontend (Svelte 5):

• ✅ Uses Svelte 5 runes ($state)
• ✅ Proper component structure and separation
• ✅ Clean, readable code
• ⚠️ Could use $derived for computed states (see issue Give clear self-host instructions #3)
• ⚠️ Error handling could be improved (see issue feat: Typescript support for scripts (alpha) #1)

---

🎯 Specific Recommendations

1. Decide on error handling strategy: Should onboarding failures block navigation or silently continue?
2. Clean up unused database field: Remove or document _first_time_user in login query
3. Add input validation: Implement maxlength on textarea
4. Consider performance optimization: Possibly include first_time_user in login response to avoid extra API call
5. Verify type generation: Ensure OpenAPI types are regenerated for the new endpoint

---

🧪 Testing Recommendations

Since this is a user-facing feature, ensure testing covers:

• ✅ First-time cloud users see onboarding
• ✅ Returning users skip onboarding
• ✅ Self-hosted instances skip onboarding
• ✅ Skip button works correctly
• ✅ Form submission handles API failures gracefully
• ✅ OAuth login flows trigger onboarding correctly
• ✅ Password login flows trigger onboarding correctly

---

📝 Reproduction Instructions for Testers

To test this feature on a cloud-hosted instance, follow these steps:

Testing the onboarding flow:

1. Create a fresh user account or reset the first_time_user flag in the database for an existing user: UPDATE password SET first_time_user = true WHERE email = 'test@example.com';
2. Log in to Windmill using either password authentication or OAuth (GitHub, Google, etc.)
3. After successful authentication, you should be automatically redirected to /user/onboarding instead of the workspace list
4. On the first step, select where you heard about Windmill from the 10 options provided (Search engine, YouTube, LinkedIn, Twitter, GitHub, Blog/Article, Word of mouth, Company, Event, or Other)
5. The interface should automatically advance to step 2 after selection
6. On step 2, enter your intended use case in the textarea (e.g., "Building AI agents and automation workflows")
7. Click "Continue" - the data should be submitted and you'll be redirected to /user/workspaces
8. Verify that subsequent logins do NOT show the onboarding flow (user is marked as completed)

Testing the skip functionality:

• On step 1, click "Skip" and verify you're redirected to workspaces without submitting data

Testing self-hosted behavior:

• On a self-hosted instance, verify that the onboarding flow never appears regardless of first_time_user status

Verifying data submission:

• Check network tab to confirm the POST request to /api/users/onboarding succeeds
• In enterprise edition, verify the data is properly stored

---

Summary

This is a well-implemented feature with clean code and good separation between OSS and EE versions. The main improvements needed are around error handling clarity, removing unused code, and minor optimizations. No blocking issues were found.

Recommendation: Approve with minor suggestions ✅

I've also left inline comments on the specific files for the most important issues.

---

[claude]

Unused Variable: The _first_time_user field is fetched from the database but not used (prefixed with _). This was previously used for cookie-based onboarding but that code was removed.

Options:

1. Remove it from the query entirely to save a tiny bit of performance
2. Keep it if there are plans to use it for other purposes
3. Add a comment explaining why it's kept

// If removing (option 1):
"SELECT email, password_hash, super_admin FROM password WHERE ..."
// Returns: Option<(String, String, bool)>

// If keeping with documentation (option 2):
// Note: first_time_user fetched but unused - kept for future analytics
if let Some((email, hash, super_admin, _first_time_user)) = email_w_h {

[claude]

Performance Consideration: This adds an extra globalWhoami() API call on every login for cloud-hosted instances.

Potential optimization: Consider including the first_time_user flag directly in the login response to eliminate this extra round-trip. This would require backend changes to the login endpoint, but could improve login performance slightly.

// Current: Login → Check globalWhoami() → Redirect
// Optimized: Login (returns first_time_user) → Redirect

Not critical, but worth considering if login performance becomes a concern.

[hugocasa]

/eeref

[windmill-internal-app]

Starting ee ref update...

[windmill-internal-app]

Successfully updated ee-repo-ref.txt