Carry the rename into the data layer - #225
Merged
Merged
Conversation
Renames the product everywhere it is user-visible, following the decision recorded in NAMING.md (#222): page titles and the landing page, both locale files, the favicon label, the exported backup filename, and the READMEs. On the iOS side the target, directory and generated project become Arabesque, the display name and the Bluetooth/local-network usage strings follow, and the bundle ID moves to com.arabesque.Arabesque — the identifier has to be right before the first App Store Connect submission, since it cannot be changed afterwards. The CoreMIDI client and port names, which other apps can see, are renamed too. The CI workflow and .gitignore follow the new target name. Deliberately left alone: - DB_NAME = 'piano-trainer' in storage.js. It is the IndexedDB database holding every user's fingerings, sessions and practice aggregates; renaming it would orphan their data. The tests that open the database by name are unchanged for the same reason. - The localStorage keys (pt-*). - The repository name and the GitHub Pages URL, so PTWebAppURL still points at the live deployment. - The historical changelog entries that mention the old name: they are a dated record of what shipped. A new entry announces the rename instead, and says explicitly that practice data is untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Renames the IndexedDB database from 'piano-trainer' to 'arabesque' and the localStorage keys from the 'pt-' / 'pt:' prefixes to a single 'arabesque:' one, rather than leaving the old name behind everywhere the data lives. Nothing is lost in the process: - storage.js moves the old database's contents into the new one on first open, then drops it. It probes with indexedDB.databases() rather than open(), since open() would create the very database it is testing for and make every future first-time visitor pay for it. The copy only happens into a database we just created, so existing data under the new name always wins. - legacyKeys.js renames the localStorage keys in place. It runs as an import side effect, imported first by i18n.js, because i18n.js reads the stored language while its module body evaluates — anything invoked from a page entry script would arrive after that read and drop the setting. - The landing page's inline redirect script reads both the new and the old key: it runs before any module, so on a returning visitor's first post-rename landing it would otherwise beat the migration and show them the pitch again. Also adds test_helper's wait_for_store. The seeding tests opened the database from Ruby after a fixed sleep, assuming the page had already created it — and opening it first creates an empty database at the same version that then never upgrades, so the stores are missing for good and the page renders nothing. They now wait on that condition instead of on the clock. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
NAMING.md landed on main with the rename still listed as pending; this branch is what does it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
NAMING.md warned against touching DB_NAME and the localStorage keys, because a bare rename orphans everyone's practice data. This branch does rename them, but behind a migration, so the warning as written is now misleading. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #224 — base branch is
rename-to-arabesque, so review that one first.#224 deliberately stopped at the surface, leaving the data under the old name.
This finishes the job without asking anyone to re-import a backup.
What moves
piano-trainer→arabesque.openDatabase()copies the olddatabase's stores into the new one on first open, then deletes it.
pt-andpt:prefixes (two separators, for noreason) collapse into a single
arabesque:.legacyKeys.jsrenames the keysin place, including the per-score
strictBpm:<url>ones.Three details worth a look
indexedDB.databases(), notopen():open()would create the very database it is testing for, making every futurefirst-time visitor pay for a database they never had.
oldVersion === 0), so ifa browser somehow has data under both names, the new one wins.
legacyKeys.jsruns as an import side effect, imported first byi18n.js, becausei18n.jsreads the stored language while its module bodyevaluates. Anything called from a page's entry script would arrive after that
read and lose the language on the first load. For the same reason the landing
page's inline redirect script — which runs before any module — reads both the
new and the old key.
Test fix
test_helper.rbgainswait_for_store. The seeding tests opened the databasefrom Ruby after a fixed
sleep, assuming the page had already created it. Thatassumption was always fragile and the extra
awaithere broke it: opening thedatabase first creates an empty one at the same version, which then never
upgrades, so the stores are missing for good and the library renders nothing.
They now wait on that condition rather than on the clock.
Verification
rake test: 66 runs, 1 failure —test_playback_starts_and_stops, whichwaits on piano samples fetched from a CDN. It is pre-existing and
order-dependent: the parent branch fails the same test identically when run
with the same seed (46255), and it passes in isolation on both. Worth fixing
separately.
🤖 Generated with Claude Code