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

524: Add filters: for_translation sql clauses and returned rows #530

Merged
merged 9 commits into from Nov 28, 2016

Conversation

Projects
None yet
4 participants
@yoavf
Member

yoavf commented Sep 5, 2016

This PR adds a gp_for_translation_clauses filter that will enable plugins to control all the various elements of the for_translation sql query.

Fixes #524

@yoavf yoavf changed the title from Add filters: `for_translation` sql clauses, `for_translations` `$rows` to Add filters: for_translation sql clauses and returned rows Sep 5, 2016

@yoavf yoavf force-pushed the Automattic:524-translation-query-rows-filters branch from 491f5e0 to 06b7738 Sep 8, 2016

@yoavf yoavf added this to the 2.3 milestone Sep 16, 2016

$sql_sort = sprintf( $sort_by, $sort_how );
$fields = 't.*, o.*, t.id as id, o.id as original_id, t.status as translation_status, o.status as original_status, t.date_added as translation_added, o.date_added as original_added';
$join = "$join_type JOIN $wpdb->gp_translations AS t ON o.id = t.original_id AND t.translation_set_id = {$translation_set->id}";

This comment has been minimized.

@akirk

akirk Oct 13, 2016

Contributor

$translation_set->id should be cast to an integer here.

*/
$clauses = apply_filters( 'gp_for_translation_clauses', compact( 'fields', 'join', 'join_where', 'where', 'orderby', 'limit' ), $translation_set, $filters, $sort );
$fields = isset( $clauses['fields'] ) ? $clauses['fields'] : '';

This comment has been minimized.

@toolstack

toolstack Oct 25, 2016

Contributor

If there is no fields setshould this default to '*' or $fields as if it's blank it will throw a SQL error?

This comment has been minimized.

@yoavf

yoavf Oct 26, 2016

Member

It's unlikely imo that a plugin would return an incomplete clauses object intentionally, but I agree that the current default value doesn't make sense if the intention is to prevent a problem. Fixing.

$sql_sort = sprintf( $sort_by, $sort_how );
$fields = 't.*, o.*, t.id as id, o.id as original_id, t.status as translation_status, o.status as original_status, t.date_added as translation_added, o.date_added as original_added';
$join = "$join_type JOIN {$wpdb->gp_translations} AS t ON o.id = t.original_id AND t.translation_set_id = " . (int) $translation_set->id;

This comment has been minimized.

@toolstack

toolstack Oct 25, 2016

Contributor

Should this be split up between $join and $join_as to make it easier to modify?

This comment has been minimized.

@yoavf

yoavf Oct 26, 2016

Member

No strong opinion here, but I think this is good enough for most cases, and keeps the code simple.

$join = isset( $clauses['join'] ) ? $clauses['join'] : '';
$join_where = isset( $clauses['join_where'] ) ? $clauses['join_where'] : '';
$where = isset( $clauses['where'] ) ? $clauses['where'] : '';
$orderby = isset( $clauses['orderby'] ) ? $clauses['orderby'] : '';

This comment has been minimized.

@toolstack

toolstack Oct 25, 2016

Contributor

If $orderby is blank won't this throw a SQL error?

This comment has been minimized.

@yoavf

yoavf Oct 26, 2016

Member

true - fixing.

*
* @type array $fields Fields to select in the query.
* @type string $join JOIN clause of the query.
* @type string $join_where Conditions for the JOIN clause.

This comment has been minimized.

@ocean90

ocean90 Nov 28, 2016

Member

@yoavf @akirk What do you think about renaming $join_where to $join_on? It's not really a WHERE clause because it's added before WHERE:

SELECT SQL_CALC_FOUND_ROWS
t.*, o.*, t.id as id, o.id as original_id, t.status as translation_status, o.status as original_status, t.date_added as translation_added, o.date_added as original_added
FROM wp_gp_originals as o
LEFT JOIN wp_gp_translations AS t
    ON o.id = t.original_id AND t.translation_set_id = 6 AND t.status != "rejected" AND t.status != "old" AND (t.status = 'current' OR t.status = 'waiting' OR t.status = 'fuzzy')
WHERE o.project_id = 3 AND o.status = "+active"
ORDER BY o.date_added DESC
LIMIT 100 OFFSET 0

This comment has been minimized.

@yoavf

yoavf Nov 28, 2016

Member

👍 from me, makes sense

@ocean90

This comment has been minimized.

Member

ocean90 commented Nov 28, 2016

@akirk Can you please approve the changes regarding your review?

@akirk

akirk approved these changes Nov 28, 2016

@ocean90 ocean90 changed the title from Add filters: for_translation sql clauses and returned rows to 524: Add filters: for_translation sql clauses and returned rows Nov 28, 2016

@ocean90 ocean90 removed the enhancement label Nov 28, 2016

@ocean90 ocean90 merged commit c198aeb into GlotPress:develop Nov 28, 2016

2 checks passed

codecov/project 36.92% (+1.27%) compared to e06e768
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment