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

Members endpoint - GET #41

Closed
r-a-y opened this issue Apr 4, 2017 · 8 comments

Comments

3 participants
@r-a-y
Copy link
Member

commented Apr 4, 2017

Note: I'm working on the Members endpoint locally for now until I have something more substantial to produce, but my plan is to piggyback on WP's already-existing WP_REST_Users_Controller class by extending it (and using a different endpoint) and overriding individual class methods to better suit BP.

I have a couple of questions. We can worry about query parameters later, but what data do we want to return in the Members List Members endpoint? GET /bp/v1/members

FWIW, this is what WP returns for GET /wp/v2/users:

  {
    "id": 9,
    "name": "x",
    "url": "",
    "description": "",
    "link": "hxxp://localhost/author/x/",
    "slug": "x",
    "avatar_urls": {
      "24": "hxxp://1.gravatar.com/avatar/HASH?s=24&d=mm&r=g",
      "48": "hxxp://1.gravatar.com/avatar/HASH?s=48&d=mm&r=g",
      "96": "hxxp://1.gravatar.com/avatar/HASH?s=96&d=mm&r=g"
    },
    "meta": [],
    "_links": {
      "self": [
        {
          "href": "hxxp://localhost/wp-json/bp/v1/members/9"
        }
      ],
      "collection": [
        {
          "href": "hxxp://localhost/wp-json/bp/v1/members"
        }
      ]
    }
  },

Inheritance

  • How much of WP's response data do we want to keep / remove?
  • url and link should be merged. Only one of them should be kept for our needs.
  • description should be removed.
  • meta needs investigation, but for our needs, could probably be removed.
  • _links needs investigation. See https://developer.wordpress.org/rest-api/using-the-rest-api/linking-and-embedding/.
  • avatar_urls needs to be modified to handle BP's generic sizes such as thumb and full since we do not offer numeric sizing options.
  • id and name will be kept
  • slug (alias of user_login) keep or rename?

