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

remove article rescrape code #798

Merged
merged 3 commits into from Nov 9, 2017

Conversation

Projects
None yet
4 participants
@timjacobi
Copy link
Collaborator

commented Nov 1, 2017

This PR:
Removes the code that triggers a rescrape for an article. Due to a recent change in the Graph API this code does not work anymore.

@pestevez

This comment has been minimized.

Copy link
Collaborator

commented Nov 8, 2017

Will the removal of the graph SDK dependency affect the use of Facebook\Exceptions\FacebookResponseException in file class-instant-articles-meta-box.php?

@timjacobi

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 8, 2017

Yes. That use statement is obsolete now. Actually I think it's been obsolete all the time since it was unused.

@timjacobi timjacobi force-pushed the timjacobi:no-rescrape branch from f30cfcf to c64a0bd Nov 8, 2017

@pestevez
Copy link
Collaborator

left a comment

:shipit:!

@pestevez

This comment has been minimized.

Copy link
Collaborator

commented Nov 8, 2017

Fixes #714

@timjacobi timjacobi merged commit 0a3714f into Automattic:master Nov 9, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@DrLightman

This comment has been minimized.

Copy link

commented Dec 8, 2017

@timjacobi

Removes the code that triggers a rescrape for an article. Due to a recent change in the Graph API this code does not work anymore.

Thanks for the heads up. Can I ask you how did you learn about that? I've a site WP site with IA plugin and I was notified of some misbehaviour with posts not appearing in instant library and this could have shed some light on the issue.

@timjacobi

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 11, 2017

There is a post with release notes for the Graph API somewhere where they say that they deprecated it.

@jerclarke

This comment has been minimized.

Copy link

commented Feb 15, 2018

Hey guys, this is the DEFINITELY WRONG way for this change to be communicated and explained.

For starters, this change, along with a detailed explanation of the Graph API changes, should have been on the Instant Articles Blog becuase it is enormous and important and that blog promises to keep us up to date.

I can imagine why you wouldn't want that though, because this change is uniquely terrible for users and publishers, and deeply embarrassing to Facebook. Still though, at least here on Github, it needed to be explained more usefully.

You should definitely be able to link to the place where the Graph API change was announced.

You should also be able to explain how the change made it impossible for you to replace this feature with the new version of the API, since it cripples the plugin to have this feature removed.

You should also have stated all the different effects this change would have had, for example that unlike before, where Instant Articles would update when the post was updated, now they will remain in the cache, stale and wrong, for 30 days. For more details see #878

This change creates fake news by making updates to articles not show on Facebook. If we correct an article, our users will see the wrong version. By not making this clear to us as publishers, you left us ignorant of the problem, and guilty of false news without even knowing it.

@jerclarke

This comment has been minimized.

Copy link

commented Feb 15, 2018

So, now I have looked through all the Graph API changes in the changelog, going back to 2016, and none of them imply that this should be broken, or that this feature should have been removed from this plugin.

So it looks like v. 2.10 - Released July 18, 2017 is the only sensible announcement that @timjacobi could have been referring to.

Here is the section that relates to scraping which does not mention deprecation

URLs
POST /{url} - This node now requires a valid access token.

GET /{url} - og_object is no longer returned by default but must be included in the fields parameter.

/{url} Scraping - GET requests will no longer scrape the URL. POST /{url} must now be used to scrape the URL.

Now, this was all in the "90-Day Breaking Changes" section, which, because the release date was July 18 2017, means that this change to the /{url} node would have taken effect on October 16, 2017. This would then, logically, be the date when the code in this plugin stopped working, and about a month later this "fix" was applied to remove the code which no longer functioned. That all makes sense as a timeline, though it's still disturbing that the developers of this plugin didn't pre-empt the problem, having been given a 90 day warning, and working for the same company. Still, I won't hound you for that, I'm looking for a fix, not to blame people for doing their best.

But how does that API change mean that we can't have scraping in this plugin?!?

I can see that it broke, but why can't you fix it?

Based on reading the code, it doesn't seem like the GET/POST change is the culprit, as the code deleted from the plugin was already using POST. This leads me to believe that what broke the plugin was "This node now requires a valid access token."

So I don't even know exactly what that implies or entails, but I know that now I want to have an access token, and I want to use it to keep my posts up to date automatically the way this plugin kept them up to date before.

What is involved in getting an access token, and why can't this plugin accomplish it for us?

Was this problem discussed in a meeting that we don't have access to? Is it impossible?

It seems like this is the specific question that should have been discussed in this ticket, or at least there should have been an explanation of it, along with a link to the changelog.

Looks to me like the feature is still there, and this is something that can be fixed.

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.