Conversation
The text editor posted itself to /texts/new or /texts with op=Save /
op=Change. It now saves through POST /api/v1/texts and
PUT /api/v1/texts/{id}, so the editor works against a configurable API
base URL instead of the page origin.
Neither endpoint existed. texts_api.ts already had a TextsApi.create()
calling POST /texts with exactly this payload shape — it had been
answering 404 since it was written, and its tests asserted a `{ id }`
response that no server ever sent. The response is now
`{ textId, bookId, message }` and those fixtures say so.
Two things the form path left implicit are explicit in the handler:
The language is checked for ownership. texts.TxLgID has a foreign key to
languages, but a foreign key proves the row exists, not that the caller
owns it, and $this->param('TxLgID') went straight into the INSERT. That
is the same gap createTermForLanguage() closed for terms. Updates were
never exposed — the text itself is user-scoped by QueryBuilder — but
creates were, so a crafted request could file a text under another
user's language on a multi-user install.
Tags arrive as names in the payload. saveTextAndReparse() called
TagsFacade::saveTextTagsFromForm(), which reads $_REQUEST, and a JSON
request does not populate it; the facade takes an optional list instead
and falls back to the request only for form callers. This also fixes the
book-split path, which read TextTags as a comma-separated list of tag
*ids* from a field that has always posted names under
TextTags[TagList][] — so tags on an auto-split import were silently
dropped.
"Check" stays a form POST. It renders a parsing report of whatever is in
the box rather than saving, and that report is server-rendered HTML. The
button names its own target with formaction/formmethod, so the form
carries no action of its own and handleTextOperation() is down to that
one branch.
The subtitle upload went with the save branch. file_import.ts has parsed
SRT/VTT in the browser into TxText for some time, so the server-side
copy was only reachable with JavaScript off — which the rest of this
two-step wizard already requires. That leaves SubtitleParserService with
no production caller; it is kept, not deleted, since removing it is a
separate call.
Psalm 0, phpcs 0/0, PHPUnit 9198, Vitest 4291, Cypress 294/294.
The three feed forms outside the manager SPA posted themselves to server
routes. They save through POST /api/v1/feeds and PUT /api/v1/feeds/{id}
now: the wizard's last step, the manual "add a feed" tab and the edit
form. /feeds/new and /feeds/{id}/edit are GET-only and render the
scaffold; FeedEditController loses both save branches.
Wizard steps 2 and 3 keep posting to /feeds/wizard. Those drive a
server-side session state machine that renders HTML previews of the
fetched page, so converting them means moving the wizard itself to the
client — a different job, and one the roadmap parks with the rest of
feeds.
Tracing step 4 turned up that it has not saved anything since 1e2216b
(2026-08-08). It posts to /feeds/edit, and that commit turned
/feeds/edit into a 302 to the manager SPA when the duplicated
server-rendered feeds list was retired. The redirect discards the body,
so completing the RSS wizard silently produced no feed. Verified against
a dev server before the fix: POST /feeds/edit with a full payload
answers 302 and news_feeds stays empty. Step 4 creates through the API
now, which is what makes it work again. Shipped in 3.4.0 and 3.4.1, so
it wants a line in the changelog as a fix, not only as a refactor.
The API path is also the only one that checks the submitted NfLgID
belongs to the caller. FeedCrudApiHandler has had that fence since it
was written; FeedEditController passed the request value straight to
FeedFacade::createFeed/updateFeed, so a crafted POST could pin a feed to
another user's language on a multi-user install. Retiring the two POST
routes closes it.
The curated-source path drops its hidden form entirely. It used to fill
six hidden inputs, wait a $nextTick for Alpine to write them, then call
form.submit(); it calls the API directly now, so both the form and the
timing dependency are gone.
New: modules/feed/api/save_feed.ts (validate, dispatch create vs update,
read a form's Nf-prefixed fields). The six tests asserting the old
hidden-field behaviour are rewritten against the API call rather than
deleted, and a success:false body is now treated as a failure — the
handler answers 200 for its own validation errors, so status alone never
proved a feed was saved.
Not covered end to end: walking the wizard to step 4 needs a live RSS
URL. The component's dispatch is unit-tested, and Cypress asserts the
rendered step no longer targets the route that swallowed it.
Psalm 0, phpcs 0/0, PHPUnit 9198, Vitest 4311, Cypress 298/298.
Steps 2 and 3 render the fetched article so the user can click the part to import. The controller passed the view the extractor's whole result — one entry per feed item — where a string belongs, so the picker printed "Array" and raised "Array to string conversion", which is fatal wherever warnings are: on those installs the wizard could not get past step 2. Present since 3.4.0. The same mismatch defeated the session cache. getFeedItemHtml() only returns strings, so the stored array read back as null and every render refetched the article; a cached render now costs 8 ms rather than a round trip to the source. Two tests asserted the broken shape, one of them mocking a ['html' => ...] return the extractor never produces, which is why this went unnoticed. Both now use the real shape, and an error entry gets a case of its own — its message names a fetched URL, so it is escaped rather than echoed as markup.
Moving the editor's save to /api/v1 (#262) reused the new-text form's autoImporting flag for every submit, and that flag also drives the "fetching the page" banner. Saving a pasted text therefore claimed to be fetching from Project Gutenberg while it was doing nothing of the sort. Track the two apart: saving gets its own flag, autoImporting keeps the banner it owns, and the submit button asks isBusy() for the state that covers both. Never released — the flag was reused earlier in this cycle.
A repair release for the RSS feed wizard, which has been unable to produce a feed since 3.4.0. Two independent faults sat in the same flow. The wizard's last step posted the finished feed to a route that 3.4.0 turned into a redirect, and a redirect discards the body, so a completed walk saved nothing. In front of that, steps 2 and 3 handed the view the extractor's result array where the article's HTML belongs: the element picker printed "Array", and the conversion notice behind it is fatal wherever warnings are, blocking the wizard at step 2 outright. Also closes a mass-assignment hole on multi-user installs: the language submitted with a new text or feed is now checked for ownership, which a foreign key alone never proved. Single-user installs were never affected. The text editor and the feed forms now save through /api/v1, so both work against a configurable API base URL rather than the page origin (#262).
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
1 participant
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.
A repair release for the RSS feed wizard, which has been unable to produce a
feed since 3.4.0, plus a mass-assignment fix for multi-user installs.
Fixed
Finishing the wizard saved nothing (#262). The last step posted the
finished feed to
/feeds/edit. 3.4.0 turned that route into a redirect to thefeeds manager when the duplicated server-rendered feeds list was retired, and a
redirect discards the body — so the walk ran to completion and produced no
feed. The manual "add a feed" tab and the curated-source browser were
unaffected.
Steps 2 and 3 showed no article to pick from. Those steps render the
fetched article so the user can click the part to import, but the controller
passed the view the extractor's whole result — one entry per feed item — where
a string belongs. The picker printed "Array", and the "Array to string
conversion" notice behind it is fatal wherever warnings are, blocking the
wizard at step 2 outright. The same mismatch defeated the session cache
(
getFeedItemHtml()only returns strings), so every render refetched thearticle; a cached render now costs 8 ms instead of a round trip to the source.
Both faults are present in 3.4.0 and 3.4.1.
Security
The language on a new text or feed is checked for ownership (#262).
texts.TxLgIDandnews_feeds.NfLgIDhave foreign keys tolanguages, but aforeign key proves the row exists, not that the caller owns it, and the form
handlers passed the submitted value straight to the facade. On a multi-user
install a crafted request could file a text or feed under another user's
language. The API endpoints these forms now use check ownership, and the
form-POST routes that skipped it are retired. Single-user installs were never
affected.
Changed
The text editor and the feed forms save through
/api/v1(#262):POST /api/v1/texts,PUT /api/v1/texts/{id},POST/PUT /api/v1/feeds—neither text endpoint existed before, and
TextsApi.create()had been callinga 404 since it was written. Both surfaces now work against a configurable API
base URL rather than the page origin, which is what a bundled mobile client
needs. Two exceptions stay server-rendered on purpose: the editor's "Check"
button, which asks for a parsing report rather than saving, and the wizard's
URL steps, which drive a server-side session state machine.
Verification
composer test:no-coverage— 9199 passednpm test— 4316 passed./vendor/bin/psalm --threads=1— 0 errors;phpcsPSR-12 clean;tscandeslintcleanEPUB), the feed forms and the curated browser against a live BBC feed
Two tests had asserted the broken preview shape — one mocking a
['html' => ...]return the extractor never produces — which is why that faultsurvived. Both now use the real shape, with a case for the error entry, whose
message names a fetched URL and so is escaped rather than echoed as markup.