feat: improve organization creation and update validation

[wilsonrivera]

Summary by CodeRabbit

• New Features

  • Organization slug max length increased to 32; slug/name inputs are trimmed before validation.
  • Additional reserved slug blocked: "account".

• Bug Fixes

  • Organization slug matching is now case-insensitive.
  • Validation produces clearer bad-request errors for invalid names/slugs.

• Tests

  • Added extensive tests covering name/slug validation edge cases and reserved-slug checks.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Checklist

• I have discussed my proposed changes in an issue and have received approval to proceed.
• I have followed the coding standards of the project.
• Tests or benchmarks have been added or updated.
• Documentation has been updated on https://github.com/wundergraph/cosmo-docs.
• I have read the Contributors Guide.

Fixes a discrepancy in validation of the organization name between the frontend and backend

[coderabbitai]

Walkthrough

Centralizes organization validation into Zod schemas in constants.ts, replaces inline validators with schema.safeParse in create/update services, removes legacy util validators, makes slug lookups case-insensitive in the repository, updates tests and frontend validation, and removes one unused import.

Changes

Cohort / File(s)	Change Summary
Schema Centralization controlplane/src/core/constants.ts	Adds organizationSlugSchema and organizationSchema (Zod): trimmed/lowercased slug, regex for alphanumerics with optional hyphens, length 3–32, reserved keywords blocked; name trimmed, length 3–32.
Service Layer Validation controlplane/src/core/bufservices/organization/createOrganization.ts, controlplane/src/core/bufservices/organization/updateOrganizationDetails.ts	Use organizationSchema.safeParse(); return ERR_BAD_REQUEST with field errors on validation failure; downstream logic uses validated data.name/data.slug; removed prior isValid... checks; added Keycloak auth step in update flow.
Utility Module Cleanup controlplane/src/core/util.ts	Removes isValidOrganizationSlug, isValidOrganizationName, and organizationSlugRegex exports and related logic.
Repository Layer controlplane/src/core/repositories/OrganizationRepository.ts	bySlug() now performs case-insensitive match using lower(${organizations.slug}) and slug.toLowerCase().
Unused Import Cleanup controlplane/src/core/bufservices/check/getChecksByFederatedGraphName.ts	Removed unused federatedGraphs import from ../../../db/schema.js.
Unit Test Updates controlplane/src/core/util.test.ts	Replaces isValidOrganizationSlug usage with organizationSlugSchema.safeParse() and updates test cases (whitespace, single-char, overlong slug cases).
Integration Test Updates controlplane/test/deactivate-org.test.ts, controlplane/test/organization/create-organization.test.ts	Minor assignment change in deactivate-org.test.ts; extensive validation tests added in create-organization.test.ts covering whitespace-only, length bounds, reserved slugs, and ERR_BAD_REQUEST assertions.
Frontend Alignment studio/src/pages/create.tsx	Trims name/slug in validation schema, increases slug max length to 32, adds "account" to reserved slugs.
Manifest package.json	(Manifest file present in diff)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Verify repository case-insensitive slug query semantics and any DB index/performance impact.
• Confirm error payload structure/messages from services remain consistent after Zod validation.
• Check for any remaining references to removed isValid... helpers.
• Ensure frontend validation behavior (trim/lowercase and reserved list) matches backend schema.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title accurately summarizes the main change: introducing validation improvements for organization creation and update operations using schema-based validation.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate docstrings

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (3)

controlplane/src/core/repositories/OrganizationRepository.ts (1)

110-136: Case‑insensitive slug lookup is correct; consider an expression index if this becomes hot

Using lower(organizations.slug) compared to slug.toLowerCase() makes lookups resilient to existing mixed‑case data and better aligned with the new lowercasing validation. If bySlug is on a hot path and the table grows large, consider adding a functional index on lower(slug) in Postgres to preserve index usage for this query.

controlplane/src/core/bufservices/organization/updateOrganizationDetails.ts (1)

11-14: Schema-based validation for updates is good; confirm callers always send both name and slug

Using organizationSchema.safeParse and normalized validatedReq.data.name/slug for uniqueness checks, Keycloak group rename, role renames, and updateOrganization keeps update behavior consistent with the create flow and the centralized rules.

One thing to double‑check: this now always validates both organizationName and organizationSlug together. If any existing client relied on partially updating just the name or just the slug (leaving the other unset/empty), those requests will now return ERR_BAD_REQUEST due to the schema’s non‑optional fields. If partial updates are not supported anywhere, this is fine; otherwise you may want a variant schema that treats one of the fields as optional.

Also note that the audit log at the end still uses authContext.organizationSlug, which will hold the old slug when a rename occurs; if you want logs to reflect the new slug, consider switching this to the updated value.

Also applies to: 63-76, 96-111, 118-120

controlplane/test/organization/create-organization.test.ts (1)

1-1: Comprehensive negative tests for org creation look good; consider adding the “account” slug case

These new tests do a good job of asserting the schema-driven behavior for whitespace-only, too-short/too-long names and slugs, and reserved slug values, including the exact error messages returned as details.