Authentication

  • WordPress limits what is shown in their List Users endpoint based on who is authenticated to the REST API. By default, non-authenticated calls limits the response to users that have created public posts, but is still publicly-queryable.
  • We should also probably limit exposure to member data as well (although we don't do this for the Members Directory via HTTP! 🤕).
  • Twitter requires authentication to query userdata - https://dev.twitter.com/rest/reference/get/users/lookup

Modifications / Enhancements / Notes

  • Adding the user registration date will be helpful, but we might want to limit this to authenticated apps (or in WP/BP terms, use bp_current_user_can( 'list_users' ) for now).
  • member_types will be added
  • As discussed on bpdevel, xprofile should have its own response top-level property (eg. xprofile.fullname). However. we should probably add support for all native BP components:
    • activity - activity.favorite_count, activity.latest_update (this will use the same schema we will settle on for activity... or not)
    • friends - friends.total_count (only offering count, listing of friends should be done in the friends endpoint)
    • groups - groups.total_count(only offering count, listing of groups should be done in the groups endpoint)
@modemlooper

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2017

xprofile endpoint should be reserved for CRUD operations of fields creation. something like /xprofile/1 would access the field with id 1. Returns what type of field etc. To access data the member has added to a field we should have /members/1/xprofile/1 = user id 1 profile field 1. Though this could be later, most apps will want to have the user AND profile fields included in one call.

basic info for /members username, avatar with the embed parameter added to include the full profile fields data and possibly any groups the user belongs to.

https://developer.wordpress.org/rest-api/using-the-rest-api/linking-and-embedding/

@r-a-y

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2017

Thanks for the reply, @modemlooper.

xprofile endpoint should be reserved for CRUD operations of fields creation.

Agreed, this ticket is solely about the GET /members endpoint. Setting and deleting xprofile data, as well as fetching individual xprofile fields and groups will be covered in the xprofile endpoint, but for the purposes of the GET /members endpoint, we should provide all xprofile data for a member.

Though this could be later, most apps will want to have the user AND profile fields included in one call.

Yes, this is what I mean with the xprofile property as part of the GET /members response

https://developer.wordpress.org/rest-api/using-the-rest-api/linking-and-embedding/

Thanks about the link to the _links parameter. I've added it to the main post.

@modemlooper

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2017

For mobile apps you want to include as much data as you can in one api request, though not needed for all web apps. The typical master detail UI you find in apps like twitter. You see the tweets list and click to the detail view. The master list view may not show all data but in the detail view you want to see it. Its bad to have another api request just to get more data on each single item view.

I suggest the embed param to get all data. BP plugin authors can add their extra data to embed as well if it is user data.

@r-a-y

This comment has been minimized.

Copy link
Member Author

commented Apr 6, 2017

So I'm implementing the xprofile property and I'm running into an issue.

At first, I had the xprofile property return this:

xprofile.{GROUP_NAME}.{FIELD_NAME} = {FIELD_VALUE}.

An example would be xprofile.base.name = Ray. The dynamic values were sanitized for JSON, but I realized that this would only really work for English installs.

So how does everyone feel about the following?

xprofile.group{GROUP_ID}.field{field_ID}.value = {FIELD_VALUE}.

This would return two values:

xprofile.group1.field1.name = Name
xprofile.group1.field1.value = Ray

This is less sexy, but does ensure that xprofile group and field integrity will always be the same since you cannot alter the xprofile group ID or the field ID.

We could also discard the group ID, so this could be shortened to:

xprofile.field1.name = Name
xprofile.field1.value = Ray

@modemlooper

This comment has been minimized.

Copy link
Contributor

commented Apr 6, 2017

I like keeping group. Returning more than the value is always good. Once app has data it can cache etc. and parse locally.

Usage though would be a full dump of all groups and fields when requesting user data.

user_id.xprofile.groups.group_id.fields.field_id

@modemlooper

This comment has been minimized.

Copy link
Contributor

commented Apr 6, 2017

[
  {
    "_links": {
      "collection": [
        {
          "href": "http:\\/\\/bptrunk.dev\\/wp-json\\/wp\\/v2\\/users"
        }
      ],
      "self": [
        {
          "href": "http:\\/\\/bptrunk.dev\\/wp-json\\/wp\\/v2\\/users\\/1"
        }
      ]
    },
    "avatar_urls": {
      "24": "http:\\/\\/1.gravatar.com\\/avatar\\/1c07be1016e845de514931477c939307?s=24&d=mm&r=g",
      "48": "http:\\/\\/1.gravatar.com\\/avatar\\/1c07be1016e845de514931477c939307?s=48&d=mm&r=g",
      "96": "http:\\/\\/1.gravatar.com\\/avatar\\/1c07be1016e845de514931477c939307?s=96&d=mm&r=g"
    },
    "description": "",
    "id": 1,
    "link": "http:\\/\\/bptrunk.dev\\/author\\/admin\\/",
    "meta": [],
    "name": "admin",
    "slug": "admin",
    "url": ""
    "xprofile": {
        "groups": {
            "1": {
                "name": "Base",
                "fields": {
                    "1": {
                        "name": "Name",
                        "value": 'Admin',
                    },
                },
            },
        },
    },
  }
]

@renatonascalves renatonascalves added this to the v1 milestone Dec 13, 2017

@renatonascalves renatonascalves changed the title Members - GET - List Users Schema Members - GET Dec 13, 2017

@renatonascalves renatonascalves changed the title Members - GET Members endpoint - GET Dec 13, 2017

@renatonascalves

This comment has been minimized.

Copy link
Member

commented Dec 13, 2017

Thanks for the feedback so far! I'll add this based on @r-a-y work here: https://github.com/r-a-y/bp-rest/blob/master/classes/Members/v1/Controller.php

PR in the next days! :)

@renatonascalves renatonascalves added this to In Progress in BP REST - V1 Dec 24, 2017

renatonascalves added a commit that referenced this issue Dec 24, 2017

@renatonascalves

This comment has been minimized.

Copy link
Member

commented Dec 24, 2017

^^

"id": 1,
"name": "Renato Alves",
"link": "http:\/\/bp.dev\/members\/admin\/",
"user_login": "admin",
"avatar_urls":
{
    "full": "\/\/www.gravatar.com\/avatar\/2d191aa2252df76ee91554475ce51b98?s=150&r=g&d=mm",
    "thumb": "\/\/www.gravatar.com\/avatar\/2d191aa2252df76ee91554475ce51b98?s=50&r=g&d=mm"
},
"member_types": [],
"xprofile":
{
    "groups":
    {
        "1":
        {
            "name": "Base",
            "fields":
            {
                "1":
                {
                    "name": "Name",
                    "value": "Renato Alves"
                }
            }
        },
        "2":
        {
            "name": "Testando",
            "fields":
            {
                "2":
                {
                    "name": "Ok!'",
                    "value": ""
                }
            }
        }
    }
},
"_links":
{
    "self": [
    {
        "href": "http:\/\/bp.dev\/wp-json\/buddypress\/v1\/members\/1"
    }],
    "collection": [
    {
        "href": "http:\/\/bp.dev\/wp-json\/buddypress\/v1\/members"
    }]
}

@renatonascalves renatonascalves moved this from In Progress to Completed in BP REST - V1 Mar 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.