Making WordPress.org

Opened 8 years ago

Closed 8 years ago

#2850 closed enhancement (fixed)

Introducing a custom taxonomy "groups" for the speakers shortcode

Reported by: kau-boy's profile Kau-Boy Owned by: coreymckrill's profile coreymckrill
Milestone: Priority: normal
Component: WordCamp Site & Plugins Keywords: dev-feedback has-patch
Cc:

Description

On WordCamps with multiple days and different session formats, the organizers might want to group the long list of speakers into several group. For WCEU for example, we want to have a list of "full session speakers", "lightning talk speakers" and "Contributor Day" speakers.

I've wrote a patch which will add new custom taxonomy "wcb_speaker_group" and which introduces a new shortcode attribute "groups". Just as with the new organizers team taxonomy, the shortcode will by default list all speakers. But using the new attribute, the organizing team can filter the list, which will enable them to create a structures page using multiple shortcodes and some headlines in between.

Attachments (3)

2850.diff (2.1 KB) - added by Kau-Boy 8 years ago.
2850.2.diff (4.3 KB) - added by Kau-Boy 8 years ago.
2850.3.diff (2.6 KB) - added by Kau-Boy 8 years ago.

Download all attachments as: .zip

Change History (12)

@Kau-Boy
8 years ago

#1 @coreymckrill
8 years ago

  • Keywords has-patch added; needs-patch removed
  • Owner set to coreymckrill
  • Status changed from new to accepted

#2 @coreymckrill
8 years ago

I think this is a good idea. Here are some suggestions for the patch:

  • When I was looking at the patch in the context of the whole file, I noticed some legacy code that was restricting the track parameter for the speakers shortcode to a particular site. I removed the restriction, and also added some sanitization for it in r5545. I recommend you do some similar sanitizing for the group parameter, since it is also a comma separated list of term slugs.
  • Also, I think the tax_query is in the wrong place. Your patch adds it to the args for a query of sessions, which happens before the query that gets the speakers. Since this is for a taxonomy that applies to speakers, I suspect tax_query should be included in the args for that query instead.
  • I can imagine speaker groups being useful for other contexts outside of the shortcode. With this in mind, I think the taxonomy should probably be included in REST API responses. Take a look at the register parameters for the wcb_sponsor_level taxonomy for an example.
  • group is a very generic term. Despite the fact that the organizer teams taxonomy also uses the extremely generic team for its rewrite and query_var, I think we should namespace group at least a little bit. How about speaker_group instead?

@Kau-Boy
8 years ago

#3 follow-up: @Kau-Boy
8 years ago

Thanks for your feedback @coreymckrill!

  • I used the same sanitization as you have done for tracks
  • The tax_query was indeed on the wrong place. As my VVV just decided yesterday, to not start MariaDB anymore, I couldn't test it :(
  • I have never added a REST API parameter for a taxonomy and the sponsors CPT looks a lot more complex. I would be glad if we could work on that together at the WCEU Contributor Day, but it shouldn't be a blocker for this ticket, don't you think?
  • The term groups is generic, but as it's only used as the shortcode args name, it should not conflict with anything. Or am I missing something? The rewrite slug for the taxonomy is already speaker-group, so that should be safe.

#4 @coreymckrill
8 years ago

@Kau-Boy looks like 2850.2.diff includes some stuff meant for #2849 ...

@Kau-Boy
8 years ago

#5 @Kau-Boy
8 years ago

@coreymckrill yeah, I just realized that a second ago and wanted to post a new patch. Looks like you were faster ;)

#6 in reply to: ↑ 3 @coreymckrill
8 years ago

Replying to Kau-Boy:

  • I used the same sanitization as you have done for tracks

Looks good!

  • I have never added a REST API parameter for a taxonomy and the sponsors CPT looks a lot more complex. I would be glad if we could work on that together at the WCEU Contributor Day, but it shouldn't be a blocker for this ticket, don't you think?

Agreed, it's not a blocker. I'd be happy to work on it with you at WCEU Contributor Day, though I don't think it will be complex. At first glance it looks like you only need to add a couple of parameters to the register_taxonomy call.

  • The term groups is generic, but as it's only used as the shortcode args name, it should not conflict with anything. Or am I missing something? The rewrite slug for the taxonomy is already speaker-group, so that should be safe.

It looks like the query_var is also set to group. Also, this is very minor, but to be consistent with the other types and taxonomies, I think it would be better if the rewrite slug was speaker_group instead of speaker-group.

#7 @Kau-Boy
8 years ago

When I added the teams taxonomy, I oriented on the attributes from this page: https://make.wordpress.org/community/handbook/wordcamp-organizer/first-steps/web-presence/custom-tools-for-building-wordcamp-content/

I thought something generic would not hurt. But if you thinks something like speaker_group would be better, feel free to commit it like this ;)

As my meta environment is broken, could you test it and update the patch if necessary?

P.S. And if you think the REST API attributes are easy to implement, go ahead and add them. I can than learn from your commit :)

#8 @Kau-Boy
8 years ago

The group shortcode argument should probably something plural though, so it's esasier to recognize, you can use a comma separated list of slugs.

#9 @coreymckrill
8 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 5546:

WordCamp Post Types: Add groups taxonomy to Speakers PT

Adds the wcb_speaker_group taxonomy to the wcb_speaker post type.
Makes the new taxonomy available in the v2 REST API speakers endpoint.
Also adds the groups parameter to the [speakers] shortcode, so that
the list of output speakers can be filtered by one or more groups.

Props Kau-Boy, coreymckrill
Fixes #2850

Note: See TracTickets for help on using tickets.