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
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
Follow-up Ie9aae938cca06e38a7a834a3f74f3e8735ab01ee.
Some fields are actually necessary when the filter isn't saved. This
would cause PHP notices when showing the editor again.
Change-Id: I2b9e0f04b3e8ad4eea8e334e16ee422bb40f0eb5
Follow-up Iabd0ae5b18571f8cad44ef2d86bcf2519e7f95ba.
This patch:
- Moves some save-related code to a separate method
- Reduces conditionals nesting
- Fixes an edge case where the content of the form would be
wiped in case the token didn't match.
- Adds another (basic) selenium test
- Standardizes return types
- Moves data load outside of buildFilterEditor
Change-Id: I89444b59f04c495c9ab59244151c8ed5d38cf0fe
This is another step needed to reduce the size of the gigantic
AbuseFilter and AbuseFilterHooks classes. It also makes many methods
non-static, for more testability.
Note, this layout is still not final. We should somehow merge the
functionality of VariableGenerator and AFComputedVariable, for which
I already have plans.
Change-Id: I366d598b69ad866496b7cb0059e0835c02e54041
RunVariableGenerator is for generating variables based on the current
action;
RowVariableGenerator is for RC entries;
VariableGenerator is the generic one.
This patch only moves the methods to the new classes, to keep the diff
easier to read, and facilitate conflict resolution. These classes will
then be revamped in I366d598b69ad866496b7cb0059e0835c02e54041.
Note that these classes are now namespaced.
One method, AbuseFilter::getEditVars, was renamed to
AbuseFilterVariableGenerator::generateEditVars, because it would
otherwise conflict with an incompatible method in RunVariableGenerator.
Change-Id: Iff412e5492873d4fae55402939a51609e64d55a8
Most of them are overwritten either in ViewEdit::loadRequest or
AbuseFilter::saveFilter. af_hit_count and af_throttled are actually
relevant for the old version, so list them explicitly. And also add
default af_group and af_global, which are later read, for import action.
Depends-On: Iabd0ae5b18571f8cad44ef2d86bcf2519e7f95ba
Change-Id: Ie9aae938cca06e38a7a834a3f74f3e8735ab01ee
Instead of having a single loadRequest method (which could end up
loading from the DB...), split it in a DB-only method and a request-only
one. Simplify the logic used to show the filter editor. Show the page
without changes or warnings if the user lost editing rights in the
meanwhile. Avoid two static properties, and pass them in when relevant
instead. Bonus: optimize a query to sort by afh_id instead of afh_timestamp to avoid filesort.
This will allow a subsequent patch to clean the $row object in
loadRequest.
Change-Id: Iabd0ae5b18571f8cad44ef2d86bcf2519e7f95ba
composer:
* mediawiki/mediawiki-codesniffer: 28.0.0 → 29.0.0
The following sniffs are failing and were disabled:
* MediaWiki.Commenting.FunctionComment.MissingParamTag
* MediaWiki.Commenting.FunctionComment.ParamNameNoMatch
npm:
* eslint-config-wikimedia: 0.13.1 → 0.15.0
* grunt-stylelint: 0.11.1 → 0.13.0
* stylelint-config-wikimedia: 0.6.0 → 0.8.0
Additional changes:
* Remove direct "stylelint" dependency in favor of "grunt-stylelint".
* Also sorted "composer fix" command to run phpcbf last.
* Removing manual reportUnusedDisableDirectives for eslint.
Change-Id: I8f73202db1333fbc36ccf556b3bb05b1e8c279cb
