Repository navigation
Conversation
|
@kotfu kindly check,thanks! |
kotfu
left a comment
There was a problem hiding this comment.
Nice first contribution, and thanks for picking this one up. The five hostnames and their region labels match aprs2.net exactly (including "Europe & Africa" and "Oceania"), existing custom configs round-trip correctly, and pulling the option list into igateServer.js with its own tests was the right instinct. Handbook updated too, which is appreciated.
One bug to fix before merge.
Selecting "Custom…" blanks the server field
If a regional server is already selected, switching the dropdown to Custom empties form.server:
saved=euro.aprs2.net -> dropdown=euro.aprs2.net, customServer=""
after selecting "Custom…": form.server=""
The load path sets customServer to an empty string whenever the saved server is a regional one:
customServer = serverSelection === CUSTOM_IGATE_SERVER ? form.server : '';and handleServerSelection then restores that empty string when you switch to Custom.
On its own that would just be an empty input, but the rest of the path makes it lossy:
- there's no client-side non-empty check on
server(onlyserver_filteris validated on save) IGateConfigRequest.ToModelstores""verbatimIGateConfigFromModelsubstitutesDefaultIGateServerwhen it reads an empty server (pkg/webapi/dto/igate.go)
So an operator running euro.aprs2.net who selects "Custom…" and saves loses their regional choice and silently lands back on rotate.aprs2.net, the server this feature exists to steer people away from. The live iGate is also reconfigured with an empty hostname in the meantime. Before this change the field was a plain text input, so there was no way to blank it by touching a dropdown.
Falling back to the current value instead of the empty string fixes it, and is better UX anyway. Switching to Custom then hands you the selected host as a starting point to edit:
form.server = next === CUSTOM_IGATE_SERVER ? (customServer || form.server) : next;Worth moving handleServerSelection into igateServer.js
The pure option-list logic got good tests, but the stateful transition stayed inline in Igate.svelte, outside the test boundary. If it moved into igateServer.js as something like nextServerState({ selection, server, customServer }, next), it would be testable under node --test alongside the rest, and this case would have been caught.
Open for discussion
Bringing in @pflarue for additional discussion. A fresh install defaults to rotate.aprs2.net, which now renders as Custom….
The original issue #593 suggests "five regional addresses plus Custom", so you implemented what was specified. However, since we
don't know what region a new graywolf install is closest to, rotate.aprs2.net is still probably the right default. What do you think about having it show up in the dropdown as "Anywhere in the world" or "Worldwide" or "Anywhere" or something similar. Then Custom isn't used for the default. What do y'all think?
|
@kotfu thanks for the review,fixed now! Screen.Recording.2026-09-20.at.12.30.01.AM.mov |
|
@kotfu can we please merge this PR? |
Fresh installs default to rotate.aprs2.net, which rendered as "Custom…" and made an untouched setting look operator-modified. Add a "Worldwide — rotate.aprs2.net" option ahead of the regional servers, default the dropdown to it before config loads, and update the handbook row to match. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merged -- thanks @shubhambhar007, nice work on this, and especially on moving the selection logic into |
Summary
Adds the five official APRS Tier 2 regional servers to the iGate settings, along with a Custom option for other hostnames.
Existing custom server configurations remain supported.
Closes #593.
Testing
Screen.Recording.2026-09-19.at.11.47.01.PM.mov