Conversation
- PageHit::hit() becomes recordHit(), avoiding the conflict with Table::hit($pk) - Manage::_authorize() makes $task optional to match its parent - InstallTemplateEntry::__invoke() matches AddTemplateEntry's signature and passes enabled=1, home=1 to the parent - SavePluginParams::__invoke() matches SaveParams' signature, using func_get_args() for the three-argument calling convention - Ftp::deleteDirectory() adds the missing $preserve parameter - Spacer::fetchTooltip() adds the default values its parent has - MysqliConnection adds the query() method its interface declares
Add the missing throw new on exception constructors in Table, Wallet, SiteController, Model and Toolbar; add the missing $this-> and $response-> on method calls in Helper, Processor and the OAuth grant types; fix the typos is_empty() to empty(), informatArrayay() to in_array() and jexit() to exit().
Remove duplicate keys in component arrays (author, parent, action, access, severity, height, wd). Comment out the earlier duplicate entries in the Mimetypes.php MIME-type table and the Password Blacklist leet-speak mappings, keeping them for reference.
Correct call sites that named methods their class does not declare: comment handlers in the blog and collections plugins, task methods in com_jobs and com_wishlist, and helper and error methods in com_collections, com_config, com_templates, Hubzero\User\Profile and several plugins.
….php Sanitizer keeps only the escapeId() method the wiki parser uses; its globals, defines and utility functions become class members. The unused utfnormalutil.php is deleted.
Add a sr-only legend to the resources search fieldset on the sample home page (row id=1). The migration only applies if the row still contains the original rsearchword field from sample.sql.
EnableTemplate.php declared class EnableComponent, a duplicate of the real EnableComponent.php. It now declares EnableTemplate, with the docblock corrected.
Replace the $$list and $$listnet variable-variables with an explicit $lists array, which static analysis can follow. The literal-slash preg_match becomes strpos, and unset plus array_push become plain appends.
Add the getLanguages() method, which returns the published content languages from #__languages. It was a Joomla JLanguageHelper method that had never been ported.
The extract($glyph, EXTR_PREFIX_ALL, 'txt') call created variables that static analysis cannot trace. Use $glyph['key'] directly.
- Course: STATE_TRASHED, which never existed, becomes STATE_DELETED (same value) - Publication: STATUS_DRAFT becomes STATE_DRAFT - Xml: self::XML_OPTION_* becomes the global XML_OPTION_* constants - Passport: add the missing PASSPORT_BADGES_URL constant - CubridDriver: PDO::CUBRID_SCH_IMPORTED_KEYS becomes the literal 2, the constant being extension-specific - The deprecation notice routes through the Log facade; \JLog does not exist - Google_Service_Drive is qualified from the namespaced plugin that reads its constant
The legend carried aria-hidden="true", which hides it from the assistive technology a legend exists for, and echoed $text, which is empty unless the module is configured with one. Drop the attribute and fall back to the label text so the fieldset is always announced.
detectExtensionName() reads $this->_option for anything implementing ControllerInterface, but only the controller base classes declare it. A controller that implements the interface directly warned on an undefined property; check property_exists() first.
Components\Jobs\Helpers\Permissions declares no constructor and reads the component name from a static property, so the argument was never used.
Use the variables the surrounding code defines: misspelled names, the $schema that eleven migrations called in up() without creating, values read from the wrong scope, and a mod_related_items reference to $this->module.
…exceptions Names that resolved to nothing at runtime: global classes and exceptions referenced unqualified from namespaced files, including catch clauses that never matched and Auth\Manager's error paths, where every failed login threw a class-not-found; missing imports; class names spelled with the wrong case; and a few references to classes that did not exist, which are added (Html\Builder\Menu) or replaced. Mail\Message's reply() call and first-address test are corrected.
Calls corrected to methods that exist: Table::_getAssetParentId, Composer::installPackage and ::removePackage (com_installer's packages controller was calling the Cli helper, which has neither), RestrictionsHelper::addPermittedSkuUser, Product::save and Expression::raw. com_publications' Datastore helper still called the dataviewer's old query_gen() and get_results(); it calls queryGen() and getResults(), and the component readme is updated to match.
Nine unrelated fixes: a missing Grammar import and a protected-method call in the database layer, DefaultRouter's constructor argument, a static Date::toSql() call, a by-reference field setup(), the ckeditor plugins' file-scope Html::behavior() call moved into onInit(), LocalProvider::reverseQuery() throwing instead of calling a missing method, an opendir() guard in com_tools, and two leftover error_log() lines.
Hubzero\Base\Object held only a namespace, and com_tags' object.php only forwarded to objct.php, where the Objct class lives and which every caller names directly. Nothing includes either file or references either name. Three Geocode\Result classes, Country and its two result factories, are referenced by nothing either.
Repository's constructor requires a loader, so the five bare
new Repository('site') calls in the cache and session storage backends
failed; they read the configuration off the container. com_config's model
is given the FileLoader its Repository needs.
Also: the memcache cache backend assigned $conf and then read $config;
com_config's model called addError() where ErrorBag supplies setError();
the memcached session backend instantiated Object, a reserved word.
Repository's constructor takes a list of paths instead of (client, loader), merging each path's config/ directory in order, and set() accepts a bare key. com_config's model is migrated to the new constructor, so saveConfig() still reads the existing values it preserves (the database password among them). ClientDetector keeps its disk-based detection rather than opening a database connection on every request. Add the Config test fixtures under Tests/Files/config, plus the MultiGroup and MultiPath sets the expanded RepositoryTest exercises.
The newsletter jobs plugin depended on com_feedaggregator, removed in 2019, and nothing references it. Its files go, and its migration is replaced by one that deletes the plugin entry.
The Config test fixture drops an eval(), and the Facades mock Application and ValidateTest are repaired.
PHP 8.2 deprecates dynamic property creation. Declare the properties on 39 component classes from com_answers through com_tools plus storefront, cart and projects, on the library classes that have no magic accessors, on the API controllers, and on the plugins and modules, which Obj had been absorbing as dynamic properties. None of the classes defines __get or __set, so no access changes meaning. projects/files and projects/publications use declared properties directly instead of get() and set() on $this. com_support's Ticketsv2_1 keeps its single, documented declarations of $acl, $config and $database, and projects/notes its own.
Plugin and Module stop extending Hubzero\Base\Obj and take the ErrorBag trait instead, so they lose get(), set(), def(), getProperties() and setProperties(). The instanceof Obj checks in the content plugins are on the article or row passed in, not on the plugin, and are unaffected.
setMetadata, addStylesheet and addstyleDeclaration become setMetaData, addStyleSheet and addStyleDeclaration, matching the declarations. PHP dispatched them case-insensitively already; the change is for static analysis.
Api\Component\Loader prefers the v1r0 controller filename and falls back to the v1_0 form, and ApiController's version reporting handles both.
Rename 60 API controllers and their classes from the v1_0 form to v1r0, the form the API loader now prefers. com_projects' filefsv1_0 controller is renamed with the rest.
Move 22 view templates under com_search, com_newsletter and com_redirect
into views/{view}/tmpl/. Hubzero\View\View searches both that directory
and its parent, and no view directory is left split between the two.
bootstrap/app.php ran as a side effect of requiring the Composer autoloader. It is required explicitly (require_once) by the six files that load the autoloader: index.php, core/bin/muse at both of its load sites, the PHPUnit and PHPStan bootstraps, and the two plugin test bootstraps. app.php guards its _HZEXEC_ and DS defines, since the test bootstraps define them first. phpunit.xml.dist bootstraps from core/tests/bootstrap.php, which sets up the container, facades and stubs, rather than from the autoloader alone.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two menu items can answer at one address. Until now which one won was decided by lft - whichever sat earlier in the menu tree. Measured by building two menus at one path and then swapping only their lft values: the winner flipped. That is not something anybody chose. Every menu's top-level items hang off one root interleaved by `ordering`, so on lucent /resources (components, ordering 1) sits next to /nav-discover (mainmenu, ordering 1) - and reordering the main menu in the admin can hand an address to a menu the admin is never shown alongside it. lft is also reassigned wholesale by any rebuild, and the two rebuilds in the tree disagree: com_menus orders children by lft, ComponentRoute by ordering then lft. So installing a component could change the answer. Now it is a rule. A menu the hub shows beats one it does not, and both beat the entry generated for a component. A longer address still wins first, and where the kind ties the old behaviour still decides, so nothing previously unambiguous moves. The point of the order: a generated entry is a floor, not a claim. It exists so every component has some address, and it steps aside the moment the hub says otherwise. Verified on lucent - a main menu item declaring /resources wins at lft 960 against the generated entry's 119, where the lower lft used to take it; delete the item and /resources goes back to the generated entry with no cleanup step. The save-time check follows the same order, so an item may take an address off a weaker claim and not off an equal one. That is how a hub puts a component in its own menu at the component's own address. And the same question backwards, which had no answer at all. Building a URL with no Itemid used the component's name, full stop - so a hub whose Resources page sat at /library still emitted /resources/browse from inside /library, losing the Itemid and with it that page's modules and template style. There is now a lookup, ranked identically, for the item that speaks for a component - counting only items whose link is the component and nothing else, since one pointing at a particular view is a page about something narrower. On a hub with no competing item this changes nothing, which is all three of ours. "muse routes check" reports addresses claimed twice, saying which answers and which is shadowed, because a rule that is deterministic is still invisible. It found a real one: welcome and mesozoic both ship two separators at /discover/n, ids 46 and 49, same parent, same alias, both titled . That is in starter.sql. Harmless - separators are not pages - but it is duplicate data and it sits where a unique key should have refused it. Not fixed here. Alias items are excluded from that report, as the router excludes them from matching: an alias sharing an address with the entry it points at is the arrangement working, not a clash. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
/resources and index.php?option=com_resources are the same page. One had an Itemid and the other had none, so one had a template style, per-page modules and a place in a breadcrumb, and the other had none of the three. Worse, a non-sef url that named its own Itemid was ignored too: index.php?option=com_x&Itemid=12 carried the Itemid as a query var and then never activated it. The menu rule returned early for a non-sef url before setActive() was ever reached - the commented-out `|| isset($query['Itemid'])` beside that return looks like somebody noticed and stopped. So: honour the Itemid when one is given, and otherwise ask the same question the builder asks - which menu item speaks for this component. That lookup needed loosening to survive real data. It counted only items whose link is the component and nothing else, which is true of entries this code generates but not of the ones a hub carries over from the old default menu: welcome has com_resources at index.php?option=com_resources&view=intro and com_support at &view=index&layout=display. A display item naming a view is a page about something narrower and still does not count, but a component menu's entry is that component's page by definition, whatever its link says. Where a hub has both, the plain one wins. Measured on all three hubs: index.php?option=com_resources now activates the same item as /resources, and index.php?option=com_content still activates nothing, because no bare com_content item exists and com_content has no generated entry. Built urls are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The twelve components that get no address were listed in the console command
and again in the backfill migration, and the dev-only lazy generation knew
about neither: routable() asked only whether a site directory existed. So with
debug on, asking for /cron, /oaipmh, /help or /media created a menu entry for
each. Measured: 28 component entries went to 32.
The list now lives in ComponentRoute, routable() consults it, and the console
command reads it from there.
Each exclusion checked rather than assumed, by asking each component at its own
name: com_cron answers with JSON, com_oaipmh with OAI-PMH XML, com_mailto 402,
com_oauth and com_media 403, com_saml, com_messages, com_system and
com_dataviewer 404 for want of a site view, com_redirect 500. com_content
routes through article paths and has never answered at /content. com_help was
the one that looked wrong - it serves a real HTML page - but its controller
does Request::setVar('tmpl', 'help'), so a template style from a menu entry
would be ignored anyway.
Excluding one does not make it unreachable. The router still resolves /cron by
component name. It means only that there is no menu entry and so no Itemid,
which is right for something nobody navigates to.
The backfill migration keeps its own copy: it is a record of what ran on hubs
that have already run it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The two separators at /discover/n were a symptom. starter.sql dropped
idx_client_id_parent_id_alias_language and added it back as a plain index, so
every hub installed with sample data has run without that constraint ever
since. Only a hub installed without sample data kept it - which is why lucent
refused a duplicate alias with an integrity error while welcome and mesozoic
quietly accepted one.
It was dropped because the sample data broke it, in two places:
- two separators under Discover, ids 46 and 49, both aliased `n`. They sit at
different points in the menu, so both are wanted; they only needed
different names.
- the main menu's Support entry, id 8, sharing the root with the component
entry it points at, id 84, both aliased `support`. Which is the pattern
this whole line of work has been about: a display item and a component
route wanting one name.
Both renamed rather than removed, and neither moves a URL: an alias item and a
separator are never matched against a request - the router skips them - so
their alias is a label. With the data no longer breaking it, starter.sql leaves
the key alone.
A migration does the same for hubs already installed, and is careful about
which of a pair it renames: the cosmetic one, never the one answering at a
path. A group with no cosmetic member is reported and left, because moving a
working address under a hub without asking is worse than leaving a duplicate.
Run on all three: welcome and mesozoic each renamed two, lucent had none, and
all three now hold the key with no duplicates and no page moved.
The install test now reads the UNIQUE and PRIMARY KEY declarations out of
schema.sql and holds the data files to them. It would have caught this the day
it was written. It lists every clash rather than the first, because a list of
what to fix is worth more than one line of it at a time.
That needed the reader to model REPLACE properly: it appended, where a database
replaces the row with the same key. starter.sql replaces assets that data.sql
already wrote, so the reader saw pairs of rows that never exist together and
the new test failed on them before reaching the real thing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…g ones The skip list was a judgement about which components are worth an address, and that kind of list ages badly: a component that is machinery today may grow a page tomorrow and nobody will remember to take it off. It is now the components with no site code to run at all - com_media, one class and a helper; com_messages, a language file and nothing else; com_system, a router and a class. Everything else gets one, endpoints included. That costs nothing: the router already resolves /cron by component name whether or not an entry exists, so all the entry adds is an Itemid, and the day one of them grows a page it has somewhere to put its modules and its template style. Note that "no site/views directory" is not the test, though it looked like a tidy mechanical one. com_dataviewer has no site/views and is a full site component - Controller.php, View/Gallery.php, View/Spreadsheet.php, its own router, a tree of assets - written in a namespaced layout rather than the old one. A rule built on that directory would have taken its address away. Measured across three hubs, seventy-eight urls before and after: seventy-two byte for byte identical. The six that moved are error pages on lucent that had been marking Home as the current menu item because nothing else matched, and stopped - eight bytes of " current" each. A migration creates the ones existing hubs are missing, so this does not wait for somebody to run "muse routes fix". It gives a switched-off component an unpublished entry, which is what EnableComponent expects to find when the component is switched back on - and "muse routes check" no longer reports that as an address left behind, because an entry that is off for a component that is off is the entry tracking the component. A published one still is reported. Also fixes a collision in survey(): the extensions rows and the map built from them were both called $installed, so the loop was writing keys into the collection it was iterating. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A menu item for a component the hub has switched off is not a page. The dispatcher already refused to run one - /citations 404s the moment com_citations is disabled - but the item stayed in the menu as a dead link, kept its Itemid, and went on holding its address against anything else that wanted it. The fix is a join rather than a second copy of the fact. The menu already joins #__extensions to get each item's component name, so it now also leaves out any item whose component is not enabled. That is right however the component was switched off - by a migration, by the extension manager, or by hand - because there is nothing to keep in step. An item with no component says so with component_id 0 and stays. Anything else has to find its component enabled, so an item left behind by a component that was uninstalled goes as well, rather than joining to nothing and being kept. Aliases go with it. An alias points at another item by id, and when that item disappears the alias is left pointing at a hole - which is how a url like /content/?Itemid=142 gets built. An alias whose target is no longer loaded is dropped too. Which makes the publish/unpublish tie redundant, and it was worse than redundant. EnableComponent and DisableComponent kept an entry's published state matching the component, so disabling by migration and re-enabling in the extension manager left the entry unpublished: the component ran, answered at its address through the router's name fallback, and had no Itemid - no modules, no template style, no breadcrumb - with nothing to say why. Those macros no longer touch the entry, and create() no longer copies the component's enabled state into it. An entry's published state goes back to meaning what it means everywhere else: whether the administrator wants the page. A migration publishes the entries that were switched off on a component's behalf, since nobody chose that. "muse routes check" stops calling an address for a switched-off component an address left behind - it is waiting, and the menu leaves it out until the component comes back. Round trip measured on welcome: disable com_resources and /resources 404s, its main menu entry and the alias pointing at it both vanish, and no dangling Itemid url is built; enable it and the page is byte for byte what it was. Across three hubs, seventy-eight urls, the only differences from before any of this are the six error pages that stopped marking Home as the current menu item. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
com_redirect takes a base64 destination in id and sends the visitor there. With
no id, base64_decode('', true) returns an empty string rather than false, so the
guard for an undecodable id did not fire and App::redirect('') was reached -
which throws, because an empty url is not a url. /redirect has been a 500 on
every hub.
An empty destination now goes where an undecodable one goes: home. And it goes
there by Request::base() rather than by pasting the Host header into a url,
because the Host header is the visitor's to set and this is the one component
whose whole job is sending people somewhere else.
Measured on all three hubs: /redirect 302s to the site root, /redirect?id= with
a valid base64 url 302s to that url, and /redirect?id=not-base64!! 302s to the
site root.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every administrator saw "jQuery is not defined" the moment they signed in. The control panel has its own template file, cpanel.php, and where index.php asks for the framework it never did - so the one page an administrator always lands on was the one page with no jQuery, while the pages they navigate to afterwards were fine. It draws js/index.js, which is jQuery's growl plugin and uses $ throughout. Found by instrumenting Html\Builder\Behavior::framework() rather than reading: a control panel request logged no call to it at all, which ruled out everything about how the push works and pointed at which file was doing the asking. Also, while in there: framework() recorded itself loaded whether or not the push happened, and the push is a no-op when there is no document yet. One early call - from bootstrap, or a plugin running before the document exists - would have meant no jQuery for the rest of that request with nothing to show for it. It now only remembers what it actually did. That was not this bug, but it is the same bug waiting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An administrator could tick some menu items, fill in the batch form and press
the button, and get a raw PHP TypeError printed on the page. Six separate
defects, none of which could be reached past the one in front of it.
1. The controller called $model->batch($vars, $pks, $contexts). Relational has
a batch(int $size) that yields chunks, so that is a TypeError before
anything happens. The model's method is batchTask().
2. batchCopy() and batchMove() opened with $this->getTable() and
$this->getDbo(), neither of which exists on Relational. Both were dead
assignments - $table is never read, and $db and $query are reassigned from
App::get('db') further down - left behind when the rest of the method was
ported.
3. batchCopy() called $this->setError(). The error trait has addError() and
setErrors(); there is no setError().
4. batchCopy() returned $newIds without initialising it.
5. batchMove() moved an item's children to the new menu and never set the
menutype on the item itself, so a move left the item behind in the menu it
came from with its own children somewhere else.
6. The form's menu picker is drawn `if ($published >= 0)`, where $published is
the status filter and "" means no filter. Until PHP 8 that compared equal to
zero and the picker was drawn. PHP 8 compares a non-numeric string against
an int as strings - '' >= '0' is false - so the one control the batch form
exists for stopped rendering unless a status was picked first.
And then the picker was wrong: menuitems() built its lookup with
$lookup[$item->menutype][] = &$item, a reference to the foreach variable, which
PHP reuses - so every entry aliased the last row read and every option in the
list was the same menu item. These are objects, already passed by handle, so
the reference bought nothing.
Verified through the admin, not by calling the methods: moving a parent with a
child moves both and keeps the nesting; copying makes "Batch Parent 2" with its
own child and leaves the original alone; and a generated component entry is
refused with the message that guard has been carrying unreachable since it was
written.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
"muse routes check" asked whether a menu row existed at a component's address and called that fine. The menu only loads published items, so an entry that is switched off leaves the component answering by component name with no Itemid - no modules, no template style, no breadcrumb - while the check reported nothing wrong. Found while testing whether the alias items could be collapsed: that plan leans on this check being the safety net, and it had a hole exactly there. It now separates the two. An address with no row is missing, as before. An address whose rows are all switched off is reported on its own, saying which item is sitting there and whether it is one this command generated. "fix" switches the generated ones back on, because it made them. It refuses the rest and says so: an item somebody unpublished on purpose is their decision, and a repair command that quietly republishes it is worse than one that leaves a component without an Itemid. Where several rows share an address it reports the generated one, since that is the one it can act on. Measured both ways on lucent. Switch off the generated /citations entry and the check names it, the page loses its active menu item, and fix puts it back. Give /search to a main menu item instead, switch that off, and the check names it as not generated while fix leaves it alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A menu item nested under a heading answers at the heading's address:
/discover/resources, not /resources. That is the point of nesting it. And once
a visitor is in that section, links to the same component should stay in it -
/discover/resources/browse - rather than dropping back to the flat component
route on the first click.
The build rule has always meant to do that. Given a link with no Itemid it
looks up the current page and, if that page belongs to the same component,
carries its Itemid through:
$item = $menu->getItem($uri->getUriVar('Itemid'));
Inside `if (is_null($itemid))`. It passes the Itemid it has just established is
null, so getItem() - which is isset($this->_items[$id]) - returns nothing, every
time, and the prefix is dropped. getActive() is the page you are on.
Measured on lucent with Resources genuinely nested under Discover:
/nav-discover/resources and /nav-discover/resources/browse both answer and the
links inside the section keep the prefix, while a link built from an unrelated
page goes to the item that speaks for the component. The generated /resources
entry still answers as the floor underneath.
I had noted this branch as deliberately left alone, on the grounds that making
urls depend on the page they were built from was undesirable. That was the
wrong call: carrying the section is the design, and dropping it is what made
nesting a menu item not worth doing.
Nothing moved on any of the three hubs, which have no nested component items -
seventy-eight urls, the same sixteen differences as before this change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Eight findings from a review of the routing work, seven confirmed on the code and one plausible. All fixed. 1. The item form could not put an item in a routing menu. The `menu` form field had been narrowed to display menus so a menu module could not be pointed at the routing table - but the item form's own Menu Location uses the same field, so routing menus vanished from it and an item already in one would silently move on save. The filter is now opt-in by attribute: mod_menu says kinds="display", the item form says kinds="display,routing". 2. The components menu could end typed display. The main menu migration makes a `components` menu before the type column exists; the later migration only retyped a `default` menu and returned early without one. Both the migration and ComponentRoute::menutype() now retype a `components` row wherever it came from. 3. MenuUnique::isUnique() always said no. TABLE_NAME = '#__menu' put the prefix inside a quoted literal, which replacePrefix() skips - so up() could not short-circuit and down() was a permanent no-op. It asks SHOW INDEX on the table as an identifier now. Proven: Non_unique=0 on both hubs. 4. Item::rebuild() selected `route` unguarded, so the admin Rebuild button and batch copy would fail on a hub whose files had been deployed before its migration ran. Guarded like its siblings. A route of "0" is a route. 5. ?option[]=x and ?Itemid[]=1 threw TypeError from the router under PHP 8. Guarded; both return 200. 6. The install-SQL reader's UNIQUE KEY regex stopped at the first ")" inside a prefix-length column, so the cart_saved_addresses key was silently never checked. It now parses as uidNumber, saToFirst, saToLast, saAddress, saZip. 7. The menu-kind ranking was written in four places. It is ComponentRoute::rank() now, and the other three call it. 8. Uninstalling a component left its generated route behind. DeleteComponentEntry pairs with AddComponentEntry: ComponentRoute::remove() takes the entry the installer made and nothing else. Plus one style finding in mod_menu.php from earlier work. Verified: item form offers a routing menu and hides the component menu while the mod_menu picker offers neither; 78-url probe unchanged against baseline; routes check clean on all three hubs; 41 install/menu tests green. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…work
A review of every commit since 2.4-main. Ten commits are mechanical sweeps
(PSR-12, namespacing, autoloading, facade imports) and were verified
mechanically: 6,731 files lint clean, PSR-12 clean across all seven core areas,
the configured suite green, every muse command resolving, every template style
on every hub pointing at a template on disk. The authored work was cut into
five slices and read by risk. This commit fixes what the February platform
slice turned up, plus one leftover from retiring the welcome template.
1. Installer could not detect a bad database password. PdoConnection's
constructor stores credentials and never connects, so the try/catch written
around it could not fire: "Test connection" said yes to anything, and the
real failure surfaced two steps later as an exception nothing caught. It
connects inside the try now, and the timeout argument that was being
dropped is passed through.
2. Registry turned every empty array into an empty object. range(0, -1) is
[0, -1], so [] failed the "is this a list" test and came back as stdClass -
[] to {} on every params round trip, and a TypeError for anything that then
did in_array() or count() on it. Proven fixed against the real class.
3. TOTP tolerance shrank from a period to a second. otphp's third argument is
a leeway in seconds, not periods; passing 1 meant a phone a few seconds off
was locked out where the replaced library accepted the adjacent 30-second
windows. The three periods are checked explicitly now.
4. The legacy configuration.php loader had two regressions: it lost its
PATH_ROOT default, so the one caller that passes nothing - saving Global
Configuration - was updating /configuration.php; and it read the file
through json_decode(json_encode()), which returns false on a single Latin-1
byte, so a Joomla-era file with an accented sitename read back as an empty
config with nothing to say why. Default restored; a byte-agnostic walk
replaces the JSON round trip.
5. The YAML config processor hard-required the PECL yaml extension, which is
declared nowhere, and the registry tries it on any string the other
processors do not claim - so a params value as ordinary as "disabled" was a
fatal error on a host without the extension. It falls back to symfony/yaml,
which the framework already ships, restoring the graceful path the
replacement had removed.
6. "muse migration status" showed already-run core migrations as pending. It
keyed history by scope literally, where migrate() treats the legacy scopes
'' and 'migrations' as core/migrations. Both agree now.
7. "muse migration refresh" rolled back oldest first, so a table was dropped
before the migration that altered it had undone its part. Down walks the
list reversed.
8. After the welcome template was retired, "muse repository flavor" set home=1
on a template that no longer exists and then home=0 on every other site
style - leaving a hub with no default template. Three copies of that block
are one helper that looks before it clears, which also fixes a missing quote
in the default case that made its second UPDATE a SQL error.
Checked and found sound on the same slice: the HZEXEC guard removal, the
markdown and Sanitizer replacements, the constants conversion, the session
change, the DDL-commits-itself transaction handling, the freed-statement
re-prepare, the identifier quote stripping, var_export config serialisation,
the mobile-template shell check, and the child-templates revert. The
installer's 8-character password salt matches the reader and the hub's own
writer; the rigid reader predates the branch.
Verified: lint and PSR-12 clean on every touched file; 41 install/menu tests
and the 2,905-test configured suite unchanged; the 78-url probe unchanged
against baseline; routes clean on all three hubs.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…found The third slice of the branch review: the component fixes and the migration framework work from September 10-11. Read from three angles - line by line, removed behaviour, and callers - with every candidate verified against the code and, where it could be, against a live hub. 1. Inserting an Expression bound it as a parameter. buildValues() and the multi-row form put '?' for every value and bound whatever it was, so Expression::now() reached PDO as its debugging string, "Expression(now)", and the column got that literal. buildSet() already rendered objects as SQL; the insert paths do the same now through one helper, and a subquery is parenthesised as it is there. The groups home-page migration was the caller that tripped it. 2. "muse migration status" listed every core migration as pending. Four one-off code paths derived a migration's scope with a bare string replacement while find() went through scopeFromPath(); on a hub where the two disagree (lucent) the status view showed 1,413 pending against 1,413 run. All four use scopeFromPath() now: 0 pending. 3. A migration that finished with warnings ran again every time. The skip test accepted only 'success', where the status view and the pending calculation both treat 'warning' as done. Eleven rows on welcome were being re-run on every migrate; the dry run is clean now. 4. The 2014 support-tickets migration's down() called modifyString() on the schema manager, which has no such method - a fatal on any rollback. It uses the same modifyColumn() chain its up() does. 5. The public projects listing asked for COM_PROJECTS_OPEN, which only the admin language file defines, so an open project's badge was the raw key. The site file has it now. 6. The read-only ticket list built the shared query folders by calling cloneCore(), which also copies the whole set onto whoever happened to load the page first. The building half is ensureCore() now and that is what the read-only branch calls; cloneCore() calls it too, so its two remaining callers are unchanged. 7. The com_usage migration moved the metrics database host from localhost to the site's host without checking either. A hub whose metrics database really is local - a replica, say - would have lost it. It connects first, the way the component's helper does, and only rewrites a host that does not answer to one that does; when neither answers it says so and leaves the settings alone. 8. Reconfiguring the database connection, interactively or from an answers file, reset the table prefix to jos_ whatever the hub used. The existing prefix is read with the rest of the existing configuration and kept. Checked and found sound: the storefront Store-alias migration (its target row ships unpublished, the three-way guard is the documented intent, and it ran cleanly on both hubs); the timezone migration's early return; the update alias for the non-MySQL drivers. Verified: lint and PSR-12 clean on every touched file; 41 install/menu tests and the 2,905-test configured suite unchanged; Expression::now() renders as NOW() in single-row, multi-row and set() inserts; the 78-url probe unchanged against baseline; routes clean on all three hubs. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The second slice of the branch review: the fifty-three February commits that
fixed undefined variables, missing methods, mismatched names, duplicate keys
and the like across the components, and reworked the config repository.
Read from three angles - line by line, removed behaviour, and callers - with
every candidate verified against the code and, where it could be, live.
Config
1. Per-client configuration was no longer loaded. The repository used to
take the client and pass it to the loader, which merges config/<client>/
over the base; the rewrite passed nothing, so an api/ or administrator/
override directory was silently ignored. The client is passed again.
2. The Joomla-era configuration.php was looked for under app/ rather than
the site root, so a hub not yet migrated to config/ booted with an empty
configuration. The loader's root path is PATH_ROOT again.
3. Saving Members > Registration with caching on threw: the repository now
refuses a bare key that is not a config group, and the cache handler name
is not one. Dot-notation, as com_cache was already changed to use. The
set() docblock described a fallback the code does not have; it now says
why the throw is the right answer.
Components and plugins
4. The storefront image form rendered a layout that has never existed - not
on this branch, not before it - so the noscript fallback and every
non-ajax upload result died. The view exists now and displayTask() reads
the object from the query string the edit pages put there.
5. Re-subscribing a newsletter address called load($id) on a Relational
model, which has no such fetch; the row was never loaded and save()
inserted a new one. It uses oneOrFail() like the tasks around it.
6. The project link preview asked voku's find($sel, 0) whether a meta
description exists; it returns a blank node, never null, so the body-text
fallback was dead. findOneOrFalse() and getAttribute().
7. Deleting a project feed comment resolved the wrong id: the link says cid,
the handler read activity. It accepts both.
8. The group Collections "Following" tab showed followers; there was no
following() at all, on this branch or before. Written from followers()
and the members plugin's equivalent, for the following template that was
already there.
9. Re-saving a tool's dev group made every developer one of its managers -
and a manager can add members, that is developers, which is the tool
admin's decision. The managers the group has are left alone.
10. Sharing a tool session copied connection details from the owner's
viewperm row without checking one exists. Guarded.
11. The forum API looked up a thread root by object and scope with no parent
filter and no ordering, so a reply could stand in for the root and the
tree walk started in the wrong place. The root is the post with no
parent, in both lookups.
12. The FTP adapter's deleteDirectory() listed bare names and deleted them
relative to the session's working directory, and accepted a $preserve
flag it ignored. Full paths; preserve honoured as Local does.
13. The jobs API carried an authorization helper nothing called, on a class
with no can() - dead code that would fatal the moment it was wired in.
Removed.
14. The promo opt-out built Awards from User::get('id'); the constructor
treats anything that is not an integer as a profile object. Cast.
15. The InstallTemplateEntry macro's signature gained two parameters in the
middle; a migration written to the old five-argument form would have put
its styles in the enabled slot. The old form is recognised and mapped.
Found live while checking: an empty search hit a preg_replace() deprecation
on every request (com_search terms, from January, outside this slice). Cast.
Checked and found sound: the Repository throw on an unknown bare key (tested
design; the shipped app.php carries every key the plugins set); the KB
comments feed link (Document::setLink() runs first); the support ticket
severity default (nothing filters on it); the members media admin check
(component-scoped, as the rest of com_members); decoupling plugins and
modules from Obj (deliberate; no in-tree or hub-local caller depends on it);
the plgX-to-namespace rewrites; the language-pack restructure (no key lost);
the API controller renames; the vendored markdown and DOM swaps.
Verified: lint and PSR-12 clean on every touched file; 113 config tests, 41
install/menu tests and the 2,905-test configured suite unchanged; site,
administrator and API clients serving on all three hubs; the 78-url probe
unchanged against baseline; routes clean on all three hubs.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…found
The fourth and last slice of the branch review: the forty-seven commits of
September 12-13 - the meridian template, the contrast and notice work on
lucent and mesozoic, the screenshot and review tooling, the front page a
template draws itself, the "no component" menu item, and the starter data's
legal and about pages. Read from three angles - line by line, removed
behaviour, and callers - with every candidate verified against the code and,
wherever it could be, against a live hub.
The "no component" menu item
1. The type could not be chosen. setTypeTask() coerced anything that was not
alias, separator or url back to component, so the chooser's new entry
produced a component item with no link; editing an existing item of the
type converted it the same way; and the items list called it "Unknown".
All three know the type now.
2. A page of that type that was not the front page was a 404. The component
provider let an empty option through only for the home item, while the
item's own form offers Display in Menu - a page the menu links to has to
answer. It answers now wherever the active item names no component.
Proven live: a temporary item of the type rendered through the template
and was a 404 again once the row was gone.
3. The chooser's tooltip still said the item never appears in menus, which
Display in Menu had made untrue.
Menus and templates
4. The three templates' error pages fetched core.js and hub.js from a
/templates/ directory that does not exist, so every error page's scripts
were 404s - the stylesheet beside them had been converted to asset() and
the scripts had not. They use asset() too; 200 on all three hubs now.
5. A menu group whose children were all hidden still drew as a disclosure
button, controlling a submenu that was never emitted. A parent is one
with children left to show.
6. With the module's toplevelLinks on - as the generated main menu ships it -
a top-level group is drawn as the button that opens it, and the phone
menu built its heading from a link that was not there: five unlabelled
rows. The heading takes the button's text when there is no link.
7. Two mesozoic system-css overrides imported the base stylesheet from one
directory too shallow, so the calendar and fancy-select widgets got the
override's five rules and none of the layout under them. Six, like the
file beside them.
8. Mesozoic's component shell linked css/component.css, which it never
shipped - asset() returns an unversioned address rather than failing, so
every tmpl=component popup was unstyled. It ships the shell stylesheet.
9. A shell that aborts - the group shell does when asked for without a
group - threw out of the document's open output buffer, so the error page
was written into a buffer nobody closed. The buffer closes on that path.
10. The main-menu migration wrote a route row with component_id 0 for a
component with no extensions row, which then passed the menu's extension
check as an item naming no component; the navigation advertised a
component that was not installed. No row, no route.
11. Past the hero, meridian's front page held the clouds wherever the last
frame inside it had put them; a jump over the boundary parked them
mid-drift. It holds the final position instead.
Data, tooling and docs
12. The footer module's "Helpful Links" heading went with the hard-coded
year, leaving four h3s under an h1 on every page of a new hub. Restored.
13. shoot.mjs wrote its screenshots relative to the working directory; run
from its own directory it put them beside itself, and seven of those were
committed while the README says they are not tracked. It resolves the
output against the repository, and the seven are gone.
14. tools/hubs/check-idempotent.sh ran `muse sampledata`, a command this
branch removed with the rest of the sample-data machinery; it would have
reported every hub as broken forever. Removed, and the README that
pointed at it says where that check belongs now.
15. The developer docs linked $this->asset() to Document/Type/Html.php; it
lives in Document/Base.php since the error document needed it.
Checked and found sound: the front page discarding the aside positions (the
template draws its own front page by design); menu_show hiding a subtree
(deliberate, and the Joomla meaning of the parameter); the meridian template's
descriptive name (lucent and mesozoic are named the same way); lucent's
retired home positions against the base data's front-page modules (a template
chooses what it draws - noted for the template docs); the sample-data
references in the untracked planning notes (not on the branch).
Verified: lint and PSR-12 clean on every touched file, node --check on every
touched script; 41 install/menu tests and the 2,905-test configured suite
unchanged; error-page assets, the component shell stylesheet and a
"no component" page checked live on the hubs; the 78-url probe unchanged
against a baseline re-taken after the error-page fix (every earlier difference
was that fix's two cache-busters); routes clean on all three hubs.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`muse repository:flavor` was written for a world this branch has left. It
knew amazonfull, amazoncmsonly, and three names for the vanilla install; it
stored the flavor as a parameter of the welcome template style, which is
retired; it made welcome or hubbasic2013 the home style, neither of which
exists here; it toggled com_store, which is gone; and it rewrote two
Joomla-era content pages that the starter data no longer ships. The help text
offered "amazon", which was not accepted.
What the flavors actually decided was one thing: does this hub run
simulation tools. That is the whole command now.
default the CMS on its own - the tools component and its usage metrics,
the My Tools and My Sessions dashboard modules, the tools column
of My Drafts, the knowledge base's Tools category and WebDAV
article, and the "tools" resource type, all off
full the same set, all on
Nothing is deleted in either direction - the resource type is made not
contributable rather than removed, the knowledge base rows are unpublished
rather than trashed - so switching back restores what was there. The
dashboard's default tiles are laid out column by column in the shipped order,
so setting the flavor a hub already has writes the same params it has.
A `status` task says which flavor the hub reads as (com_tools decides), what
is on and off, and which pieces disagree with the flavor.
The routing work earlier on this branch means disabling com_tools takes the
Tools route and its menu alias out of the navigation by itself; verified on
the welcome hub: set default gave /tools a 404 and dropped the route, set
full brought both back, and the probe against baseline is unchanged after the
round trip. The one /tools link that remains in the default flavor is in a
shipped content module ("See what is available"), which is data rather than
routing.
docs/reference/muse.md regenerated.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…point
The last commit reduced `muse repository:flavor` to two hard-coded flavors.
This one takes the hard-coding out. A flavor is now a description in a JSON
file of the levers that make one hub differ from another, and the command
reads, resolves and applies whatever it finds.
The levers
description, extends, template
components {enable, disable}
modules {enable, disable, params}
plugins {enable, disable, params} named folder/element
dashboard {tiles} the member dashboard's defaults
kb {categories, articles} published states by alias
content {articles} published states by alias
resource_types columns by alias
Every lever is a switch on rows that exist - an enabled flag, a parameter, a
state, a column. Nothing is added or removed, so applying one flavor after
another leaves the hub as the second describes it. Anything in a file that
is not a lever is refused by name, with the list of what is.
Where flavors come from
A file holds any number of flavors, an object keyed by name. Files are read
from three places in order: a directory named with --path (or the
HUBZERO_FLAVORS environment variable), then the hub's app/flavors, then the
CMS's core/flavors. The first definition of a name wins, so a hub redefines a
shipped flavor by writing one of the same name.
Flavors cascade. One that extends another starts from its parent's levers
and overrides them: a scalar replaces; a name the child enables leaves the
inherited disable list, and the other way round; parameters and states merge
key by key; a dashboard layout replaces the layout, since a layout is one
thing. A flavor may extend one defined in any of the directories, so a hub's
own flavor can start from a shipped one. A circle is refused with the circle
named; so is a parent that is defined nowhere.
The CMS ships default and full in core/flavors/flavors.json. Full is written
as "default, with the tool pieces back on" - it names what it turns on and
inherits the rest - which is the cascade earning its keep on line one.
The command
muse repository:flavor list what there is, and from where
muse repository:flavor show <name> the levers, resolved
muse repository:flavor set <name> pull them
muse repository:flavor status [<name>] how the hub differs; or, given no
name, which flavor it matches and
which is nearest when none does
The installer
`muse install` applies a flavor after the migrations, when every table a
lever touches is there. The answer file's `flavor` names one (and `flavors`
a directory to read it from); so do --flavor and --flavors on the command
line; and when neither says and somebody is there to ask, the install offers
the default flavor and leaves the hub as the data shipped it if declined. So
a site can be stood up from a script pointed at its own flavor directory and
come up already shaped. `muse install flavor` runs that step on its own.
Verified: lint and PSR-12 clean; ten unit tests over reading, cascading,
cross-directory redefinition and extension, circles, unknown parents,
unknown levers, misshapen levers and the tile layout; and live on the welcome
hub - set default took com_tools and its route out (/tools 404) and set full
brought everything back with the dashboard params byte-identical to an
untouched hub; a directory of the hub's own redefined full and added a quiet
flavor through both --path and HUBZERO_FLAVORS; a malformed file was refused
by name; `muse install flavor` applied and refused as it should. The 78-url
probe is unchanged against baseline. docs/reference/muse.md regenerated.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Two additions to the data-driven flavors, and one thing the second showed. YAML. A flavor file may be *.yml or *.yaml as well as *.json; the two read alike, and a flavor in either may extend one in the other. JSON stays the shipped format because it needs nothing, but a hub's own file usually wants a comment beside the switch saying why it is set - which YAML can carry and JSON cannot. Parsing goes through symfony/yaml, which the CMS already ships; a file that does not parse is refused with its name and the parser's reason. Within a directory, files are read in name order. Menu items. A `menu` lever publishes or hides site menu items by path - the address the router matches, which is unique among a client's items where an alias is not - so a flavor can take an entry out of the navigation without touching the component behind it. It is meant for navigation entries; a generated component route (the `components` menu) is what `muse routes fix` republishes, so the way to lose a component's address is to disable the component, and the docs say so. States merge path by path down the cascade like the knowledge base and content levers. Rows a flavor names that the hub does not have. Trying the menu lever on a path the hub did not have showed that check() treated a missing row as a match - so a mistyped path would have passed silently. A row the flavor names and the hub lacks is now a note, not a difference: the hub is not in the wrong state, there is no state. `set` says "No menu item x to set" as it goes; `status` lists what the flavor names that is not on this hub after its verdict. The same holds for knowledge base and content aliases and resource types. Verified: lint and PSR-12 clean; twelve unit tests (two new: YAML and JSON in one directory extending across directories, with menu items merging by path; and a broken YAML file refused by name); and live on the welcome hub through a YAML file of its own - `quiet` hid the discover/courses entry (/discover/courses 404, /courses still answering), the mistyped comunity/nowhere was reported by `set` and by `status`, `loud` put the entry back and `status full` matched throughout, as it should since full says nothing about that item. The 78-url probe is unchanged against baseline. docs/reference/muse.md regenerated; the developer book has the YAML example and the menu lever. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…er keys
A `modules.items` lever publishes or hides site module instances. A number
names one instance by id; anything else is a title and names every instance
with it, since the shipped data places the same module in more than one
template's positions under one title ("What you will find here" is two
rows on a new hub). This is the lever that reaches a shipped content
module - the front-page pitch, a dashboard tile - which the module switch
could not without disabling every instance of mod_custom.
Trying it by id showed that the cascade renumbered integer keys: 92 came
out as 0, because array_merge() reindexes integer keys, and a module id or
an all-digit alias is an integer key once YAML or JSON has read it. Every
key-keyed merge in the cascade - parameters, states, menu paths, resource
type columns, module instances - now goes through array_replace(), which
keeps keys, keeps the parent's order and lets the child's values win.
Also: the two PHP notices the configured suite has carried since before
this branch. QueryBuilderIntegrationTest asserted that update() and
delete() "return affected rows" with `$result > 0`, but Query::execute()
hands back the driver, as every Driver::query() does, so the comparison was
an object against an int - a notice, and an assertion that could never
fail. The tests now assert the statement executed and ask the driver for
the row count, which is what they meant. The suite is clean for the first
time on this branch: OK (2917 tests, 8588 assertions).
Verified: lint and PSR-12 clean; the flavor tests cover instances by id and
title merging down the cascade; live on the welcome hub a YAML flavor hid
"What you will find here" by title (both instances, and the front page lost
the pitch), then by id, and the Resources tile by id, and put them back;
`status full` matched throughout. The 78-url probe is unchanged.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… it lost Rebuilding the welcome hub from scratch stopped 449 migrations in, on a fatal: AddComponentEntry calls $this->rebuildMenu() after inserting a component's admin menu entry, and that method is not on it. It was, until "One place that makes a component route" moved the nested-set walk into Hubzero\Menu\ComponentRoute; the macro's other caller was updated and this one was not. Nothing on an existing hub runs it - those migrations have run once and are not run again - so three hubs, the full test suite and a four-slice review all passed over it. It fails only where it matters most: every new install, partway through, with the schema half built. The macro asks ComponentRoute for the walk now, through one memoised helper that the route creation already used. The whole tree, not a client's part of it: one root carries both the site and the administrator menus, and an entry inserted with zeroes for its bounds is an entry the admin menu cannot place. MacrosTest checks, for all 23 macros, that every $this->method() call resolves to a method the class has, and that each macro is invokable. It fails on the unfixed file with "AddComponentEntry calls rebuildMenu() on itself, which it does not have" - verified by setting the fix aside and running it - and passes with the fix. A macro runs during an install or a migration, which is the worst place to learn that a method moved. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
muse install writes site_template into app/config and never touches #__template_styles. The style marked home is what a hub actually wears; the config value is only the fallback the loader reaches for when there is no such row, and the base data always ships one. So an answer file asking for kimera produced a hub running meridian, silently, and the setting the installer had just written was inert. tools/hubs/newsite.sh knew this and worked around it, ending every build with raw SQL against jos_template_styles - an UPDATE, an INSERT ... SELECT FROM DUAL guarded by a NOT EXISTS subquery, and another UPDATE. That is the workaround, not the fix, and it only helped hubs built by that script. Hubzero\Template\Style is now the one place that says which template a client wears, the way Hubzero\Menu\ComponentRoute is the one place that makes a component route. It finds the style, adds one for a template installed without a style rather than refusing to use it, clears home across the client and sets it. Both callers come through it: the installer's new step, and a flavor's `template` lever, which had its own copy of the same walk. muse install template --template=meridian muse install (honours the answer file's site_template) The site template only. administrator_template has never been acted on, and what is written there is not always an administrator template - newsite.sh has been putting `kimera`, a site template, in that key on every hub it has ever built. Reading it now would have put a site template on the admin and broken the one screen somebody would use to put it back. Fixed in the script to say kameleon, and the installer leaves the admin template alone either way. StyleTest covers the guard that decides whether a template is there to be worn. It caught that is_dir() was happy to follow '../components' out of the templates directory: a template is named, not addressed, so the name has to look like a name before anything is done with it. Verified by rebuilding all three hubs from their answer files with --reset: welcome asked meridian and got meridian, mesozoic mesozoic, lucent meridian, each serving that template on the front page and kameleon on the admin, all 1413 migrations, routes clean, no /tools link on any front page, and twelve fresh requests across the three adding nothing to the error log. newsite.sh no longer runs any SQL of its own. Co-Authored-By: Claude Opus 5 <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
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.
Reformat the PHP code base to PSR-12 and add phpcs.xml.dist, which runs
PSR12 over the code directories minus the two sniffs that flag
underscore-prefixed and snake_case method names, since those are public
API in places. Structural fixes the formatter cannot make are included
(visibility keywords, marked fall-throughs, template file-header order),
private members lose their underscore prefix, and a few classes take the
names their files imply. JavaScript and CSS are untouched. A bare
core/vendor/bin/phpcs from the repository root reports nothing.