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

Opengraph integration for Yoast SEO 14.0 #491

Closed
wants to merge 8 commits into from
Closed

Opengraph integration for Yoast SEO 14.0 #491

wants to merge 8 commits into from

Conversation

Chouby
Copy link
Contributor

@Chouby Chouby commented Apr 6, 2020

Yoast SE0 changes the way to handle Opengraph tags in the version 14.0. This PR adapts our compatibility to this new way. The old method is kept for backward compatibility with version older than 14.0, at least for a while.

It also re-introduces integation tests, although for we now use the action wpseo_head instead of the action wp_head in former version (see b5686e2).

Fix #487

@Chouby Chouby added this to the 2.7 milestone Apr 6, 2020
as wp.org still provides the RC1.
@Chouby Chouby requested a review from manooweb April 6, 2020 06:39
@Chouby Chouby modified the milestones: 2.7, 2.7.x Apr 6, 2020
@jdevalk
Copy link

jdevalk commented Apr 28, 2020

Are you anywhere near releasing this? We've released Yoast SEO 14.0 and getting customer complaints.

@Chouby
Copy link
Contributor Author

Chouby commented Apr 28, 2020

@jdevalk Thank you for the heads-up.

I have an issue with the phpunit integration test. It was working with the version 14.0-rc6, but doesn't work anymore with the final version. Not sure what change woudl have caused this.

@Chouby
Copy link
Contributor Author

Chouby commented Apr 28, 2020

I believed that I had found the culprit in the migration status and setting the relevant option did fix the error on my local environment.
But now the test fails with this errors:
PDOException: SQLSTATE[42S02]: Base table or view not found: 1146 Table 'wordpress_test.wptests_yoast_indexable' doesn't exist.
Have you an idea of what's happening?

@jdevalk
Copy link

jdevalk commented Apr 28, 2020

Hey @Chouby due to some other issues we're completely removing the PDO requirement.

If you check out replace-orm-with-wpdb-version from wordpress-seo you should be able to work with that.

@Chouby
Copy link
Contributor Author

Chouby commented Apr 29, 2020

I have restested against 14.0.2-rc1. The test doesn't fail anymore. However I now get database errors:

WordPress database error Table 'wordpress_test.wptests_yoast_indexable' doesn't exist for query SELECT * FROM `wptests_yoast_indexable` WHERE `object_type` = 'home-page' LIMIT 1 made by PHPUnit_TextUI_Command::main, PHPUnit_TextUI_Command->run, PHPUnit_TextUI_TestRunner->doRun, PHPUnit_Framework_TestSuite->run, PHPUnit_Framework_TestSuite->run, PHPUnit_Framework_TestSuite->run, PHPUnit_Framework_TestCase->run, PHPUnit_Framework_TestResult->run, PHPUnit_Framework_TestCase->runBare, PHPUnit_Framework_TestCase->runTest, ReflectionMethod->invokeArgs, WPSEO_Test->test_opengraph, do_action('wpseo_head'), WP_Hook->do_action, WP_Hook->apply_filters, Yoast\WP\SEO\Integrations\Front_End_Integration->present_head, Yoast\WP\SEO\Memoizers\Meta_Tags_Context_Memoizer->for_current_page, Yoast\WP\SEO\Repositories\Indexable_Repository->for_current_page, Yoast\WP\SEO\Repositories\Indexable_Repository->find_for_home_page, Yoast\WP\Lib\ORM->find_one, Yoast\WP\Lib\ORM->_run, Yoast\WP\Lib\ORM::_execute
WordPress database error Table 'wordpress_test.wptests_yoast_indexable' doesn't exist for query SELECT * FROM `wptests_yoast_indexable` WHERE `object_type` = 'home-page' LIMIT 1 made by PHPUnit_TextUI_Command::main, PHPUnit_TextUI_Command->run, PHPUnit_TextUI_TestRunner->doRun, PHPUnit_Framework_TestSuite->run, PHPUnit_Framework_TestSuite->run, PHPUnit_Framework_TestSuite->run, PHPUnit_Framework_TestCase->run, PHPUnit_Framework_TestResult->run, PHPUnit_Framework_TestCase->runBare, PHPUnit_Framework_TestCase->runTest, ReflectionMethod->invokeArgs, WPSEO_Test->test_opengraph, do_action('wpseo_head'), WP_Hook->do_action, WP_Hook->apply_filters, Yoast\WP\SEO\Integrations\Front_End_Integration->present_head, Yoast\WP\SEO\Memoizers\Meta_Tags_Context_Memoizer->for_current_page, Yoast\WP\SEO\Repositories\Indexable_Repository->for_current_page, Yoast\WP\SEO\Repositories\Indexable_Repository->find_for_home_page, Yoast\WP\Lib\ORM->find_one, Yoast\WP\Lib\ORM->_run, Yoast\WP\Lib\ORM::_execute
WordPress database error Table 'wordpress_test.wptests_yoast_indexable' doesn't exist for query INSERT INTO `wptests_yoast_indexable` (`object_type`, `title`, `breadcrumb_title`, `permalink`, `blog_id`, `description`, `is_robots_noindex`, `open_graph_title`, `open_graph_image`, `open_graph_image_id`, `open_graph_description`, `permalink_hash`, `created_at`, `updated_at`) VALUES ('home-page', '%%sitename%% %%page%% %%sep%% %%sitedesc%%', 'Home', 'http://example.org/', '1', 'Just another WordPress site', '0', '', '', '', '', '19:d0d92cdc9b30c1300fbc3c7ebd304be0', '2020-04-29 12:12:15', '2020-04-29 12:12:15') made by PHPUnit_TextUI_Command::main, PHPUnit_TextUI_Command->run, PHPUnit_TextUI_TestRunner->doRun, PHPUnit_Framework_TestSuite->run, PHPUnit_Framework_TestSuite->run, PHPUnit_Framework_TestSuite->run, PHPUnit_Framework_TestCase->run, PHPUnit_Framework_TestResult->run, PHPUnit_Framework_TestCase->runBare, PHPUnit_Framework_TestCase->runTest, ReflectionMethod->invokeArgs, WPSEO_Test->test_opengraph, do_action('wpseo_head'), WP_Hook->do_action, WP_Hook->apply_filters, Yoast\WP\SEO\Integrations\Front_End_Integration->present_head, Yoast\WP\SEO\Memoizers\Meta_Tags_Context_Memoizer->for_current_page, Yoast\WP\SEO\Repositories\Indexable_Repository->for_current_page, Yoast\WP\SEO\Repositories\Indexable_Repository->find_for_home_page, Yoast\WP\SEO\Builders\Indexable_Builder->build_for_home_page, Yoast\WP\SEO\Builders\Indexable_Builder->save_indexable, Yoast\WP\SEO\Models\Indexable->save, Yoast\WP\Lib\Model->save, Yoast\WP\Lib\ORM->save, Yoast\WP\Lib\ORM::_execute

