Repository navigation
feat: Usenet settings in Speed & data limits, with an on/off switch - #229
Conversation
The news server gets a fixed place: a Usenet page in Speed & data limits shows it and opens the same server dialog the download screens use. Its switch is saved with the server (usenet-server.json; a file without it reads as on). Off, an NZB link probes as a plain file, an NZB file can't be added, and NZB downloads stop and don't connect until it's on again.
|
Arpit-Khandelwal
left a comment
Arpit-Khandelwal
left a comment
There was a problem hiding this comment.
Thanks @dxmoc. A fixed place for the server makes sense: right now you can only get to it from an NZB. I merged current main (no conflicts). Typecheck node/web/e2e, lint, format:check and the build are clean. nzb.spec.ts passes (18), and so do limits, ui, scheduleQueue, probeRedirects, taskbarProgress and download (75). The switch does what it says: usenetTurnedOff only fails downloads that are running, paused and queued ones fail before they connect, and the test checks that no article is asked for while it's off. With #230 on top, I checked that switching off stops the backup server's connections too.
One thing before I approve:
- Saving the switch rebuilds the server from a fixed list of fields (
LimitsDialog.tsx:758):{ host, port, tls, username, connections, enabled }. Any field the server gets later goes back to its default whenever someone flips the switch. #231 already adds one: with it,deleteArchiveswould be turned back on every time Usenet is switched off or on. A smallsetUsenetEnabled(enabled)call that only changes the flag would avoid this. Or send the saved server as it is:saveUsenetServer({ ...usenet, password: undefined, enabled: usenetEnabled }).checkedServeralready drops the fields it doesn't know.
Nits:
probe.ts:227:usenetEnabled()now readsusenet-server.jsonand unseals the password on every HTTP probe, including links that aren't NZBs. It only needs to run when the link looks like an NZB, e.g.if (isNzbLink(...) && (await usenetEnabled())), and the same before the XML check.LimitsDialog.tsx:559: "Saved in its own dialog." reads a bit odd. Maybe: "Changes to the server are saved in its own window, right away."
Scope: @anmolkapil, this is the first of the three follow-ups proposed in #133 (this settings page, a backup server in #230, par2/unpack in #231). You said those would be discussed separately, so a quick yes from you on giving Usenet a page in Speed & data limits would be good. #233 also puts non-limit settings in this dialog.
| } | ||
| try { | ||
| if (usenet && usenetEnabled !== undefined && usenetEnabled !== usenet.enabled) { | ||
| const { host, port, tls, username, connections } = usenet |
There was a problem hiding this comment.
This rebuilds the server from a fixed list of fields, so any field added later (like deleteArchives in #231) goes back to its default when the switch is saved. A setUsenetEnabled(enabled) call, or saveUsenetServer({ ...usenet, password: undefined, enabled: usenetEnabled }), would keep everything else as saved.
There was a problem hiding this comment.
Fixed in 87affcf. The switch now saves the server as it was loaded, { ...usenet, enabled: usenetEnabled }, so a field added later (like deleteArchives in #231) keeps its value when the switch is flipped. No password is in the view, so the stored one stays. I also moved this save to come after the other settings. Switching off stops running NZBs, and that shouldn't happen if the rest of the save fails.
| const suggestedFileName = fileNameFromHeaders(response.headers, current) | ||
| if (isNzbLink(response.headers, current, suggestedFileName)) { | ||
| // Off, an NZB is a file like any other: for another newsreader to take. | ||
| const usenet = await usenetEnabled() |
There was a problem hiding this comment.
Nit: this reads the server file and unseals the password for every HTTP probe. It's only needed when the link looks like an NZB: if (isNzbLink(response.headers, current, suggestedFileName) && (await usenetEnabled())), and the same before the XML sniff below.
There was a problem hiding this comment.
Done in 2993832. The probe checks isNzbLink(...) first and reads the switch only after that. The XML sniff does the same once it finds <nzb. usenetEnabled() now only reads the JSON through checkedServer and no longer unseals the password.
| <section className={sectionClass}> | ||
| <h4 className={headingClass}>News server</h4> | ||
| <p className={hintClass}> | ||
| Where NZB downloads come from, as your Usenet provider gives it. Saved in its own dialog. |
There was a problem hiding this comment.
Nit: "Saved in its own dialog." reads a bit odd. Maybe: "Changes to the server are saved in its own window, right away."
There was a problem hiding this comment.
Changed to your wording: "Changes to the server are saved in its own window, right away."
…e it last The switch rebuilt the server from a fixed list of fields, so any field added later went back to its default on every flip. It now sends the server as loaded. It is also saved after the other settings: switching off stops running NZBs, which shouldn't happen when the rest failed.
usenetEnabled() now reads the switch without unsealing the password.
… is saved without one Without a keyring at login (or with one that can't decrypt it), the saved password reads as none, and flipping the switch dropped it from the file.
Switching Usenet off failed the running NZBs, which freed their slots, so pump started the next queued NZB, which failed on the switch too, and so on until every queued NZB was in error, each with a notification. While the switch is off, pump now leaves queued NZBs waiting (HTTP and torrent downloads still start), and switching it back on starts them. The switch is read once at startup and kept as it is saved. A download failed by the switch gets no notification: the user switched it off.
…orm is saved The form sent an empty password whenever the session had none to show, so on Linux without a keyring (or with another keychain) editing any other field of the server deleted the sealed password. A blank field now always means "keep the stored one", as it already did when the password could be shown.
Switching Usenet off failed the running NZBs while the queued ones waited, so on switching it back on the queued ones started by themselves and the ones further along stayed in error until each was resumed by hand. Running NZBs now go back in the queue the way the download schedule stops downloads, and start again with the rest once the switch is on. Resuming one by hand while off still fails with the switch's message.
|
Thanks. All three points are fixed, on current main. A second look turned up a few more things, also fixed:
Checks: typecheck, lint, format:check and the build pass. Full e2e suite in a Linux container: the only failure was #230 is rebuilt on this. #231 still reads |
Arpit-Khandelwal
left a comment
Arpit-Khandelwal
left a comment
There was a problem hiding this comment.
Thanks @dxmoc, all three points are fixed, and the extra fixes look right to me. The switch now saves { ...usenet, enabled }, so every server field stays as saved, and the probe only reads the switch after isNzbLink (and usenetEnabled() no longer unseals the password). Putting running NZBs back in the queue instead of failing them is a better fit, and the new test shows nothing is asked for while it's off and both finish once it's back on. I checked it on current main (#224, #234, #127 are in): typecheck node/web/e2e, lint, format:check and the build are clean, and nzb, limits, scheduleQueue, telegram, ui and probeRedirects pass (62).
Re-checked after #227 and #192 landed: it still merges cleanly, and nzb, limits, ui and disk pass (48/48).
Usenet settings in Speed & data limits
Follow-up to #133 (issue #126). In #133 the news server could only be reached from an NZB: the New download dialog, or the server button on a download or error screen. This gives it a fixed place, as discussed there: a Usenet page in Speed & data limits, with an on/off switch.
What changed
UsenetServerDialogthe other entry points use, so there is still only one form.usenet-server.json. A file saved before this change has no flag, so it reads as on.checkedServervalidates the flag the same way it handlestls..nzbfile itself gets saved (for another newsreader, say). Indexer errors are still caught..nzbshows "Usenet downloads are off. Turn them on in Speed & data limits."Main-process changes:
usenetServer.ts(flag,usenetEnabled()),probe.ts,downloadManager.ts/nzbTransfer.ts(refuse to connect when off),handlers.ts(stop running NZB downloads when it's saved as off). No new IPC channels: the switch is saved throughsaveUsenetServer, and leaving out the password keeps the saved one.How to test
.nzbfile: you get the "Usenet downloads are off" message.e2e: three new tests in
e2e/nzb.spec.tsunder "Usenet settings":http, and a running download stops and doesn't reconnect when resumed; back on, it finishesnpx playwright test e2e/nzb.spec.ts: 18 passed. The smoke suite passes exceptui.spec.ts:288, which fails the same way on main in my Linux container.