Skip to content

refactor: categorize leftover config options#682

Merged
steveiliop56 merged 2 commits into
mainfrom
refactor/leftover-config
Mar 2, 2026
Merged

refactor: categorize leftover config options#682
steveiliop56 merged 2 commits into
mainfrom
refactor/leftover-config

Conversation

@steveiliop56
Copy link
Copy Markdown
Member

@steveiliop56 steveiliop56 commented Mar 2, 2026

Summary by CodeRabbit

  • Chores

    • Configuration variables renamed and standardized (database path, resources path, analytics, UI warnings); defaults clarified and resources path set to ./resources.
    • Analytics, resources, and UI warnings are enabled by default in the shipped configuration.
  • Bug Fixes

    • UI warning display and redirect gating updated to use the new warnings setting, ensuring consistent warning behavior.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 2, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4caa6bf and f882fe9.

📒 Files selected for processing (1)
  • internal/config/config.go

📝 Walkthrough

Walkthrough

Refactors configuration names from negation-based to affirmation-based, nests flat config fields into composite types (Database, Analytics, Resources), and updates backend controllers, bootstrap, and frontend schema/components to use the new fields and semantics.

Changes

Cohort / File(s) Summary
Env file
​.env.example
Renamed environment variables and defaults: TINYAUTH_RESOURCESDIRTINYAUTH_RESOURCES_PATH (./resources), TINYAUTH_DATABASEPATHTINYAUTH_DATABASE_PATH (./tinyauth.db), replaced negation flags (TINYAUTH_DISABLEANALYTICS, TINYAUTH_DISABLERESOURCES, TINYAUTH_UI_DISABLEWARNINGS) with affirmation flags (TINYAUTH_ANALYTICS_ENABLED, TINYAUTH_RESOURCES_ENABLED, TINYAUTH_UI_WARNINGSENABLED) and set defaults to enabled.
Config types & defaults
internal/config/config.go
Replaced flat fields with nested structs: DatabaseConfig{Path}, AnalyticsConfig{Enabled}, ResourcesConfig{Enabled, Path}; removed DatabasePath, ResourcesDir, DisableAnalytics, DisableResources; renamed UI DisableWarningsWarningsEnabled; updated NewDefaultConfiguration defaults.
Bootstrap
internal/bootstrap/app_bootstrap.go, internal/bootstrap/router_bootstrap.go
Switched to nested config access (e.g., app.config.Database.Path, app.config.Analytics.Enabled, app.config.Resources.Path/Enabled); router bootstrap now maps WarningsEnabled, Path, and Enabled into controller configs.
Controllers & tests
internal/controller/context_controller.go, internal/controller/context_controller_test.go, internal/controller/resources_controller.go, internal/controller/resources_controller_test.go
Renamed public struct fields: context controller DisableUIWarningsWarningsEnabled (JSON tag warningsEnabled); resources controller ResourcesDirPath, ResourcesDisabledEnabled; updated initialization, checks, and tests (inverted logic where necessary to preserve behavior).
Frontend schema & UI
frontend/src/schemas/app-context-schema.ts, frontend/src/components/layout/layout.tsx, frontend/src/pages/continue-page.tsx
Renamed app context schema field disableUiWarningswarningsEnabled; updated components to use warningsEnabled for gating DomainWarning and redirect logic (condition polarity adjusted accordingly).
Manifests
package.json, go.mod
Minor manifest adjustments referenced in bootstrap/config changes (lines changed small).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 I hopped through configs, tidy and bright,
From negation to yes, I made names light.
Paths and flags nested, all snug in their places,
Frontend and backend with clearer faces,
I nibble a carrot and celebrate the new bytes. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'refactor: categorize leftover config options' accurately describes the main change: reorganizing configuration fields into nested composite types (Database, Analytics, Resources) and renaming boolean flags for consistency.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/leftover-config

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.

Copy link
Copy Markdown
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.

🧹 Nitpick comments (1)
internal/controller/resources_controller.go (1)

20-27: Consider guarding file server initialization when resources are disabled.

The fileServer is initialized unconditionally even when config.Path is empty or config.Enabled is false. While the handler correctly guards against serving in these cases, creating the file server is unnecessary overhead.

♻️ Optional: Guard fileServer initialization
 func NewResourcesController(config ResourcesControllerConfig, router *gin.RouterGroup) *ResourcesController {
-	fileServer := http.StripPrefix("/resources", http.FileServer(http.Dir(config.Path)))
+	var fileServer http.Handler
+	if config.Enabled && config.Path != "" {
+		fileServer = http.StripPrefix("/resources", http.FileServer(http.Dir(config.Path)))
+	}
 
 	return &ResourcesController{
 		config:     config,
 		router:     router,
 		fileServer: fileServer,
 	}
 }

Note: If you apply this, also add a nil check in resourcesHandler before calling fileServer.ServeHTTP.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/controller/resources_controller.go` around lines 20 - 27,
NewResourcesController currently always creates fileServer even when resources
are disabled or Path is empty; update NewResourcesController so it only sets
fileServer when config.Enabled is true and config.Path is non-empty (e.g., leave
fileServer nil otherwise), and then add a nil check in resourcesHandler before
calling fileServer.ServeHTTP to avoid dereferencing a nil pointer when resources
are disabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@internal/controller/resources_controller.go`:
- Around line 20-27: NewResourcesController currently always creates fileServer
even when resources are disabled or Path is empty; update NewResourcesController
so it only sets fileServer when config.Enabled is true and config.Path is
non-empty (e.g., leave fileServer nil otherwise), and then add a nil check in
resourcesHandler before calling fileServer.ServeHTTP to avoid dereferencing a
nil pointer when resources are disabled.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 24c5b35 and 4caa6bf.

📒 Files selected for processing (11)
  • .env.example
  • frontend/src/components/layout/layout.tsx
  • frontend/src/pages/continue-page.tsx
  • frontend/src/schemas/app-context-schema.ts
  • internal/bootstrap/app_bootstrap.go
  • internal/bootstrap/router_bootstrap.go
  • internal/config/config.go
  • internal/controller/context_controller.go
  • internal/controller/context_controller_test.go
  • internal/controller/resources_controller.go
  • internal/controller/resources_controller_test.go

Comment thread internal/config/config.go Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 2, 2026

Codecov Report

❌ Patch coverage is 9.52381% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 15.31%. Comparing base (c48181a) to head (f882fe9).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
internal/config/config.go 0.00% 11 Missing ⚠️
internal/bootstrap/router_bootstrap.go 0.00% 3 Missing ⚠️
internal/bootstrap/app_bootstrap.go 0.00% 2 Missing ⚠️
internal/controller/resources_controller.go 33.33% 0 Missing and 2 partials ⚠️
internal/controller/context_controller.go 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #682      +/-   ##
==========================================
- Coverage   15.39%   15.31%   -0.09%     
==========================================
  Files          50       50              
  Lines        3643     3663      +20     
==========================================
  Hits          561      561              
- Misses       3025     3045      +20     
  Partials       57       57              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@steveiliop56 steveiliop56 merged commit cd410b6 into main Mar 2, 2026
8 checks passed
@Rycochet Rycochet deleted the refactor/leftover-config branch April 1, 2026 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants