-
-
Notifications
You must be signed in to change notification settings - Fork 301
feat(backend): add support to disable creation of default demo items #1118
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds a boolean configuration option (CreateDefaultDemoItems) to control automatic creation of default demo items, wires it from config through service initialization into UserService, and makes demo-item bootstrap optional and non-fatal (errors logged) when enabled. Changes
sequenceDiagram
autonumber
participant Operator
participant Config as "Config (conf.go)"
participant Main as "main.go"
participant Services as "services.New / options"
participant UserSvc as "UserService"
Operator->>Config: set CreateDefaultDemoItems (env/CLI)
Config->>Main: provide Options
Main->>Services: New(..., WithCreateDefaultDemoItems(cfg.Options.CreateDefaultDemoItems))
Services->>UserSvc: construct with createDefaultDemoItems flag
UserSvc->>UserSvc: on RegisterUser / registerOIDCUser -> if createDefaultDemoItems then create demo labels/locations
alt creation errors
UserSvc-->>UserSvc: log errors (do not abort)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Security recommendations
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/en/configure/index.md (1)
47-47: Clarify description and hint at production usageThe description is clear, but you might explicitly mention that setting this to
falseskips seeding demo labels/locations, and recommend disabling it in hardened/production deployments to avoid unnecessary sample data being created automatically.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/app/api/main.go(1 hunks)backend/internal/core/services/all.go(3 hunks)backend/internal/core/services/service_user.go(3 hunks)backend/internal/data/repo/repo_item_templates.go(2 hunks)backend/internal/data/repo/repo_items_test.go(0 hunks)backend/internal/sys/config/conf.go(2 hunks)docs/en/configure/index.md(2 hunks)
💤 Files with no reviewable changes (1)
- backend/internal/data/repo/repo_items_test.go
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-07T07:51:13.533Z
Learnt from: JeffResc
Repo: sysadminsmedia/homebox PR: 996
File: backend/app/api/providers/oidc.go:156-170
Timestamp: 2025-09-07T07:51:13.533Z
Learning: The HomeBox authentication system uses email as the primary user identifier with a unique database constraint. Email-based OIDC identity binding was chosen over issuer+subject pairs to avoid major architectural refactoring, despite some theoretical security trade-offs.
Applied to files:
docs/en/configure/index.md
🧬 Code graph analysis (3)
backend/app/api/main.go (2)
backend/internal/core/services/all.go (1)
WithCreateDefaultDemoItems(37-41)backend/internal/sys/config/conf.go (1)
Options(36-46)
backend/internal/core/services/all.go (2)
backend/internal/core/currencies/currencies.go (1)
Currency(74-80)backend/internal/core/services/service_user.go (1)
UserService(23-26)
backend/internal/core/services/service_user.go (1)
backend/internal/data/repo/repos_all.go (1)
AllRepos(11-22)
⏰ 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). (18)
- GitHub Check: Frontend Tests / Integration Tests
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 4/4
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 1/4
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 3/4
- GitHub Check: Frontend Tests / Integration Tests PGSQL 16
- GitHub Check: Frontend Tests / Integration Tests PGSQL 15
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 2/4
- GitHub Check: Frontend Tests / Integration Tests PGSQL 17
- GitHub Check: Backend Server Tests / Go
- GitHub Check: build (linux/arm/v7)
- GitHub Check: build (linux/arm64)
- GitHub Check: build (linux/arm/v7)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/arm64)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/arm64)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/arm/v7)
🔇 Additional comments (6)
docs/en/configure/index.md (1)
269-269: CLI flag docs align with the new optionThe
--options-create-default-demo-itemsflag and default value match the new config option and env var naming; looks consistent.backend/internal/data/repo/repo_item_templates.go (1)
46-58: Struct tag adjustments preserve existing API semanticsThe tag layout for the
Default*andDefaultLabelIDsfields remains consistent with pointer +omitempty+x-nullableusage, so the JSON/validation behavior stays intact. No issues from these changes.Also applies to: 75-87
backend/app/api/main.go (1)
172-180: Config flag is correctly wired into service initializationPassing
services.WithCreateDefaultDemoItems(cfg.Options.CreateDefaultDemoItems)here cleanly exposes the new option to the service layer while keeping defaults inservices.New. Looks good.backend/internal/sys/config/conf.go (1)
36-46: New CreateDefaultDemoItems option is consistent with existing config surface
CreateDefaultDemoItemswithyaml:"create_default_demo_items" conf:"default:true"fits the existingOptionspattern and matches the documented env/CLI names. The other tagged lines here appear to be format-only; no functional changes.Also applies to: 69-77, 84-90
backend/internal/core/services/all.go (1)
19-23: Option plumbing for createDefaultDemoItems is sound and backward-compatibleThe new
createDefaultDemoItemsfield, its default oftrue, and theWithCreateDefaultDemoItemsoption follow the existing options pattern and ensureUserServicesees the configured value viaservices.New. No issues from a wiring or initialization perspective.Also applies to: 37-41, 55-59, 65-77
backend/internal/core/services/service_user.go (1)
23-26: Gating demo bootstrap is correct; consider harmonizing error handling and verify construction sitesThe new
createDefaultDemoItemsflag onUserServiceand the gating in bothRegisterUserandregisterOIDCUserachieve the goal of making demo data creation configurable while defaulting to current behavior.Two follow‑ups to consider:
Error handling consistency
RegisterUserstill aborts user creation if any default label/location insert fails, whereasregisterOIDCUsernow logs and continues. For reliability (and to avoid blocking account creation on non‑critical demo data), you may want to treat demo item creation as best‑effort in both paths, or at least document the intentional difference.Ensure all UserService instances honor the config flag
services.Newcorrectly passescreateDefaultDemoItems, but a direct&UserService{repos: ...}elsewhere would now silently disable demo creation (zero valuefalse). It's worth confirming that all constructions go throughservices.New.From a security/ops angle, this new flag is a nice knob: operators can (and probably should) disable demo item seeding in locked‑down or production environments to avoid unnecessary sample data creation.
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: 0
🧹 Nitpick comments (2)
docs/en/configure/index.md (2)
47-47: Clarify behavior when disabled + minor security noteThe description is correct but a bit one‑sided. To make behavior explicit and help admins reason about setups (especially in production), consider wording that also states the “false” behavior, e.g.:
-| HBOX_OPTIONS_CREATE_DEFAULT_DEMO_ITEMS | true | create default demo labels and locations when a new user creates a group | +| HBOX_OPTIONS_CREATE_DEFAULT_DEMO_ITEMS | true | when true, create default demo labels and locations when a new user creates a group; when false, no demo items are created |This also subtly nudges production operators to disable demo data when not needed, which is slightly better from a security/least‑data standpoint.
270-270: Keep CLI flag description consistent with env var semanticsThe new flag is wired through correctly in the docs; for consistency and operator clarity, you might mirror the env var wording here so it’s obvious what toggling it does, e.g.:
- --options-create-default-demo-items <bool> (default: true) + --options-create-default-demo-items <bool> (default: true) # when false, do not create default demo labels/locations on new group creationClear toggle semantics reduce misconfiguration risk, especially on hardened/production deployments that want to avoid demo data.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/en/configure/index.md(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-07T07:51:13.533Z
Learnt from: JeffResc
Repo: sysadminsmedia/homebox PR: 996
File: backend/app/api/providers/oidc.go:156-170
Timestamp: 2025-09-07T07:51:13.533Z
Learning: The HomeBox authentication system uses email as the primary user identifier with a unique database constraint. Email-based OIDC identity binding was chosen over issuer+subject pairs to avoid major architectural refactoring, despite some theoretical security trade-offs.
Applied to files:
docs/en/configure/index.md
⏰ 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). (16)
- GitHub Check: build (linux/arm/v7)
- GitHub Check: build (linux/arm64)
- GitHub Check: build (linux/amd64)
- GitHub Check: Frontend Tests / Integration Tests PGSQL 17
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 2/4
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 3/4
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 1/4
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 4/4
- GitHub Check: Frontend Tests / Integration Tests PGSQL 16
- GitHub Check: Frontend Tests / Integration Tests
- GitHub Check: build (linux/arm64)
- GitHub Check: build (linux/arm/v7)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/arm64)
- GitHub Check: build (linux/arm/v7)
What type of PR is this?
What this PR does / why we need it:
This PR adds a new configuration option to disable the creation of default demo labels and locations when new users register and create a group.
Changes made:
backend/internal/sys/config/conf.go: AddedCreateDefaultDemoItems boolfield to theOptionsstruct with default valuetrueto maintain backward compatibilitybackend/internal/core/services/all.go:createDefaultDemoItemsfield to the internal options structWithCreateDefaultDemoItems()option function to pass the config to servicesUserServiceinitialization to accept and store this configuration optionbackend/internal/core/services/service_user.go:UserServicestruct to store thecreateDefaultDemoItemsflagRegisterUser()method to check the config flag before creating default labels and locations (lines 99-116)registerOIDCUser()method to check the config flag before creating default labels and locations (lines 303-317)backend/app/api/main.go: Updated service initialization to passcfg.Options.CreateDefaultDemoItemsto the servicesdocs/en/configure/index.md:HBOX_OPTIONS_CREATE_DEFAULT_DEMO_ITEMSenvironment variable entry to the configuration table (default:true)--options-create-default-demo-itemsto the CLI arguments sectionDesign decisions:
trueto maintain backward compatibility - existing behavior is preserved unless explicitly disabledRegisterUser()andregisterOIDCUser()methods to ensure consistent behavior for both regular and OIDC user registration flowsAutoIncrementAssetID) using the options patternWhich issue(s) this PR fixes:
Fixes #659
Special notes for your reviewer:
Testing
Default behavior (enabled):
Disabled behavior:
HBOX_OPTIONS_CREATE_DEFAULT_DEMO_ITEMS=falseenvironment variableSummary by CodeRabbit
New Features
Behavior / Bug Fix
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.