Add filtering of lesson plans by language #1500

adamwoodnz · 2023-04-18T23:56:19Z

Closes #874

This PR adds admin and frontend filtering of Lesson Plans, expanding on the functionality previously added for Tutorials.

Screenshots

Admin	Frontend	Cards

How to test

IMPORTANT before you start

In the Lesson Plan list if the Language field displays English [en_US] it is most likely that the Lesson Plan actually has no meta language set and the column is displaying the default value. The easiest way to fix this is to bulk edit all these entries and set them to English. This has already been done in prod, so if you're testing in sandbox things should just work.

Admin

Ensure you have Lesson Plans with multiple languages in your environment
In the admin, navigate to the Lesson Plan list. Check that the filter language dropdown has options covering all the items listed. If you find there is no English option, ensure you have read the note above.
Filter the admin list. Changing post status should update the languages in the filter dropdown.
Add a new Lesson Plan and leave the language as the default
Navigate to the Tutorial list page. Smoke test that the filtering still works there, matching the behaviour for Lesson Plans.

Frontend

Open the frontend Lesson Plan archive at http://localhost:8888/lesson-plans/?_view=all
Note the new language filter at the top of the sidebar
Test filtering and check the results against the admin
Check that the cards in the left have the correct language displayed
Filter by English [en_US] and check that the Lesson Plan you created in step 4 above is displayed
Test the 'Clear all filters' action. You should stay on the archive page and not be redirected to the Lesson Plan landing page.
Navigate to the Tutorials archive and smoke test filtering using the language and subtitle dropdowns

adamwoodnz · 2023-04-20T00:27:40Z

wp-content/plugins/wporg-learn/inc/post-meta.php

+	// This language meta field is rendered in the editor sidebar using a PluginDocumentSettingPanel block,
+	// which won't save the field on publish if it has the default value.
+	// Our filtering by locale depends on it being set, so we force it to be updated after saving:
+	$language         = get_post_meta( $post_id, 'language', true );
+	$language_default = 'en_US';
+	if ( ! isset( $language ) || $language_default === $language ) {
+		update_post_meta( $post_id, 'language', $language_default );
+	}


This follows the same approach that we had to take with Tutorials: https://github.com/WordPress/Learn/blob/trunk/wp-content/plugins/wporg-learn/inc/post-meta.php#L525

$language can be read as the default value of en_US even if it's not set, hence forcing save in this case as well, on line 362.

It should stay on the archive page rather than redirecting to the landing page

Enables admin page post type filtering to also filter the locale list

jonathanbossenger · 2023-04-20T12:32:04Z

Nice work @adamwoodnz I've made a note to review this ASAP.

iandunn

Looks great! I left a minor suggestion, but I don't feel strongly about it.

iandunn · 2023-04-21T21:03:41Z

wp-content/plugins/wporg-learn/inc/post-meta.php

+	$results = isset( $post_status )
+		? $wpdb->get_col( $wpdb->prepare(
+			"
+				SELECT DISTINCT postmeta.meta_value
+				FROM {$wpdb->postmeta} postmeta
+					JOIN {$wpdb->posts} posts ON posts.ID = postmeta.post_id AND posts.post_type = %s AND posts.post_status = %s 
+				WHERE postmeta.meta_key = %s
+			",
+			$post_type,
+			$post_status,
+			$meta_key
+		) )
+		: $wpdb->get_col( $wpdb->prepare(
+			"
+				SELECT DISTINCT postmeta.meta_value
+				FROM {$wpdb->postmeta} postmeta
+					JOIN {$wpdb->posts} posts ON posts.ID = postmeta.post_id AND posts.post_type = %s 
+				WHERE postmeta.meta_key = %s
+			",
+			$post_type,
+			$meta_key
+		) );


I worry that duplicating the query here could lead to them accidentally getting out of sync, and it seems less readable to me.

What do you think about integrating $post_status into the query conditionally? That seems like the common approach to me. Here's a few examples:

https://github.com/WordPress/wordcamp.org/blob/e3638a232799b31827d4cb107ec503919b0b04ff/public_html/wp-content/plugins/wordcamp-payments-network/includes/payment-requests-list-table.php#L50-L109

https://github.com/WordPress/WordPress/blob/ead3a2ec81f88842c2cba8b8a693871d18522104/wp-includes/canonical.php#L962-L964

https://github.com/WordPress/WordPress/blob/ead3a2ec81f88842c2cba8b8a693871d18522104/wp-includes/class-wp-query.php#L2536-L2537

https://github.com/WordPress/WordPress/blob/ead3a2ec81f88842c2cba8b8a693871d18522104/wp-includes/canonical.php#L940-L950

Yeah it's a fair point. I was trying to remove the need for phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared, but it seems to be common practice. Changed to something closer resembling the previous code, and updated the comment.

jonathanbossenger

The functionality works great, nice one @adamwoodnz

adamwoodnz · 2023-04-26T23:26:23Z

wp-content/plugins/wporg-learn/inc/post-meta.php

 	global $wpdb;

 	$and_post_status = '';
