Add support to attach Flows & Layouts at the OU level#4302
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughOrganization units now carry default authentication, registration, recovery, and sign-out flow identifiers. The service validates configured flows, persistence stores the identifiers and flags, HTTP handlers and imports propagate them, and startup injects the flow resolver. ChangesOrganization Unit Default Flows
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant OUHandler
participant organizationUnitService
participant OUFlowResolver
participant OUStore
Client->>OUHandler: Create or update OU with flow fields
OUHandler->>organizationUnitService: Forward OU request
organizationUnitService->>OUFlowResolver: Validate configured flow IDs
OUFlowResolver-->>organizationUnitService: Validity or ServiceError
organizationUnitService->>OUStore: Persist validated OU metadata
OUStore-->>organizationUnitService: Stored organization unit
organizationUnitService-->>OUHandler: Organization unit response
OUHandler-->>Client: JSON response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@backend/pkg/thunderidengine/providers/model.go`:
- Around line 62-65: Document the new Organization Unit flow fields AuthFlowID,
RegistrationFlowID, RecoveryFlowID, and SignOutFlowID in docs/content/apis.mdx
for create, update, and response payloads. Add or update a guide under
docs/content/guides/ explaining OU-level default authentication, registration,
recovery, and sign-out flows and layouts, and document these fields in the
relevant import/declarative-resource guide.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 353e718d-2878-42f9-8bad-bf88d982fef2
⛔ Files ignored due to path filters (2)
backend/tests/mocks/oumock/ConfigurableOUService_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/oumock/OUFlowResolver_mock.gois excluded by!**/*_mock.go
📒 Files selected for processing (18)
backend/cmd/server/servicemanager.gobackend/dbscripts/entitydb/postgres.sqlbackend/dbscripts/entitydb/sqlite.sqlbackend/internal/ou/ConfigurableOUService_mock_test.gobackend/internal/ou/OUFlowResolver_mock_test.gobackend/internal/ou/error_constants.gobackend/internal/ou/file_based_store_test.gobackend/internal/ou/handler.gobackend/internal/ou/handler_test.gobackend/internal/ou/model.gobackend/internal/ou/service.gobackend/internal/ou/service_test.gobackend/internal/ou/store.gobackend/internal/ou/store_constants.gobackend/internal/ou/store_test.gobackend/internal/system/importer/service_adapters.gobackend/internal/system/importer/service_test.gobackend/pkg/thunderidengine/providers/model.go
7a09f57 to
fee38c8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@backend/dbscripts/entitydb/postgres.sql`:
- Around line 11-17: Align the registration-flow schema defaults with the
application’s false/zero contract by changing IS_REGISTRATION_FLOW_ENABLED from
default '1' to default '0' in backend/dbscripts/entitydb/postgres.sql lines
11-17 and backend/dbscripts/entitydb/sqlite.sql lines 11-17. Leave the recovery
and signout defaults unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9b32bd1d-63e1-49cb-8a15-1978c3ff8939
⛔ Files ignored due to path filters (2)
backend/tests/mocks/oumock/ConfigurableOUService_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/oumock/OUFlowResolver_mock.gois excluded by!**/*_mock.go
📒 Files selected for processing (20)
backend/cmd/server/config/resources/server_configs/cors.yamlbackend/cmd/server/deployment.yamlbackend/cmd/server/servicemanager.gobackend/dbscripts/entitydb/postgres.sqlbackend/dbscripts/entitydb/sqlite.sqlbackend/internal/ou/ConfigurableOUService_mock_test.gobackend/internal/ou/OUFlowResolver_mock_test.gobackend/internal/ou/error_constants.gobackend/internal/ou/file_based_store_test.gobackend/internal/ou/handler.gobackend/internal/ou/handler_test.gobackend/internal/ou/model.gobackend/internal/ou/service.gobackend/internal/ou/service_test.gobackend/internal/ou/store.gobackend/internal/ou/store_constants.gobackend/internal/ou/store_test.gobackend/internal/system/importer/service_adapters.gobackend/internal/system/importer/service_test.gobackend/pkg/thunderidengine/providers/model.go
🚧 Files skipped from review as they are similar to previous changes (14)
- backend/internal/system/importer/service_adapters.go
- backend/internal/ou/file_based_store_test.go
- backend/internal/ou/handler.go
- backend/cmd/server/servicemanager.go
- backend/internal/system/importer/service_test.go
- backend/internal/ou/error_constants.go
- backend/internal/ou/store_constants.go
- backend/internal/ou/OUFlowResolver_mock_test.go
- backend/internal/ou/store.go
- backend/internal/ou/ConfigurableOUService_mock_test.go
- backend/internal/ou/handler_test.go
- backend/internal/ou/service.go
- backend/internal/ou/model.go
- backend/internal/ou/store_test.go
fee38c8 to
959f6fd
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@backend/internal/ou/service.go`:
- Around line 392-405: The Organization Unit flow fields and default-flow
validation behavior need documentation. Update docs/content/apis.mdx to describe
authFlowId, registrationFlowId, recoveryFlowId, signOutFlowId, and their
enablement fields, and update the relevant guide under docs/content/guides/ with
validation rules, accepted flow types, and invalid-flow responses.
- Around line 339-342: In CreateOrganizationUnit, run the OU authorization check
via checkOUAccess before invoking validateDefaultFlows and resolving flows
through flowMgtService.IsValidFlow. Ensure unauthorized callers are rejected
before any flow ID validity or type information is accessed, while preserving
the existing validation behavior for authorized requests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9e55fffa-2ec3-46c4-9024-7cae4e258b07
⛔ Files ignored due to path filters (2)
backend/tests/mocks/oumock/ConfigurableOUService_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/oumock/OUFlowResolver_mock.gois excluded by!**/*_mock.go
📒 Files selected for processing (18)
backend/cmd/server/servicemanager.gobackend/dbscripts/entitydb/postgres.sqlbackend/dbscripts/entitydb/sqlite.sqlbackend/internal/ou/ConfigurableOUService_mock_test.gobackend/internal/ou/OUFlowResolver_mock_test.gobackend/internal/ou/error_constants.gobackend/internal/ou/file_based_store_test.gobackend/internal/ou/handler.gobackend/internal/ou/handler_test.gobackend/internal/ou/model.gobackend/internal/ou/service.gobackend/internal/ou/service_test.gobackend/internal/ou/store.gobackend/internal/ou/store_constants.gobackend/internal/ou/store_test.gobackend/internal/system/importer/service_adapters.gobackend/internal/system/importer/service_test.gobackend/pkg/thunderidengine/providers/model.go
🚧 Files skipped from review as they are similar to previous changes (16)
- backend/cmd/server/servicemanager.go
- backend/internal/system/importer/service_adapters.go
- backend/internal/ou/handler.go
- backend/dbscripts/entitydb/sqlite.sql
- backend/internal/ou/file_based_store_test.go
- backend/internal/ou/handler_test.go
- backend/internal/ou/OUFlowResolver_mock_test.go
- backend/pkg/thunderidengine/providers/model.go
- backend/internal/ou/model.go
- backend/internal/ou/ConfigurableOUService_mock_test.go
- backend/dbscripts/entitydb/postgres.sql
- backend/internal/ou/store_constants.go
- backend/internal/system/importer/service_test.go
- backend/internal/ou/store.go
- backend/internal/ou/store_test.go
- backend/internal/ou/service_test.go
765b48d to
634d056
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
634d056 to
5c17a19
Compare
| type: string | ||
| format: uuid | ||
| description: "Layout configuration ID for the organization unit" | ||
| authFlowId: |
There was a problem hiding this comment.
This uses short format and other configs uses the expanded full name. Better to stick to a consistent pattern
There was a problem hiding this comment.
Application also uses the similar format. @coderabbitai Create an issue to track this refactor.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
gh issue create \
--repo thunder-id/thunderid \
--title "Standardize Organization Unit flow configuration naming" \
--assignee brionmario \
--body "$(cat <<'EOF'
## Summary
Standardize the naming convention used for Organization Unit default flow configuration so it is consistent with the corresponding application configuration and the established expanded-name pattern.
## Rationale
The Organization Unit API and related models currently use a short naming form for the new default-flow configuration. Aligning these names with the broader configuration convention will improve consistency and reduce ambiguity for API consumers and maintainers.
## Affected areas
- Organization Unit OpenAPI schemas in `api/ou.yaml`
- Organization Unit request/response models and handlers
- Declarative/YAML import mappings
- Persistence mappings, tests, and any clients or documentation that expose these fields
## Required changes
1. Inventory the existing Organization Unit and application flow configuration field names.
2. Define and apply one consistent expanded naming convention for the Organization Unit flow configuration fields.
3. Update affected API contracts, backend mappings, declarative import support, and tests together.
4. Assess and document compatibility or migration requirements for existing API consumers and persisted/declarative configurations before renaming externally visible fields.
## Acceptance criteria
- [ ] Organization Unit flow configuration uses the agreed consistent naming convention across API contracts, models, handlers, imports, and persistence.
- [ ] API/OpenAPI documentation and declarative/YAML mappings reflect the final names.
- [ ] Relevant unit and integration tests cover the standardized names.
- [ ] Compatibility or migration behavior for existing consumers is explicitly documented.
Requested by: `@brionmario`
Related pull request: https://github.com/thunder-id/thunderid/pull/4302
Related review comment: https://github.com/thunder-id/thunderid/pull/4302#discussion_r3640692447
EOF
)"Length of output: 2266
@brionmario, created follow-up issue #4314 to track the Organization Unit flow configuration naming refactor.
🧠 Learnings used
Learnt from: AnuradhaSK
Repo: thunder-id/thunderid PR: 3528
File: backend/internal/ou/handler_test.go:1949-2079
Timestamp: 2026-06-25T09:59:08.858Z
Learning: In thunder-id/thunderid, the rendered API reference content is generated from the root-level OpenAPI specs in `api/*.yaml` (combined via `pnpm generate:api-specs` / `scripts/merge-openapi-specs.mjs` and consumed by Scalar). During API-change reviews, treat changes to the relevant `api/*.yaml` files as the documentation update for the API reference, and do not treat `docs/content/apis.mdx` as per-endpoint API documentation (it’s only a thin wrapper that renders `ApiVersionReference`).
Learnt from: AnuradhaSK
Repo: thunder-id/thunderid PR: 3528
File: backend/internal/role/init.go:73-74
Timestamp: 2026-06-25T10:17:36.544Z
Learning: For API changes, update the source OpenAPI specs under `api/*.yaml` (the authoritative docs). Do not make manual per-endpoint documentation edits in `docs/content/apis.mdx`, since it’s only a thin wrapper for the Scalar viewer. `pnpm generate:api-specs` merges the `api/*.yaml` files via `scripts/merge-openapi-specs.mjs` into `docs/static/api/combined.yaml`, which Scalar serves at the `/api` route—so the required review check is that the relevant `api/*.yaml` specs reflect the change.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@api/ou.yaml`:
- Around line 1479-1503: Expose the authentication, registration, recovery, and
sign-out flow IDs and enablement flags through OrganizationUnitListResponse by
adding these properties to OrganizationUnitBasic, or by changing its list item
reference to OrganizationUnit. Keep the property types, UUID formats, and
descriptions consistent with the existing OrganizationUnit definitions so
generated clients include them.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 342dbc50-7945-418a-94aa-5b1c238a74d4
⛔ Files ignored due to path filters (2)
backend/tests/mocks/oumock/ConfigurableOUService_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/oumock/OUFlowResolver_mock.gois excluded by!**/*_mock.go
📒 Files selected for processing (20)
api/ou.yamlbackend/cmd/server/servicemanager.gobackend/dbscripts/entitydb/postgres.sqlbackend/dbscripts/entitydb/sqlite.sqlbackend/internal/ou/ConfigurableOUService_mock_test.gobackend/internal/ou/OUFlowResolver_mock_test.gobackend/internal/ou/error_constants.gobackend/internal/ou/file_based_store_test.gobackend/internal/ou/handler.gobackend/internal/ou/handler_test.gobackend/internal/ou/model.gobackend/internal/ou/service.gobackend/internal/ou/service_test.gobackend/internal/ou/store.gobackend/internal/ou/store_constants.gobackend/internal/ou/store_test.gobackend/internal/system/i18n/core/defaults.gobackend/internal/system/importer/service_adapters.gobackend/internal/system/importer/service_test.gobackend/pkg/thunderidengine/providers/model.go
🚧 Files skipped from review as they are similar to previous changes (16)
- backend/cmd/server/servicemanager.go
- backend/internal/ou/handler.go
- backend/internal/system/importer/service_adapters.go
- backend/dbscripts/entitydb/postgres.sql
- backend/internal/ou/file_based_store_test.go
- backend/internal/ou/OUFlowResolver_mock_test.go
- backend/internal/system/importer/service_test.go
- backend/internal/ou/ConfigurableOUService_mock_test.go
- backend/internal/ou/store.go
- backend/internal/ou/handler_test.go
- backend/pkg/thunderidengine/providers/model.go
- backend/internal/ou/model.go
- backend/internal/ou/store_constants.go
- backend/internal/ou/service.go
- backend/internal/ou/store_test.go
- backend/internal/ou/service_test.go
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@backend/internal/ou/store_constants.go`:
- Around line 191-207: The ORGANIZATION_UNIT metadata migration must preserve
legacy themeId and layoutId values. Update the relevant organization-unit
read/create flow around buildOrganizationUnitFromResultRow and the affected SQL
queries to backfill legacy columns into METADATA or retain a temporary
compatibility fallback, ensuring existing records return non-empty
themeId/layoutId.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 66541776-a3a9-4199-95d0-6ba9dd1e5394
⛔ Files ignored due to path filters (2)
backend/tests/mocks/oumock/ConfigurableOUService_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/oumock/OUFlowResolver_mock.gois excluded by!**/*_mock.go
📒 Files selected for processing (18)
api/ou.yamlbackend/.mockery.public.ymlbackend/dbscripts/entitydb/postgres.sqlbackend/dbscripts/entitydb/sqlite.sqlbackend/internal/ou/ConfigurableOUService_mock_test.gobackend/internal/ou/OUFlowResolver_mock_test.gobackend/internal/ou/file_based_store_test.gobackend/internal/ou/handler.gobackend/internal/ou/handler_test.gobackend/internal/ou/model.gobackend/internal/ou/service.gobackend/internal/ou/service_test.gobackend/internal/ou/store.gobackend/internal/ou/store_constants.gobackend/internal/ou/store_test.gobackend/internal/system/importer/service_adapters.gobackend/internal/system/importer/service_test.gobackend/pkg/thunderidengine/providers/model.go
💤 Files with no reviewable changes (8)
- backend/internal/system/importer/service_adapters.go
- backend/internal/ou/handler.go
- backend/internal/ou/model.go
- backend/dbscripts/entitydb/postgres.sql
- backend/internal/system/importer/service_test.go
- backend/internal/ou/file_based_store_test.go
- backend/dbscripts/entitydb/sqlite.sql
- backend/internal/ou/handler_test.go
🚧 Files skipped from review as they are similar to previous changes (5)
- backend/internal/ou/ConfigurableOUService_mock_test.go
- backend/pkg/thunderidengine/providers/model.go
- backend/internal/ou/service_test.go
- backend/internal/ou/service.go
- backend/internal/ou/store_test.go
Add support to attach Flows & Layouts at the OU level
a02f2ca to
5b6f531
Compare
Purpose
Approach
Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes