refactor!: modernize Article, Category and StructureElement#6531
Merged
Conversation
Same modernization pattern applied as in the language and mediapool PRs:
- typed promoted readonly properties on `StructureElement` (camelCase),
`additionalData` array carries the remaining cache fields (meta-info etc).
- subclass-specific properties (`Article::$categoryId`/`$templateKey`/
`$startArticle`, `Category::$parentId`) sit on the subclass and are
populated by its own `private __construct(array $data)` after the
parent constructor is invoked — keeps `readonly` while avoiding the
forwarding of every parent argument.
- abstract `fromCache(array): ?static` factory shared via
`StructureElement::get`; `Category::fromCache` returns null for
non-start-article rows (former type guard inside `get` is gone).
- new `require(int $id, ?int $clang = null): static` next to `get()`.
- `path` is now `list<int>` (mirroring `MediaCategory`), `getPath()` /
`getPathAsArray()` removed; `getValue('path')` still returns the
pipe-string for BC.
- `getParent` only lives on `Category` publicly; `Article` has a
protected `getParent` returning the containing or parent category
(used internally by `getClosest`/`getClosestValue`/
`isOnlineIncludingParents`).
- `isStartArticle`/`isSiteStartArticle`/`isNotFoundArticle`/`hasTemplate`
moved out of the base class — they are article concepts.
`hasTemplate` removed entirely; check `null !== $article->templateKey`.
- `metaInfoPrefix` is an abstract get-only property on the base.
- `Article` no longer carries a `parentId`; it has `categoryId` (nullable).
- removed dead `last_update_stamp` cache column and the
`StructureElement::$classVars` / `getClassVars` / `resetClassVars`
machinery that only existed for the old dynamic-property design.
- rector rules cover the visible signature changes
(`getTemplateKey -> templateKey`, `getCategoryId -> categoryId`,
`getPath/getPathAsArray -> path`, `hasTemplate -> templateKey`,
StructureElement getters -> properties, etc.).
- `StructurePermission::hasCategoryPerm` accepts `?int` now (mirrors
the nullable `Article::$categoryId`).
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.
Summary
Applies the same modernization pattern as the language (#6521) and mediapool (#6516) PRs to the structure-element classes.
StructureElement(abstract base)parentId→ removed from base,clangId,createDate, …)additionalDataarray carries the remaining cache fields (meta-info etc.)require(int $id, ?int $clang = null): staticnext toget(), analogous toLanguage::require()fromCache(array): ?staticfactory dispatched fromStructureElement::get$pathis nowlist<int>(mirroringMediaCategory);getPath()/getPathAsArray()removed,getValue('path')still returns the pipe-string for BCmetaInfoPrefixis an abstract get-only property on the base (PHP 8.4 syntax)StructureElement::$classVars/getClassVars/resetClassVars(only existed to support the old dynamic-property design); the one caller (ArticleHandler::article2startarticle) uses the SQL field-name list directlylast_update_stampcache columnArticle(final)private __construct(array $data)parses the cache row, callsparent::__construct(...)once with named args, then sets its ownreadonlyproperties — no parent-arg duplication, single-phase initcategoryId(nullable),templateKey,startArticleas own propertiesparentId(semantically didn't make sense — see comment thread)getCategory()exposes the containing/own category;getParent()isprotectedand used internally for the tree walkisSiteStartArticle(),isNotFoundArticle(),isStartArticle()live here (article concepts)hasTemplate()removed — checknull !== $article->templateKeyinsteadCategory(final)private __construct(array $data)likeArticleparentId(nullable for root) sits heregetParent()ispublic(visibility widened fromprotectedbase)fromCache()returnsnullfor non-start-article cache rows (formerCategory::class === static::classguard inside the baseget()is gone)name/priority/template/startarticlefields — those are article-only; cross-type access viagetValuereturns nullRector rules
Cover the visible signature changes for downstream addons:
Article::getCategoryId→categoryId(changedint→?int)Article::getTemplateKey→templateKeyArticle::hasTemplate→templateKey(changedbool→?string, callers using the bool need manual adjustment)StructureElement::getPath→path(changedstring→list<int>)StructureElement::getPathAsArray→pathStructureElementgetters → properties (getId,getParentId,getClangId,getName,getPriority,getCreateDate/getUpdateDate/getCreateUser/getUpdateUser)Category::getName/getPriority→name/priority(both classes havename/priorityproperties — Article fromname/prioritycolumns, Category fromcatname/catpriority)Other
StructurePermission::hasCategoryPerm(?int)— accepts null (mirrors the now-nullableArticle::$categoryId)CategoryTreeRenderer::formatLi()removed — only one caller (ArticleList::listItem) and only ever invoked withArticle, deadCategorybranch eliminated; logic inlined inArticleListArticleTestextended withgetClosest/getClosestValue/isOnlineIncludingParentsdata-provider tests, mirroringCategoryTest