In order for PagedMovedEvent to replace PageMoveCompleteHook implementations, it should carry article redirect creation data.
Redirects can be created as part of the move action, and are not currently captured by before/after page snapshot.
References
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| PageMovedEvent: add page redirect information. | mediawiki/core | master | +66 -2 |
I'd suggest adding the following method to PageMovedEvent: getRedirectPage(): ?PageRecord. If would return null if no redirect was created.
I'm not sure about the name, it could be more descriptive getNewRedirectPageCreatedAtOriginalLocation() is a bit long...
Is a ?PageRecord a better type than ?WikiPage? WikiPage is the type carried over from MovePage::moveToInternal.
Naming wise, what do you think of
No strong opinions on my end though. I don't have a good feeling for MW core conventions, so I'll take your advice here!
Is a ?PageRecord a better type than ?WikiPage? WikiPage is the type carried over from MovePage::moveToInternal.
Yes. WikiPage is a stateful monster object, it should be avoided if possible. It's not safe to serialize either. But WikiPage implements the Pagerecord interface, so you can just use the WikiPage object you have in MovePage. Best call $redirectArticle->toPageRecord() to get an immutable object, to avoid surprises caused by the object's state chaning before listeners get invoked (unlikely in this case, but as a general rule, events should only contain immutable objects).
getRedirectArticle(): ?WikiPage (follows the convention for local vars in MovePage.
"Article" and "page" have been used interchangeably in the code base in the past, causing confusion. We have been phasing out the use of "article" in favor of "page". I'd strive for consistency with public interfaces. That reminds me - perhaps the key in the Status object shouldn't use "Article" either.
having a createdRedirect(): bool that tests whether a redirect at the old title was left.
Sounds good, if a bit redundantl. I'd probably go for wasredirectCreated, because boolean getters generally start with "is" or "was".
@daniel Thanks for clarifying. +1 for returning an immutable object. I'll update my wip.
Change #1145851 had a related patch set uploaded (by Gmodena; author: Gmodena):
[mediawiki/core@master] PageMovedEvent: add page redirect information.
Change #1145851 merged by jenkins-bot:
[mediawiki/core@master] PageMovedEvent: add page redirect information.