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.

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

Change API according to Eran's request #5

Closed
dafnarosenblum opened this issue Feb 27, 2019 · 7 comments
Closed

Change API according to Eran's request #5

dafnarosenblum opened this issue Feb 27, 2019 · 7 comments
Assignees

Comments

@dafnarosenblum
Copy link
Collaborator

Replace the get route with a route that receives a list of ids and returns the once that are bots - from the given list.

@eranshmil
Copy link
Member

The request should be something like that:

{
  userIds: number[],
  loggedInUserId: number
}

The route will do the following:

  1. Check the statuses of each of the userIds.
  2. Check whether the loggedin user is a team member.

The response should be something like that:

{
  isTeamMember: boolean,
  userStatuses: { [userId: number]: Status }[]
}

@mderazon
Copy link
Collaborator

mderazon commented Mar 3, 2019

I can help with that.
What's the use case for this endpoint ? I'm asking because it's usually discouraged having a body in GET requests and on the other hand, making it a POST loses the semantic meaning of this endpoint.

If we can get by on putting this in query param that would be better.
Something like

GET /bots?q=userid1,userid2,userid3,...

In the future you might want to filter by other things as well

GET /bots?q=userid1,userid2&isTeamMember=false

But if the list of users you query for is really big, it will surpass the url limit (2,083 characters)

@eranshmil
Copy link
Member

I think that we won't reach that limit in the query string.
But, the isTeamMember is part of the response, not a filtering option.

The query string should accept two params: userIds and loggedInUserId .

@mderazon
Copy link
Collaborator

mderazon commented Mar 3, 2019

Okay, so i'll put it in the query, loggedInUserId should not be in the query param though, should come from any sort of authentication, probably in the header once we have this: #1

So maybe i'll work on #1 first

@eranshmil
Copy link
Member

This is not the meaning of the param, just a bad naming. :)
It suppose to be the twitter user id, so it will check if he is part of the team, and show him some indication in the page that the extension is still working as expected.

@eranshmil eranshmil assigned mderazon and unassigned mderazon Mar 3, 2019
@liorn
Copy link
Collaborator

liorn commented Mar 5, 2019

@eranshmil I can take this.
A couple of (n00b?) questions, just want to make sure we're synced...

  1. Right now, how do we know who's a team member? Should I create a new DB table which holds team member IDs? (and then we fill it with another tool, not from the extension itself)?
  2. Can I open a new route for it? Or should I reuse "confirmed"?

@eranshmil
Copy link
Member

eranshmil commented Mar 5, 2019

There is two issues to complete before this one.
#7 and #8.

@haimkastner haimkastner self-assigned this Mar 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants