After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 772243 - Bump the facebook API to 2.7
Bump the facebook API to 2.7
Status: RESOLVED FIXED
Product: libgfbgraph
Classification: Other
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: GFBGraph maintainer(s)
GFBGraph maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2016-09-30 10:31 UTC by Krzesimir Nowak
Modified: 2018-02-13 10:46 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Graph: Updated to Graph API v.2.10 (2.16 KB, patch)
2017-10-03 13:31 UTC, Álvaro Peña
none Details | Review
Removed duplicated email permission in GTest (2.16 KB, patch)
2017-10-16 22:08 UTC, Álvaro Peña
none Details | Review
Removed, really, duplicated email permission in GTest (3.67 KB, patch)
2017-10-23 22:20 UTC, Álvaro Peña
committed Details | Review

Description Krzesimir Nowak 2016-09-30 10:31:16 UTC
Currently the Graph API used is 2.3, which will be turned off in July 8th, 2017. See https://developers.facebook.com/docs/apps/changelog#versions for details.

There is one catch in bumping the version - just calling <GRAPH>/v2.7/me returns a minimal JSON object with an ID and a name. To get more fields, one must specify them explicitly via fields parameter. This is a change compared to v2.3 where <GRAPH>/v2.3/me returned all the fields current permissions allows one to retrieve them. I'm not sure if this is limited only to user instances or rather to all of them.

This may need some (breaking?) changes in the GFBGraphConnectable interface to allow specifying the fields to be retrieved.
Comment 1 Umang Jain 2016-12-09 14:08:43 UTC
Hi Krzesimir Nowak,

I am going to add facebook photo upload in gnome-photos[1] so need to work on this library. I think we will need the new implementations of v2.8 in the lib for future compatibility too.

[1] https://bugzilla.gnome.org/show_bug.cgi?id=766031
Comment 2 Michael Catanzaro 2017-05-10 03:55:52 UTC
So libgfbgraph is used by gnome-maps, gnome-online-miners, and gnome-photos. Rishi is already CCed here, so adding Marcus. Looks like Facebook integration is set to die on July 8 unless someone fixes libgfbgraph.
Comment 3 Álvaro Peña 2017-05-10 12:40:53 UTC
I am going to check it up ASAP, the changes are not really hard to do. But any help is welcome.
Comment 4 Álvaro Peña 2017-10-03 13:31:24 UTC
Created attachment 360839 [details] [review]
Graph: Updated to Graph API v.2.10

Updated endpoint URL to the new Graph version.

