garden-stream / garden

shared resources
0 stars 2 forks source link

User response varies #15

Closed keawade closed 7 years ago

keawade commented 7 years ago

The order of items returned from a call to /api/user is inconsistent.

NathanBland commented 7 years ago

it is sorted by last updated.

keawade commented 7 years ago

Last updated includes when I last followed or unfollowed them. That is a problem.

If I have a list of those guys and I follow one, they jump to the top of the list.

NathanBland commented 7 years ago

Indeed. That is how it is currently though. I'm just saying that the response isn't inconsistent. :laughing:

keawade commented 7 years ago

Technically true, but still a bug. Consider a case where we have a hundred users. Each of them doing things. Every time I refresh a page that displays a list of users, it will vary wildly and seemingly inconsistently. Scale up to a thousand users, or more, and it becomes even more chaotic. Even if it technically is consistent, this is a problem.

NathanBland commented 7 years ago

Is it a problem? For user discovery a list that changes frequently and shows different users all the time seems beneficial to me. It isn't a user search api, in which you would expect the closest match to be first, and the same going on. For just browsing users, random order seems like a pro to me.

keawade commented 7 years ago

Yes. Especially if it is the only way to get users.

I need a consistent list of users. A consistent list can always be scrambled with minimal effort. A scrambled list takes much more work to make ordered.

NathanBland commented 7 years ago

See to me, it sounds like you actually want a search api. Most services don't even offer a browse feature. In fact none do that I can find.

Most offer a suggested, which is probably an aggregation of people who are either followed by your followers, or other people you follow. None provide just a normalized list of users.

keawade commented 7 years ago

If you want to make a search api part of 1.0, I'm fine with that. I think it would be simpler to just tweak the return order for now.

NathanBland commented 7 years ago

Ok. See #16 for search api progress.

crodeheaver commented 7 years ago

Just jumping in late here - If Keith wants a consistent response, sort by id or last name or first name or anything that makes it consistent. Then make the client sort it. Then each client can do what it want. I personally don't think it's the servers responsibility to sort crap for you.

crodeheaver commented 7 years ago

Also, I'd like to make a distinction between sorting and searching. Searching narrows results, sorting (obviously) orders them.

keawade commented 7 years ago

If we're eventually planning on limiting the number of results, having the client sort will no longer be an option. I'd rather have a more consistent default now so we don't break our apps when we make that change.

crodeheaver commented 7 years ago

Why can't the client sort based on a limited number of results?

keawade commented 7 years ago

Because if that limited number of results is practically random, then no matter what consistency I try to impose, it will be limited to the consistency of the limited set I'm working with.

NathanBland commented 7 years ago

It isn't random, see comment above. Paging would work fine here when it comes to that.

crodeheaver commented 7 years ago

@keawade I could argue, but I won't. ;P I get your point. I do think the results you get back should be consistent, but I don't think it's the servers responsibility to sort them. In other words, if the server sends back the users in a completely random nature, there's nothing to stop you from sorting them after you get them.

crodeheaver commented 7 years ago

And like Nathan said, the results aren't actually random

keawade commented 7 years ago

Given enough active users, the list will be indistinguishable from randomness to a user. I don't care if it technically isn't random. I care that it looks random. It looks like you get a completely different list every time you refresh the page.

Now, if I have all the data returned when I make the call, I can impose order on the set. However, once we start paginating, I lose the ability to apply a meaningful sort to this psuedo-random data unless I increase my data set size, which defeats the purpose of paginating in the first place.

@crodeheaver The server is already sorting the results by modified date. We just need to change that to something that won't change as much.

In order to ensure the future utility of this functionality, we need to impose a more consistent order.

NathanBland commented 7 years ago

Well. There are two problems here.

  1. We don't have that many users, even if we did, people don't follow/unfollow that often, so the list wouldn't be as inconsistent as you think.
  2. You are basically saying this is a bug on something that isn't even implemented yet. As the label already suggests, for this endpoint this functionality is not changing. If you want a consistent list, use the search api.
crodeheaver commented 7 years ago

Going along with what @NathanBland said, search api + client sorting = problem solved.

keawade commented 7 years ago

We don't have that many users

Irrelevant. We should build to scale. If we don't, then as soon as we do have that many users, we're screwed.

people don't follow/unfollow that often

Users are ordered by the user's modified date. Every change to any user will likely change the list's order.

for this endpoint this functionality is not changing

Since this is the primary method for retrieving users, I think we should reconsider that.

NathanBland commented 7 years ago

Users are ordered by the user's modified date. Every change to any user will likely change the list's order.

Are you really suggesting that people will change their password/displayname/email more often than they will follow people? Because I don't believe that will happen.

Since this is the primary method for retrieving users, I think we should reconsider that.

It is not. see #17

keawade commented 7 years ago

Okay, I thought we were still storing posts under the user object.

Why make a new endpoint when we could just extend this endpoint? Wouldn't it make more sense to get users through /api/users with a query rather than through the generic route api/search?

NathanBland commented 7 years ago

No, that changed as of commit a1d3d4a.

What else are you going to search? I don't think we'll ever make posts searchable, and there are no other resources currently planned... I can perhaps adjust search to be /api/search/users if it would make you feel better.