Repository navigation
feat(upgrade): register admin settings, permissions and pages with Extend.Admin - #61
Merged
Merged
Conversation
…tend.Admin Flarum 2.0 registers an extension's admin settings, permissions and page with the Admin extender in admin/extend. - A new step moves the registry chains an admin initializer runs into admin/extend, with the imports and literal constants they use, when they would do the same from there. Otherwise all of the extension's registrations stay, with a @todo saying why. - The misc frontend step renames every app.extensionData in a file, not only the first, and getAllExtensionPermissions() to the getAllPermissions() 2.0 renamed it.
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.
Fixes: no issue.
Changes proposed in this pull request:
Flarum 2.0 registers an extension's admin settings, permissions and page with
Extend.Admininadmin/extend. A new step, after the misc frontend step, moves theapp.extensionData.for(…)/app.registry.for(…)chains an admin initializer runs intoadmin/extend(a new one, or the one the index already exports) assetting(() => …),customSetting(…),permission(() => …, type, priority)andpage(…), along with the imports and literal constants they use. It moves them only when they would do the same thing from there. Otherwise all of the extension's registrations stay where they are, with a@TODOsaying why. The initializer itself stays, because other extensions may check for it by name.The misc step now renames every
app.extensionDatain a file, not just the first. Nine real 1.x files use it more than once (fof/blog, for example), and in 2.0 the uses it missed crash. It also renamesgetAllExtensionPermissions()togetAllPermissions(), the method's 2.0 name.Reviewers should focus on:
When it leaves things:
if, or in another admin file);Moving only some would change their order, so it is all or nothing for each extension.
I tested it on 197 real 1.x admins (bundled, 84 FoF, 98 third-party): 161 moved, 23 were left with a
@TODO, and 13 had nothing to move. In every one that moved, each registration became exactly one extender method, and nothing lost its import.For 40 FoF extensions the result is the same as their hand-ported 2.x
extendfile. The other 24 differ only where the port changed things: pages rewritten as settings, new settings, new priorities.Screenshot
N/A
Necessity
Confirmed
@TODO. Running the step again changed nothing.)yarn testinjs/). (yarn test: 288 tests, plus Prettier, ESLint andtsc.)composer test). (N/A)Required changes:
None.