@Prtksxna and I discussed the behavior if users try to watch beyond the 6 month maximum via hacky behavior. We prefer that, if the user tries to exceed the 6 month maximum for watched items, the items are automatically set to the permitted maximum temporary watch period (6 months) rather than permanent. This is because the user has explicitly chosen a temporary rather than permanent period, and we should therefore allow the maximum available. The user would then see the 6 months watched in the success message. We would also clarify this behavior in the documentation.

Technical notes from the engineering meeting:

• Keep WatchedItemStore::addWatchBatchForUser() return value as-is (boolean)
• Consumers of WatchedItemStore::addWatchBatchForUser() are responsible for disallowing expiries that exceed the maximum, using common logic in WatchedItem class or something.
• If WatchedItemStore::addWatchBatchForUser() gets an expiry that's too long, it will instead go with the maximum (6 months)
• API can show a warning, and watch for the maximum.

@DannyS712 Did you want to take this on? Let me know if anything doesn't make sense. I left some technical notes above (T249672#6057433)

Change 591203 had a related patch set uploaded (by MusikAnimal; owner: MusikAnimal):
[mediawiki/core@master] [WIP] Enforce a maximum watchlist expiry duration

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

Tagging Platform Team Workboards (Clinic Duty Team) as I'm told your help has been offered. If you're able to, we could use some review on https://gerrit.wikimedia.org/r/591203 given it is not a very well known nook of MediaWiki core (specifically the TypeDef stuff). Many thanks!

@Pchelolo just a polite poke that https://gerrit.wikimedia.org/r/591203 is ready for re-review! Many thanks :)

@Krinkle I had one question for you at https://gerrit.wikimedia.org/r/c/mediawiki/core/+/591203/8/includes/watcheditem/WatchedItemStore.php#956 whenever you get a chance. Much appreciated!

Change 591203 merged by jenkins-bot:
[mediawiki/core@master] WatchedItemStore: Enforce a maximum watchlist expiry duration

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

Patch merged! I didn't get that logging added yet. Let me know if you think we need it, Krinkle, and I'll make a follow-up patch. No opposition, I just didn't know where to log.

Notes for QA -- the action=watch is the only API that returns a warning when the expiry exceeds the limit (it will be added to other APIs in separate patches). However we're enforcing the maximum duration in the storage layer, so if you put 10 years as the expiry for any API, or trick the web UI to use 10 years, it should be saved as 6 months instead.

I tested the basic functionality with API:Watch and action=watch:

• If the expiry value is less than or equal to $wgWatchlistExpiryMaxDuration, page is watched for the expiry length
• If the expiry value is greater than $wgWatchlistExpiryMaxDuration, the page is watched for the value of the latter (defaults to 6 months)
• If the expiry is "infinite", the page is watched permanently
• If there is no expiry value, the page is watched permanently or the expiry value is unchanged (if the page is already being watched)
• If the value of $wgWatchlistExpiryMaxDuration is null, the expiry value can be any length of time

For API:Watch, if you exceed the maximum value it returns a helpful message of the form:
Given value "7 months" for parameter "expiry" exceeds the maximum of "6 months". Using maximum instead.

This isn't the case for action=watch, which returns the same success message it does now, and silently watches for the maximum amount of time.

Of course, you can extend the expiry period of a temporarily watched page beyond the max duration by watching it again before it expires.

For regression purposes, I set $wgWatchlistExpiry = false and tested some core functionality, including different ways of watching/unwatching pages.

You can set $wgWatchlistExpiryMaxDuration to lots of different types of values, including:

• relative values like "last Monday"
• absolute values like "3 June 2020" or even unix timestamps like '@1591094928'

If $wgWatchlistExpiryMaxDuration is an invalid value (such as an invalid date or something that isn't a string), the behaviour appears to be the same as when the value is null (i.e. no limit on expiry length).

When a user inputs an expiry value, if it is not a valid date in English, I believe it gets ignored.

For example, there is an error in the French translations of the expiry values for action=watch, which means any expiry value you put in gets ignored.

@AMooney, should this task be moved back to "In Progress"?

@Naike I wanted to check in to see why you may want to move this to "In Progress." As far as Community Tech is concerned, this work has been completed, but I'll wait to hear from you before marking it as done. Thanks!

As far as Community Tech is concerned, this work has been completed...

@ifried - According to dom, it sounds like the API error part either hasn't been done or doesn't work. From the task description: "When I attempt to set an expiry that exceeds this, either through the API or elsewhere, it should show an error message."

In T249672#6215712, @kaldari wrote:

As far as Community Tech is concerned, this work has been completed...

@ifried - According to dom, it sounds like the API error part either hasn't been done or doesn't work. From the task description: "When I attempt to set an expiry that exceeds this, either through the API or elsewhere, it should show an error message."

Oh, sorry, I think I see the confusion. I didn't choose my words very carefully :). When I said action=watch I was referring to the UI (e.g. https://en.wikipedia.beta.wmflabs.org/wiki/Gaspar_de_Santa_Coloma?action=watch). I forgot that action=watch could also refer to the API.

The error message for API has been done. If you set an expiry that exceeds the max, it currently shows a message like: Given value "7 months" for parameter "expiry" exceeds the maximum of "6 months". Using maximum instead.

However, there is no such error message for the UI, which MusikAnimal points out in T249672#6171005. I assume it will be done in another ticket.

@dom_walden - Ah thanks for clarifying that!