Spell the permanent link the way the page reads it back - #114
Conversation
Camera#permalink wrote `&var=` and Cameras::SocsController#show has only ever read `params[:ver]`, so the edition was the single field the permanent link dropped. A visitor who picked Ultimate on a 32MB chip and followed their own link -- or sent it to someone else, which is what the link is for -- got the wizard back on Lite, with the flash type intact and no sign anything had changed. Every other field round-tripped, which is why it went unnoticed. permalink now emits `ver`. `show` accepts `var` as well, because the old spelling is the only one any shared link carries; `ver` wins where a link somehow has both. The seven near-identical reader lines are now one table. The bug was a key disagreeing with a field, and a table is where that disagreement is visible; insertion order carries the precedence. Pulling them out of `show` also drops that action below the Metrics/AbcSize and Metrics/CyclomaticComplexity thresholds it had been over, and its length from 20 lines to 14 -- two fewer rubocop offences in the touched files than before this change. Four tests. The two model tests pin the whole key set against the reader rather than the one key that broke, so the next field to drift fails here. Reverting permalink to `var` fails both; dropping `var` from the table fails the shared-link test. Closes #111. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YFg9PRy2T6cr983S8gran5
PR Summary by QodoFix permanent-link edition round trips with legacy compatibility
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
Code Review by Qodo
1. Round-trip omits net and sd
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Qodo's review of the change: accepting `var` back made a combination reachable that was not before. `show` had no server-side size rule -- the menu applies it in JavaScript, after this action has already decided which options open selected -- so `?rom=nor8m&var=ultimate` rendered the form on nor8m + Ultimate, which enforce_eight_meg_limit refuses on submit. Confirmed by rendering it: `nor8m, ultimate`. Silent here, unlike in `update`. There the edition decides what gets flashed and the visitor is told when it changes; here it only decides where a form opens. Same carve-out as enforce_eight_meg_limit: a SoC published as Ultimate and nothing else keeps it, because opening on a Lite tarball upstream never built is worse than the size warning `update` will give. Folded together with the existing flash-type guard, since both are the same job -- bring what arrived in the query string back inside what the menu offers. That drops `show` under Metrics/AbcSize as well: 42 offences in the touched files against 44 on master. Qodo also asked the round-trip test to cover `net` and `sd`. It cannot, and the reason is worth recording rather than papering over: the selects for both are commented out at show.html.erb:86-87, so the rendered form carries five fields, not seven. `net` and `sd` are set on the camera by the link and then rendered nowhere and posted nowhere -- `update` falls back to eth/nosd whatever the link said. Asserting them would need `assigns`, which this app has no rails-controller-testing for, and would pin a value that goes no further than that one request. The round-trip test now sends a link carrying wifi/sd anyway, so it covers `show` accepting them, and the helper says plainly why it stops there. 198 runs, 750 assertions. Dropping the new guard fails the 8MB test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YFg9PRy2T6cr983S8gran5
|
Both findings addressed in 062bc87. One fixed, one answered. 1. Permalinks preserve 8MB Ultimate — real, and introduced by this PRConfirmed by rendering it: Fixed, and folded together with the flash-type guard that was already there, Folding the two also took 2. Round-trip omits net and sd — cannot be asserted, and the reason mattersThe rendered form carries five fields, not seven: The selects for So the honest coverage is: the round-trip test now sends a link carrying That 198 runs, 750 assertions, 0 failures. Dropping the new guard fails the 8MB test. |
Regression from #114, found while triaging website bugs filed against OpenIPC/firmware. A permanent link names every field whether or not it has a value, so `?...&rom=nor16m&var=&sd=sd` is what a link built from a camera with no edition chosen looks like -- and the menu produces exactly that, because allowedEditions falls back to '' for a chip with nothing published. OpenIPC/firmware#1912 carries a real one. `if params[key]` treats the empty string as an answer, so the edition became "" and the dropdown rendered with nothing selected at all. Before #114 this was unreachable: `show` read `ver`, which permalink never wrote, so an empty `var=` was simply ignored. Accepting `var` -- and emitting `ver` -- made both spellings live. `present?` instead. Blank in a query string means "not specified", so the constructor default stands, which is what it did before. JavaScript covers for it on load, since '' is not in the allowed list and checkFlashSize moves off it, and a browser submits the first option of a select with nothing marked. So this never produced wrong instructions -- but it rendered a form the visitor had to notice and correct, on a link the site generated itself. Two tests, one per spelling. Reverting to bare truthiness fails both. 21 rubocop offences in the touched files, unchanged from master. Claude-Session: https://claude.ai/code/session_01YFg9PRy2T6cr983S8gran5 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 participant
Closes #111.
Camera#permalinkwrote&var=;Cameras::SocsController#showhas only everread
params[:ver]. The edition was the one field the permanent link dropped —pick Ultimate on a 32MB chip, follow your own link, and the wizard comes back on
Lite with the flash type intact and nothing to say it changed.
mac,cip,sip,net,romandsdall round-tripped, which is why it went unnoticed.permalinknow emitsver.showstill acceptsvar, because the oldspelling is the only one any already-shared link carries;
verwins if a linksomehow has both.
The reader is a table now
The seven near-identical
x = params[:k] if params[:k]lines are one frozenhash. The defect was a key disagreeing with a field, and a table is where that
disagreement is legible; insertion order carries the
var-then-verprecedence. The MAC stays out of it — unlike the rest it is rewritten rather
than copied, and applied whether or not the link carried one.
That also takes
showbelow two thresholds it had been over:Metrics/AbcSize(show)Metrics/CyclomaticComplexity(show)Metrics/MethodLength(show)42 rubocop offences in the touched files, down from 44. No new categories.
Verification
bin/rails test— 196 runs, 742 assertions, 0 failures (192 before).The two model tests pin the whole key set against the reader rather than the
one key that broke, so the next field to drift fails here rather than in
someone's shared link.
permalinkback to&var=vardropped from the table🤖 Generated with Claude Code
https://claude.ai/code/session_01YFg9PRy2T6cr983S8gran5