Allowing filtering of speakers by track

[hlashbrooke]

As discussed with Ian Dunn over email.

Currently the [speakers] shortcode shows all the published speakers. It would be helpful if you could filter them by track so that, for example, you can display your regular session speakers in a different format/location to your workshop instructors.

My solution adds a new meta field to the wcb_speaker post type called _wcpt_speaker_tracks. This is processed and added to all speakers whenever a session is saved. I have then modified the [speakers] shortcode to allow for a track parameter that accepts a comma-separated list of track slugs so only speakers in those tracks are displayed. The parameter defaults to all. In addition, I have added each track slug as class to each .wcorg-speaker div so each track's speakers can be styled differently.

[hlashbrooke]

Solution as described in ticket body

[iandunn]

Thanks Hugh, that sounds pretty good. I'm curious about the new meta field, though. Doesn't the data needed to generate the list of speakers already exists in the _wcpt_speaker_id fields (plural) on the Session posts. It seems to me that it'd be simpler and more straightforward to use that, rather than duplicating it in a new field. What do you think?

[hlashbrooke]

The reason I used a new meta field is because with the [speakers] shortcode you're doing a query using the wcb_speaker post type and the _wcpt_speaker_id meta field is saved on the wcb_session posts, so I couldn't find an intuitive way to handle that with WP_Query. I added the new meta field to make the query easier so now the shortcode now just adds a meta_query to the existing query.

The other option would have been to make the shortcode query the wcb_session posts for the sessions in the specified tracks and then take the speakers from the post meta there. The problem with that is it would not have been possible to retain the ordering parameters that the [speakers] shortcode currently accepts. That's why I went for the method in the patch. If you're happy to can the ordering parameters then I can write a new patch that does things that way instead.

[iandunn]

What about something like this?

$speaker_ids    = array();
$track_sessions = get_posts( array(
        'post_type'   => 'wcb_session',
        'numberposts' => -1,
        'tax_query'   => array(
                array(
                        'taxonomy' => 'wcb_track',
                        'field'    => 'slug',
                        'terms'    => explode( ',', $track_slugs ),
                ),
        ),
) );

foreach ( $track_sessions as $session ) {
        $session_speaker_ids = get_post_meta( $session->ID, '_wcpt_speaker_id' );
        $speaker_ids         = array_merge( $speaker_ids, $session_speaker_ids );
}

$speakers = get_posts( array(
        'post_type'   => 'wcb_speaker',
        'numberposts' => -1,
        'post__in'    => $speaker_ids,

        // order, etc parameters can still go here
) );

[hlashbrooke]

Yeah - that would work. I was trying to avoid doing multiple queries like that though as it affects performance on the frontend, which is where it counts. If that's the way you would prefer though then I can submit a new patch (or you can just add it of course) - I'm on my phone right now, so can look at it again in the morning.

[iandunn]

Yeah, I figured we can cache the end result in a transient for an hour.

[hlashbrooke]

I've updated the patch to handle the queries like you suggested - it stores the array of speaker IDs in a transient for 1 hour and I've allowed a force refresh of the transient by adding a refresh parameter to the page URL.

[hlashbrooke]

Using multiple queries, but storing data in transient

[iandunn]

Looks good :)

Instead of refreshing the transient via a URL parameter, I think we can do it automatically with something like this:

add_action( 'save_post', array( $this, 'refresh_wordcamp_speaker_ids_transient' ), 10, 2 );
public function refresh_wordcamp_speaker_ids_transient( $post, $post_raw ) {
        $relevant_post_types = array( 'wcb_speaker', 'wcb_session' );

        if ( has_shortcode( 'speakers', $post ) || in_array( $post->post_type, $relevant_post_types ) ) {
                delete_transient( 'wordcamp_speaker_ids' );
        }
}

What do you think?

Other than that, the only thing I'd mention is that we can use HOUR_IN_SECONDS instead of 3600, to make it a little more obvious.

Still waiting to hear back from Andrea, but I'll post an update when I do.

[hlashbrooke]

An issue just occurred to me here that would affect my use case. We plan on using [speakers] shortcode twice on the same page. The first instance would be [speakers track="talks"] and the second would be [speakers track="workshops"]. If we set a transient in the first instance then those same speaker IDs would be shown in the second instance, which would pretty much defeat the point of the whole thing. It would be the same if you used the shortcode on two different pages - moving from one page to the other would use the same set of speakers IDs.