-	if ( $published_only ) {
-		$and_post_status = "AND posts.post_status = 'publish'";
+	if ( in_array( $post_status, get_post_stati(), true ) ) {


@iandunn are you satisfied with the security of this approach? $post_status is passed in from INPUT_GET here.

This line attempts to ensure that only safe strings matching post_status values in the system are included in the query.

post_status is also only sourced from INPUT_GET in the admin. On the frontend post_status is always hard coded to publish.

Yeah, that looks like a great way to guarantee it's a valid value 👍🏻

iandunn

Looks good!

iandunn · 2023-05-01T15:09:00Z

wp-content/plugins/wporg-learn/inc/post-meta.php

 *
 * @return array
 */
-function get_available_workshop_locales( $meta_key, $label_language = 'english', $published_only = true ) {
+function get_available_post_type_locales( $meta_key, $post_type, $label_language = 'english', $post_status ) {


I get this notice on PHP 8.0.28. We don't run that on prod yet, but will in the near future, so it'd be good to be prepared now IMO.

PHP Deprecated: Required parameter $post_status follows optional parameter $label_language in wp-content/plugins/wporg-learn/inc/post-meta.php on line 269

Fixed in 4cb6519

iandunn · 2023-05-01T15:13:17Z

wp-content/plugins/wporg-learn/inc/post-meta.php

 	global $wpdb;

 	$and_post_status = '';
-	if ( $published_only ) {
-		$and_post_status = "AND posts.post_status = 'publish'";
+	if ( in_array( $post_status, get_post_stati(), true ) ) {


Yeah, that looks like a great way to guarantee it's a valid value 👍🏻

Optional parameter should be last in PHP 8

adamwoodnz self-assigned this Apr 18, 2023

adamwoodnz force-pushed the add/874-lesson-plans-filter-by-language branch from 948c26f to 226c779 Compare April 19, 2023 02:30

adamwoodnz commented Apr 20, 2023

View reviewed changes

adamwoodnz force-pushed the add/874-lesson-plans-filter-by-language branch from 019928f to d181ba8 Compare April 20, 2023 02:06

adamwoodnz marked this pull request as ready for review April 20, 2023 02:08

adamwoodnz requested review from jonathanbossenger and iandunn April 20, 2023 02:08

adamwoodnz force-pushed the add/874-lesson-plans-filter-by-language branch from d181ba8 to ca29dbd Compare April 20, 2023 03:03

adamwoodnz added 7 commits April 20, 2023 15:10

Fix clear all filters button

99f0cdd

It should stay on the archive page rather than redirecting to the landing page

Expand get_available_workshop_locales to work for any post type

8417db5

Add language filter to Lesson Plan archive

7b942e6

Add language to lesson plan meta on cards

085e011

Add admin table language filter for Lesson Plans

13ab2e4

Filter locales by any post type

463ea9d

Enables admin page post type filtering to also filter the locale list

Force the default meta language value to be saved

4c1afee

adamwoodnz force-pushed the add/874-lesson-plans-filter-by-language branch from ca29dbd to 4c1afee Compare April 20, 2023 03:10

adamwoodnz added [Type] Enhancement New feature request for the Learn website. [Component] Learn Theme Website development issues related to the Learn theme. [Component] Learn Plugin Website development issues related to the Learn plugin. labels Apr 21, 2023

iandunn approved these changes Apr 21, 2023

View reviewed changes

jonathanbossenger approved these changes Apr 24, 2023

View reviewed changes

adamwoodnz force-pushed the add/874-lesson-plans-filter-by-language branch from 32b84ce to b36d679 Compare April 26, 2023 02:27

Remove duplicated query

6f881b8

adamwoodnz force-pushed the add/874-lesson-plans-filter-by-language branch from b36d679 to 6f881b8 Compare April 26, 2023 02:31

Use built in function to get post statuses

6807634

adamwoodnz marked this pull request as draft April 26, 2023 02:37

adamwoodnz commented Apr 26, 2023

View reviewed changes

adamwoodnz marked this pull request as ready for review April 26, 2023 23:29

adamwoodnz requested a review from iandunn April 26, 2023 23:29

iandunn approved these changes May 1, 2023

View reviewed changes

Update get_available_post_type_locales parameter order

4cb6519

Optional parameter should be last in PHP 8

adamwoodnz merged commit 83713e4 into trunk May 3, 2023
1 check passed

adamwoodnz deleted the add/874-lesson-plans-filter-by-language branch May 3, 2023 23:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add filtering of lesson plans by language #1500

Add filtering of lesson plans by language #1500

adamwoodnz commented Apr 18, 2023 •

edited

adamwoodnz Apr 20, 2023 •

edited

jonathanbossenger commented Apr 20, 2023

iandunn left a comment

iandunn Apr 21, 2023

adamwoodnz Apr 26, 2023

jonathanbossenger left a comment

adamwoodnz Apr 26, 2023

adamwoodnz Apr 26, 2023 •

edited

iandunn May 1, 2023

iandunn left a comment

iandunn May 1, 2023

adamwoodnz May 2, 2023

iandunn May 1, 2023

Add filtering of lesson plans by language #1500

Add filtering of lesson plans by language #1500

Conversation

adamwoodnz commented Apr 18, 2023 • edited

Screenshots

How to test

IMPORTANT before you start

Admin

Frontend

adamwoodnz Apr 20, 2023 • edited

Choose a reason for hiding this comment

jonathanbossenger commented Apr 20, 2023

iandunn left a comment

Choose a reason for hiding this comment

iandunn Apr 21, 2023

Choose a reason for hiding this comment

adamwoodnz Apr 26, 2023

Choose a reason for hiding this comment

jonathanbossenger left a comment

Choose a reason for hiding this comment

adamwoodnz Apr 26, 2023

Choose a reason for hiding this comment

adamwoodnz Apr 26, 2023 • edited

Choose a reason for hiding this comment

iandunn May 1, 2023

Choose a reason for hiding this comment

iandunn left a comment

Choose a reason for hiding this comment

iandunn May 1, 2023

Choose a reason for hiding this comment

adamwoodnz May 2, 2023

Choose a reason for hiding this comment

iandunn May 1, 2023

Choose a reason for hiding this comment

adamwoodnz commented Apr 18, 2023 •

edited

adamwoodnz Apr 20, 2023 •

edited

adamwoodnz Apr 26, 2023 •

edited