Use facebook api v2.10 #128
Conversation
Use current facebook api version v2.10. Beginning with v2.9, the api requires the fields parameter to be set if it shall return not only the id but additional fields, and the fields have been reorganised and renamed.
|
Travis-ci failed but the reasons seem not to be related to my PR, or are they? Can some maintainer check? |
|
Ok, I saw for PHP 7.1 travis tells format errors. I will correct and commit in a minute. |
Fix too long lines
|
Now Travis succeeded for PHP 7.0. The others fail with errors which seem to be related to the test setup and not to this PR, but those don't show any code style errors anymore, so I assume this PR is OK now. |
src/Backend/Facebook.php
Outdated
| } | ||
| if (isset($data['share']) && isset($data['share']['share_count'])) { | ||
| return $data['share']['share_count']; | ||
| if (isset($data['engagement']) && isset($data['engagement']['reaction_count']) |
There was a problem hiding this comment.
(isset($data['engagement']) is not needed when having all those other checks
There was a problem hiding this comment.
it was like that in the present code, too:
if (isset($data['share']) && isset($data['share']['share_count'])) { ...
There was a problem hiding this comment.
do you know why travis fails?
There was a problem hiding this comment.
https://travis-ci.org/heiseonline/shariff-backend-php/jobs/297185279#L601
Failed to clone the git@github.com:heiseonline/shariff-backend-php.git repository
Seems to hit some github limitation. For sure not your fault.
it was like that in the present code, too
Does not mean that it was "correct" like that. ;-)
Simplifying code makes it better readable.
Simplify isset check and add some new lines for better readability
|
@liayn Done, isset code corrected. Can you also have a look on my comment here? I have meanwhile checked the other shariff backends. For the Perl backend I could do a PR with the same changes as done here, but the Node backend still seems to use the meanwhile unsupported FQL, so the change there would be a bit bigger, and I am not sure if I can do that since not being an expert in JS. Question: What shall I do regarding this? Make PR for other backends, too, whereever I can, or ignore them because soemone else will do that anyway sooner or later? Another possible problem with Facebook's recent API versions is that if an object never has been scraped before by the FB crawler, a GET request to the open graph (OG) object containing the og_object in the list of fields to get will return an empy og_object. In this case Facebook say it needs a POST request to initialise the OG object for that particular API version. See https://developers.facebook.com/blog/post/2017/07/18/graph-api-v2.10/ , section "URL API Changes":
I have no idea if this is a problem for the counters, too, but if so, it could be a problem for shariff-backend-php when merging this PR here. A solution would be to change thos PR so it used API v2.9, which has the same format of counters but according to the cited text above does not have the changed behavior described above. Later then we could extend the backend so it can be used with v2.10 and later and will include the og_object in the fields list so it can check if that is empty and can issue the POST request. But if someone created his facebook app with version v2.10 or later, the counter on shariff backend will not work for him if it uses a lower version. Question: Shall I change this PR to API version v2.9 to be on the safer side? Or should it be taken as it is? |
src/Backend/Facebook.php
Outdated
| } | ||
| if (isset($data['share']) && isset($data['share']['share_count'])) { | ||
| return $data['share']['share_count']; | ||
| if (isset($data['engagement']['reaction_count']) |
There was a problem hiding this comment.
You can combine this to isset($foo, $bar, $baz)
There was a problem hiding this comment.
@core23 You are right. Done.
|
@liayn Forget the 2nd part "Another possible problem" of my comment above. The counters will be zero for a new object, and first scraping by facebook crawler will then be issued after having clicked the button by Facebook share dialog. |
Correct multi-line function call
|
Travis fails seem not to be related to this PR. The logs do not show code style errors. |
|
@richard67 I'm not a official maintainer nor do I use any of the other implementations. Hence, I can only say "I do not care too much". |
There seems to be a problem with composer and GitHub. I am looking into this. |
|
Any idea, when this will be merged? My users (I created a little extension for Pagekit CMS) are waiting for this :) |
Apparently Travis does not set "hidden" or encrypted environment variables anymore on builds originating from forked repositories. It looks like this causes problems with composer and GitHub API rate limits. |
Use current facebook api version v2.10 and adapt to changes in v2.9.
This also solves issue #127.
Beginning with v2.9, the api requires the fields parameter to be set if it shall return not only the
id but additional fields, and the fields have been reorganised and renamed.
See https://developers.facebook.com/docs/graph-api/changelog/version2.9#gapi-changes-general:
The Facebook Like button shows then the total of all these counts, except of the comment_plugin_count, which seems to belong to the deprecated comment plugin, so I did not include this one into the total.
See following examples from a test (the token parameter in the URL has to be replaced by a valid token you can get at the Facebook Api Explorer).
How to test:
Verify that for an URL for which shariff-backend-php works well, the shariff-backend-php without this pull request applied and the and shariff-backend-php with this pull request applied show the same count for facebook if queried at the same time.
Why to change to latest api version:
When creating an app_id on the Facebook app dashboard, it is bound to the current API version, i.e. you can't select an api version lower than that. So if you today create an app, you can select only v2.10 as api version in the dashboard, and querying that app on Facebook Graph Object Explorer using api version v2.8 and a key made from the app_id and the secret of the app and not of a current user, you will not get a result.
Additional info:
When using a dedicated Facebook API version, e.g. "https://graph.facebook.com/v2.10/", and not only "https://graph.facebook.com/" without version, then the result is always in the format for that version, i.e. the check in the source code for array elements belonging to older API versions is not necessary. Therefore it is sufficient to check only for the result of that API version and not the older things "if (isset($data['data']) && isset($data['data'][0]) && isset($data['data'][0]['total_count']))" for very old versions which meanwhile are not available anymore and "if (isset($data['share']) && isset($data['share']['share_count']))" for version v2.8 in addition, like it is now in the source code, which checks both old alternatives. Maybe this was some remainder from old code when no version folder "/vX.Y" (like now "/v2.8") was used in the query URL, so the result could be returned in different formats depending on the version used by the APP belonging to the app_id parameter.