Page MenuHomePhabricator

Change the user-created list name if it the same name as the default list
Closed, ResolvedPublic1 Estimated Story Points

Description

PROBLEM
Reading list names must be unique. However, there may be some existing Android reading list users who have a reading list which happens to be the same name as the new default list of 'Saved' which will be shown to all users per T180239.
This is a problem since:

  • It is confusing for users to have two 'Saved' lists, one that is editable and one that is not.
  • If if they are synced both lists may be merged unintentionally.

Proposed solution

  1. On updating the app to introduce the default 'Saved' lists, the system changes the name of any user-created list to be 'Saved_[user created]'
  2. As per current behavior, if user tries to create/change a list name which is the same as an existing list, show an error message: "Reading list "<list name>" already exists"
    image.png (1×1 px, 163 KB)
Steps to QA

Pre-requisite: Prior to updating to the reading list sync version of the app, a user has a reading list called 'Saved'

  1. Update to the reading list sync version of the app.
  2. Navigate to view reading lists. It is expected that the empty default 'Saved' list will now be shown, with the existing user-created 'Saved' list renamed 'Saved_[user created]', with the same articles saved.
  3. Taps to rename the 'Saved_[user created]' list and try to rename it to 'Saved'. It is expected that an inline error message will be shown in the text field advising that "Reading list "Saved" already exists"

Event Timeline

Is this (list name uniqueness) something that should be enforced on the server side?

+ @Fjalapeno @cmadeo

Hi @Tgr - this ticket was per my understanding that (a) the default list name cannot be changed, and (b) user-created list names are unique. For Android users that already have reading lists, the chance is not zero that a reading list they already have my be the same name "Saved" being shown (at least in en) to them, so wanted to ensure users are not ever shown two lists called "Saved", one which is editable and one which is not.

To your question about list name enforcement on server side, not certain but I think this is the case as well per the following scenario:

  1. A logged in user (who has NOT yet enabled sync) creates a reading list called "Alpha" on device #1, and saves 5 articles to that list.
  2. On a separate device #2, the same user (logged in to the same account) creates a list also called "Alpha", but adds 10 different articles to this list.
  3. User enables the sync service on their account

Expected behaviour:
There is a single list called "Alpha" synced across both devices, containing 15 articles (merging the 5 articles on device #1 and the 10 articles on device #2).

Current server behavior is to identify the lists by ID (which is something that gets assigned the first time they are submitted to the server). So in that scenario, assuming a naive sync implementation in the apps, you would end up with two different lists called Alpha. (With not-so-naive logic it is of course possible to work around that.)

Ah imho seems like the server should enforce the name uniqueness as well, but will leave it to @Dbrant @Fjalapeno et al to hash out..

Charlotte renamed this task from Change the user-created list name if it the same name as the default list to Change the user-created list name if it the same name as the default list - S.Jan 2 2018, 6:18 PM
Charlotte triaged this task as Medium priority.

Change 401637 had a related patch set uploaded (by Sharvaniharan; owner: Sharvaniharan):
[apps/android/wikipedia@master] Existing user created reading list named 'saved'

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

Charlotte renamed this task from Change the user-created list name if it the same name as the default list - S to Change the user-created list name if it the same name as the default list.Jan 3 2018, 4:44 PM
Charlotte set the point value for this task to 1.

@RHo the server really doesn't care about list names ad @Tgr mentioned.

I would probably put this on the client, because it's much more efficient for the client just to check other list names and never let the user submit the name in the UI. Also the display name of the "default" list will be controlled by the clients and localized, so the server doesn't even know what the clients will be showing users for the name of the default list in for instance, Spanish.

Related: I "think" lists can be added and deleted while the user is offline - is this right? Or do you prevent users from manipulating lists until they have a connection? If so, then client side checking is really the way to go. Otherwise it would be very difficult to resolve this "later".

Hey @Fjalapeno - yep you can create and add articles to lists offline (at least in Android anyway - is this right @cmadeo or @JoeWalsh on iOS?)... I have no strong feelings about client or server side handling de-duping names, so long as the following scenario is as expected even when a user has devices on both platforms:

  1. A logged in user (who has NOT yet enabled sync) creates a reading list called "Alpha" on device #1, and saves 5 articles to that list.
  2. On a separate device #2, the same user (logged in to the same account) creates a list also called "Alpha", but adds 10 different articles to this list.
  3. User enables the sync service on their account

Expected behaviour:
There is a single list called "Alpha" synced across both devices, containing 15 articles (merging the 5 articles on device #1 and the 10 articles on device #2).

@RHo I talked to the iOS team about this today also. I had a misunderstanding of how the Default lists were identified by the server. They are identified simply by the name property, so the service does need to make sure that it rejects setting the name to "Default" for any list that is not the actual default list. Thanks for being on top of this…

@Tgr can you make sure that the service rejects changing the name of a list to "Default"? (also not sure if there is any other work - like should we add a column for marking a list as default?)

TL;DR

@RHo duplicate names in general will still be expected to be handled client side - its just much easier for clients to reject that before committing to the server and with the overhead of error handling on the client.
@Dbrant even though we will be doing server side checking for the Default name, clients should still reject the name "default" in the UI for the delayed syncing reason listed above. cc @JoeWalsh

Let me know if this addresses most of the concerns here - thanks all

Ok, that didn't take long… I am re-wrong again 🤦‍♂️

So after chatting with @Tgr he mentioned there is indeed a property to identify the default list.

@JoeWalsh @Dbrant can you verify that you see this in the response and it is working as you would expect?

If this property exists, then actually there is no reason to reject this name on the server. We actually don't care that the name default even exists… in fact that name could be blank or whatever. Sorry @RHo I am back tracking a little here, but in the end it shouldn't matter. The root issue (as I understand it) is that we want to make sure that clients can always identify the default list. It seems that does exist so I think we are ok here pending responses from the engineers. Let me know if this is not the case.

Yes, the default property is working the way we expect.

Also confirmed default is working as expected.

@Fjalapeno @Tgr @Dbrant @RHo I believe the API should enforce uniqueness of list names in addition to the client-side logic. This is a public API, so anyone could write a client that allows duplicate list names. Even if no one makes a third party client, there's a chance a bug in one of our own clients introduces duplicate lists.

Change 401637 merged by jenkins-bot:
[apps/android/wikipedia@master] Existing user created reading list named 'saved'

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

ABorbaWMF subscribed.

Working for me after a bunch of testing on v2.7.224-alpha-2018-01-26
An example:

Screenshot_2018-02-02-15-22-23.png (1×768 px, 142 KB)