fix(logging): derive band from frequency consistently across logging forms - #232
Merged
Merged
Conversation
…forms The New Contact page and QuickLogCard each carried their own copy of a frequency→band mapper that had drifted from the shared, tested frequencyToBand() in src/lib/adif.ts. As a result, typing a frequency on several real amateur bands failed to auto-fill the band, and on the New Contact page the copies' narrower ranges made frequency *validation* wrongly reject valid QSOs as "outside amateur radio bands": - 60M below 5.33 MHz (the US channelised sub-band, e.g. 5.107 MHz FT8) - 4M (70 MHz), 23CM (1296 MHz), 13CM, 33CM - the LF/MF digital bands (2200M, 630M) Extract the band plan into a new server-import-free module src/lib/bands.ts (frequencyToBand + an ordered AMATEUR_BANDS list) so both the server-side ADIF importer and the client logging forms share one source of truth — adif.ts can't be imported into client components because it pulls in the pg pool. Both forms now import from @/lib/bands; adif.ts re-exports frequencyToBand for backwards compatibility. Band pickers use AMATEUR_BANDS so any auto-derived band is a selectable option. The 'OTHER' sentinel is handled at each call site (no auto-fill / validation failure). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Owner
Author
Review: PR #232 — approve (verdict; could not post as formal review — GitHub blocks self-approval since this PR's author matches the reviewing account)Reviewed the diff and the task context. This is a clean, well-scoped fix. What it does: Consolidates three drifted copies of frequency→band mapping (New Contact page, QuickLogCard, and the ADIF importer) into a single source of truth in Verified:
No issues found. Recommend approving. |
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.
Problem
The two ham-radio logging forms — the New Contact page (
/new-contact) and the dashboard Quick Log card — each carried their own private copy of afreqToBand()helper. Those copies had drifted from the shared, unit-testedfrequencyToBand()insrc/lib/adif.ts(the one used for ADIF import), so the same frequency could be mapped differently depending on where you were in the app.The drift caused two user-facing bugs when logging a contact by typing a frequency:
validateFrequency()used the same narrow copy, so typing e.g.1296.1or5.107produced "Frequency is outside amateur radio bands" and blocked the log entry.Example of the drift (60M): the form copies used
5.33–5.408 MHz; the shared/ADIF mapper uses the full ADIF enumeration5.06–5.45 MHz.Solution
Consolidate onto a single source of truth.
src/lib/bands.ts— a pure, server-import-free module holdingfrequencyToBand()(moved verbatim fromadif.ts) plus an orderedAMATEUR_BANDSlist. It has nopg/db imports, so it can be imported into client components without dragging the database driver into the browser bundle (which is exactly whyadif.tscouldn't be reused directly).src/lib/adif.tsnow imports and re-exportsfrequencyToBandfrom@/lib/bands, so existing importers and tests that source it from@/lib/adifkeep working unchanged.frequencyToBand+AMATEUR_BANDSfrom@/lib/bands, deleting their local copies. The band pickers renderAMATEUR_BANDSso any auto-derived band is a selectable option. The'OTHER'sentinel returned for out-of-band frequencies is handled at each call site — it means "don't auto-fill" for derivation and "outside amateur radio bands" for validation, preserving the prior behaviour.Net: −77 / +17 lines across the three touched files; two duplicated mappers become one.
Testing performed
npm run typecheck— cleannpm run lint— cleannpm run build— succeedsnpx playwright test tests/bands.spec.ts tests/frequency-to-band.spec.ts— 10 passed. Addedtests/bands.spec.tscovering: every band the mapper emits is present inAMATEUR_BANDS(and in ascending order, no dupes, no'OTHER'); the@/lib/adifre-export stays in lock-step with the source of truth; and the specific bands the standalone forms previously missed (60M channels, 4M, 23CM, 13CM, LF). The existingfrequency-to-band.spec.tsstill passes unchanged via the re-export.Backwards compatibility
No API or DB changes.
frequencyToBand's ranges and outputs are identical to before (moved, not modified); only the two client forms change behaviour — and only to gain the correct bands they were missing.Future follow-up
src/components/charts/BandDistributionChart.tsxand the filter-chip band lists (search,filter-chips-demo) are separate hard-coded lists that could also adoptAMATEUR_BANDSin a later consistency sweep.🤖 Generated with Claude Code