fix: #779 + #780 — missing i18n keys + service worker scope#781
Conversation
…780) The service worker has been failing to register since Story 18.5 landed. The script was served from /beatify/static/sw.js but registered with scope '/beatify/' — browsers reject that (a SW's max scope is its own path or deeper). Result: [Admin] SW registration failed: SecurityError on every page load, offline/cache-first was dead, and every prior CACHE_VERSION bump in sw.js was a no-op. - Added SwJsView that reads www/sw.js and serves it as JS at /beatify/sw.js with no-cache headers so CACHE_VERSION bumps propagate without waiting for HTTP cache TTL on the bootstrap itself. - Updated the three registration call sites (admin.js, dashboard.js, player-core.js) to the new URL. - Ruff cleaned up four imports in views.py that were left over from an earlier refactor and flagged as unused. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
admin.html:311 and :754 reference two keys that had no translations —
non-English users silently got the HTML element's fallback text ("All"
/ "Skip") and the console logged [i18n] Missing translation key: on
every load.
Added: en/de/es/fr/nl under the "admin" namespace.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bumped manifest + sw.js CACHE_VERSION + admin.min.js?v= + dashboard.min.js?v= + player.bundle.min.js?v= cache-busters. All three bundles were regenerated because the serviceWorker.register URL changed inside them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request updates Beatify to version 3.3.1-rc4, resolving a service worker activation bug by serving the script from a new /beatify/sw.js endpoint and adding missing i18n keys for admin actions. Feedback highlights a critical discrepancy where player.html references player.bundle.min.js while the PR updates player-core.min.js, which could break the player page. Additionally, the SwJsView implementation should be optimized by pre-calculating the file path and handling potential null values from file reads to avoid runtime exceptions.
| <script src="/beatify/static/js/i18n.min.js"></script> | ||
| <script src="/beatify/static/js/utils.min.js"></script> | ||
| <script type="module" src="/beatify/static/js/player.bundle.min.js?v=3.3.0"></script> | ||
| <script type="module" src="/beatify/static/js/player.bundle.min.js?v=3.3.1-rc4"></script> |
There was a problem hiding this comment.
There is a discrepancy between the file referenced in the HTML (player.bundle.min.js) and the minified file updated in this pull request (player-core.min.js). The PR description and changelog both mention player.bundle.min.js being regenerated, but the patch contains player-core.min.js. If player.html continues to use player.bundle.min.js, it will not receive the updated service worker registration URL, leaving the player page broken for service worker activation.
| def __init__(self, hass: HomeAssistant) -> None: | ||
| """Initialize the sw.js view.""" | ||
| self.hass = hass | ||
|
|
||
| async def get(self, request: web.Request) -> web.Response: # noqa: ARG002 | ||
| """Serve the service worker script.""" | ||
| sw_path = Path(__file__).parent.parent / "www" / "sw.js" | ||
| try: | ||
| content = await self.hass.async_add_executor_job(_read_file, sw_path) | ||
| except OSError: | ||
| _LOGGER.error("Service worker script not found: %s", sw_path) | ||
| return web.Response(text="Service worker not found", status=500) | ||
| # Must be served as JS. No-cache so CACHE_VERSION bumps propagate without | ||
| # waiting for HTTP cache to expire on the SW bootstrap itself. | ||
| return web.Response( | ||
| text=content, | ||
| content_type="application/javascript", | ||
| headers=_NO_CACHE_HEADERS, | ||
| ) |
There was a problem hiding this comment.
The sw_path is calculated on every request in the get method. Since this path is static, it should be calculated once in the __init__ method and stored as an instance attribute to improve efficiency. Additionally, if _read_file returns None on failure (consistent with the behavior of _get_html), the current implementation will raise a TypeError when creating the web.Response. Adding a check for None and returning a 404 status is safer and more standard for a missing resource.
def __init__(self, hass: HomeAssistant) -> None:
"""Initialize the sw.js view."""
self.hass = hass
self._sw_path = Path(__file__).parent.parent / "www" / "sw.js"
async def get(self, request: web.Request) -> web.Response: # noqa: ARG002
"""Serve the service worker script."""
try:
content = await self.hass.async_add_executor_job(_read_file, self._sw_path)
except OSError:
_LOGGER.error("Service worker script not found: %s", self._sw_path)
return web.Response(text="Service worker not found", status=404)
if content is None:
_LOGGER.error("Service worker script could not be read: %s", self._sw_path)
return web.Response(text="Service worker not found", status=404)
# Must be served as JS. No-cache so CACHE_VERSION bumps propagate without
# waiting for HTTP cache to expire on the SW bootstrap itself.
return web.Response(
text=content,
content_type="application/javascript",
headers=_NO_CACHE_HEADERS,
)
Fixes the two pre-existing issues spotted during the #772 live test.
Closes #780 — Service worker scope mismatch
Root cause: Since Story 18.5 landed,
sw.jshas been registered from/beatify/static/sw.jswith scope/beatify/. Browsers reject this — a SW's max scope is its own path or deeper. Every page load logged:```
[Admin] SW registration failed: SecurityError: ... scope ('/beatify/') is not
under the max scope allowed ('/beatify/static/')
```
Impact: The SW never activated. All of Story 18.5's offline/cache-first benefits were dead, and every `CACHE_VERSION` bump (rc1/rc2/rc3 and every prior RC cycle) was a no-op at the SW layer — only the HTML `?v=` cache-busters actually forced refreshes.
Fix (Option A from #780): Added `SwJsView` that reads `www/sw.js` and serves it as `application/javascript` at `/beatify/sw.js` (no-cache headers so bootstrap updates propagate instantly). Updated the three registration call sites (`admin.js`, `dashboard.js`, `player-core.js`) to the new URL. Regenerated the three affected minified bundles (`admin.min.js`, `dashboard.min.js`, `player.bundle.min.js`).
Closes #779 — Missing i18n keys `admin.filterAll` / `admin.skipRound`
Two keys referenced in `admin.html` but absent from every locale JSON — non-English users silently got the HTML fallback text. Added to all 5 locales:
Test plan
Version
🤖 Generated with Claude Code