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.

Sign up for GitHub

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

Fix/219 clarify filter names #320

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Fix/219 clarify filter names #320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
81090bc
Add tests for creating empty translations
simonwheatley Aug 7, 2015
ec98986
Change filter names to make consistent with the rest of Babble
simonwheatley Aug 7, 2015
be65a9c
Change method name to `create_empty_translations`
simonwheatley Aug 7, 2015
946e81b
Call the method to create empty translations during the test
simonwheatley Aug 13, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions class-jobs.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public function __construct() {
add_action( 'add_meta_boxes', array( $this, 'add_meta_boxes' ) );
add_action( 'add_meta_boxes_bbl_job', array( $this, 'add_meta_boxes_bbl_job' ), 999 );
add_action( 'admin_init', array( $this, 'admin_init' ) );
add_action( 'babble_create_empty_translation', array( $this, 'create_empty_translation' ) );
add_action( 'bbl_cron_create_empty_translations', array( $this, 'create_empty_translations' ) );
add_action( 'bbl_translation_post_meta_boxes', array( $this, 'bbl_translation_post_meta_boxes' ), 10, 3 );
add_action( 'bbl_translation_submit_meta_boxes', array( $this, 'bbl_translation_submit_meta_boxes' ), 10, 2 );
add_action( 'bbl_translation_terms_meta_boxes', array( $this, 'bbl_translation_terms_meta_boxes' ), 10, 2 );
Expand Down Expand Up @@ -1081,11 +1081,11 @@ public function get_job_objects( $job ) {
}

/**
* Create empty translations of a post for all languages. Called via WP-Cron on the `babble_create_empty_translation` hook.
* Create empty translations of a post for all languages. Called via WP-Cron on the `bbl_cron_create_empty_translations` hook.
*
* @param array $args Args array containing a `post_id` element.
*/
public function create_empty_translation( array $args ) {
public function create_empty_translations( array $args ) {
global $bbl_post_public;

if ( !$post = get_post( $args['post_id'] ) ) {
Expand Down Expand Up @@ -1130,9 +1130,9 @@ public function create_post_jobs( $post_id, array $lang_codes ) {
if ( bbl_get_default_lang_code() == $lang_code )
continue;

if ( apply_filters( 'bbl_create_empty_translation', false, $post ) ) {
if ( apply_filters( 'bbl_trigger_create_empty_translations', false, $post ) ) {

wp_schedule_single_event( time(), 'babble_create_empty_translation', array(
wp_schedule_single_event( time(), 'bbl_cron_create_empty_translations', array(
array(
'post_id' => $post->ID,
)
Expand Down
3 changes: 3 additions & 0 deletions class-post-public.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1298,6 +1298,9 @@ public function get_shadow_post_types( $base_post_type ) {
* Returns the post in a particular language, or the fallback content
* if there's no post available.
*
* @FIXME: Currently this function is broken by Babble_Jobs::create_empty_translations
* See: test_canonical_content_fallback
*
* @param int|WP_Post $post Either a WP Post object, or a post ID
* @param string $lang_code The language code for the required language
* @param boolean $fallback If true: if a post is not available, fallback to the default language content (defaults to true)
Expand Down
14 changes: 14 additions & 0 deletions tests/test-translations.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,4 +133,18 @@ public function test_term_translations() {

}

public function test_canonical_content_fallback() {
$this->assertSame( 'en_US', get_locale() );

$en = $this->factory->post->create_and_get();
// In normal operation, the create_empty_translations method is called on an immediate single cron job
$GLOBALS['bbl_jobs']->create_empty_translations($en->ID);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need changing to the following when #312 is merged:

Babble::get( 'jobs' )->create_empty_translations( $en->ID );

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

$fr = bbl_get_post_in_lang( $en->ID, 'fr_FR', true );

// @FIXME: These tests fail due to the interaction of Babble_Post_Public::get_post_in_lang and Babble_Jobs::create_empty_translations
$this->assertSame( $en->post_title, $fr->post_title );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@simonwheatley looking through the code, this test seems to be expecting the title to default to the default post, however what I understand, there is only logic in place to fallback to the default lang if a post in that language does not exist. In this case, we do have a translated post in fr_FR so no "fallback" of content will happen. See https://github.com/Automattic/babble/blob/develop/class-post-public.php#L1337

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joehoyle Originally fallback content was envisaged as more granular than that within a single post, so title, content, and excerpt can all fallback independently of each other:

babble/class-post-public.php

Lines 545 to 571 in 3d211c8

// Get fallback content in the default language
$subs_index = array();
foreach ( $posts as & $post ) {
if ( empty( $post->post_title ) || empty( $post->post_excerpt ) || empty( $post->post_content ) ) {
if ( $default_post = $this->get_default_lang_post( $post->ID ) )
$subs_index[ $post->ID ] = $default_post->ID;
}
if ( ! $this->get_transid( $post ) && bbl_get_default_lang_code() == bbl_get_post_lang_code( $post ) )
$this->set_transid( $post );
}
if ( ! $subs_index )
return $posts;
// @FIXME: Alternative approach: hook on save_post to save the current value to the translation, BUT content could get out of date – in post_content_filtered
foreach ( $posts as & $post ) {
// @TODO why does this only override the title/excerpt/content? Why not override the post object entirely?
if( isset( $subs_index[ $post->ID ] ) ) {
$default_post = get_post( $subs_index[ $post->ID ] );
if ( empty( $post->post_title ) )
$post->post_title = $default_post->post_title;
if ( empty( $post->post_excerpt ) )
$post->post_excerpt = $default_post->post_excerpt;
if ( empty( $post->post_content ) )
$post->post_content = $default_post->post_content;
}
}
return $posts;

$this->assertSame( $en->post_content, $fr->post_content );

}

}
2 changes: 1 addition & 1 deletion translation-show-pre-translation.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@

*/

add_action( 'bbl_create_empty_translation', '__return_true' );
add_action( 'bbl_trigger_create_empty_translations', '__return_true' );