@jdevalk
Copy link

jdevalk commented Apr 29, 2020

We've just released 14.0.2, can you test against that? We've fixed a lot since RC1.

@Chouby
Copy link
Contributor Author

Chouby commented Apr 29, 2020

Sorry. I did not see that 14.0.2 was released, but in fact it looks like the tests were ran against 14.0.2. Our tests always use https://downloads.wordpress.org/plugin/wordpress-seo.zip which should correspond to the trunk branch.

I just restarted them to be sure but I still get the database error (even if tests don't fail). Here is the Travis result https://travis-ci.org/github/polylang/polylang/builds/680693875

@jdevalk
Copy link

jdevalk commented Apr 29, 2020

Looks like that build passed?

@Chouby
Copy link
Contributor Author

Chouby commented Apr 29, 2020

Yes. The tests all pass. But if you look at the reports, you will see the database errors. Not sure why phpunit doesn't mark the test as errored in such case.

@jdevalk
Copy link

jdevalk commented Apr 29, 2020

What you could do is run a wp cli command before running your tests:

wp yoast index

this would index the entire database.

@Chouby
Copy link
Contributor Author

Chouby commented Apr 30, 2020

Is there a way to create this table with a PHP function? I looked at _wpseo_activate() and found the method to create the tables yoast_seo_links and yoast_seo_meta but unless I missed something, it looks like yoast_indexable is not created there.

@jdevalk
Copy link

jdevalk commented Apr 30, 2020

It is created from PHP, through our Ruckusing migrations.

You should be able to run do_action( '_yoast_run_migrations' );

Not that that is an internal API, it might break at some point.

@Chouby
Copy link
Contributor Author

Chouby commented Apr 30, 2020

It doesn't work. I suppose that the Migration_Runner class is not instantiated and thus nothing is hooked to this action.

What I don't understand is that I had this table in my local test environment. So at some point - I suppose when I tested all the RC - the table was created and this explains why the test was ok locally. On a fresh install as in Travis tests, the table is not created anymore.

_wpseo_activate() has always been present in my setUp(). Was the creation of the table done by _wpseo_activate() in a previous RC?

@yunusbayraktaroglu
Copy link

FYI, after updating 14.0.4, my yoast breadcrumb translations and URL's are totally messed up.

@bluisier
Copy link

bluisier commented May 15, 2020

I'm not exactly sure it's related to this issue but I have Polylang 2.7.2 and Yoast seo 14.1 installed on my different websites. On one of them, I have the option "Your homepage displays" set as "your latest posts". On the english version of my homepage my metas are notre translated (https://www.novo-monde.com/en/) event if they are translated in my section "string translations".

I'm also missing the string translations for the yoast social media settings for facebook on the front page (title and description)

And finally, since I updated to 14.1, I'm missing the og tag <meta property=”og:locale:alternate” content=”...”> Not sure if it is intended or not

@Chouby Chouby removed this from the 2.7.x milestone May 15, 2020
@Chouby
Copy link
Contributor Author

Chouby commented May 15, 2020

@bluisier The first part of your issue is not related. It is is due to Yoast having broken their own integration with WPML and Polylang since Yoast 14.0. See also my comment here: #505 (comment)

This PR is to update our own integration for Opengraph only, the fact that you are miising the <meta property=”og:locale:alternate” content=”...”>. But it is currently blocked because we don't have a way to correctly test this integration code. The tests are necessary to detect when Yoast modifies it code.

@Chouby Chouby changed the title Compatibility with Yoast SEO 14.0 Opengraph integration for Yoast SEO 14.0 May 15, 2020
As they break again with a new error in Yoast 14.1:
WordPress database error Can't reopen table: 'wyi' for query DELETE wyi
FROM wptests_yoast_indexable wyi INNER JOIN wptests_yoast_indexable wyi2
WHERE wyi2.object_id = wyi.object_id AND wyi2.object_type =
wyi.object_type AND wyi2.id < wyi.id;
@Chouby
Copy link
Contributor Author

Chouby commented May 20, 2020

Closed in favor of #509.

@Chouby Chouby closed this May 20, 2020
@Chouby Chouby deleted the issue#487 branch May 20, 2020 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Opengraph integration broken by Yoast SEO 14.0
4 participants