I think, due to the nature of the data that is displayed, we could probably do away with the transient to be honest. WordCamps are generally 3 days long at the most (in most cases they're less), so the amount of speakers that will actually be shown isn't so drastic that the page load times will be hugely affected by doing two queries for each shortcode instance.

I'm happy to can the transient and will submit an updated patch if thats OK with you. Unless you can think of another way around the issue? If you could use wildcards in delete_transient() then it would be easy, but that's not possible at the moment.

[iandunn]

It should be easy to get around that by making sure the transient key is unique to the data, which is a best practice anyway. Something like:

$concatenated_data = $track_slugs . implode( ',', $speaker_ids );
$transient_key     = 'wordcamp_speaker_ids_' . substr( md5( $concatenated_data ), 0, 24 ); // transient keys are limited to 45 characters

Truncating the MD5 hash technically opens it up to collisions, but it's probably rare enough that it won't be an issue in reality.

[hlashbrooke]

I was thinking of something like that - the only issue is that it doesn't give you a way to force refresh the transient, but having thought about it I don't think that's entirely necessary to be honest seeing as though it will refresh every hour anyway.

Adding an updated patch below.

[hlashbrooke]

Setting transient per shortcode

[iandunn]

In 779:

WordCamp Post Types: Add 'track' parameter to [speakers] shortcode.

The id attribute on each speaker div is now deprecated, because the shortcode now supports use cases where the
shortcode is used multiple times on a single page, and assigning duplicate IDs violates the HTML spec.

Fixes #561
props hlashbrooke

[iandunn]

Sorry the for delay Hugh, but this is committed now. Thanks for the patch :)

Let me know how it works out when you use it on your site.

[hlashbrooke]

Awesome - thanks! I've been on leave for the past week, but I'm going to be looking into this again this week and will let you know if I encouter any issues.

[hlashbrooke]

Replying to iandunn:

Sorry the for delay Hugh, but this is committed now. Thanks for the patch :)

Let me know how it works out when you use it on your site.

I've tested this out on 2014.capetown and it doesn't seem to be working quite right. You can see the output here: ​http://2014.capetown.wordcamp.org/speakers/.

The 'Session Speakers' block only includes speakers in the 'talks' tracks and that's working correctly - none of the workshop instructors are appearing there. However, in the 'Workshop Instructors' section it includes everyone - the workshop instructors as well as all the speakers who have already appeared in the first section. There will be some overlap as some speakers should appear in both sections, but right now it is just showing everyone.

I'm going to have a look at the code and see what I can work out, but if this could get sorted out soon that would be awesome - we want to start selling tickets next week :)

[hlashbrooke]

OK - it's working now. Looks like it wasn't working purely because I copied and pasted the first shortcode instance and the 'track' parameter was obviously escaped incorrectly. Either way - it's sorted now and the update is working perfectly :)

[iandunn]

Glad to hear it's working. Thanks for testing it out :)

As a followup to #561 some fatal were encountered in a cron task.

It turns out that the flow is something like this:

1. Cron Load Pattern Directory
2. This kicks in, realises that it should load Gutenberg-16.8
3. The Pattern Directory code switches to the Translate.wordpress.org site, and includes the translate plugins
4. Translate.wordpress.org doesn't have the Gutenberg plugin active
   • active_plugins[ array_search() = false ] translates to active_plugins[0] = gutenberg-16.8/..., which was previously glotpress, not gutenberg.
   • It then loads the other GlotPress plugins that require GlotPress to be active first, and fatals.

Supporting loading plugins for a switched site is not something that WordPress really allows for; but is something that can be done "good enough" if the two sites are running an almost different set of plugins. For example; the Pattern Directory does it like this:
​https://github.com/WordPress/pattern-directory/blob/43e0eedfba131cd1fdfc24ac2939a48629acff2e/public_html/wp-content/plugins/pattern-translations/includes/makepot.php#L166-L189

This PR just verifies that the $from plugin is actually in the array before mangling it.

I considered using blog_id checks or checking that $check exists in the array at filter time; but this isn't enough.
Consider the following, SiteA should be filtered to gutenberg-16.8 and if the plugins for SiteB get loaded, it should also load that same version of Gutenberg to avoid conflicts.

• SiteA: gutenberg, plugin-that-causes-gutenberg-16.8-to-load.
• SiteB: gutenberg, translation-plugin

Consider the following, SiteA should be filtered to gutenberg-16.8 and if the plugins for SiteB get loaded, it should also load that same version of Gutenberg to avoid conflicts.

SiteA: gutenberg, plugin-that-causes-gutenberg-16.8-to-load.
SiteB: gutenberg, translation-plugin

This still doesn't "fix" that if the problematic plugin is network activated on SiteA, and per-site activated on SiteB..
To fix that, we could always do the add_filter() for both, if the plugin is detected in either..

The 2nd commit resolves that. Just in case gutenberg ever gets activated on the Translate site (It's network activated, so this code doesn't try to include it at present)

But then to take it a step further, should filter_the_filters be run on switch, to pick up cases like.. loading siteB and then switching to SiteA.. but if the incompatible plugin is already loaded, there's not much we can do...

I think I'm happy with this latest change, as it supports things best we currently can, and should avoid fataling except in the most edge-casey situation.