We didn't check if the provided ID was valid. While editing an existing
filter (or creating a new one), we check the ID in SpecialAbuseFilter,
so it's guaranteed to get an integer in ViewEdit, and the case of a
non-existing filter is handled later, in buildFilterEditor.
But for links like Special:AbuseFilter/history/foobarbaz/item/1 (where
"foobarbaz" should be the filter ID), no validation was performed. This
caused a useless query to be carried out on the abuse_filter_history table (which would likely return false), then accessing properties of a non-object ('$row->afh_id'), and we ended up showing filter 1. This was spotted because we actually got notices in production.
Bug: T231632
Change-Id: I6436c7d2df8c1f0fc971f4a4079dac9118aa8209
Some of these are transformed into real unit tests, while the
AbuseFilterSaveTest class is refactored to avoid using the DB and to use
a lot more of mocks and DI.
Depends-On: I22743557e162fd23b3b4e52951a649d8c21109c8
Change-Id: Id8412e2b8a4e873fd4821ecc1a3c95710be9a870
There are lots of calls to $user->isAllowed which could be simplified
using available accessors like canEdit(). So simplify those calls and
avoid duplication.
Note that using canEdit also fixes a bug which affected blocked users:
we used to show e.g. the import link, and not to display as disabled
several text fields, while blocked users cannot actually edit filters.
Depends-On: I22743557e162fd23b3b4e52951a649d8c21109c8
Change-Id: I62779e940949ef49018a9c6d901bb6e10aa81da8
There are lots of cases where we can inject a User object without
additional efforts. Now $wgUser is only used inside AFComputedVariable,
which is a little bit harder to handle because some instances of that
class are serialized in the DB, and thus we cannot easily change the
constructor until T213006 is resolved.
This partly copies what Ia474f02dfeee8c7d067ee7e555c08cbfef08f6a6 tried
to do, but adopting a different approach for various can*() methods:
they're now static methods in the AbuseFilter class, so future callers
don't need to instantiate an AbuseFilterView class. This also allows to
re-use those methods in an API module for editing filters (T213037).
Bug: T213037
Bug: T159299
Change-Id: I22743557e162fd23b3b4e52951a649d8c21109c8
After having removed the export link in
I72f46247f4323fb5bfe7fa74f332076dbd346187, we don't have any tool to
show for new filters. So avoid outputting an empty section.
Change-Id: Ia07bccdbadb7b874397135bc3f7468d6e0b9eb13
This was broken in I34c040dbeb3ab01158fb3db22496def6ccaf72d9. I thought
the members of that object were always arrays, but I was wrong.
Plus typehint a few array parameters and make a couple of methods
private since they're only used in this class.
Bug: T230639
Change-Id: I0c51359769c4b3054f95755a96e7e0a2d8e5bf15
Now it's always wider, and so is the "notes" field. Moreover, the
fallback textarea has the exact same size. Plus removed a parameter
which only made it hard to write a CSS rule for the textarea. Since the
textarea is generated by the same code, and we're always using it for
the same thing (filter syntax, regardless of the final goal), make it
always use the same name.
Bug: T230591
Change-Id: Ibb308e80d954c0e81aa09249c38c39572f157948
As data could be "old" and it may have no meaning.
Also remove a superfluous isset(), as $row->af_hidden is always set.
Depends-On: I2d8c8f8278073a9420e3eb373fb89a655925618a
Change-Id: I072363706c61f272c4c3691de4078e2a19148424
They're currently stored separately, so move matches count together with
other per-filter data to keep it consistent. This also removes a
parameter from filterMatchesKey, as it's not needed anymore.
Split from child patch by Dragons flight.
Bug: T53294
Depends-On: I8f47beb73cfc1b63c4b3c809fc6d65a1e66ee334
Change-Id: I2d8c8f8278073a9420e3eb373fb89a655925618a
This changes the buildFilterEditor function to be protected and to
behave consistently: so, instead of adding stuff to OutputPage inside it
and also returning other stuff to be added by the caller, the function
now adds everything itself.
Also, the message "you're editing an old version of the filter" is now
shown only if the user can see the filter.
Change-Id: I1f40af41c5de0f63aa6210a261928892da0b3f69
The current form is awkward. They're all like
[ actionname => [ 'action' => actionname, 'parameters' => params ] ]
This is greatly confusing since adds a nesting level, and just
duplicates the actionname information (also, we actually never retrieve
it from the internal array). Instead, change all of them to be
[ actionname => params ]
which is a lot shorter and clearer (and easier to handle).
A similar case is handled in I8134ecc41fbecdbed99faf406e9e3ca91b6123b9
(see PS 8..10).
Change-Id: I34c040dbeb3ab01158fb3db22496def6ccaf72d9
To make the switch to afl_filter_id and afl_global easier.
Bug: T227095
Depends-On: Ie550889495232b534c0f9aec31039cf21b2135b1
Change-Id: If557bad8f5c1a6d15e3556e4bfbd0330d7d49c59
This is fixing potential bugs where invalid strings with more than one
comma have silently been accepted.
Change-Id: Ib1e7d0c99973f243ef6faad6389bab688187c1cf
I find it obvious that a file called "AbuseFilterTokenizerTest" is a
"test for the AbuseFilterTokenizer class". A comment that is just
repeating this information is typicalls not helpful, but distracting
and a potential source of mistakes, e.g. when stuff is copy-pasted,
but the comment not adjusted.
Change-Id: I1d4cc06e9e5631955ff73bf675090cf9c33c9390
This variable was introduced to selectively enable profiling because
stats recording was bad for performance. Nowadays, stats are recorded in
a deferredupdate and don't harm performance anymore. Thus, this variable
can be removed and profiling be enabled by default.
Bug: T191039
Depends-On: Ib5fdeb75c1324f672b4ded39681f006fde34b4d1
Change-Id: Ia5c477edc8733bb1994cb6d01e1371ed496c8bcb
Since double-equals are evil. I left some of them in place where I
wasn't sure, but I may be changed some which were intended to be
doubles. It could be a good idea to delay merging this patch until we'll
have more code coverage.
Change-Id: I1721a3ba532d481e3ecf35f51099c1438b6b73b2
Follow-up of I982d67aa62a899916a26452aceb9646df8c31232. The help text
was meant to be localized, and I probably forgot to do so in the
mentioned patch.
Change-Id: If394b02819911f9c97519b5c972977c38e6d83fa
If "tag" option is selected and the form is submitted without adding any
tag, just show it blank instead of adding an empty tag to the topbar.
Separately validate the empty tag case (and added a test for it).
Bug: T203353
Depends-On: I3b2e763bd8835207dc5df1db43d3e1881e6961c3
Change-Id: I8884b739fd17fa2eace5aac8775d3524aa606f1f
Adding PHPdocs to every class members, in every file. This patch only
touches comments, and moved properties on their own lines. Note that
some of these properties would need to be moved, somehow changed, or
just removed (either because they're old, unused leftovers, or just
because we can move them to local scope), but I wanted to keep this
patch doc-only.
Change-Id: I9fe701445bea8f09d82783789ff1ec537ac6704b
So that they're easier to read, and because readonly is semantically
more appropriate.
Bug: T217143
Change-Id: I76be8e7fb1cf46efd0c03cde74344be6cb2a0902
Some ObjectCache:: methods are soft deprecated since 1.28. Remove them
now, since the replacement is easy.
Change-Id: I713781d5e98238a1c194e97b5faae488a8ac190d
Several people have reported throttle groups being hard to use, mostly
because the field doesn't have options with the usable groups. This is
because users can combine valid groups in many ways, and thus we don't
provide options. However, let's add an help link pointing to mw.org.
Change-Id: I982d67aa62a899916a26452aceb9646df8c31232
While editing filters, sometimes it happen that you make some change,
forget about it and then reload/close the page, and no warnings will be
issued. This patch makes use of the core module used for normal page
editing to display a warning if trying to leave a filter editing page
with any unsaved change (both to the filter pattern or other form
elements).
Change-Id: I78d79215565d5c82028b1a2a4276497ccbffdea2
Simplify some logic constructs, reduce the amount of return statements
inside methods, explicitly declare variables before using them, reduce
code duplication, add names to JS anonymous function to produce clearer
stack traces.
Change-Id: Ife4546a91c30d4c519d09a712ba56a2f33abe579
This reverts commit 1ed75b4ae0.
Fixed the one which caused errors, by making articleFromTitle
only use WikiPage, instead of silently mixing WikiPage and Article.
Note for reviewers: this patch is identical to the one which was
previously +2ed, which was mostly correct. To see the actual change,
diff AFComputedVariable with 1..current.
Change-Id: I6747eaed861af6c40a3b1610aebcc1174296e9ed
Long (sigh) explanation in T203587#4569698. Also, simplified the way
TagMultiselect are generated, this one and the one for change tags.
This new selector is back-compat both with the old textarea and the OOUI
checkboxMultiselect; actually, this one is //fully// compatible with the
old textarea.
Add validation for throttle parameters and unit tests for validation
(split from I976c95658cddb2585910b6f8a5f047aadc4e4d47).
Added a trim when retrieving throttle identifier to allow syntax like
'ip, user'.
Improved the message shown on history.
Re-added the maintenance script to clean DB.
As I wrote in the task, a review by two other people would be great, at
least for the maintenance script (it could potentially break the DB).
Bug: T203587
Bug: T203336
Bug: T203584
Bug: T203585
Depends-On: I3b2e763bd8835207dc5df1db43d3e1881e6961c3
Change-Id: I7831dbb0bab55807392ac1f7915d6cb0cb713593
The OutputPage::parse/parseInline() methods emit untidy output and
are often used with the wrong user interface/content language
selection. Replace with new methods added in 1.33 which are
tidy and consistent.
Bug: T198214
Depends-On: Ica79c2acbc542ef37f971c0be2582ae771a23bd0
Change-Id: Iec8071f4e50f169356e4f68ccb746c55f1606ea6
