Page MenuHomePhabricator

Update Product Infrastructure-owned products that may be affected by IP Masking
Open, Needs TriagePublic

Description

IP Masking will affect lots of our products, features, tools, gadgets, etc. This task is for tracking work to update those that are owned by Product Infrastructure, ahead of IP Masking being enabled on WMF sites.

See T326816: Update features for temporary accounts, particularly What will be affected.

A preliminary investigation (T326759) has found that the following may be affected:

  • ContactPage
  • ReadingLists

Event Timeline

The Content-Transform-Team never actually worked on ReadingLists, as far as I know. We could use some help here, since we're not familiar with these code bases at all.

@cscott We are not familiar with those codebases either. Do you know who might be able to help?

Perhaps @Dbrant knows? I remember Jdlrobson mentioning that they did some work together on this.

I believe @Tgr built the initial iteration of this extension, and might say more definitively how it would be impacted.

From what I understand of IP masking so far, since temporary accounts are full-fledged accounts, they will automatically be able to "use" reading lists, and be able to use the API in the usual way, so there shouldn't be any breaking changes in that regard. (I've done some testing on beta-dewiki, without problems)

So then, from a product perspective, any reading lists saved by a temporary user will become inaccessible when the user expires. Therefore the question becomes: should it be left up to the clients (i.e. Apps) to discourage or disable creating reading lists if logged in as a temporary user, or should we make changes to the extension itself to return an error when creating reading lists as a temporary user?

Yeah, by default temporary users will be treated same as normal users for pretty much everything unless explicit checks are added to the code.

We have decided against migrating user data when a temporary user registers, mostly for privacy and legal reasons. For private data like reading lists a migration probably wouldn't be problematic, and not too much effort (we could just add a migration hook, similar to UserMerge, and make it update the user ID). Probably not something to try for the MVP version, though.

Therefore the question becomes: should it be left up to the clients (i.e. Apps) to discourage or disable creating reading lists if logged in as a temporary user, or should we make changes to the extension itself to return an error when creating reading lists as a temporary user?

It's a one-line change so why not. But the apps would still have to hide the UI from temporary users.

Our guidance for IP Masking impacted products at the moment is to keep their experience consistent with current logged-out editor experience to avoid introducing drastic changes without prior community input.
With that in mind, I think it would be good to disable reading lists for temporary accounts and remove references to them in the UI.

Sounds good, thanks @Niharika! It sounds like we'll pursue making a small change to the extension to prevent a temporary account from setting-up reading lists.

And from the client side, just fyi, the apps do allow users to accumulate reading lists locally even if logged out. It's only when logged into an account, we sync the reading lists to the server, using the ReadingLists extension. It should be very simple for the clients to maintain the same behavior for a temporary account (i.e. don't sync reading lists) as for a logged-out user.

And from the client side, just fyi, the apps do allow users to accumulate reading lists locally even if logged out. It's only when logged into an account, we sync the reading lists to the server, using the ReadingLists extension. It should be very simple for the clients to maintain the same behavior for a temporary account (i.e. don't sync reading lists) as for a logged-out user.

That sounds fine to me. Thanks @Dbrant. Do you know who will be responsible for updating the client behavior? Making sure this doesn't fall through the cracks.

I've created a task (T331912) that will represent this work on the apps side.

Ok, that seems to cover ReadingLists. What about ContactPage?

ContactPage has some logic to format messages from logged-in users and anons differently (e.g. username vs. IP). It should probably just treat temp users as logged-in.