Add direct backup download action to executions menu#177
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a reusable download-link helper for execution views, wires a download action into the executions row menu, and adds test coverage for when the menu item renders. ChangesExecution download action
Estimated review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/view/web/dashboard/executions/show_execution_test.go (1)
15-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise
listExecutionswith a named table, not just the helper.This only tests
downloadExecutionDropdownButtonin isolation, so it would still pass if the row-menu wiring at Line 82 ininternal/view/web/dashboard/executions/list_executions.godisappeared. Converting this into a named slice-of-structs table that renders the row menu would both cover the PR’s regression target and satisfy the test-file convention. As per coding guidelines,**/*_test.go: Write table-driven tests using t.Run with a slice of structs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/view/web/dashboard/executions/show_execution_test.go` around lines 15 - 49, The current test only covers downloadExecutionDropdownButton directly, so it misses regressions in listExecutions row-menu wiring. Refactor this into a named table-driven test using t.Run with a slice of structs that exercises listExecutions and renders the row menu, not just the helper, so the dashboard executions menu behavior is covered end-to-end. Use the existing symbols listExecutions and downloadExecutionDropdownButton to locate the relevant code and keep the assertions focused on the rendered menu output.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/view/web/dashboard/executions/show_execution_test.go`:
- Around line 29-33: The execution download link assertion in
show_execution_test.go is hardcoding the unprefixed route, so it should be
updated to match the production URL construction used by the dashboard view. Use
pathutil.BuildPath with the execution download route and executionID when
building the expected href in the test, so the assertion remains correct when a
path prefix is configured. Locate the check around the Download backup link in
TestShowExecution.
---
Nitpick comments:
In `@internal/view/web/dashboard/executions/show_execution_test.go`:
- Around line 15-49: The current test only covers
downloadExecutionDropdownButton directly, so it misses regressions in
listExecutions row-menu wiring. Refactor this into a named table-driven test
using t.Run with a slice of structs that exercises listExecutions and renders
the row menu, not just the helper, so the dashboard executions menu behavior is
covered end-to-end. Use the existing symbols listExecutions and
downloadExecutionDropdownButton to locate the relevant code and keep the
assertions focused on the rendered menu output.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4de0e07f-7a89-4966-b373-a35c0efb5fba
📒 Files selected for processing (3)
internal/view/web/dashboard/executions/list_executions.gointernal/view/web/dashboard/executions/show_execution.gointernal/view/web/dashboard/executions/show_execution_test.go
1 participant
Summary
Download backupaction to each downloadable execution row menu on/dashboard/executions/dashboard/executions/{id}/downloadroute/handlerCloses #176
Verification
/tmp/go1.26.4/go/bin/gofmt -w internal/view/web/dashboard/executions/list_executions.go internal/view/web/dashboard/executions/show_execution.go internal/view/web/dashboard/executions/show_execution_test.goPATH=/tmp/go1.26.4/go/bin:$PATH /tmp/go1.26.4/go/bin/go test ./internal/view/web/dashboard/executions ./...PATH=/tmp/go1.26.4/go/bin:$PATH /tmp/go1.26.4/go/bin/go build -o ./dist/app ./cmd/app/.PATH=/tmp/go1.26.4/go/bin:$PATH /tmp/go1.26.4/go/bin/go build -o ./dist/change-password ./cmd/changepw/.npm run prettier -- --check . --ignore-path .prettierignore.tmp(local.omx/and generatedtmp/excluded)git diff --checkNotes
Summary by CodeRabbit