This feature didn't work and even if we fixed it as suggested
in the task, it would still be bogus. For deterministic paging,
the afh_user_text field should be in an index together with
another field(s). But currently it's indexed alone.
By the way, the indexes on abuse_filter_history should be fixed
anyway. Special:AbuseFilter/history also allows filtering by
filter/user which require index on the fields. They are present
but are not composite, so either the sorting is done
inefficiently without an index or there is a fullscan.
Also remove the getIndexField override. TablePager knows best
what value can be used there, we don't really have to override
it.
Bug: T204210
Change-Id: I7335f82c917a1d219fd7f0999da5b62433f14bd8
This was a means to bypass the limitation to filter by
triggered filter (for example, when a group contains
a single filter).
Change-Id: Icd7b0b64ff16b4ce26f4d52ad9d9abce62972e60
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
The array_filter is likely meant to empty the array if the empty string
was exploded ( `explode( "\n", '' ) === [ '' ]` ). However, it can also
remove other stuff, e.g. the string '0'. An explicit comparison is
easier to read & interpret, marginally faster, and avoids rare but not
impossible edge cases.
Change-Id: Ie77d65b56319664a2ac370f32341dc72b619a635
Previously, the cached value would depend on the tags
parameter to be updated. The provided value may be
different for each call, so callers may receive
unexpected values.
For example, while core usually calls this with core-defined
hooks, our method AbuseFilter::isAllowedTag calls this
providing an empty array. If core's call happened shortly
after ours and hit cache, its array would be overwritten
with only AbuseFilter's tags, the rest would be lost.
Also do some clean up:
- only call array_filter on explode'd array
- call array_unique on the value, since it's usual that
multiple filters share the same tag
Noticed when thinking about moving this to a service.
Change-Id: I4f4322e80ec89e48458a3bf46a1146863bec8237
af_actions and af_hidden are treated in the same way, so avoid
duplicating that code. Some of the remaining cases are also quite
similar (although not identical), so we might want to merge them in the
future.
Change-Id: I1b48502e077e58eb9ff459326bba18bb1d127242
The only exception is mwexamples-comparisons.t which intentionally
includes examples with = and == to test the "weak" version of the
comparison operator.
Bug: T262063
Change-Id: I6f92aadc69489da481a606bfda89617b8efbb261
Copying my investigation from I8c93e2ae7e7bd4fc561c5e8490ed2feb1ef0edc2:
This code was introduced in 2009, see rEABF0f1eb8db78bfa83ddb93427f39aad619523d8f25:
$display = wfMsg( "abusefilter-action-$action" );
$display = wfEmptyMsg( "abusefilter-action-$action", $display ) ? $action : $display;
And wfEmptyMsg looked like this:
function wfEmptyMsg( $msg, $wfMsgOut ) {
return $wfMsgOut === htmlspecialchars( "<$msg>" );
}
so this made sense. But then, in 2010 (rMWae3ced88e535c7fd046f0ad6f0710cc87f0004ea) the function was changed:
function wfEmptyMsg( $key ) {
global $wgMessageCache;
return $wgMessageCache->get( $key ) === false;
}
without anyone removing the parameter from AbuseFilter.
Finally, in 2012 (rEABF176227e721c9475de2c2163d3b6e20ca4769c406) the usage of wfEmptyMsg was removed, and $display became a parameter to wfMessage().
Long story short, no need to pass that parameter.
Change-Id: Iad875f0c0ab5aaa06c795232638f52e9ca62786e
Ideally, this might live in MediaWikiIntegrationTestCase. For the
createaccount one, AuthManager should also provide a method to log the
creation, because currently we are forced to copypaste that code here.
- Add the missing tests for 'upload' in RCVariableGenerator, and adjust
the existing ones (delete file afterwards, more tablesUsed, use the
right extension).
- Exclude from the coverage report a couple of lines which should
theoretically be unreachable. Escalate logging to WARN level, where it's
more likely to be spotted.
- Remove an unused method (RCVariableGenerator::newFromID). This denies
the need to maintain and cover it. We also don't want this generator
to act as a factory.
Overall, this change brings the coverage for RCVariableGenerator to 100%
Bug: T201193
Change-Id: I425c3d9f6800f74eb6e4eda483b90cfb3bbbcb51
This was also long overdue. Also fix a bug that caused page creations to
not be shown when examining past edits (using rc_last_oldid doesn't work
for page creations).
Bug: T201193
Bug: T262903
Change-Id: I5f7a994add12332c950904146248c5de7c2beee5
- 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
This is I4df27f3d02432c201c04d9fa118f0129b0a79778 striking again. Fool
me once, shame on thee, fool me twice...
Change-Id: Icea025a2c81e3b413b7bd9ece52866aeaf42937d
- Include an attempt to restore the dump in case the text table
contains a truncated dump (not 100% sure that this can really
happen, nor do I know the cause, but it shouldn't hurt)
- Remove a check for 'action'. The variable might be missing in case of
a corrupted dump. Having an array at that point can only mean "new
format".
- Don't assume that old_wikitext and new_wikitext are set when showing
past filter hits (again, might be unset due to data corruption).
Bug: T264513
Change-Id: I7510d28fc3f43f985a1283e23b413f07adfe7921
This is a simple change but with tons of benefits:
- Easier to track usages for IDEs
- Easier to understand in static analysis (phan)
- Can be analyzed by phpda
- Ensures no typos
- These classes can be namespaced without affecting readability here
Change-Id: Ic04d19dfbe9184baf2ef4bac53011521e2e44953
- 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
These have been saved in the parent class for quite some time.
Refactor accessors in method overrides.
Change-Id: I9819caa5ab87ac3a8e47efb32b00d89c3e2a61af
It's possible that we try clicking this button before it's ready. This
theory is hard to verify because I get no problem locally, but this
shouldn't hurt.
Also specialize input selectors for a couple elements that might not be
univocally determined within the page.
Change-Id: Ida65c3c5fd4d8b3b35ecbee7e99977c71c7c4b96
These scripts were already included in the updater (and hence executed)
several MW versions ago. There's no need to write tests for them right
now, so exclude these from coverage.
Change-Id: I43e46f06b98bb3b9b9d61a45baaf232e2a99c308
The current code was more of a subpar, temporary solution. However, we
need a stable solution in case more variables will be deprecated in the
future (T213006 fixes the problem for the past deprecation round). So,
instead of setting a hacky property, directly translate all variables
when loading the var dump. This is not only stable, but has a couple
micro-performance advantages:
- Calling getDeprecatedVariables happens only once when loading the
dump, and not every time a variable is accessed
- No checks are needed when retrieving a variable,
because names can always assumed to be new
Some simple benchmarks reveals a runtime reduction of 8-15% compared to
the old code (8% when it had varsVersion = 2, 15% for varsVersion = 1),
which comes at no cost together with increased readability and
stability. It ain't much, but it's honest work.
Change-Id: Ib32a92c4ad939790633aa63eb3ef8d4629488bea
The editing view is currently full of tech debt, brittle and surprising
code and whatnot. It's basically a miracle if it works without problem,
and it'd be an even bigger miracle if you could change something there
without breaking anything.
For these reasons, and because that class must be refactored as part of
the upcoming overhaul, this patch adds a bunch of selenium tests to test
the main functionality of that page.
In particular, these tests cover all possible cases (each corresponding
to a data source) for which buildFilterEditor can be called, which FTR are:
1 - View the result of importing a filter
2 - Create a new filter
3 - Load the current version of an existing filter
4 - Load an old version of an existing filter
5 - Show the user input again if saving fails after one of the steps
above
Having automated tests to cover these cases means that we don't have to
manually test all the scenarios manually each time the class is touched.
Bug: T201193
Change-Id: I408e0a132905416effe0d6d6dc0921991edd66bd