It was required to use the "fields" param (see
https://developers.facebook.com/docs/graph-api/using-graph-api#reading)
to retrieve the email of an user and added the permission "email" too.
Comment 5 Umang Jain 2017-10-05 17:27:23 UTC
Review of attachment 360839 [details] [review]:

::: gfbgraph/gfbgraph-common.c
@@ +22,3 @@
 #include <rest/rest-proxy.h>
 
+#define FACEBOOK_ENDPOINT "https://graph.facebook.com/v2.10"

Don't we need to keep this in one place so that other files can use this macro as common via #include ?

or you expect to have a local FACEBOOK_ENDPOINT in .c file only (wherever required) ?
Comment 6 Álvaro Peña 2017-10-05 17:45:12 UTC
(In reply to Umang Jain from comment #5)
> Review of attachment 360839 [details] [review] [review]:
> 
> ::: gfbgraph/gfbgraph-common.c
> @@ +22,3 @@
>  #include <rest/rest-proxy.h>
>  
> +#define FACEBOOK_ENDPOINT "https://graph.facebook.com/v2.10"
> 
> Don't we need to keep this in one place so that other files can use this
> macro as common via #include ?
> 
> or you expect to have a local FACEBOOK_ENDPOINT in .c file only (wherever
> required) ?

The endpoint is only required by the rest call, which requires an authorizer (simple o goa or whatever), why do you need the endpoint to be placed in a header?

Perhaps it will be fine to have a GFBGraphConfig or similar with the endpoint url, the api version and thinks like that...
Comment 7 Umang Jain 2017-10-05 18:05:28 UTC
> 
> Perhaps it will be fine to have a GFBGraphConfig or similar with the
> endpoint url, the api version and thinks like that...

Exactly. The macro FACEBOOK_ENDPOINT should be pulled from one source only rather than sprinkled over in all *.c files; will make version bumps easier when we will have many functionalities using the endpoint.
Comment 8 Álvaro Peña 2017-10-06 00:02:24 UTC
(In reply to Umang Jain from comment #7)
> > 
> > Perhaps it will be fine to have a GFBGraphConfig or similar with the
> > endpoint url, the api version and thinks like that...
> 
> Exactly. The macro FACEBOOK_ENDPOINT should be pulled from one source only
> rather than sprinkled over in all *.c files; will make version bumps easier
> when we will have many functionalities using the endpoint.

What I don’t see your point. The main idea to have just one facebook endpoint definition, and make it private, is to reduce the possible errors with two differents endpoints...

At this moment you can read the endpoint throw the proxy call obeject.
Comment 9 Umang Jain 2017-10-07 05:52:29 UTC
Ok now, I understand. I guess I am under the impression of something else here. Thanks for clarifications
Comment 10 Umang Jain 2017-10-07 05:58:45 UTC
Review of attachment 360839 [details] [review]:

::: gfbgraph/gfbgraph-user.c
@@ +269,1 @@
 

++

::: tests/gtestutils.c
@@ +50,1 @@
 

"email" duplicated.
Comment 11 Álvaro Peña 2017-10-16 22:08:37 UTC
Created attachment 361698 [details] [review]
Removed duplicated email permission in GTest

Graph: Updated to Graph API v.2.10

Updated endpoint URL to the new Graph version.

It was required to use the "fields" param (see
https://developers.facebook.com/docs/graph-api/using-graph-api#reading)
to retrieve the email of an user and added the permission "email" too.
Comment 12 Umang Jain 2017-10-17 02:05:45 UTC
Review of attachment 361698 [details] [review]:

::: tests/gtestutils.c
@@ +50,1 @@
 

The "email" duplication is still present.
Comment 13 Umang Jain 2017-10-17 02:07:49 UTC
Review of attachment 361698 [details] [review]:

::: tests/gtestutils.c
@@ +49,1 @@
+#define FACEBOOK_TEST_USER_PERMISSIONS "email,user_about_me,user_photos,email,publish_actions"

Did you attach the previous patch again? :)
Comment 14 Álvaro Peña 2017-10-23 22:20:32 UTC
Created attachment 362123 [details] [review]
Removed, really, duplicated email permission in GTest
Comment 15 Álvaro Peña 2017-10-24 22:41:25 UTC
Updated Facebook API support to 2.10
Comment 16 Marcus Lundblad 2018-02-13 10:33:29 UTC
Btw, could we see a new release of the library soon to get this out?
Comment 17 Umang Jain 2018-02-13 10:44:00 UTC
(In reply to Marcus Lundblad from comment #16)
> Btw, could we see a new release of the library soon to get this out?

Yes for sure. Also, it would be nice if these patches for facebook uploads gets reviewed and merged before the release. Ping:  Álvaro Peña. GNOME Photos is blocked on this feature for months now. It would be great to land this asap.

Thanks
Comment 18 Umang Jain 2018-02-13 10:44:43 UTC
(In reply to Umang Jain from comment #17)
> (In reply to Marcus Lundblad from comment #16)
> > Btw, could we see a new release of the library soon to get this out?
> 
> Yes for sure. Also, it would be nice if these patches for facebook uploads
> gets reviewed and merged before the release. Ping:  Álvaro Peña. GNOME
> Photos is blocked on this feature for months now. It would be great to land
> this asap.
> 
> Thanks

Sorry, forgot to include Bug URL. https://bugzilla.gnome.org/show_bug.cgi?id=776098
Comment 19 Debarshi Ray 2018-02-13 10:46:13 UTC
Yay! Thanks, Álvaro.