refactor: move dashboard out of separate package#24
Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThe changes consolidate the separate dashboard service module into the main analytics binary. Dashboard-specific files are removed, functionality is integrated into Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
Dockerfile (1)
13-21: Consider reducing manualCOPYdrift risk.
At Line 13–Line 21, the explicit per-file copy list is easy to miss during future refactors. PreferCOPY . .with a strict.dockerignoreto avoid accidental container build breaks when new source files are added.Possible simplification
-COPY ./cache.go ./ -COPY ./dashboard.html ./ -COPY ./dashboard_handler.go ./ -COPY ./favicon.ico ./ -COPY ./health_handler.go ./ -COPY ./instances_handler.go ./ -COPY ./main.go ./ -COPY ./rate_limiter.go ./ -COPY ./queries ./queries +COPY . .🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 13 - 21, The Dockerfile currently lists many explicit COPY entries (e.g., COPY ./cache.go, ./main.go, ./queries, ./dashboard_handler.go, etc.), which risks missing new source files during refactors; replace the multiple per-file COPY lines with a single COPY . . to copy the whole build context and then add a strict .dockerignore to exclude local artifacts (binaries, .git, node_modules, go/pkg, temp files) and any secrets so the image size and build context remain controlled; ensure the Dockerfile still sets the correct WORKDIR and build steps and verify that the ./queries directory and other needed files are included by the new COPY . . approach.main.go (1)
103-108: Consider gating static asset routes behind DashboardEnabled.
/favicon.icoand/robots.txtare registered unconditionally, but they appear to be dashboard-related assets. If the dashboard is disabled, serving these routes may be unnecessary. Consider moving them inside theif config.DashboardEnabledblock for consistency.Proposed change
if config.DashboardEnabled { router.Get("/dashboard", dashboardHandler.Dashboard) + router.Get("/favicon.ico", dashboardHandler.Favicon) + router.Get("/robots.txt", dashboardHandler.Robots) } - - router.Get("/favicon.txt", dashboardHandler.Favicon) - router.Get("/robots.txt", dashboardHandler.Robots)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main.go` around lines 103 - 108, Routes for favicon and robots are registered unconditionally but are dashboard-related; update the registration so that router.Get("/favicon.txt", dashboardHandler.Favicon) and router.Get("/robots.txt", dashboardHandler.Robots) are moved inside the existing if config.DashboardEnabled { ... } block (the same block that registers router.Get("/dashboard", dashboardHandler.Dashboard)) so these assets are only served when config.DashboardEnabled is true and by the same dashboardHandler functions.dashboard_handler.go (2)
81-85: Parse template once at initialization for better performance.The template is parsed on every request. Since
dashboardTemplateis an embedded constant, parse it once when creating the handler.Proposed refactor
type DashboardHandler struct { queries *queries.Queries + tmpl *template.Template } func NewDashboardHandler(queries *queries.Queries) *DashboardHandler { + tmpl, err := template.New("dashboard").Parse(dashboardTemplate) + if err != nil { + panic(fmt.Sprintf("failed to parse dashboard template: %v", err)) + } return &DashboardHandler{ queries: queries, + tmpl: tmpl, } }Then in
Dashboard:- tmpl, err := template.New("dashboard").Parse(dashboardTemplate) - if err != nil { - http.Error(w, "Internal server error", http.StatusInternalServerError) - return - } - - err = tmpl.Execute(w, versionStats) + err = h.tmpl.Execute(w, versionStats)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dashboard_handler.go` around lines 81 - 85, The template parsing currently done inside the request handler (template.New("dashboard").Parse(dashboardTemplate)) should be moved to initialization: parse dashboardTemplate once (e.g., in init() or when constructing the handler) and store the resulting *template.Template in a package-level variable or a field on the Dashboard handler; handle parse errors at startup (log or panic) so runtime requests use the pre-parsed template, and update the Dashboard function to Execute that stored template instead of calling Parse on every request.
95-105: Handlew.Writeerror returns.Per static analysis, the error returns from
w.Writeare not checked. While failures here are typically unrecoverable (client disconnected), handling them follows best practices and silences linter warnings.Proposed fix
func (h *DashboardHandler) Favicon(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "image/x-icon") w.WriteHeader(http.StatusOK) - w.Write(faviconData) + _, _ = w.Write(faviconData) } func (h *DashboardHandler) Robots(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "text/plain") w.WriteHeader(http.StatusOK) - w.Write([]byte("User-agent: *\nDisallow: /")) + _, _ = w.Write([]byte("User-agent: *\nDisallow: /")) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dashboard_handler.go` around lines 95 - 105, Both Favicon and Robots ignore the (int, error) return from w.Write; capture and handle the error by assigning (_, err := w.Write(...)) and if err != nil log it with context. Update DashboardHandler.Favicon (writing faviconData) and DashboardHandler.Robots (writing the byte slice) to check the error and call a logger if available (e.g. h.logger.Errorf("Favicon write failed: %v", err) / h.logger.Errorf("Robots write failed: %v", err)), otherwise fallback to log.Printf, to satisfy the linter and record client write failures.dashboard.html (2)
137-139: Hardcoded copyright year will become stale.The year "2026" is hardcoded and will need manual updates. Since this is server-rendered via Go templates, you could pass the current year from the handler, or use JavaScript to set it dynamically.
- <p class="text-sm font-medium text-center"> - Copyright © 2026 Tinyauth - </p> + <p class="text-sm font-medium text-center"> + Copyright © <span id="year"></span> Tinyauth + </p>And add to the script section:
document.getElementById('year').textContent = new Date().getFullYear();Alternatively, add a
Yearfield toversionStatsand use{{.Year}}.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dashboard.html` around lines 137 - 139, Replace the hardcoded "2026" in dashboard.html with a dynamic value: either add an element id (e.g., id="year") and set its text via client-side JS (document.getElementById('year').textContent = new Date().getFullYear()) or, server-side, add a Year int field to the versionStats struct and render it with {{.Year}} in the template; update the template paragraph to reference the chosen approach and ensure the handler populates versionStats.Year when using the server-side option.
10-10: Pin Chart.js to a specific version for stability.Loading Chart.js without a version specifier pulls the latest version, which could introduce unexpected breaking changes. Pin to the current stable version (4.5.1) or a version compatible with your requirements.
Suggested diff
- <script src="https://cdn.jsdelivr.net/npm/chart.js"></script> + <script src="https://cdn.jsdelivr.net/npm/chart.js@4.5.1"></script>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dashboard.html` at line 10, Change the Chart.js CDN import to pin to a specific stable version instead of the floating latest tag: update the <script src="https://cdn.jsdelivr.net/npm/chart.js"> entry to reference the desired version (e.g. 4.5.1) and use the UMD/minified build (chart.umd.min.js or chart.min.js) so the page always loads the pinned Chart.js release.README.md (1)
29-29: Table alignment inconsistency.The
DASHBOARD_ENABLEDrow has an extra space before the closing|in the name column compared to other rows. This is a minor formatting nit.-| `DASHBOARD_ENABLED` | boolean | Whether to enable the dashboard. | `true` | +| `DASHBOARD_ENABLED` | boolean | Whether to enable the dashboard. | `true` |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 29, The table row for the configuration key DASHBOARD_ENABLED has an extra space before the closing pipe in the name column causing misalignment; edit the Markdown table row containing DASHBOARD_ENABLED to remove the stray space so the pipes align consistently with other rows and ensure spacing around the pipes matches the rest of the table.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dashboard_handler.go`:
- Line 45: Fix the typo in the initial value of the mostUsed variable: change
the string "unkown" to "unknown" where mostUsed is declared/initialized so the
variable contains the correct word.
- Line 79: Remove the leftover debug call fmt.Println(versionStats) in
dashboard_handler.go; either delete the line or replace it with a structured log
call using the project's logger (e.g., logger.Debugf/Infof) and include context
about "versionStats" so the output is consistent with other logs and can be
filtered/parsed (locate the fmt.Println(versionStats) call near the dashboard
handler and update it accordingly).
In `@dashboard.html`:
- Around line 148-162: The template is embedding Go slice values (VersionLabels,
VersionValues) which render as Go-style strings like "[v1.0 v1.1]" and break
Chart.js; update the server-side code that prepares data for the template (where
VersionLabels and VersionValues are set) to JSON-encode those slices (e.g.,
using json.Marshal or a helper that returns a JSON string) or register a
template function (e.g., toJSON) that returns properly formatted JavaScript
arrays, then pass those JSON strings into the template so the Chart
initialization block (the new Chart(...) dataset labels and data) receives valid
JS arrays like ["v1.0","v1.1"] and [1,2,3].
In `@main.go`:
- Around line 107-108: The route path for the favicon is incorrect: change the
registration that calls router.Get("/favicon.txt", dashboardHandler.Favicon) to
use "/favicon.ico" so it matches the <link> in dashboard.html; locate the
router.Get call that binds dashboardHandler.Favicon and update the route string
to "/favicon.ico" (no other code changes required).
In `@README.md`:
- Line 7: Replace the phrase "on start up" with the single-word noun "on
startup" in the README description of the heartbeat; locate the sentence
containing "The heartbeat contains the UUID generated by Tinyauth on start up
and the version information" and update it to read "...generated by Tinyauth on
startup and the version information" so the noun form is correct.
---
Nitpick comments:
In `@dashboard_handler.go`:
- Around line 81-85: The template parsing currently done inside the request
handler (template.New("dashboard").Parse(dashboardTemplate)) should be moved to
initialization: parse dashboardTemplate once (e.g., in init() or when
constructing the handler) and store the resulting *template.Template in a
package-level variable or a field on the Dashboard handler; handle parse errors
at startup (log or panic) so runtime requests use the pre-parsed template, and
update the Dashboard function to Execute that stored template instead of calling
Parse on every request.
- Around line 95-105: Both Favicon and Robots ignore the (int, error) return
from w.Write; capture and handle the error by assigning (_, err := w.Write(...))
and if err != nil log it with context. Update DashboardHandler.Favicon (writing
faviconData) and DashboardHandler.Robots (writing the byte slice) to check the
error and call a logger if available (e.g. h.logger.Errorf("Favicon write
failed: %v", err) / h.logger.Errorf("Robots write failed: %v", err)), otherwise
fallback to log.Printf, to satisfy the linter and record client write failures.
In `@dashboard.html`:
- Around line 137-139: Replace the hardcoded "2026" in dashboard.html with a
dynamic value: either add an element id (e.g., id="year") and set its text via
client-side JS (document.getElementById('year').textContent = new
Date().getFullYear()) or, server-side, add a Year int field to the versionStats
struct and render it with {{.Year}} in the template; update the template
paragraph to reference the chosen approach and ensure the handler populates
versionStats.Year when using the server-side option.
- Line 10: Change the Chart.js CDN import to pin to a specific stable version
instead of the floating latest tag: update the <script
src="https://cdn.jsdelivr.net/npm/chart.js"> entry to reference the desired
version (e.g. 4.5.1) and use the UMD/minified build (chart.umd.min.js or
chart.min.js) so the page always loads the pinned Chart.js release.
In `@Dockerfile`:
- Around line 13-21: The Dockerfile currently lists many explicit COPY entries
(e.g., COPY ./cache.go, ./main.go, ./queries, ./dashboard_handler.go, etc.),
which risks missing new source files during refactors; replace the multiple
per-file COPY lines with a single COPY . . to copy the whole build context and
then add a strict .dockerignore to exclude local artifacts (binaries, .git,
node_modules, go/pkg, temp files) and any secrets so the image size and build
context remain controlled; ensure the Dockerfile still sets the correct WORKDIR
and build steps and verify that the ./queries directory and other needed files
are included by the new COPY . . approach.
In `@main.go`:
- Around line 103-108: Routes for favicon and robots are registered
unconditionally but are dashboard-related; update the registration so that
router.Get("/favicon.txt", dashboardHandler.Favicon) and
router.Get("/robots.txt", dashboardHandler.Robots) are moved inside the existing
if config.DashboardEnabled { ... } block (the same block that registers
router.Get("/dashboard", dashboardHandler.Dashboard)) so these assets are only
served when config.DashboardEnabled is true and by the same dashboardHandler
functions.
In `@README.md`:
- Line 29: The table row for the configuration key DASHBOARD_ENABLED has an
extra space before the closing pipe in the name column causing misalignment;
edit the Markdown table row containing DASHBOARD_ENABLED to remove the stray
space so the pipes align consistently with other rows and ensure spacing around
the pipes matches the rest of the table.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e4e538f3-baf2-4ed2-9be2-6efd3c1dfc86
⛔ Files ignored due to path filters (1)
favicon.icois excluded by!**/*.ico
📒 Files selected for processing (23)
.github/workflows/release.yml.gitignore.vscode/launch.jsonDockerfileLICENSEREADME.mddashboard.htmldashboard/.gitignoredashboard/Dockerfiledashboard/README.mddashboard/go.moddashboard/main.godashboard_handler.godocker-compose.dev.ymldocker-compose.ymlhealth_handler.goinstances_handler.gomain.goqueries.sqlqueries/db.goqueries/models.goqueries/queries.sql.gosqlc.yaml
💤 Files with no reviewable changes (6)
- dashboard/.gitignore
- dashboard/go.mod
- dashboard/README.md
- .vscode/launch.json
- dashboard/Dockerfile
- dashboard/main.go
Summary by CodeRabbit
New Features
/dashboardDASHBOARD_ENABLEDconfiguration option (enabled by default)Changes
tinyauth-analyticstoanalyticsDocumentation
DASHBOARD_ENABLEDconfiguration option