Fix instance admin sign-up with form-encoded telemetry value#9405
Fix instance admin sign-up with form-encoded telemetry value#9405nicksan222 wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe instance admin sign-up endpoint now converts form-encoded telemetry values into booleans. A contract test covers false and true inputs, successful setup, redirect behavior, and administrator record creation. ChangesInstance admin sign-up
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Pull request overview
This PR fixes first-run instance administrator sign-up failing when is_telemetry_enabled is submitted as a form-encoded string by coercing the form value to a Python boolean before persisting it, and adds a contract test to ensure setup completes and the telemetry flag is stored correctly.
Changes:
- Coerce
is_telemetry_enabledfromrequest.POSTinto a boolean ("1"/"true"→True, otherwiseFalse) prior to saving theInstance. - Add a contract test that exercises the sign-up redirect flow and asserts the instance becomes configured and telemetry is persisted as expected.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/api/plane/license/api/views/admin.py | Coerces is_telemetry_enabled POST form value to a boolean before saving the instance. |
| apps/api/plane/tests/contract/app/test_instance_admin.py | Adds a contract test validating sign-up completes and telemetry preference persists correctly. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/api/plane/tests/contract/app/test_instance_admin.py (1)
19-22: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the documented
"1"truthy form value.The endpoint explicitly supports both
"true"and"1", but this contract only exercises"true"and"false". Add("1", True)so the second supported truthy representation is protected by regression coverage.Proposed test update
("form_value", "expected_value"), - [("false", False), ("true", True)], + [("false", False), ("true", True), ("1", True)],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/plane/tests/contract/app/test_instance_admin.py` around lines 19 - 22, Add the documented ("1", True) case to the parameter list in the instance-admin contract test, preserving the existing "true" and "false" cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/api/plane/tests/contract/app/test_instance_admin.py`:
- Around line 19-22: Add the documented ("1", True) case to the parameter list
in the instance-admin contract test, preserving the existing "true" and "false"
cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c2d04425-904a-43ff-a183-1d19bd73d224
📒 Files selected for processing (2)
apps/api/plane/license/api/views/admin.pyapps/api/plane/tests/contract/app/test_instance_admin.py
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Description
Fixes instance administrator sign-up failing when
is_telemetry_enabledis submitted as a form-encoded string.The endpoint now converts supported truthy form values (
"true"and"1") into a Python boolean before assigning the value to the instance model. This prevents PostgreSQL from rejecting string values for the boolean column and allows the existing atomic setup flow to complete successfully.A contract test covers both
"true"and"false"form values and verifies that:Type of Change
Screenshots and Media (if applicable)
Not applicable.
Test Scenarios
is_telemetry_enabled=falseand verify setup completes with telemetry disabled.is_telemetry_enabled=trueand verify setup completes with telemetry enabled.References
Fixes #9370
Summary by CodeRabbit
true,false, and1), and persist the normalized setting during instance setup.