Skip to content

Conversation

@Robert27
Copy link
Contributor

@Robert27 Robert27 commented Dec 8, 2025

What type of PR is this?

  • feature

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: Added CreateDefaultDemoItems bool field to the Options struct with default value true to maintain backward compatibility
  • backend/internal/core/services/all.go:
    • Added createDefaultDemoItems field to the internal options struct
    • Created WithCreateDefaultDemoItems() option function to pass the config to services
    • Updated UserService initialization to accept and store this configuration option
  • backend/internal/core/services/service_user.go:
    • Updated UserService struct to store the createDefaultDemoItems flag
    • Modified RegisterUser() method to check the config flag before creating default labels and locations (lines 99-116)
    • Modified 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 pass cfg.Options.CreateDefaultDemoItems to the services
  • docs/en/configure/index.md:
    • Added HBOX_OPTIONS_CREATE_DEFAULT_DEMO_ITEMS environment variable entry to the configuration table (default: true)
    • Added corresponding CLI argument --options-create-default-demo-items to the CLI arguments section

Design decisions:

  • Default value is true to maintain backward compatibility - existing behavior is preserved unless explicitly disabled
  • The flag is checked in both RegisterUser() and registerOIDCUser() methods to ensure consistent behavior for both regular and OIDC user registration flows
  • The implementation follows the existing pattern used for other configuration options (e.g., AutoIncrementAssetID) using the options pattern

Which issue(s) this PR fixes:

Fixes #659

Special notes for your reviewer:

  • The default behavior remains unchanged (defaults are created), ensuring backward compatibility
  • Both regular user registration and OIDC user registration paths respect the new configuration option
  • The implementation follows existing patterns in the codebase for configuration options
  • All code compiles successfully and follows the existing code style

Testing

  1. Default behavior (enabled):

    • Started Homebox with default configuration
    • Registered a new user and created a group
    • Verified that default labels (Appliances, IOT, Electronics, Servers, General, Important) and locations (Living Room, Garage, Kitchen, Bedroom, Bathroom, Office, Attic, Basement) were created
  2. Disabled behavior:

    • Set HBOX_OPTIONS_CREATE_DEFAULT_DEMO_ITEMS=false environment variable
    • Registered a new user and created a group
    • Verified that no default labels or locations were created

Summary by CodeRabbit

  • New Features

    • Added a configurable option (enabled by default) to control automatic creation of default demo labels and locations when a group/user is created.
  • Behavior / Bug Fix

    • Demo item bootstrapping is now optional and non-fatal: individual failures are logged and no longer block user or OIDC-based user creation.
  • Documentation

    • Configuration docs updated with the new CLI/env option and default setting.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 8, 2025

Walkthrough

Adds 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

Cohort / File(s) Summary
Configuration & Documentation
backend/internal/sys/config/conf.go, docs/en/configure/index.md
Add CreateDefaultDemoItems bool (yaml:"create_default_demo_items", default true); document env var HBOX_OPTIONS_CREATE_DEFAULT_DEMO_ITEMS and CLI flag --options-create-default-demo-items.
Service Initialization & Options
backend/app/api/main.go, backend/internal/core/services/all.go
Add option function WithCreateDefaultDemoItems(v bool) and pass cfg.Options.CreateDefaultDemoItems into services.New(...); default the option to true in services.New.
User Service Registration Logic
backend/internal/core/services/service_user.go
Add createDefaultDemoItems bool field to UserService; gate demo-item bootstrap in RegisterUser and registerOIDCUser on this flag; relax individual demo-item creation errors to logging only when gated.
Data Model Formatting
backend/internal/data/repo/repo_item_templates.go
Reorder/adjust struct tags for ItemTemplateCreate and ItemTemplateUpdate (spacing/tag order changes only; no semantic field changes).
Test Cleanup
backend/internal/data/repo/repo_items_test.go
Remove trailing blank lines (formatting only).
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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Review focus:
    • service_user.go: confirm gating logic and that logged errors are appropriate and non-fatal.
    • all.go / main.go: confirm correct propagation of the new option and the intended default=true.
    • conf.go and docs: validate naming, YAML tag, and default semantics.
    • repo_item_templates.go: ensure tag reordering does not affect codegen/validation pipelines.

Security recommendations

  • Ensure logs from demo-item creation do not expose sensitive user data (IDs, tokens, or PII). Use structured logs with redaction where needed.
  • Consider validating config at startup and emitting a clear, non-sensitive audit/log message when CreateDefaultDemoItems is false.
  • If demo items trigger DB writes at registration, ensure rate/abuse protections and transactional consistency are appropriate to avoid partial bootstrap states.

Poem

✨ A tiny switch in config lights the way,
Demo labels whisper "stay" or "play."
Errors now logged, no sudden fall,
New accounts choose — create or stall. 🎛️

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 (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main feature: adding support to disable creation of default demo items, which matches the primary change across all modified files.
Description check ✅ Passed The description comprehensively covers all required template sections: PR type (feature), detailed explanation of changes with file-by-file breakdown, linked issue (#659), design decisions, and testing methodology.
Linked Issues check ✅ Passed The PR successfully implements the primary objective from issue #659: preventing default locations (and labels) from being added when creating an account through a new configuration flag exposed as an environment variable.
Out of Scope Changes check ✅ Passed Minor formatting changes in repo_item_templates.go (struct tag reordering) and repo_items_test.go (trailing whitespace removal) are incidental and not out-of-scope, as they don't alter functionality or introduce unrelated features beyond the stated objective.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

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

@Robert27 Robert27 changed the title feat: add support to disable creation of default demo items feat(backend): add support to disable creation of default demo items Dec 8, 2025
@coderabbitai coderabbitai bot added the ⬆️ enhancement New feature or request label Dec 8, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 usage

The description is clear, but you might explicitly mention that setting this to false skips 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

📥 Commits

Reviewing files that changed from the base of the PR and between 05a2700 and 4064ced.

📒 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 option

The --options-create-default-demo-items flag 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 semantics

The tag layout for the Default* and DefaultLabelIDs fields remains consistent with pointer + omitempty + x-nullable usage, 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 initialization

Passing services.WithCreateDefaultDemoItems(cfg.Options.CreateDefaultDemoItems) here cleanly exposes the new option to the service layer while keeping defaults in services.New. Looks good.

backend/internal/sys/config/conf.go (1)

36-46: New CreateDefaultDemoItems option is consistent with existing config surface

CreateDefaultDemoItems with yaml:"create_default_demo_items" conf:"default:true" fits the existing Options pattern 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-compatible

The new createDefaultDemoItems field, its default of true, and the WithCreateDefaultDemoItems option follow the existing options pattern and ensure UserService sees the configured value via services.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 sites

The new createDefaultDemoItems flag on UserService and the gating in both RegisterUser and registerOIDCUser achieve the goal of making demo data creation configurable while defaulting to current behavior.

Two follow‑ups to consider:

  1. Error handling consistency

    • RegisterUser still aborts user creation if any default label/location insert fails, whereas registerOIDCUser now 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.
  2. Ensure all UserService instances honor the config flag

    • services.New correctly passes createDefaultDemoItems, but a direct &UserService{repos: ...} elsewhere would now silently disable demo creation (zero value false). It's worth confirming that all constructions go through services.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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 note

The 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 semantics

The 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 creation

Clear 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

📥 Commits

Reviewing files that changed from the base of the PR and between 00cf819 and ef1430e.

📒 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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⬆️ enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Disable default locations

1 participant