Page MenuHomePhabricator

Watchlist Expiry: Add expiry type to ParamValidator [medium]
Closed, ResolvedPublic

Description

As a Watchlist Expiry user, I want an expiry type added to ParamValidator, so that we can keep parsing & validation in one place.

Background: See T248407 for context. We now have many APIs that accept an expiry, so we should introduce a new parameter type for this type of value to keep parsing and validation in one place. The watchlist expiry is slightly different than some others in that a blank value means "don't change", not "indefinite". Our refactoring should keep in this mind, as other APIs unrelated to watchlisting (such as the userrights API) treat blank/null expiries differently.

Resources:

Acceptance Criteria:

  • Add expiry type to ParamValidator

Event Timeline

Restricted Application added a project: Platform Engineering. · View Herald TranscriptMar 25 2020, 8:09 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Anomie added a subscriber: Anomie.Mar 25 2020, 9:08 PM

The watchlist expiry is slightly different than some others in that a blank value means "don't change", not "indefinite". Our refactoring should keep in this mind as other APIs unrelated to watchlisting (such as the userrights API) treat blank/null expiries differently.

That probably doesn't matter for this task. A TypeDef for parsing expiries should only concern itself with parsing actual user input, and the empty string should probably be treated as an invalid expiry. "No input" isn't handled by the TypeDef, it just results in the API module receiving null.

The watchlist expiries would handle receiving that null as "no change". Something that wanted to interpret it as "indefinite" would probably just set PARAM_DEFAULT => 'indefinite' so it would never receive the null in the first place. Something that wanted to force the user to always explicitly specify the expiry would set PARAM_REQUIRED => true.

ifried renamed this task from Add expiry type to ParamValidator to Watchlist Expiry: Add expiry type to ParamValidator.Mar 26 2020, 4:24 PM
ifried updated the task description. (Show Details)
ifried updated the task description. (Show Details)Mar 26 2020, 4:27 PM
ARamirez_WMF renamed this task from Watchlist Expiry: Add expiry type to ParamValidator to Watchlist Expiry: Add expiry type to ParamValidator [medium].Mar 26 2020, 5:32 PM
ARamirez_WMF moved this task from To Be Estimated/Discussed to Estimated on the Community-Tech board.

Change 585297 had a related patch set uploaded (by MusikAnimal; owner: MusikAnimal):
[mediawiki/core@master] [WIP] Add expiry type to ParamValidator

https://gerrit.wikimedia.org/r/585297

Change 585297 merged by jenkins-bot:
[mediawiki/core@master] Add expiry type to ParamValidator

https://gerrit.wikimedia.org/r/585297

dom_walden added a subscriber: dom_walden.

I briefly tested the validation of the expiry parameter for API:Watch and Special:ApiSandbox.

For regression, because we modified the code for wfIsInfinity(), I tested the expiry parameter of Special:Block.

ifried closed this task as Resolved.May 6 2020, 4:51 PM
ifried moved this task from Product sign-off to Done on the Community-Tech (Kanban-2019-20-Q4) board.
ifried added a subscriber: ifried.

There is nothing user-facing to test with this ticket, and Dom has done some general tests. For this reason, I'm marking this work as Done.

Aklapper removed a subscriber: Anomie.Fri, Oct 16, 5:38 PM