fix(schema): register steps 361-367 so an upgrade can leave the schema page - #1340
Merged
Merged
Conversation
…a page #1338 raised FOG_SCHEMA 360 -> 367 and added no steps. Its seven UPDATE statements were appended before the file's final `];` on the assumption that commons/schema.php is one large array. It is not -- it is ~326 separate `$this->schema[] = [...]` statements -- so they landed INSIDE the previous step's array and the step count never moved. The consequence is total, and it is not "the icons stay wrong": count($this->schema) == mySchema == 360 -> $hasIndexed false, so the updater applies nothing -> the stored version is written from the loop variable, so it is never advanced past 360 -> mySchema < FOG_SCHEMA stays true forever -> DatabaseManager::establish() redirects EVERY request to ?node=schema So an upgraded server cannot leave the schema page, while that page reports there is nothing to do and answers the POST with 204 -- which jQuery reports as statusText "nocontent" and $.notifyFromAPI renders as a red "Generic Error" toast on a two-second loop. Reported from a live deploy. A FRESH install was unaffected, because the seven statements still ran as part of that last step. That is why every schema job in CI stayed green: they build from scratch and never replay an upgrade from an existing version. The statements are now seven real `$this->schema[] = [...]` entries, labelled 361-367, so array_slice($this->schema, 360) yields them and the last one sets the version to 367. FOG_SCHEMA is unchanged at 367, which was always the right number -- 325 literal appends plus the 35 the keySequences loop writes was 360 before this. tests/schema-gate.test.php already existed for this bug class and did not catch it, which is the more useful half of this commit. It compared `$schema < $highest`, so it enforced only FOG_SCHEMA being too LOW. #1338 made it too HIGH -- with no new label and no new append, there was nothing for its other two checks to see either. Its own docblock already said the high side is "worse than stranding a step". It now compares for equality and explains which direction failed. Verified by reproducing #1338's exact shape, which fails it with the cause named, and by the low side, which still fails as before. Co-Authored-By: Claude <noreply@anthropic.com>
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.
Regression from #1338, reported from a live deploy. An upgraded server cannot leave
?node=schema.What I got wrong
commons/schema.phpis not one large array — it is ~326 separate$this->schema[] = [...]statements. I appended #1338's seven UPDATEs before the file's final];, so they joined the previous step's array and the step count never moved, whileFOG_SCHEMAwent 360 → 367.The schema page then reports nothing to do and answers the POST
204, which jQuery reports as statusTextnocontentand$.notifyFromAPIrenders as a red "Generic Error" toast, looping.A fresh install was fine — the statements still ran inside that last step. That is exactly why every schema job in CI stayed green: they build from scratch and never replay an upgrade from an existing version.
The fix
Seven real
$this->schema[] = [...]entries, labelled 361-367, soarray_slice($this->schema, 360)yields them and the last sets the version to 367.FOG_SCHEMAis unchanged — 367 was always the right number (325 literal appends + the 35 thekeySequencesloop writes = 360 before this).The part that matters more
tests/schema-gate.test.phpalready existed for this exact bug class and did not catch it. It compared$schema < $highest, enforcing only the case where FOG_SCHEMA is too low. #1338 made it too high, and with no new label and no new append there was nothing for its other two checks to see either.Its own docblock already said the high side is "worse than stranding a step" — a server cannot leave the schema page to fix itself. The comparison is now equality, and the message says which direction failed and names the usual cause.
Mutation-verified both ways:
Full suite:
144 passed, 0 failed.