Make FilterProfiler::getFilterProfile return stats unchanged,
in a structured way. Move computations to AbuseFilterViewEdit,
as they are only useful there. Don't return false on cache
misses, return arrays with zero values instead.
Bug: T266531
Change-Id: I8718cc31a5004340bf742315c7075e10a61fcbfd
This deals with data inconsistencies in buildFilterEditor. Every
property of $row was tested in all 5 scenarios (also using Selenium) to
check when it's set. The result is in the normalizeRow method, which
aims to remove any inconsistencies, so that buildFilterEditor always
receives a "complete" row with all defaults set.
The code in buildFilterEditor is now cleaner (because there are no
isset() checks), and it gives us a unique place where we can set
defaults (rather than partly doing that in
loadRequest/loadFilterData/loadImport, and partly relying on isset).
This will be especially useful when introducing value objects to
represent filters, because now you just have to look at normalizeRow()
to tell which properties are allowed to be missing, and thus what "kind"
of filter object you need (see
I5f33227887c035e301313bbe24d1c1fefb75bc6a).
Additionally, reduce the properties that get passed around during
export/import, and make the selenium test try a roundtrip, rather than
relying on hardcoded data that may get outdated. A future patch will
refactor the import/export code.
Change-Id: Id52c466baaf6da18e2981f27a81ffdad3a509e78
The previous code would call getUserGroups again once creating the log
entry, but this was slightly flawed: we're updating groups on master,
but the read happens on a replica that might be outdated, hence
resulting in broken logging. Instead of reading from master, we can just
keep a list of the groups that were actually added, and use that
afterwards.
Change-Id: I7cc282e15561de3a3d3e183808a65991aa27d2bb
This service is responsible for the blockautopromote feature:
(un)block autopromotion and check status.
The patch mostly moves code from static methods to the new class
and relaxes type hints (e.g. from User to UserIdentity).
Change-Id: I79a72377881cf06717931cd09af12f3b8e5f3e3f
- Add a helper method to output an unrecoverable error, comprising a
button to go back to the filters list;
- Move the token check to attemptSave, so to make the conditionals
easier to read, and group errors together
- Make buildFilterEditor take an HTML parameter for the error, so the
caller can specify whether it's error or warning
- Move the check for non-existing filters out of buildFilterEditor
- Add a bunch of typehints
- Don't set af_throttled and af_hit_count in the empty row template, but
set af_deleted (these are only used in buildFilterEditor)
- Make AbuseFilter::translateFromHistory consistently include the af_global
property (previously it would only be set for global filters; this error
was introduced when first implementing global filters)
- The only user-facing change is that, when trying to use a custom
warning/disallow message on a global filter, this is now considered a
non-fatal error, so we now show the editing interface (and not just an
unrecoverable error).
The next step is resolving the @todo in buildFilterEditor about null
checks.
Change-Id: I9d217dcac3f4cc0b26e53eca735cc327d5efc76d
This commit avoids direct queries on the DB, which is already an
improvement. It also adds some TODO comments for future improvements,
mostly things that depend on core changes.
Bug: T265224
Change-Id: I8eb76a0c463751976c2c5deedb3570305f1ab4f0
So that sysadmins can further customize the extension. It was also wrong
to use the same variable for many different things.
Note that there's no associated patch in wmf-config because we use the
defaults. However, before merging this patch, please recheck that
AbuseFilterRestrictions and AbuseFilterDisallowGlobalLocalBlocks aren't
used there (https://codesearch.wmflabs.org/operations/?q=AbuseFilterDisallowGlobalLocalBlocks%7CAbuseFilterRestrictions&i=nope&files=&repos=)
Bug: T175221
Change-Id: I7581b3ee6d9d11a6cf1599b8ff874e8c3d54adf4
This patch addresses two issues observed in WMF production:
- Specifying a search mode without a search pattern would result in a
call to mb_stripos (in AbuseFilterPager) with an empty delimiter,
which triggers a PHP warning. Avoid this by checking that the search
pattern is not the empty string, and unset the search mode if that's
the case.
- Trying to use an invalid search mode would result in an unhandled
LogicException. We have some code in place to check the validity of
the URL parameter, but the relevant code didn't reset the search mode
to null, hence AbuseFilterPager would throw before we can show a
pretty error to the user.
Bug: T265994
Change-Id: Ib19d36d6265981097bbb551783fdac8bdaa98854
This method was divided into multiple, shorter methods. We now have a
dedicated method for imports, and one for everything else, plus a method
for loading actions. Merged a conditional for when the token didn't
match. Avoid returning Status objects with data inside as it's too
difficult to properly infer types for those.
This is still not perfect, and another round of simplification might be
necessary before this class can be updated to use the upcoming Filter
value objects.
Change-Id: I2de1de1982105e5b9b817a893c357615ffb7db86
While this might seem a small change, it removes the last remaining
coupling between SpecialAbuseFilter and the *View classes, that were
forming a huge tangle.
Change-Id: I5a9d6516e3fa2d3efc4bb2e19b05379dc33cd84d
- Depend on a generic IContextSource rather than SpecialAbuseFilter
(lower coupling);
- Inject a LinkRenderer (IContextSource doesn't have a ::getLinkRenderer
method)
- Add a helper method in SpecialAbuseFilter to get the page title, that
can also be used elsewhere (and the name constant can be made private
now)
- Pull down the mFilter property (and rename it to just 'filter') to
classes that actually need it. Some classes didn't need this at all
and the types were different among subclasses
Now the only cause of coupling between the View classes and
SpecialAbuseFilter is the static call in getTitle.
Change-Id: I3df0c3a7621f0cc9a64a16b0a402a15aae2d5d73
This service should act as a mediator between the AF code and the
permission manager, and it should know what are the permissions required
by each action.
Change-Id: Ieb177d9992147b11fa7b8f05929da6c182cc2286
In particular, the interface shouldn't generate links to
"Special:AbuseFilter/history/0" (AbuseFilterHistoryPager::getTitle,
can be seen when visiting "Special:AbuseFilter/history").
Change-Id: Id3dc1bb4fc3c5e853603bf0ec04a6b1751f7d862
PHP is not strongly typed, so it's not a good idea to use scalars of
different types (here it's an integer vs the string 'new') to represent
different possibilities. This can have bad effects when type juggling
occurs, and it's also harder to figure out what the type of the
parameter can be (because a numeric ID might have been passed as a
string). Using integer vs null avoids all of this, and also allows us to
use nullable typehints.
These changes were partly copied from
If981cb35bf19a8469aa6c43c907e107cf8c65bc2 and should help with the
migration to the Filter value objects.
Change-Id: I8837d46c3c33761fea53f67b530b721dc7bd49b0
This patch removes the dependency of saveFilter on the ContextSource
kitchen sink. It also removes some unneded dependency, and adds
$originalRow/$originalActions as parameter, rather than hacky properties
in $newRow that are easy to forget. The related test can also be greatly
simplified.
This also introduces a behaviour change: checking $newRow instead of the Request allows us
to account for values normalization done in
AbuseFilterViewEdit::loadRequest, and to also work correctly for imports
(and generally speaking, it makes the method suitable for an
AbuseFilterEdit API module, too).
Next step is moving this method to a service. Some signatures,
indenting, name choices etc. are subpar, but this is just because these
methods are temporary anyway.
Bug: T213037
Change-Id: I235b928d7b9c2ef1c46ea0bf3e3ed212500b4161
- Make a separate method which determines the view
to be shown from subpage syntax and test it.
- Reduce circular dependency between SpecialAbuseFilter
and AbuseFilterView. Use params to transfer information
to views.
Change-Id: Ib9442ea5f9990a5c48f9b9e04055aa22bf7e456e
- Use null instead of empty strings
- Check the mode, and not the pattern, to decide whether the user
searched for something
- The call to parent::__construct can now be moved up
- Note in a comment how this code is problematic due to "smart"
constructors
- Avoid caching the headers, as that's not going to work anymore.
Change-Id: I420ab0215d53354a67d9d130ebd8d85dfbd2778b
When the user selects to see global rules and it's a remote wiki, hide the rule search field. (Note that the list of search modes needs to listen to this setting as well.)
This was discussed during reviewing I0771fa048.
Also move local/global filters setting to the top as it's more important than that for disabled and deleted filters (which will both stay together).
Change-Id: I0912aa1f5d7a5d75e6ae5a2a3362b8d38260c611
This will decouple a bit the huge and chaotic tangle of AF classes. Some
boilerplate code for AbuseFilter services is also added with this patch.
Note that this requires injecting a KeywordsManager in
AbuseFilterVariableHolder, or unit tests would fail. This is still
incomplete, and the Manager is only injected in tests, because
VariableHolder still has to be refactored.
The test for the UpdateVarDumps script had to be updated, because
serializing VHs in there was a bad choice. As pointed out in a comment,
the test is likely going to break again once we remove the BC code, but
I hope that we'll be able to remove the test at that point.
Change-Id: I12a656a310adb8c5f75cab63f6db9e121e109717
These methods had no reals reason to be static and belong to the
AbuseFilter class. Most of them were moved to Parser class as common
variations of the existing entry points. One was specific to the
EvalExpression API module and was moved there.
This change comes at no cost, and will make it possible to inject a
parser where needed.
Change-Id: Ifd169cfc99df8a5eb4ca94ac330f301ca28a2442
The <pre> element is now hidden with CSS, and is only shown after the
user clicks the "Eval" button.
Moreover, make the button primary and progressive, as to indicate that
it activates the primary function of that page.
Bug: T253492
Change-Id: I300ce6ec0a84ea73025a5af9173024df7c291e03
We have many topnav links, and future patches may add others (e.g.
Ia5fd4f0b35fcabf045a7b49fa40fa85b72c92544). The "import" feature is
probably the less used, and is also pretty similar to creating a new
filter.
Thus, remove its link from the topbar and move it to a button next to
the "Create a new filter" button.
Note that the old message is reusable, and thus it should be moved on
translatewiki after merge.
Change-Id: I52042d62b2bab7e4a1e9bbc027e7de5addec8157
When using RecentChange::getQueryInfo() it should be used to instance a
RecentChange class
RCDatabaseLogEntry is only useful in context of LogFormatter
This is a breaking change for the hook variable,
but "RC entry" refers more to the RecentChange class than to the
RCDatabaseLogEntry class
Change-Id: I3af1e42594f8235be815ce38e3411c762ae01092
Additional changes:
* Removed phan-taint-check-plugin from extra, now inherited from mediawiki-phan-config.
Change-Id: Ib63be75df4bfdbd2c5b97de5f80dbec715108c01
Using var_export for better visual effect, especially for arrays.
The result from /tools is much clearer and the 'wrong syntax' message is
a bit more explicative than before.
Bug: T190653
Bug: T239972
Change-Id: I79a17305c7f19f7900f896f895e9365bb5f2fd58
At the moment there's no validation for import data, so it's totally
possible to insert rubbish in the field, and the code will produce other
rubbish. For instance, it's not so uncommon to see lots of PHP notices
on logstash for ViewEdit code trying to access members of the imported
data as if it were an object.
Change-Id: If9d783f0f9242d3d1bc297572471e62f51ee0e40
