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

Ignore locale when sorting for shortcode "camptix_attendees" #1310

Open
iworks opened this issue May 2, 2024 · 6 comments
Open

Ignore locale when sorting for shortcode "camptix_attendees" #1310

iworks opened this issue May 2, 2024 · 6 comments

Comments

@iworks
Copy link

iworks commented May 2, 2024

Describe the bug

Showing attendees by camptix_attendees shortcode ignore local sorting.

To reproduce

Steps to reproduce the behavior:

  1. Go to https://krakow.wordcamp.org/2024/czym-w-ogole-jest-wordcamp/uczestnicy/
  2. Scroll down to the last atendees.
  3. See error - at the end you can se "Łukasz Jasiński" and "Łukasz Mastalski" - they should be shown after the "L" letter before "M" letter.

Expected behavior

Proper sorting by name according to the site locale.

Screenshots / Screencasts

Zrzut ekranu z 2024-05-02 12-38-27

WordCamp

If this is a problem on a specific WordCamp's site, list the site or page URL here.

@pkevan
Copy link
Contributor

pkevan commented May 7, 2024

I think this is actually due to the data storage type, as we don't do any special for sorting in locales, just on post_title which in this case is the attendee name (

public function sanitize_attendees_atts( $attr ) {
$attr = shortcode_atts(
array(
'order' => 'asc',
'orderby' => 'title',
'posts_per_page' => 10000,
'tickets' => false,
'columns' => 3,
'questions' => '',
),
$attr,
'camptix_attendees'
);
).

Unsure how we've done this before, but I think we'd have to change that in the database (which might require some assistance to do so) and there is a chance of data loss.

@dd32 have you known us do this before?

@pkevan
Copy link
Contributor

pkevan commented May 7, 2024

Actually - if we sort by polish collation (utf8mb4_polish_ci) then the results look correct, but unsure if we can push this through wp_query.

@pkevan
Copy link
Contributor

pkevan commented May 7, 2024

Ah - this isn't possible through WP Query, due to the santitization of the orderby parameter: https://github.com/WordPress/wordpress-develop/blob/6.5/src/wp-includes/class-wp-query.php#L1663-L1685

@dd32
Copy link
Member

dd32 commented May 7, 2024

I think this is actually due to the data storage type

Yup, because it's stored as utf8mb4_unicode_ci it's going to be sorted by that collation - Which apparently uses an older set of weight keys.

I think we'd have to change that in the database

My initial response is "noooo". However, it turns out that core does use the utf8mb4_unicode_520_ci collation when possible:
https://github.com/WordPress/wordpress-develop/blob/473e2554db8e547a07a16f73080ca49c0c30b89f/src/wp-includes/class-wpdb.php#L894-L897

(utf8mb4_unicode_ci uses v4 utf8 weighting keys for sort, utf8mb4_unicode_520_ci uses v5.2 utf8 weighted sorts)

AFAIK we don't use that, because a combination of HyperDB + how utf8mb4 is forced on..

Upon looking into it, $wpdb->has_cap( 'utf8mb4_520' ) returns false on WordCamp. But HyperDB appears to support it But it turns out not to, because $wpdb->db_version() return 5.5.5 as WordPress/wordpress-develop@fed98bd is not applied to HyperDB..

The above won't "fix" this though; it'll just let new tables be created using 520 AFAIK, we could adjust existing tables though.

isn't possible through WP Query

This would be possible via the posts_request SQL filter, for example, the following query sorts as you'd expect:

SELECT post_title FROM wc_posts WHERE post_type = 'tix_attendee' ORDER BY post_title COLLATE 'utf8mb4_unicode_520_ci' ASC

@pkevan
Copy link
Contributor

pkevan commented May 8, 2024

Thanks @dd32 - I think I prefer the non-update options via the posts_request filter.

We'd need to adjust the query here:

$attendee_args = apply_filters(
'camptix_attendees_shortcode_query_args',
array_merge(
array(
'post_type' => 'tix_attendee',
'posts_per_page' => 200,
'post_status' => array( 'publish', 'pending' ),
'paged' => $paged,
'order' => $attr['order'],
'orderby' => $attr['orderby'],
'fields' => 'ids', // ! no post objects
'cache_results' => false,
),
$query_args
),
$attr
);
$attendees = get_posts( $attendee_args );
to include a suppress_filters to false since the filter wouldn't run, but otherwise this could work.

@dd32
Copy link
Member

dd32 commented May 9, 2024

to include a suppress_filters to false

I forgot get_posts sets that by default.. As cray-cray as it sounds, if that introduces problems, I would then say use the query filter..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants