Skip to content

fix: coerce is_telemetry_enabled form values on instance admin signup#9415

Open
pranav-afk wants to merge 1 commit into
makeplane:previewfrom
pranav-afk:fix/9370-telemetry-bool-signup
Open

fix: coerce is_telemetry_enabled form values on instance admin signup#9415
pranav-afk wants to merge 1 commit into
makeplane:previewfrom
pranav-afk:fix/9370-telemetry-bool-signup

Conversation

@pranav-afk

@pranav-afk pranav-afk commented Jul 13, 2026

Copy link
Copy Markdown

Description

Fixes instance admin first-run signup when is_telemetry_enabled is posted as a form string.

POST /api/instances/admins/sign-up/ reads telemetry from the form:

is_telemetry_enabled = request.POST.get("is_telemetry_enabled", True)

When the field is omitted, the default is a real bool. When the form sends is_telemetry_enabled=false / true, Django returns a string. That string was assigned to Instance.is_telemetry_enabled (BooleanField) and instance.save() could 500.

Because the handler creates the user and InstanceAdmin before that save, a failure left the instance half-configured:

• user + instance admin created
• instances.is_setup_done never set
• retry → ADMIN_ALREADY_EXIST
• normal login → INSTANCE_NOT_CONFIGURED
• no UI/API recovery without a manual DB update

Change: coerce form values to a real boolean before assignment:

• already a bool → keep as-is
• otherwise treat truthy strings as true: 1, true, yes, on (case-insensitive)
• everything else (including "false") → False

User/admin/instance writes already run inside transaction.atomic(), so a later failure no longer strands a partial admin signup the same way.

Type of Change
• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] Feature (non-breaking change which adds functionality)
• [ ] Improvement (change that would cause existing functionality to not work as expected)
• [ ] Code refactoring
• [ ] Performance improvements
• [ ] Documentation update

Screenshots and Media (if applicable)
N/A — backend auth/bootstrap only.

Test Scenarios
• Fresh instance (no admin, is_setup_done false)
• Sign up with is_telemetry_enabled=false (form-encoded) → 200 redirect, setup completes, can sign in
• Fresh instance again: omit is_telemetry_enabled → defaults to enabled, setup completes
• Sign up with is_telemetry_enabled=true / True / 1 → telemetry stays enabled
• Confirm instances.is_setup_done is true and is_telemetry_enabled matches the intended value
• Confirm a second signup still returns ADMIN_ALREADY_EXIST (guard unchanged)

References
Fixes #9370
• Related: #8828 (same endpoint / telemetry form string pattern)

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of the telemetry setting during instance setup.
    • Ensures telemetry preferences are consistently interpreted and saved as true or false values.

Form-encoded signup can post is_telemetry_enabled as the string false or
true. Assigning that string to a BooleanField caused save() to 500 after
creating the admin user, leaving the instance half-configured. Coerce the
value to a real bool before writing. Writes already run in transaction.atomic.

Fixes makeplane#9370
@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 57d8992f-44e4-4e48-a97a-4c07831d1b5e

📥 Commits

Reviewing files that changed from the base of the PR and between dc9d80b and 440dc21.

📒 Files selected for processing (1)
  • apps/api/plane/license/api/views/admin.py

📝 Walkthrough

Walkthrough

The admin sign-up endpoint now converts the is_telemetry_enabled form value into a boolean before including it in error payloads or saving it to the instance.

Changes

Telemetry flag normalization

Layer / File(s) Summary
Normalize sign-up telemetry input
apps/api/plane/license/api/views/admin.py
The endpoint preserves boolean inputs and converts case-insensitive string values to booleans using recognized truthy inputs.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested reviewers: dheeru0198, pablohashescobar

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: coercing form values for is_telemetry_enabled during instance admin signup.
Description check ✅ Passed The description follows the template with Description, Type of Change, Screenshots, Test Scenarios, and References filled in.
Linked Issues check ✅ Passed The change matches #9370 by normalizing telemetry form values before saving and addressing the signup failure described.
Out of Scope Changes check ✅ Passed No unrelated code changes are indicated; the diff stays focused on telemetry value handling in the signup endpoint.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

[bug] Instance admin sign-up 500s when is_telemetry_enabled is posted as a form string, leaving the instance half-configured (is_setup_done never set)

1 participant