Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support for watchlist expiry #1090

Merged
merged 2 commits into from
Nov 28, 2020
Merged

Support for watchlist expiry #1090

merged 2 commits into from
Nov 28, 2020

Conversation

Amorymeltzer
Copy link
Collaborator

@Amorymeltzer Amorymeltzer commented Aug 7, 2020 •
edited
Loading

This is still pretty rough — I whipped it up some time last month — but wanted to put this up as a starting point, in case there's any discussion. Watchlist expiries in core can be tested at testWiki, but aren't enabled on enWiki (I'm not sure there's been any discussion about that process).

Basically, this:

  • Adds a few default options to watchlistEnums in twinkleconfig
  • Tweaks a few config options, even adding a few as necessary
  • Overhauls how morebits handles watchlists, namely by expanding setWatchlist and doing away with setWatchlistFromPreferences.

The major sticking points are:

  1. Does it work?
  2. Is any of this a good idea?
  3. Handling bespoke watchlist API actions (AFAICT action=watch has no way to default to prefs, so can either easily be mostly correct or rather convlutedly be correct (see twinklexfd)).

@MusikAnimal This is your baby as far as I see, so I'd love your thoughts/feedback on any/all of this at your leisure. I only played with it minimally after it first went live, so I may have missed some specifics in the implementation...

@Amorymeltzer Amorymeltzer added Feature request Module: morebits The morebits.js library Module: twinkle The twinkle.js global gadget file Module: config deprecations Functions, etc. from Morebits that are deprecated and should be checked for usage before removal labels Aug 7, 2020
@MusikAnimal
Copy link
Collaborator

I think it's a great idea! Gadgets like Twinkle is what we suspected would be a major consumer. I certainly have no need to watch someone's user talk indefinitely after issuing them a single warning, same with an XfD I created, but I would for a temporary period if I could. I think people will appreciate this functionality in Twinkle.

AFAICT action=watch has no way to default to prefs

The preferences are tied to specific actions such as delete/rollback/etc., so for action=watch there's no preference to fallback to.

I have not done much for code review, but as long as the "watchlist" and "watchlistexpiry" parameters are set it the APIs should accept it. Note also you can supply any arbitrary value accepted by PHP's strtotime, just like you can for block durations. E.g. "1 day", "next Thursday", or an exact timestamp. It may be worth it to provide a way to enter custom expiries, but I assume the pre-supplied options are enough for most people. The only caveat is $wgWatchlistExpiryMaxDuration, currently set to 6 months. This limit may be removed in the future.

Thanks for doing this work!

@Amorymeltzer
Copy link
Collaborator Author

Amorymeltzer commented Aug 11, 2020 •
edited
Loading

The preferences are tied to specific actions such as delete/rollback/etc., so for action=watch there's no preference to fallback to.

It'd be nice I suppose to supply an action to the watch API and have it use the default pref for that action. Pie in the sky! (ETA: T262912)

@MusikAnimal Do you know if there's a timeline or plan for rollout (to enwiki or elsewhere)? Just wondering if/when I'll need to start debugging, etc.

@MusikAnimal
Copy link
Collaborator

Do you know if there's a timeline or plan for rollout (to enwiki or elsewhere)?

I don't know that we have an exact timeline yet, but I expect it wouldn't get to any group 2 wikis for another month or so. There'll be a post at WP:VPT a week or two in advance.

@Amorymeltzer Amorymeltzer mentioned this pull request Sep 12, 2020
Merged
@jackmcbarn jackmcbarn mentioned this pull request Sep 15, 2020
Closed
@Amorymeltzer
Copy link
Collaborator Author

Amorymeltzer commented Sep 15, 2020 •
edited
Loading

Rebased, etc. Moved 3 commits into 2, so that each is logically and functionally sound. Only major change from above is to support unwatch, although we don't use it.

I should add that one other way of doing this would be to expand morebits' functions to take a second parameter, and use it if necessary. That'd simplify things here a little bit, but I think it's preferable to bring morebits more in line with how the API actually treats things (noting that things are still in flux).

@Amorymeltzer Amorymeltzer mentioned this pull request Sep 19, 2020
Merged
@Amorymeltzer Amorymeltzer changed the title WIP: Support for watchlist expiry Support for watchlist expiry Nov 1, 2020
@Amorymeltzer Amorymeltzer marked this pull request as ready for review November 1, 2020 20:31
@Amorymeltzer
Copy link
Collaborator Author

