174 lines
6.1 KiB
Markdown
174 lines
6.1 KiB
Markdown
# TASK: Bug fixes from v0.6.2 code scan
|
||
|
||
## Context
|
||
|
||
Comprehensive code scan of felhom-controller v0.6.2 found 4 minor bugs across templates, shell scripts, and Go code. None are critical, but all should be fixed for correctness.
|
||
|
||
**Current state:** Controller v0.6.2 running on demo-felhom.eu.
|
||
|
||
All changes in this task are in the **deploy-felhom-compose** repo only.
|
||
|
||
---
|
||
|
||
## Bug 1: Missing `require_arg` for `--hdd-path` in docker-setup.sh
|
||
|
||
**File:** `scripts/docker-setup.sh`
|
||
|
||
**Problem:** The `--hdd-path` flag parsing doesn't use `require_arg` validation like all other flags do. Under `set -u`, if `--hdd-path` is the last argument and has no value, `$2` is unbound and the script crashes with a cryptic bash error instead of a friendly message.
|
||
|
||
**Current code** (in the argument parsing `while` loop):
|
||
|
||
```bash
|
||
--hdd-path) HDD_PATH="$2"; shift 2 ;;
|
||
```
|
||
|
||
**Fix:** Add `require_arg` call, matching the pattern used by all other flags:
|
||
|
||
```bash
|
||
--hdd-path)
|
||
require_arg "$1" "${2:-}"
|
||
HDD_PATH="$2"; shift 2 ;;
|
||
```
|
||
|
||
**Verification:** Search for other flags in the same `while` loop — they all use `require_arg`. Confirm `require_arg` is defined earlier in the script (it is).
|
||
|
||
---
|
||
|
||
## Bug 2: Implicit `event` variable in `stackAction()` (layout.html)
|
||
|
||
**File:** `controller/internal/web/templates/layout.html`
|
||
|
||
**Problem:** The `stackAction` JavaScript function references `event.currentTarget` to get the clicked button, but `event` is never passed as a parameter. It relies on the implicit global `window.event` object, which is non-standard and deprecated. Works in Chrome/Firefox today but is not guaranteed.
|
||
|
||
**Current code:**
|
||
|
||
```javascript
|
||
async function stackAction(name, action) {
|
||
const btn = event.currentTarget;
|
||
```
|
||
|
||
**Fix — Step 1:** Change the function signature to accept `event`:
|
||
|
||
```javascript
|
||
async function stackAction(event, name, action) {
|
||
const btn = event.currentTarget;
|
||
```
|
||
|
||
**Fix — Step 2:** Update ALL `onclick` call sites in the same file that call `stackAction` to pass `event` as the first argument. Search for `stackAction(` in the template — each call looks like:
|
||
|
||
```html
|
||
onclick="stackAction('{{.Name}}', 'start')"
|
||
```
|
||
|
||
Change each to:
|
||
|
||
```html
|
||
onclick="stackAction(event, '{{.Name}}', 'start')"
|
||
```
|
||
|
||
There are multiple call sites (start, stop, restart buttons in the stacks section). Update **all** of them.
|
||
|
||
**Verification:** Search the entire file for `stackAction(` — every call site must pass `event` as the first argument. No other functions in the codebase call `stackAction`.
|
||
|
||
---
|
||
|
||
## Bug 3: Missing separator in page title (layout.html)
|
||
|
||
**File:** `controller/internal/web/templates/layout.html`
|
||
|
||
**Problem:** The `<title>` tag concatenates `.Title` and "Felhom.eu" with no separator, rendering as e.g. `"VezérlőpultFelhom.eu"` instead of `"Vezérlőpult — Felhom.eu"`.
|
||
|
||
**Current code:**
|
||
|
||
```html
|
||
<title>{{.Title}}Felhom.eu</title>
|
||
```
|
||
|
||
**Fix:**
|
||
|
||
```html
|
||
<title>{{.Title}} — Felhom.eu</title>
|
||
```
|
||
|
||
Uses em dash (U+2014) with spaces on both sides. This is a single-character change in the template.
|
||
|
||
**Edge case:** If `.Title` is empty, the title becomes ` — Felhom.eu` (leading space + dash). Check if any handler sets an empty `.Title`. If so, consider using a conditional:
|
||
|
||
```html
|
||
<title>{{if .Title}}{{.Title}} — {{end}}Felhom.eu</title>
|
||
```
|
||
|
||
Check all handlers that call `render()` or `renderTemplate()` — if every handler always sets a non-empty `.Title`, the simple fix (without conditional) is fine.
|
||
|
||
---
|
||
|
||
## Bug 4: `nextPruneLabel` edge case on Sunday before 4am (funcmap.go)
|
||
|
||
**File:** `controller/internal/web/funcmap.go`
|
||
|
||
**Problem:** The `nextPruneLabel` function calculates when the next weekly prune (Sunday 4:00) will occur. On Sunday before 4am, `daysUntilSunday` computes to 0, but the function returns the date in `"2006-01-02"` format instead of `"ma"` (Hungarian for "today"). Every other "today" scenario in the codebase uses the `"ma"` label.
|
||
|
||
**Current code:**
|
||
|
||
```go
|
||
daysUntilSunday := (7 - int(now.Weekday())) % 7
|
||
if daysUntilSunday == 0 && now.Hour() >= 4 {
|
||
daysUntilSunday = 7
|
||
}
|
||
next := time.Date(now.Year(), now.Month(), now.Day()+daysUntilSunday, 4, 0, 0, 0, now.Location())
|
||
return next.Format("2006-01-02")
|
||
```
|
||
|
||
The logic breakdown:
|
||
- Sunday, hour >= 4: `daysUntilSunday` = 0 → set to 7 (next week). Correct.
|
||
- Sunday, hour < 4: `daysUntilSunday` = 0 → stays 0, returns today's date as `"2006-01-02"`. Should return `"ma"`.
|
||
- Any other day: `daysUntilSunday` > 0 → returns future date. Correct.
|
||
|
||
**Fix:**
|
||
|
||
```go
|
||
daysUntilSunday := (7 - int(now.Weekday())) % 7
|
||
if daysUntilSunday == 0 {
|
||
if now.Hour() >= 4 {
|
||
daysUntilSunday = 7 // Already ran today, next week
|
||
} else {
|
||
return "ma" // Today (Sunday), hasn't run yet
|
||
}
|
||
}
|
||
next := time.Date(now.Year(), now.Month(), now.Day()+daysUntilSunday, 4, 0, 0, 0, now.Location())
|
||
return next.Format("2006-01-02")
|
||
```
|
||
|
||
**Verification:** Mentally walk through all cases:
|
||
- Monday–Saturday: `daysUntilSunday` is 1–6, returns future date ✓
|
||
- Sunday 03:00: returns `"ma"` ✓
|
||
- Sunday 04:00: `daysUntilSunday` = 7, returns next Sunday ✓
|
||
- Sunday 23:00: `daysUntilSunday` = 7, returns next Sunday ✓
|
||
|
||
---
|
||
|
||
## Build & Deploy
|
||
|
||
After all fixes, commit and deploy as v0.6.3:
|
||
|
||
```bash
|
||
# 1. Commit
|
||
cd /e/git/deploy-felhom-compose
|
||
git add -A && git commit -m "fix: require_arg for --hdd-path, explicit event in stackAction, title separator, nextPruneLabel Sunday edge case" && git push
|
||
|
||
# 2. Build (only needed for bugs 2-4 which affect the controller binary/templates)
|
||
ssh kisfenyo@192.168.0.180 "cd ~/build/felhom-controller && ./build.sh 0.6.3 --push"
|
||
|
||
# 3. Deploy to demo node
|
||
ssh kisfenyo@192.168.0.162 "docker pull gitea.dooplex.hu/admin/felhom-controller:0.6.3 && cd /opt/docker && docker compose up -d"
|
||
|
||
# 4. Verify
|
||
ssh kisfenyo@192.168.0.162 "docker logs felhom-controller --tail 5"
|
||
```
|
||
|
||
## Post-deploy checklist
|
||
|
||
- [ ] Page title shows separator: "Vezérlőpult — Felhom.eu" (check browser tab)
|
||
- [ ] Stack start/stop/restart buttons still work (Bug 2 didn't break onclick handlers)
|
||
- [ ] `docker-setup.sh --hdd-path` without value shows friendly error (test locally)
|
||
- [ ] Backup page shows "ma" on Sunday before 4am (only testable at that time, or adjust system clock) |