WordPress.org

Making WordPress.org

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#561 closed enhancement (worksforme)

Allowing filtering of speakers by track

Reported by: hlashbrooke Owned by: iandunn
Milestone: Priority: normal
Component: WordCamp Site & Plugins Keywords: has-patch
Cc:

Description

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.

Attachments (3)

561.diff (4.4 KB) - added by hlashbrooke 5 years ago.
Solution as described in ticket body
561.2.diff (3.6 KB) - added by hlashbrooke 5 years ago.
Using multiple queries, but storing data in transient
561.3.diff (4.1 KB) - added by hlashbrooke 5 years ago.
Setting transient per shortcode

Download all attachments as: .zip

Change History (19)

@hlashbrooke
5 years ago

Solution as described in ticket body

#1 @iandunn
5 years ago

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?

#2 @hlashbrooke
5 years ago

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.

#3 @iandunn
5 years ago

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
) );

#4 @hlashbrooke
5 years ago

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.

#5 @iandunn
5 years ago

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

#6 @hlashbrooke
5 years ago

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
5 years ago

Using multiple queries, but storing data in transient

#7 @iandunn
5 years ago

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.

#8 @hlashbrooke
5 years ago

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.

#9 @iandunn
5 years ago

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.

#10 @hlashbrooke
5 years ago

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
5 years ago

Setting transient per shortcode

#11 @iandunn
5 years ago

  • Owner set to iandunn
  • Resolution set to fixed
  • Status changed from new to closed

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

#12 follow-up: @iandunn
5 years ago

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.

#13 @hlashbrooke
5 years ago

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.

#14 in reply to: ↑ 12 @hlashbrooke
5 years ago

  • Keywords needs-refresh added
  • Resolution fixed deleted
  • Status changed from closed to reopened

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 :)

#15 @hlashbrooke
5 years ago

  • Keywords needs-refresh removed
  • Resolution set to worksforme
  • Status changed from reopened to closed

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 :)

#16 @iandunn
5 years ago

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

Note: See TracTickets for help on using tickets.