Amorymeltzer commented Nov 1, 2020 •
edited
Loading

The latest release schedule has this slated to head out everywhere December 1. I wonder if there's any value in pushing out most of this beforehand, hopefully flushing out any bugs in the background reworking. Refamiliarizing myself with it, I think what'd be needed is mainly just removing the expiry options from Twinkle.config.watchlistEnums. Thoughts? I'll try and test it out beforehand, but I think it'll make things simpler.

This is live on testwiki atm, I'll try and keep that updated with this.

@Amorymeltzer Amorymeltzer force-pushed the watchlistexpiry branch 8 times, most recently from 2c88cd3 to 2deba92 Compare November 16, 2020 13:27
@Amorymeltzer Amorymeltzer force-pushed the watchlistexpiry branch 6 times, most recently from 0fa66a0 to 893a36e Compare November 23, 2020 19:22
@Amorymeltzer
morebits: Add basic watchlist expiry support, do away with setWatchli…
8373ca3 
…stFromPreferences

Predictably, adds `ctx.watchlistExpiry`.

More notably, this also restructures how watchlisting is handled.  In particular, `setWatchlist` has been expanded to incorporate `setWatchlistFromPreferences`, which is thus obsoleted and done away with.  Correspondingly, `xfd`, `arv`, and `image` have had their ad hoc systems replaced with simple `setWatchlist` calls.  Support for the `unwatch` option has also been added, although remains unused by Twinkle itself (note the potential linguistic confusion between "no" and "unwatch").  `undelete`, which does not support watching a page ([phab:T247915]]), of course does not support watchlist expiry.

- arv: Work with new enums/remove fromPreferences (spiWatchReport)
- image: Work with new enums (deliWatchPage, deliWatchUser)
- xfd: Work with new enums (xfdWatchPage, xfdWatchDiscussion, xfdWatchList, xfdWatchUser, xfdWatchRelated)
@Amorymeltzer
config: Support watchlist expiry in modules
677d243 
The meat of the backend was done in `twinkleconfig`.  New expiry options have been added to `watchlistEnums`, at the levels of: 1 week, 1 month, 3 months, and 6 months (6 months is the current maximum: https://phabricator.wikimedia.org/T249672).  Additionally, I've converted any watchlist booleans to enums; this is not strictly necessary to enable watchlist expiry, but it gives every preference the same level of granularity, and I think would/should be expected.  To do this, though, I've added a bit of code to handle conversion of booleans to enums (`true` to `yes`, `false` to `no`).  As such, I've bumped the config version to 2.1; there's no major change to the overall structure, but it indicates that a number of preferences have been changed and will be processed differently going forward.  That is, old values will work, but new values will not necessarily work on old code.

This also adds new options for speedy and fluff (`watchSpeedyExpiry` and `watchRevertedExpiry`) to allow users to specify watchlist behavior, including expiry; both modules had non-standard watch options - fluff's agf/norm/vand and speedy's list of CSD criteria.  The conversion of boolean options (prod, tag, warn, welcome) required no changes due to the changes made to morebits' `setWatchlist` in the commit before this (85bb43d).

xfd, on the other hand, needed some extra love when nominating modules.  Since the `watch` API action only supports watch/unwatch and not `preferences` (T262912), we have to check it ourself using `mw.user.options.get('watchdefault')`.

Changes:
- xfd: Handle expanded enum options in watching of modules
- fluff: New expiry option `watchRevertedExpiry`
- speedy: New expiry option `watchSpeedyExpiry`

Config changes:
- prod: `prodWatchPages` no longer boolean
- tag: `watchTaggedPages`, `watchMergeDiscussions` no longer boolean
- warn: `watchWarnings` no longer boolean
- welcome: `watchWelcomes` no longer boolean
@Amorymeltzer Amorymeltzer merged commit ed1b903 into wikimedia-gadgets:master Nov 28, 2020
@Amorymeltzer
Copy link
Collaborator Author

Deployed! I think having the expiry options available and used would just lead to a swallowed API warning, but I've disabled them for the nonce.

@Amorymeltzer Amorymeltzer added this to the December 2020 update milestone Dec 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
deprecations Functions, etc. from Morebits that are deprecated and should be checked for usage before removal Feature request Module: config Module: morebits The morebits.js library Module: twinkle The twinkle.js global gadget file
Projects
None yet
Milestone
December 2020 update
Development

Successfully merging this pull request may close these issues.
2 participants
@Amorymeltzer @MusikAnimal