Refactor and optimize: deduplicate, batch queries, fix bugs

- Remove unused DATABASE_PATH import from games.py
- Extract _new_game_context(db) helper to deduplicate GET/POST error path
- Extract _parse_hole_form() helper to deduplicate hole validation in create/edit course
- Batch N+1 player handicap and tee queries in create_game into two IN() queries
- Fix hole count import bug: use max() across all tees instead of first match
- Fix migration exception handling: re-raise unless "duplicate column name"
- Cache calcSkins() result in _skinsCache; invalidate on score/skip changes
- Collapse two identical Next buttons into one in new_game.html
- Fix stale skinDoubleBackNine checkbox: reset on mode change, use x-model

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Rolf
2026-03-23 22:52:45 +01:00
parent d81d6a2d3c
commit 0be6e908e6
5 changed files with 101 additions and 118 deletions
+3 -2
View File
@@ -139,5 +139,6 @@ async def init_db():
try: try:
await db.execute(migration) await db.execute(migration)
await db.commit() await db.commit()
except Exception: except Exception as e:
pass # column already exists if "duplicate column name" not in str(e).lower():
raise
+37 -55
View File
@@ -51,6 +51,36 @@ async def _get_course_detail(cid: str, db: aiosqlite.Connection):
return dict(course), holes, tees return dict(course), holes, tees
def _parse_hole_form(form, hole_count: int) -> tuple[dict, dict, dict]:
"""Parse par/SI fields from a form. Returns (pars, stroke_indices, errors)."""
pars: dict[int, int] = {}
stroke_indices: dict[int, int | None] = {}
errors: dict[str, str] = {}
for i in range(1, hole_count + 1):
raw = form.get(f"par_{i}", "4")
try:
p = int(raw)
if p < 3 or p > 5:
errors[f"par_{i}"] = f"Hole {i}: par must be 35."
else:
pars[i] = p
except (ValueError, TypeError):
errors[f"par_{i}"] = f"Hole {i}: invalid par."
raw_si = form.get(f"si_{i}", "")
if raw_si:
try:
si = int(raw_si)
if si < 1 or si > hole_count:
errors[f"si_{i}"] = f"Hole {i}: SI must be 1{hole_count}."
else:
stroke_indices[i] = si
except (ValueError, TypeError):
errors[f"si_{i}"] = f"Hole {i}: invalid SI."
else:
stroke_indices[i] = None
return pars, stroke_indices, errors
# ── List ────────────────────────────────────────────────────────────────────── # ── List ──────────────────────────────────────────────────────────────────────
@router.get("/admin/courses", response_class=HTMLResponse) @router.get("/admin/courses", response_class=HTMLResponse)
@@ -91,30 +121,8 @@ async def create_course(request: Request,
errors["holes"] = "Holes must be 9 or 18." errors["holes"] = "Holes must be 9 or 18."
hole_count = holes if holes in (9, 18) else 18 hole_count = holes if holes in (9, 18) else 18
pars = {} pars, stroke_indices, hole_errors = _parse_hole_form(form, hole_count)
stroke_indices = {} errors.update(hole_errors)
for i in range(1, hole_count + 1):
raw = form.get(f"par_{i}", "4")
try:
p = int(raw)
if p < 3 or p > 5:
errors[f"par_{i}"] = f"Hole {i}: par must be 35."
else:
pars[i] = p
except (ValueError, TypeError):
errors[f"par_{i}"] = f"Hole {i}: invalid par."
raw_si = form.get(f"si_{i}", "")
if raw_si:
try:
si = int(raw_si)
if si < 1 or si > hole_count:
errors[f"si_{i}"] = f"Hole {i}: SI must be 1{hole_count}."
else:
stroke_indices[i] = si
except (ValueError, TypeError):
errors[f"si_{i}"] = f"Hole {i}: invalid SI."
else:
stroke_indices[i] = None
if errors: if errors:
return templates.TemplateResponse( return templates.TemplateResponse(
@@ -187,16 +195,12 @@ async def import_course(api_id: int, request: Request,
else: else:
location = (loc.get("address") or "").strip() location = (loc.get("address") or "").strip()
# Determine hole count from tees # Determine hole count from tees — use max so a course with any 18-hole tee stays 18
tees_male = data.get("tees", {}).get("male") or [] tees_male = data.get("tees", {}).get("male") or []
tees_female = data.get("tees", {}).get("female") or [] tees_female = data.get("tees", {}).get("female") or []
all_tees = tees_male + tees_female all_tees = tees_male + tees_female
holes_count = 18 valid_counts = [t.get("number_of_holes") for t in all_tees if t.get("number_of_holes") in (9, 18)]
for t in all_tees: holes_count = max(valid_counts, default=18)
n = t.get("number_of_holes")
if n in (9, 18):
holes_count = n
break
# Hole par + course-level stroke index from first male tee with hole data (fallback for display) # Hole par + course-level stroke index from first male tee with hole data (fallback for display)
hole_pars: dict[int, int] = {} hole_pars: dict[int, int] = {}
@@ -286,30 +290,8 @@ async def edit_course_submit(cid: str, request: Request,
errors["holes"] = "Holes must be 9 or 18." errors["holes"] = "Holes must be 9 or 18."
hole_count = holes if holes in (9, 18) else 18 hole_count = holes if holes in (9, 18) else 18
pars = {} pars, stroke_indices, hole_errors = _parse_hole_form(form, hole_count)
stroke_indices = {} errors.update(hole_errors)
for i in range(1, hole_count + 1):
raw = form.get(f"par_{i}", "4")
try:
p = int(raw)
if p < 3 or p > 5:
errors[f"par_{i}"] = f"Hole {i}: par must be 35."
else:
pars[i] = p
except (ValueError, TypeError):
errors[f"par_{i}"] = f"Hole {i}: invalid par."
raw_si = form.get(f"si_{i}", "")
if raw_si:
try:
si = int(raw_si)
if si < 1 or si > hole_count:
errors[f"si_{i}"] = f"Hole {i}: SI must be 1{hole_count}."
else:
stroke_indices[i] = si
except (ValueError, TypeError):
errors[f"si_{i}"] = f"Hole {i}: invalid SI."
else:
stroke_indices[i] = None
if errors: if errors:
return templates.TemplateResponse( return templates.TemplateResponse(
+50 -55
View File
@@ -6,7 +6,7 @@ import aiosqlite
from fastapi import APIRouter, Depends, Form, Request, WebSocket, WebSocketDisconnect from fastapi import APIRouter, Depends, Form, Request, WebSocket, WebSocketDisconnect
from fastapi.responses import HTMLResponse, RedirectResponse from fastapi.responses import HTMLResponse, RedirectResponse
from app.database import get_db, DATABASE_PATH from app.database import get_db
from app.dependencies import get_current_user from app.dependencies import get_current_user
from app.templates_config import templates from app.templates_config import templates
@@ -140,6 +140,29 @@ def _totals(players: list, scores: dict) -> dict:
return totals return totals
async def _new_game_context(db: aiosqlite.Connection) -> dict:
async with db.execute(
"SELECT id, name, avatar, handicap FROM users WHERE is_onboarded = 1 ORDER BY name"
) as c:
all_users = [dict(r) for r in await c.fetchall()]
async with db.execute(
"SELECT c.id, c.name, c.holes, c.location, COALESCE((SELECT SUM(par) FROM course_holes WHERE course_id=c.id),0) AS par, COALESCE((SELECT SUM(par) FROM course_holes WHERE course_id=c.id AND hole_number<=9),0) AS par_9 FROM courses c ORDER BY c.name"
) as c:
courses = [dict(r) for r in await c.fetchall()]
async with db.execute(
"SELECT id, course_id, name, slope, rating, gender, front_slope, front_rating, back_slope, back_rating FROM course_tees ORDER BY course_id, gender, name"
) as c:
tee_rows = [dict(r) for r in await c.fetchall()]
tees_by_course: dict[str, list] = {}
for t in tee_rows:
tees_by_course.setdefault(t["course_id"], []).append(
{"id": t["id"], "name": t["name"], "slope": t["slope"], "rating": t["rating"],
"gender": t["gender"], "front_slope": t["front_slope"], "front_rating": t["front_rating"],
"back_slope": t["back_slope"], "back_rating": t["back_rating"]}
)
return {"all_users": all_users, "courses": courses, "tees_by_course": tees_by_course}
# ── Routes ──────────────────────────────────────────────────────────────────── # ── Routes ────────────────────────────────────────────────────────────────────
@router.get("/games", response_class=HTMLResponse) @router.get("/games", response_class=HTMLResponse)
@@ -196,32 +219,9 @@ async def games_list(request: Request, user: dict = Depends(get_current_user),
@router.get("/games/new", response_class=HTMLResponse) @router.get("/games/new", response_class=HTMLResponse)
async def new_game_page(request: Request, user: dict = Depends(get_current_user), async def new_game_page(request: Request, user: dict = Depends(get_current_user),
db: aiosqlite.Connection = Depends(get_db)): db: aiosqlite.Connection = Depends(get_db)):
async with db.execute( ctx = await _new_game_context(db)
"SELECT id, name, avatar, handicap FROM users WHERE is_onboarded = 1 ORDER BY name"
) as c:
all_users = [dict(r) for r in await c.fetchall()]
async with db.execute(
"SELECT c.id, c.name, c.holes, c.location, COALESCE((SELECT SUM(par) FROM course_holes WHERE course_id=c.id),0) AS par, COALESCE((SELECT SUM(par) FROM course_holes WHERE course_id=c.id AND hole_number<=9),0) AS par_9 FROM courses c ORDER BY c.name"
) as c:
courses = [dict(r) for r in await c.fetchall()]
async with db.execute(
"SELECT id, course_id, name, slope, rating, gender, front_slope, front_rating, back_slope, back_rating FROM course_tees ORDER BY course_id, gender, name"
) as c:
tee_rows = [dict(r) for r in await c.fetchall()]
# Group tees by course_id for JSON embed in template
tees_by_course: dict[str, list] = {}
for t in tee_rows:
tees_by_course.setdefault(t["course_id"], []).append(
{"id": t["id"], "name": t["name"], "slope": t["slope"], "rating": t["rating"],
"gender": t["gender"], "front_slope": t["front_slope"], "front_rating": t["front_rating"],
"back_slope": t["back_slope"], "back_rating": t["back_rating"]}
)
return templates.TemplateResponse( return templates.TemplateResponse(
"games/new_game.html", "games/new_game.html", {"request": request, "user": user, **ctx},
{"request": request, "user": user, "all_users": all_users,
"courses": courses, "tees_by_course": tees_by_course},
) )
@@ -260,25 +260,10 @@ async def create_game(request: Request, holes_count: int = Form(...),
error = "All players must select a tee." error = "All players must select a tee."
if error: if error:
async with db.execute( ctx = await _new_game_context(db)
"SELECT id, name, avatar, handicap FROM users WHERE is_onboarded = 1 ORDER BY name"
) as c:
all_users = [dict(r) for r in await c.fetchall()]
async with db.execute(
"SELECT id, course_id, name, slope, rating, gender, front_slope, front_rating, back_slope, back_rating FROM course_tees ORDER BY course_id, gender, name"
) as c:
tee_rows = [dict(r) for r in await c.fetchall()]
tees_by_course: dict[str, list] = {}
for t in tee_rows:
tees_by_course.setdefault(t["course_id"], []).append(
{"id": t["id"], "name": t["name"], "slope": t["slope"], "rating": t["rating"],
"gender": t["gender"], "front_slope": t["front_slope"], "front_rating": t["front_rating"],
"back_slope": t["back_slope"], "back_rating": t["back_rating"]}
)
return templates.TemplateResponse( return templates.TemplateResponse(
"games/new_game.html", "games/new_game.html",
{"request": request, "user": user, "all_users": all_users, {"request": request, "user": user, **ctx, "error": error},
"courses": courses, "tees_by_course": tees_by_course, "error": error},
) )
game_id = secrets.token_urlsafe(10) game_id = secrets.token_urlsafe(10)
@@ -326,19 +311,29 @@ async def create_game(request: Request, holes_count: int = Form(...),
# Use played-par×2 as estimated 18-hole par. # Use played-par×2 as estimated 18-hole par.
par_for_formula = course_par_played * 2 if nine_of_eighteen else course_par_played par_for_formula = course_par_played * 2 if nine_of_eighteen else course_par_played
for uid in player_ids: # Batch-fetch all player handicaps
async with db.execute("SELECT handicap FROM users WHERE id = ?", (uid,)) as c: ph_placeholders = ",".join("?" * len(player_ids))
row = await c.fetchone() async with db.execute(
hc = row["handicap"] if row else None f"SELECT id, handicap FROM users WHERE id IN ({ph_placeholders})", player_ids
tee_id = form.get(f"tee_{uid}") or None ) as c:
handicaps_map = {r["id"]: r["handicap"] for r in await c.fetchall()}
tee_row = None # Batch-fetch all selected tees
if tee_id: tee_id_map = {uid: (form.get(f"tee_{uid}") or None) for uid in player_ids}
async with db.execute( tee_ids_needed = [t for t in tee_id_map.values() if t]
"SELECT slope, rating, stroke_indices, front_slope, front_rating, back_slope, back_rating FROM course_tees WHERE id = ?", tees_map: dict[str, aiosqlite.Row] = {}
(tee_id,), if tee_ids_needed:
) as c: t_placeholders = ",".join("?" * len(tee_ids_needed))
tee_row = await c.fetchone() async with db.execute(
f"SELECT id, slope, rating, stroke_indices, front_slope, front_rating, back_slope, back_rating FROM course_tees WHERE id IN ({t_placeholders})",
tee_ids_needed,
) as c:
tees_map = {r["id"]: r for r in await c.fetchall()}
for uid in player_ids:
hc = handicaps_map.get(uid)
tee_id = tee_id_map[uid]
tee_row = tees_map.get(tee_id) if tee_id else None
playing_handicap = None playing_handicap = None
strokes_allocation_json = None strokes_allocation_json = None
+7 -1
View File
@@ -307,6 +307,7 @@ function gameApp() {
touchStartX: 0, touchStartX: 0,
dragSrc: null, dragSrc: null,
dragOver: null, dragOver: null,
_skinsCache: null,
init() { init() {
this.players = PLAYERS_DATA; this.players = PLAYERS_DATA;
@@ -339,6 +340,7 @@ function gameApp() {
this.scores[h][uid] = s; this.scores[h][uid] = s;
} }
this.totals = msg.totals; this.totals = msg.totals;
this._skinsCache = null;
} }
}; };
this.ws.onclose = () => setTimeout(() => this.connectWs(), 2000); this.ws.onclose = () => setTimeout(() => this.connectWs(), 2000);
@@ -358,6 +360,7 @@ function gameApp() {
const cur = this.scores[hole][uid].strokes; const cur = this.scores[hole][uid].strokes;
this.scores[hole][uid].strokes = cur ? Math.max(1, cur + delta) : (HOLE_PARS[hole] || 4); this.scores[hole][uid].strokes = cur ? Math.max(1, cur + delta) : (HOLE_PARS[hole] || 4);
this.dirty.add(hole); this.dirty.add(hole);
this._skinsCache = null;
}, },
setBingo(hole, uid) { setBingo(hole, uid) {
@@ -441,6 +444,7 @@ function gameApp() {
} }
// Alpine reactivity workaround for Set // Alpine reactivity workaround for Set
this.skipped = new Set(this.skipped); this.skipped = new Set(this.skipped);
this._skinsCache = null;
}, },
isSkipped(hole) { isSkipped(hole) {
@@ -503,6 +507,7 @@ function gameApp() {
touchDragEnd() { this.dropPlayer(this.dragOver ?? this.dragSrc); }, touchDragEnd() { this.dropPlayer(this.dragOver ?? this.dragSrc); },
calcSkins() { calcSkins() {
if (this._skinsCache) return this._skinsCache;
const skins = {}; const skins = {};
const money = {}; const money = {};
PLAYERS_DATA.forEach(p => { skins[p.id] = 0; money[p.id] = 0; }); PLAYERS_DATA.forEach(p => { skins[p.id] = 0; money[p.id] = 0; });
@@ -535,7 +540,8 @@ function gameApp() {
if (!SKINS_CARRYOVER) { pot = 0; moneyPot = 0; } if (!SKINS_CARRYOVER) { pot = 0; moneyPot = 0; }
} }
} }
return { skins, money, holes, unclaimed: pot, unclaimedMoney: moneyPot }; this._skinsCache = { skins, money, holes, unclaimed: pot, unclaimedMoney: moneyPot };
return this._skinsCache;
}, },
isSkinWinner(hole, uid) { isSkinWinner(hole, uid) {
+4 -5
View File
@@ -163,7 +163,7 @@
<div x-show="skinValue && holesMode === '18'"> <div x-show="skinValue && holesMode === '18'">
<label style="display:flex; align-items:center; gap:0.5rem; cursor:pointer;"> <label style="display:flex; align-items:center; gap:0.5rem; cursor:pointer;">
<input type="checkbox" name="skin_double_back_nine" value="1" style="width:auto;" <input type="checkbox" name="skin_double_back_nine" value="1" style="width:auto;"
@change="skinDoubleBackNine = $event.target.checked"> x-model="skinDoubleBackNine">
<span style="font-weight:400; color:var(--text-muted);">Double value on back nine (holes 1018)</span> <span style="font-weight:400; color:var(--text-muted);">Double value on back nine (holes 1018)</span>
</label> </label>
</div> </div>
@@ -226,11 +226,11 @@
<span>18 holes</span> <span>18 holes</span>
</label> </label>
<label class="radio-card" style="flex:1;"> <label class="radio-card" style="flex:1;">
<input type="radio" name="_holes_mode" value="front9" @change="holesMode = 'front9'"> <input type="radio" name="_holes_mode" value="front9" @change="holesMode = 'front9'; skinDoubleBackNine = false">
<span>Front 9</span> <span>Front 9</span>
</label> </label>
<label class="radio-card" style="flex:1;"> <label class="radio-card" style="flex:1;">
<input type="radio" name="_holes_mode" value="back9" @change="holesMode = 'back9'"> <input type="radio" name="_holes_mode" value="back9" @change="holesMode = 'back9'; skinDoubleBackNine = false">
<span>Back 9</span> <span>Back 9</span>
</label> </label>
</div> </div>
@@ -268,8 +268,7 @@
</div> </div>
<div style="position:sticky; bottom:calc(var(--nav-height) + env(safe-area-inset-bottom)); background:var(--bg); padding:0.75rem 0; margin-top:0.5rem;"> <div style="position:sticky; bottom:calc(var(--nav-height) + env(safe-area-inset-bottom)); background:var(--bg); padding:0.75rem 0; margin-top:0.5rem;">
<button type="button" x-show="hasTees()" class="btn btn-primary" @click="step = 2" :disabled="!holesMode">Next →</button> <button type="button" class="btn btn-primary" @click="hasTees() ? step = 2 : goToConfirm()" :disabled="!holesMode">Next →</button>
<button type="button" x-show="!hasTees()" class="btn btn-primary" @click="goToConfirm()" :disabled="!holesMode">Next →</button>
</div> </div>
</div> </div>