In retrospect, I rushed the previous patch: we really need a way to tell
which deletion an ID belong to (and whether it was scheduled). So make
both result getters return an array with known keys that can be used
programmatically. Right now the class can only delete a single page, and
thus there's a single constant and this change is effectively a noop.
The deletionWasScheduled() method, introduced in 1.37, was
hard-deprecated out of an abundance of caution. There are no known uses
on codesearch, so it can probably just be removed in the next release.
Also reorder constructor params to DeletePage -- BacklinkCacheFactory is
a service injected by the factory, hence it shouldn't be grouped
together with value objects injected by the caller.
Change-Id: I32679b7cacc638ec3e9dc5b8dfe9bcc794b22ecf
- Use DeletePage in FileDeleteForm instead of
WikiPage::doDeleteArticleReal
- Properly handle scheduled deletions in FileDeleteForm: previously, a
null status value could indicate a missing page OR a scheduled
deletion, but the code always assumed the first, and it would generate
a duplicated log entry. The API response would also not contain the
"delete-scheduled" message. This has been broken since the introduction
of scheduled deletion.
- In ApiDelete, for file deletions, check whether the status is OK not
good. The two might be equivalent, but this way it's more consistent.
- Add some documentation for the Status objects returned by file-related
methods. This is still incomplete, as there are many methods using
Status and none of them says what the status could be. In particular,
this means that for now we keep checking whether the status is OK
instead of good, even though it's unclear what could produce a
non-fatal error.
- In LocalFileDeleteBatch, avoid using a class property for the returned
status, as that's hard to follow. Instead, use a local variable and
pass it around when needed.
Bug: T288758
Change-Id: I22d60c05bdd4a3ea531e63dbb9e49efc36935137
Move all output-related code from Article to DeleteAction.
Article::doDelete is now deprecated, because there are some callers in
the wild, although I don't think any caller should need it.
Kill some ancient-PHP-style pass-by-refs that are useless and only make
the code more error-prone for both the caller and the callee.
Bug: T288282
Change-Id: Ic1de0ed8ebba15da5ed9f5cd11625017360a7672
This is where the UI logic is supposed to live. The remaining methods in
FileDeleteForm will later be deprecated in favour of a dedicated command
object.
This patch tries to move the code to the new class with minimal changes,
with cleanup left to do for later.
Also change return typehints and docs for getFile(): WikiFilePage::loadFile()
is guaranteed to load a file. The "|bool" would confuse PHPStorm and
phan.
Note that FileDeleteForm is unused everywhere, except for some static
methods not touched by this patch.
Bug: T288282
Change-Id: I4fa1dfa096cd2ae7a58067764b12a275b1cce1d5
FileDeleteForm is a UI class, and it already uses injected OutputPage
as ContextSource, so just inject ContextSource properly.
Manual tests done:
- Delete old version of a file
- Delete current version of a file
- Try to delete with no permissions.
Change-Id: I3b1f0573ea9ccd5aa9a36d13bd2fc76a5546c426
These are styles for actions that do not have a dedicated style module.
Given that the amount of css it contains it marginal, creating a
dedicated module for each action would be overkill.
Bug: T278504
Change-Id: Id03c81e7d5ebf179731649aa230def2e8e21ac02
User::getBoolOption() is deprecated and should be replaced with UserOptionsLookup::getBoolOption()
Bug: T277600
Change-Id: Ife3c721237258d50852bbf764def74657cc70428
This is micro-optimization of closure code to avoid binding the closure
to $this where it is not needed.
Created by I25a17fb22b6b669e817317a0f45051ae9c608208
Change-Id: I0ffc6200f6c6693d78a3151cb8cea7dce7c21653
Class is not @newable or safe for creation, so breaking changes
can be made to the constructor.
Bug: T252979
Change-Id: Ia16a0d3eaabbc450dfd779b71ce1cc28f8d4f3b4
Class isn't safe to create (@newable) and not providing a User is
also explicitly hard deprecated.
Bug: T245581
Change-Id: I9859cbfb21e8fbe012980a381e7cde28b8837bc6
Reasons for oversighter should only used by them,
so allow them to be added to the end of the list if the user is allowed.
There is no extra separator between both lists,
because the reason list can be structured by */**
Done for page deletion and file deletion
Similar to I7cb8bc17a297285da83b3b4c664b634fe0db660c
Bug: T250631
Change-Id: I854a390c9abf8bb78fec520cd59b41ba6b7513b5
Deprecating something means to say something nasty about it, or to draw
its character into question. For example, "this function is lazy and good
for nothing". Deprecatory remarks by a developer are generally taken as a
warning that violence will soon be done against the function in question.
Other developers are thus warned to avoid associating with the deprecated
function.
However, since wfDeprecated() was introduced, it has become obvious that
the targets of deprecation are not limited to functions. Developers can
deprecate literally anything: a parameter, a return value, a file
format, Mondays, the concept of being, etc. wfDeprecated() requires
every deprecatory statement to begin with "use of", leading to some
awkward sentences. For example, one might say: "Use of your mouth to
cough without it being covered by your arm is deprecated since 2020."
So, introduce wfDeprecatedMsg(), which allows deprecation messages to be
specified in plain text, with the caller description being optionally
appended. Migrate incorrect or gramatically awkward uses of wfDeprecated()
to wfDeprecatedMsg().
Change-Id: Ib3dd2fe37677d98425d0f3692db5c9e988943ae8
Migrate all callers of Hooks::run() to use the new
HookContainer/HookRunner system.
General principles:
* Use DI if it is already used. We're not changing the way state is
managed in this patch.
* HookContainer is always injected, not HookRunner. HookContainer
is a service, it's a more generic interface, it is the only
thing that provides isRegistered() which is needed in some cases,
and a HookRunner can be efficiently constructed from it
(confirmed by benchmark). Because HookContainer is needed
for object construction, it is also needed by all factories.
* "Ask your friendly local base class". Big hierarchies like
SpecialPage and ApiBase have getHookContainer() and getHookRunner()
methods in the base class, and classes that extend that base class
are not expected to know or care where the base class gets its
HookContainer from.
* ProtectedHookAccessorTrait provides protected getHookContainer() and
getHookRunner() methods, getting them from the global service
container. The point of this is to ease migration to DI by ensuring
that call sites ask their local friendly base class rather than
getting a HookRunner from the service container directly.
* Private $this->hookRunner. In some smaller classes where accessor
methods did not seem warranted, there is a private HookRunner property
which is accessed directly. Very rarely (two cases), there is a
protected property, for consistency with code that conventionally
assumes protected=private, but in cases where the class might actually
be overridden, a protected accessor is preferred over a protected
property.
* The last resort: Hooks::runner(). Mostly for static, file-scope and
global code. In a few cases it was used for objects with broken
construction schemes, out of horror or laziness.
Constructors with new required arguments:
* AuthManager
* BadFileLookup
* BlockManager
* ClassicInterwikiLookup
* ContentHandlerFactory
* ContentSecurityPolicy
* DefaultOptionsManager
* DerivedPageDataUpdater
* FullSearchResultWidget
* HtmlCacheUpdater
* LanguageFactory
* LanguageNameUtils
* LinkRenderer
* LinkRendererFactory
* LocalisationCache
* MagicWordFactory
* MessageCache
* NamespaceInfo
* PageEditStash
* PageHandlerFactory
* PageUpdater
* ParserFactory
* PermissionManager
* RevisionStore
* RevisionStoreFactory
* SearchEngineConfig
* SearchEngineFactory
* SearchFormWidget
* SearchNearMatcher
* SessionBackend
* SpecialPageFactory
* UserNameUtils
* UserOptionsManager
* WatchedItemQueryService
* WatchedItemStore
Constructors with new optional arguments:
* DefaultPreferencesFactory
* Language
* LinkHolderArray
* MovePage
* Parser
* ParserCache
* PasswordReset
* Router
setHookContainer() now required after construction:
* AuthenticationProvider
* ResourceLoaderModule
* SearchEngine
Change-Id: Id442b0dbe43aba84bd5cf801d86dedc768b082c7
Move the user parameter to be before the other optional parameters,
and hard deprecate not passing a user. Interim signature is fully
backwards compatible, emitting deprecation noticed when a user is
not passed but otherwise accepting parameters in the old order as well
as the old order, including handling of defaults.
File::delete is a soft deprecated wrapper for the new File::deleteFile,
and File::deleteOld is a hard deprecated wrapper for the new
File::deleteOldFile. Both of the former methods accepted optional user
parameters; both of the new methods require a user, so that the call to
construct a LocalFileDeleteBatch can pass a user.
Bug: T245710
Change-Id: I9cde7831e16a719c22f093f95248b8055d9ac6fd
There is only 1 call for creating a new FileDeleteForm, and it is
also updated; not passing a user is hard deprecated.
Also removes use of Title::getUserPermissionsErrors
Bug: T244929
Bug: T245233
Change-Id: Iaa3002ccc91f795bb1c6728163867e4b7a98a261
Deletion is only possible for files on the local wiki, so this is always
a LocalFile. The static self::doDelete already requires a LocalFile.
Caught by PhanTypeMismatchArgument, to be enabled with I34d65fe3ff191.
Change-Id: Iee0774340208b493b075085485343e05f922751c
$tags can be `null` as seen in most cases and this causes failures,
so, the safer path is to make sure if $tags is a null, do nothing.
Change-Id: I5b7e39adba5d08fdcd42c437a72a391be98c8695
This removes most of the pre-CommentStore text columns, and the
$wgCommentTableSchemaMigrationStage setting that used to determine
whether the columns were used.
rev_comment remains in the code, as on Wikimedia wikis the revision
table is too large to alter at this time. A future change will combine
that with the removal of rev_user_text, rev_content_model, and
rev_content_format (and the addition of rev_comment_id and rev_actor).
CommentStore's constructor continues to take a $stage parameter, and
continues to have the logic for handling it, for the benefit of
extensions that might need their own migration process.
Bug: T166733
Change-Id: I1479c73774e01ead1490adf6128f820c09bce9d4
This ensures that broken messages can't break the <div> wrapper and
that the output is tidy.
Bug: T205624
Change-Id: I2511adf593a13528e205a82d9fcdc8a524d0a95f