Since organizationSlugSchema also treats "account" as a reserved keyword, you might want to extend the test.each table to include it, to keep test coverage aligned with the full reserved list:

-  test.each(['login', 'create', 'signup'])(
+  test.each(['login', 'create', 'signup', 'account'])(

Also applies to: 43-139

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 90e1bd1 and 3af1ebb.

📒 Files selected for processing (10)

• controlplane/src/core/bufservices/check/getChecksByFederatedGraphName.ts (0 hunks)
• controlplane/src/core/bufservices/organization/createOrganization.ts (4 hunks)
• controlplane/src/core/bufservices/organization/updateOrganizationDetails.ts (4 hunks)
• controlplane/src/core/constants.ts (2 hunks)
• controlplane/src/core/repositories/OrganizationRepository.ts (1 hunks)
• controlplane/src/core/util.test.ts (2 hunks)
• controlplane/src/core/util.ts (0 hunks)
• controlplane/test/deactivate-org.test.ts (1 hunks)
• controlplane/test/organization/create-organization.test.ts (2 hunks)
• studio/src/pages/create.tsx (2 hunks)

💤 Files with no reviewable changes (2)

• controlplane/src/core/util.ts
• controlplane/src/core/bufservices/check/getChecksByFederatedGraphName.ts

🧰 Additional context used

🧠 Learnings (2)

📚 Learning: 2025-08-29T10:28:04.846Z

Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
Learning: In the controlplane codebase, authentication and authorization checks (including organization scoping) are handled at the service layer in files like unlinkSubgraph.ts before calling repository methods. Repository methods like unlinkSubgraph() in SubgraphRepository.ts can focus purely on data operations without redundant security checks.

Applied to files:

• controlplane/src/core/bufservices/organization/updateOrganizationDetails.ts
• controlplane/src/core/bufservices/organization/createOrganization.ts
• controlplane/src/core/repositories/OrganizationRepository.ts

📚 Learning: 2025-10-17T16:35:24.723Z

Learnt from: Aenimus
Repo: wundergraph/cosmo PR: 2287
File: composition/tests/v1/federation-factory.test.ts:569-583
Timestamp: 2025-10-17T16:35:24.723Z
Learning: In the wundergraph/cosmo repository's Vitest test suite, `toHaveLength()` works with Map objects and `expect(map.has(key))` without an explicit matcher correctly asserts the boolean result in the test assertions for `composition/tests/v1/federation-factory.test.ts`.

Applied to files:

• controlplane/test/organization/create-organization.test.ts

🧬 Code graph analysis (5)

controlplane/src/core/bufservices/organization/updateOrganizationDetails.ts (1)

controlplane/src/core/constants.ts (1)

• organizationSchema (32-41)

controlplane/test/organization/create-organization.test.ts (2)

controlplane/test/test-util.ts (1)

• SetupTest (66-418)

controlplane/src/core/test-util.ts (1)

• genID (53-55)

controlplane/src/core/bufservices/organization/createOrganization.ts (1)

controlplane/src/core/constants.ts (1)

• organizationSchema (32-41)

controlplane/src/core/util.test.ts (1)

controlplane/src/core/constants.ts (1)

• organizationSlugSchema (15-30)

controlplane/src/core/repositories/OrganizationRepository.ts (1)

controlplane/src/db/schema.ts (1)

• organizations (1266-1289)

⏰ 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). (6)

• GitHub Check: Analyze (javascript-typescript)
• GitHub Check: build_push_image
• GitHub Check: Analyze (go)
• GitHub Check: build_test
• GitHub Check: build_push_image
• GitHub Check: build_test

🔇 Additional comments (4)

controlplane/test/deactivate-org.test.ts (1)

105-109: LGTM: capturing createOrganization result keeps behavior unchanged

Storing the createOrganization response in a local constant does not alter the test flow; nothing further needed here.

studio/src/pages/create.tsx (1)

83-107: Frontend validation now aligned with backend org schema

Trimming name/slug, lowercasing slug, extending length to 32, and blocking reserved slugs (including "account") bring the UI validation in line with the centralized backend schema, which should prevent most client/server mismatch issues.

controlplane/src/core/bufservices/organization/createOrganization.ts (1)

17-18: Centralized schema validation and use of normalized name/slug look solid

Validating the request with organizationSchema.safeParse and short‑circuiting with ERR_BAD_REQUEST plus field‑level messages is a clear improvement. Using validatedReq.data.name/slug for Keycloak seeding and createOrganization ensures the stored slug is the normalized, lowercased value enforced by the schema.

Also applies to: 30-39, 68-71, 82-85

controlplane/src/core/constants.ts (1)

1-1: Centralized org name/slug schemas are well-structured and match usage elsewhere

Defining organizationSlugSchema (trim + lowercase + regex + 3–32 length + reserved slugs) and organizationSchema here gives a single source of truth that both bufservices and tests consume, which is exactly what you want for consistent validation and error messaging. The frontend schema in studio/src/pages/create.tsx mirrors these rules closely, so future changes should mostly happen in this file.

Also applies to: 15-41

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

controlplane/src/core/bufservices/organization/createOrganization.ts (1)

17-39: Centralized schema validation looks good; consider narrowing the validated payload and error reporting

Good call moving name/slug validation into organizationSchema and failing fast with ERR_BAD_REQUEST. This keeps the handler consistent with other schema-driven validations.

Two small, optional tweaks you might consider:

1. Validate only the fields you care about
   Right now you call organizationSchema.safeParse(req), while the schema only describes { name, slug }. To avoid relying on how unknown keys are treated and to decouple from future changes to CreateOrganizationRequest, you could explicitly project the payload:

-    const validatedReq = organizationSchema.safeParse(req);
+    const validatedReq = organizationSchema.safeParse({
+      name: req.name,
+      slug: req.slug,
+    });

2. Expose or log multiple field errors (optional)
   fieldErrors.name?.[0] || fieldErrors.slug?.[0] only surfaces the first available message. If both fields are invalid, you might want to either:
   • Concatenate messages into a single string, or
   • Prefer one field deterministically (e.g., slug-first) depending on UX expectations, and optionally log the full validatedReq.error at debug level for easier diagnosis.

These are non-blocking; the current behavior is functionally correct.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3af1ebb and 85be4dc.

📒 Files selected for processing (4)

• controlplane/src/core/bufservices/organization/createOrganization.ts (4 hunks)
• controlplane/src/core/bufservices/organization/updateOrganizationDetails.ts (4 hunks)
• controlplane/src/core/constants.ts (2 hunks)
• controlplane/src/core/util.test.ts (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (3)

• controlplane/src/core/bufservices/organization/updateOrganizationDetails.ts
• controlplane/src/core/util.test.ts
• controlplane/src/core/constants.ts

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-08-29T10:28:04.846Z

Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
Learning: In the controlplane codebase, authentication and authorization checks (including organization scoping) are handled at the service layer in files like unlinkSubgraph.ts before calling repository methods. Repository methods like unlinkSubgraph() in SubgraphRepository.ts can focus purely on data operations without redundant security checks.

Applied to files:

• controlplane/src/core/bufservices/organization/createOrganization.ts

🧬 Code graph analysis (1)

controlplane/src/core/bufservices/organization/createOrganization.ts (1)

controlplane/src/core/constants.ts (1)

• organizationSchema (33-42)

⏰ 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). (6)

• GitHub Check: build_push_image
• GitHub Check: build_test
• GitHub Check: build_test
• GitHub Check: build_push_image
• GitHub Check: Analyze (javascript-typescript)
• GitHub Check: Analyze (go)

🔇 Additional comments (1)

controlplane/src/core/bufservices/organization/createOrganization.ts (1)

70-85: Using validated slug/name for Keycloak seeding and org creation is correct

Switching seedGroup and createOrganization to use validatedReq.data.slug and validatedReq.data.name ensures that any schema-level transforms (e.g., trimming, normalization) are applied consistently across Keycloak and the database. This avoids subtle mismatches between what was validated and what is persisted / propagated.

No changes requested here; this looks solid.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

controlplane/src/core/util.test.ts (1)

204-222: Slug tests now match the 3–32 char schema; consider a couple of extra cases

The updated expectations and new cases ('25CharactersLong123456789' as valid, whitespace-only, too-short, and too-long slugs) correctly reflect organizationSlugSchema (trim + lowercase, 3–32 chars, regex constraints), and checking safeParse(...).success against expected is sound.

If you want to harden coverage a bit further, you could optionally:

• Add a case for a reserved keyword (e.g. 'login') expecting false.
• Add one test that asserts the normalized output (e.g. 'ACME-Corp' parses successfully and parsed.data === 'acme-corp').

Not required for this PR, but would more tightly pin the schema’s behavior.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 85be4dc and 04da435.

📒 Files selected for processing (1)

• controlplane/src/core/util.test.ts (2 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-10-17T16:35:24.723Z

Learnt from: Aenimus
Repo: wundergraph/cosmo PR: 2287
File: composition/tests/v1/federation-factory.test.ts:569-583
Timestamp: 2025-10-17T16:35:24.723Z
Learning: In the wundergraph/cosmo repository's Vitest test suite, `toHaveLength()` works with Map objects and `expect(map.has(key))` without an explicit matcher correctly asserts the boolean result in the test assertions for `composition/tests/v1/federation-factory.test.ts`.

Applied to files:

• controlplane/src/core/util.test.ts

🧬 Code graph analysis (1)

controlplane/src/core/util.test.ts (1)

controlplane/src/core/constants.ts (1)

• organizationSlugSchema (15-31)

⏰ 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). (6)

• GitHub Check: build_test
• GitHub Check: build_push_image
• GitHub Check: build_push_image
• GitHub Check: build_test
• GitHub Check: Analyze (go)
• GitHub Check: Analyze (javascript-typescript)

🔇 Additional comments (1)

controlplane/src/core/util.test.ts (1)

2-3: Import changes correctly switch tests to the schema-based validator

Dropping the old slug validator import and using organizationSlugSchema keeps tests aligned with the new centralized validation logic in constants.ts. No issues here.

[StarpTech]

LGTM