-
Notifications
You must be signed in to change notification settings - Fork 5
Reduce Minimum Org name length in Onboard API #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
Conversation
WalkthroughRelaxed minimum length validation for OnboardingRequest.organization_name in backend/app/models/onboarding.py from 3 to 1. No other logic or control flow changes. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 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 (
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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: 1
🧹 Nitpick comments (3)
backend/app/models/onboarding.py (3)
69-81: Normalize names pre-validation (trim/collapse spaces).To avoid
"\tA B "variants and accidental empties after trim, add a pre-validation normalizer.You can add this validator (outside the shown lines) above
set_defaults:@model_validator(mode="before") @classmethod def _normalize(cls, data): if isinstance(data, dict): for key in ("organization_name", "project_name"): v = data.get(key) if isinstance(v, str): # trim and collapse internal whitespace v = re.sub(r"\s+", " ", v.strip()) data[key] = v return dataAnd keep the
pattern=r".*\S.*"guard to reject all-whitespace after trimming.
25-29: Add targeted tests for the relaxed constraint.Cover edge cases: 1-char ok, whitespace-only rejected (with the new pattern), long names near limit.
Example pytest cases (API route):
def test_onboard_org_name_single_char(client, superuser_token_headers): r = client.post("/api/v1/onboard", json={"organization_name": "A", "project_name": "P"}, headers=superuser_token_headers) assert r.status_code == 201 def test_onboard_org_name_whitespace_rejected(client, superuser_token_headers): r = client.post("/api/v1/onboard", json={"organization_name": " ", "project_name": "P"}, headers=superuser_token_headers) assert r.status_code == 422Want me to add these in backend/app/tests/api/routes/test_onboarding.py?
25-29: Align onboarding.organization_name max_length with other modelsThe
organization_namefield inbackend/app/models/onboarding.pyis currently capped atmax_length=100, while all core models usemax_length=255for organization and project names. To maintain consistency and avoid surprising validation errors during onboarding, update this limit to 255.• File:
backend/app/models/onboarding.py
Change:organization_name: str = Field( description="Name of the organization to be created or linked", - min_length=1, - max_length=100, + min_length=1, + max_length=255, )
📜 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 (1)
backend/app/models/onboarding.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/app/models/onboarding.py (3)
backend/app/models/organization.py (1)
OrganizationUpdate(29-31)backend/app/tests/crud/test_onboarding.py (1)
test_onboard_project_new_organization_project_user(26-90)backend/app/tests/api/routes/test_onboarding.py (1)
test_onboard_project_with_auto_generated_defaults(131-160)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: checks (3.11.7, 6)
🔇 Additional comments (1)
backend/app/models/onboarding.py (1)
25-29: Change aligns with PR objective; OK to ship.Lowering
organization_namemin length to 1 matches the requirement and is safe from a model perspective.
| organization_name: str = Field( | ||
| description="Name of the organization to be created or linked", | ||
| min_length=3, | ||
| min_length=1, | ||
| max_length=100, | ||
| ) |
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
Block whitespace-only org names.
With min_length=1, inputs like " " will pass validation. Add a regex constraint to require at least one non-whitespace character.
Apply this minimal diff:
organization_name: str = Field(
description="Name of the organization to be created or linked",
- min_length=1,
+ min_length=1,
+ pattern=r".*\S.*",
max_length=100,
)📝 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.
| organization_name: str = Field( | |
| description="Name of the organization to be created or linked", | |
| min_length=3, | |
| min_length=1, | |
| max_length=100, | |
| ) | |
| organization_name: str = Field( | |
| description="Name of the organization to be created or linked", | |
| min_length=1, | |
| pattern=r".*\S.*", | |
| max_length=100, | |
| ) |
🤖 Prompt for AI Agents
In backend/app/models/onboarding.py around lines 25 to 29, the organization_name
Field allows whitespace-only strings because min_length=1 accepts " " — add a
regex constraint such as regex=r".*\S.*" to require at least one non-whitespace
character (i.e. change the Field call to include regex=r".*\S.*" while keeping
min_length and max_length).
Checklist
Before submitting a pull request, please ensure that you mark these task.
fastapi run --reload app/main.pyordocker compose upin the repository root and test.Notes
Please add here if any other information is required for the reviewer.
Summary by CodeRabbit