Introduce shorter methods, one for each steps, so that it's easier to
understand what the code is doing and figure out if the order makes
sense. The ConsequencesExecutor test is now a proper unit test. Also
simplify AbuseFilterConsequencesTest, removing old/wrong logic and
fixing two expected values that were actually wrong (but worked because
of the aforementioned wrong logic).
The only functional changes should be:
- We pick the longest block *after* checking the ConsequenceDisabler
consequences, so e.g. if a filter has a long block + warn and another
filter has a shorter block, we still keep the second one if warn will
disable the block.
- Remove disallow in presence of dangerous actions after checking
ConsequenceDisabler's and deduplicating blocks. Otherwise we may
remove disallow for filters where block (etc.) doesn't end up being
disabled. We may also want to consider not removing disallow at all,
now that messages are customizable.
Bug: T303059
Change-Id: If00adbf2056758222eaaea70b16d3b4f89502c20
- Use a /64 range for IPv6 instead of /16.
- Fix a curious and serious bug for IPv6, where grouping by range
would only use the first (!) number of the IP address, due to the
'v6-' prefix returned by IP::toHex.
- Fail hard if the identifier is unknown -- it's not something that's
supposed to happen.
- Include the type name in each identifier, instead of prefixing all
type names to all identifiers. This makes it easier to understand the
parts of the key.
- Test the whole lot.
Bug: T211101
Change-Id: I54c4209f2f0d5a4c5e7b81bed240ca3e28a2ded7
This is a plain value object that represents the action being filtered,
replacing associative arrays that were being used up to this point.
We should now check whether it's possible to make it not require an
accountname (which complicates things), and then use it in related
classes as well, e.g. Parameters.
Change-Id: I9550c14819b600c97c46b632cc1c2d447972d69c
UserEditTracker::getUserEditCount now allows anonymous users,
but it returns null and phan is aware of this. Suppress this
warning until at least 1.37 is required.
Change-Id: I9962abe08fa31d55421d8bdda23ea0a1c0471a86
The block log entry will be automatically suppressed, until we can
implement a better solution.
Bug: T152394
Change-Id: I8bae477ad7e4d0190335363ac2decf28e4313da1
Requires injecting a temporary block factory, and excluding
ManualLogEntry::insert from the test, but it's now much cleaner and
quicker.
It still cannot be a unit test due to the usage of User.
Change-Id: Iba9732d6d79733b31b45eb4d0187b1c8a82499dc
This will not be correct if the target already has a partial block
applied (which is very rare BTW). Leaving a TODO because this is low
priority.
Also keep returning the status in tests, because it makes tests easier
to write.
Change-Id: Ifac795125927d584a31d95e1b4c4241eef860fa1
In particular, this brings stronger typing for getID(), and we can get
rid of many phan suppressions.
Change-Id: Icbf3a6f7db8105082646ec227f62c09449fb165d
This makes VariableHolder a true value object, and introduces a
stateless service, VariableManager, to operate on it.
Note, in theory, this new service is still cyclically coupled with
LazyVariableComputed. However, it's now two stateless service being
coupled, not two smart/god value objects, so we've still earned
something. For now, the dependency is hidden by using a callback. Some
alternatives for that are mentioned in a code comment.
Bug: T261069
Change-Id: I2f2c84c8e91472ba36084a8bbb4a923f6e04354b
I think either all or none should consider global filters.
Are there any backwards compatibility concerns?
Change-Id: I22b664e9752588edc195dc4e4f5369392f91ad23
Introduce ReversibleConsequence interface for Consequence classes
whose potentially destructive actions can be reverted using
Special:AbuseFilter/revert. This allows moving reverting logic from
AbuseFilterViewRevert to individual Consequence classes and testing.
Unfortunately, the code is definitely not very clean now.
Change-Id: I558da711f1645ccf64792c6102cf743827171320
