fix(api): remove user_id session check from dashboard settings

[s0up4200]

Summary by CodeRabbit

• Refactor
  • Simplified dashboard settings handler architecture by streamlining internal request handling and data access patterns.
  • Updated database schema for dashboard settings to improve data integrity and table structure consistency.

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

[coderabbitai]

Walkthrough

This change removes session management from the DashboardSettingsHandler by eliminating the sessionManager field and its constructor parameter. Authentication logic is replaced with a hardcoded user ID (1). A database migration removes a foreign key constraint from the dashboard_settings table via table recreation (required by SQLite's limitations).

Changes

Cohort / File(s)	Summary
Handler Session Removal internal/api/handlers/dashboard_settings.go	Removed sessionManager field and updated NewDashboardSettingsHandler constructor to accept only store parameter. Modified Get and Update methods to use hardcoded user ID (1) instead of retrieving from session. Removed scs/v2 import and adjusted error logging.
Constructor Update internal/api/server.go	Updated NewDashboardSettingsHandler call to remove sessionManager argument, now passing only the store.
Database Migration internal/database/migrations/027_remove_dashboard_settings_fk.sql	SQL migration that removes foreign key from dashboard_settings table by recreating it with stricter user_id UNIQUE constraint, copying existing data, and adding trg_dashboard_settings_updated trigger for updated_at refresh.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Review the hardcoded user ID (1) logic in dashboard_settings.go to confirm intended behavior and potential implications for multi-user systems
• Verify the migration correctly preserves all data during table recreation and that the trigger logic is sound
• Ensure no other code paths depend on the removed sessionManager parameter

Poem

🐰 Sessions fade like morning dew,
One user ID, the constrained view,
Foreign keys fall, rebuilt anew,
Simpler schemas, less to review!

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
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: removing session-based user ID validation from dashboard settings handlers.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/oidc-dashboard-settings

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1aa7360 and b61999f.

📒 Files selected for processing (3)

• internal/api/handlers/dashboard_settings.go (2 hunks)
• internal/api/server.go (1 hunks)
• internal/database/migrations/027_remove_dashboard_settings_fk.sql (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-11-19T20:04:51.737Z

Learnt from: s0up4200
Repo: autobrr/qui PR: 606
File: internal/database/migrations/016_add_instance_reannounce_settings.sql:21-27
Timestamp: 2025-11-19T20:04:51.737Z
Learning: In the autobrr/qui repository, PRAGMA recursive_triggers is never enabled in the SQLite setup. The AFTER UPDATE trigger pattern that issues a separate UPDATE statement on the same table (e.g., to update an updated_at timestamp) is used consistently across migrations and does not cause recursion with recursive_triggers disabled (the SQLite default). This pattern should remain consistent across all migrations.

Applied to files:

• internal/database/migrations/027_remove_dashboard_settings_fk.sql

🧬 Code graph analysis (2)

internal/api/server.go (1)

internal/api/handlers/dashboard_settings.go (1)

• NewDashboardSettingsHandler (19-23)

internal/api/handlers/dashboard_settings.go (2)

internal/models/dashboard_settings.go (1)

• DashboardSettingsStore (50-52)

internal/api/handlers/helpers.go (1)

• RespondError (34-38)

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

• GitHub Check: Run tests

🔇 Additional comments (5)

internal/database/migrations/027_remove_dashboard_settings_fk.sql (2)

28-32: Trigger pattern is correct.

The AFTER UPDATE trigger that issues a separate UPDATE statement follows the established pattern in this codebase and is safe without recursive_triggers enabled.

Based on learnings, this pattern is consistent with other migrations in the repository.

---

7-26: This review comment is based on a misunderstanding of SQL UNIQUE constraints and does not identify an actual issue.

The UNIQUE constraint on user_id does not create a "single shared dashboard settings row for all users." Instead, it enforces that there is one settings row per distinct user_id value—allowing user 1 to have their settings row, user 2 to have theirs, and so on. With the handler hardcoding userID = 1, this is the correct design: one application user with their own settings.

This is a deliberate single-user application architecture where OIDC handles external authentication while the application runs as a single internal user (user_id=1). The UNIQUE constraint on user_id existed in the original migration (026_add_dashboard_settings.sql) and was not introduced by this migration. Migration 027 correctly removes only the FOREIGN KEY constraint to support OIDC-only installs that lack a user table.

The code is working as intended and requires no changes.

Likely an incorrect or invalid review comment.

internal/api/handlers/dashboard_settings.go (2)

19-23: Constructor change removes session management.

The constructor signature has been simplified to remove the sessionManager parameter, aligning with the removal of per-user session-based authentication for dashboard settings.

---

26-26: The hardcoded userID=1 is correct by design for this single-user system.

This is not a multi-user system and not a behavior change. The database schema enforces a single-user constraint with CHECK (id = 1) on the user table. Both password and OIDC authentication methods authenticate to the same user account (ID=1). Dashboard settings correctly use this hardcoded ID, which is consistent with how all user operations in the codebase function (e.g., UserStore.Get(), UserStore.UpdatePassword() also hardcode ID=1). The middleware extracts a username from context for display/logging purposes, but the system architecture is intentionally single-user.

Likely an incorrect or invalid review comment.

internal/api/server.go (1)

271-271: Constructor call correctly updated.

The NewDashboardSettingsHandler call has been correctly updated to match the new signature that no longer requires a sessionManager parameter.

Note: The dashboard settings routes (lines 357-358) remain protected by the IsAuthenticated middleware, ensuring only authenticated users can access them. However, the handler itself uses a hardcoded userID = 1, meaning all authenticated users share the same settings.

Please clarify the intended user model for dashboard settings:

• Is this application single-user only?
• Should dashboard settings be global/shared across all users?
• Is per-user dashboard settings support planned for the future?

This will help determine if the current implementation aligns with product requirements.

---

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