Make WordPress Core

Opened 15 years ago

Closed 9 years ago

#9227 closed enhancement (fixed)

in get_the_category_list(), filter categories before constructing list

Reported by: kevinb's profile KevinB Owned by: wonderboymusic's profile wonderboymusic
Milestone: 4.4 Priority: normal
Severity: normal Version: 2.8
Component: Taxonomy Keywords: has-patch
Focuses: template Cc:

Description

The template function get_the_category_list returns a concatenated string of category links for an individual post. The only filtering available is on the full concatenated sting, via 'the_category' hook.

Since filtering the category array directly is greatly preferable for most uses, please consider adding the attached patch, or something like it.

Whatever hook is added should somehow indicate the data context - in this case for display. In filtering post categories, there is the potential for non-display situations in which the actual post categories must be maintained even though the display filtering hides some from the current user. What we should not do is apply a single filter inside get_the_category or wp_get_object_terms, since themes and plugins may already call those functions for multiple data contexts.

I had considered calling the new hook 'get_the_category_for_list'. Instead I'm suggesting a more generic 'get_the_category' hook, passing context as the second argument. That would leave the API consistent for possible future use in filtering other contexts.

Attachments (5)

get_the_category_display.patch (577 bytes) - added by KevinB 15 years ago.
direct filtering of the post categories array in get_the_category list
9227.diff (810 bytes) - added by ericlewis 10 years ago.
9227.2.diff (875 bytes) - added by ericlewis 10 years ago.
9227.3.diff (983 bytes) - added by ericlewis 10 years ago.
9227.4.diff (1.1 KB) - added by ericlewis 10 years ago.

Download all attachments as: .zip

Change History (32)

@KevinB
15 years ago

direct filtering of the post categories array in get_the_category list

#1 @KevinB
15 years ago

  • Keywords changed from category template filter to category, template, filter

#2 in reply to: ↑ description ; follow-up: @filosofo
15 years ago

Replying to KevinB:

I had considered calling the new hook 'get_the_category_for_list'. Instead I'm suggesting a more generic 'get_the_category' hook, passing context as the second argument. That would leave the API consistent for possible future use in filtering other contexts.

So wouldn't it make more sense to add the filter to get_the_category()'s return?

#3 in reply to: ↑ 2 @KevinB
15 years ago

Replying to filosofo:

Replying to KevinB:

I had considered calling the new hook 'get_the_category_for_list'. Instead I'm suggesting a more generic 'get_the_category' hook, passing context as the second argument. That would leave the API consistent for possible future use in filtering other contexts.

So wouldn't it make more sense to add the filter to get_the_category()'s return?

The problem I see is that get_the_category() does not know the context it's called in, whereas get_the_category_list() would always be a display context. Since get_the_category() returns an array, some themes or plugins would not be unreasonable in using it in a raw/db context - for a maintenance operation which pertains to all stored post categories regardless of the currently logged user. Is this a valid concern, or would you just assume that get_the_category() is always for display purposes and all backend operations use wp_get_object_terms()?

I want to filter post categories in a display context for the current user (who may be restricted by my plugin), so I need a hook which will prevent me from fibbing to someone about the actual post categories.

#4 @FFEMTcJ
15 years ago

  • Keywords has-patch added
  • Milestone changed from Unassigned to 2.8

#5 @Denis-de-Bernardy
15 years ago

+1. would naming the hook get_the_category_list instead make more sense?

#6 @Denis-de-Bernardy
15 years ago

  • Keywords tested commit added

see also #8704 and #9719

#7 @Denis-de-Bernardy
15 years ago

#8704 got fixed, we can probably close this one as fixed too.

#8 @kevinb
15 years ago

  • Version changed from 2.7 to 2.8

The wp_get_object_terms filter added in #8704 is not, as the seasoned professionals say, a "robust" solution to the issue I raised here.

The wp_get_object_terms filter alone (and the term_cache which it loads) does not allow for any distinction between terms which an object has and terms which the current user can view.

#9 @Denis-de-Bernardy
15 years ago

  • Milestone changed from 2.8 to 2.9

punting per IRC discussion

#10 @Denis-de-Bernardy
15 years ago

patch still applies clean

#11 @ryan
14 years ago

  • Milestone changed from 2.9 to Future Release

#12 @scribu
14 years ago

  • Keywords template commit removed

#13 @nacin
10 years ago

  • Component changed from Template to Taxonomy
  • Focuses template added

@ericlewis
10 years ago

@ericlewis
10 years ago

#14 @ericlewis
10 years ago

  • Keywords category filter tested removed

In attachment:9227.2.diff I suggest using a new filter named the_category_list.

#15 @wonderboymusic
10 years ago

  • Owner set to DrewAPicture
  • Status changed from new to assigned

Please review the inline docs.

#16 @wonderboymusic
10 years ago

  • Milestone changed from Future Release to 4.0

#17 @DrewAPicture
10 years ago

  • Keywords needs-patch added; has-patch removed

9227.2.diff looks OK except that the mixed type on the @param lines should be bool, not false. The description should be used to describe where false would be passed for each.

Edit: No "returns" here.

Last edited 10 years ago by DrewAPicture (previous) (diff)

@ericlewis
10 years ago

#18 @ericlewis
10 years ago

attachment:9227.3.diff fixes the types of the @params and gives a more verbose description to the $categories parameter.

#19 @DrewAPicture
10 years ago

@ericlewis Can fix aligning the 2nd+ lines with the beginning of the parameter description instead of the variable?

@param type $var This is a very long description that has the possibility
            of spanning multiple lines.

vs.

@param type $var This is a very long description that has the possibility
                 of spanning multiple lines.

Also, the $post_id param line needs a description.

@ericlewis
10 years ago

#20 @ericlewis
10 years ago

attachment:9227.4.diff fixes alignment and gives $post_id a description.

#21 @DrewAPicture
10 years ago

  • Keywords has-patch commit added; needs-patch removed
  • Owner changed from DrewAPicture to wonderboymusic

9227.4.diff works for me.

#22 follow-up: @SergeyBiryukov
10 years ago

  • Keywords 2nd-opinion added; commit removed
  1. In 9227.4.diff, $categories should be an array rather than array|bool, because get_the_category() always returns an array. The docs need to be adjusted as well.
  2. get_the_categories filter was added in [16332]. Is there a use case that cannot be achieved using that filter? You can conditionally apply it on front-end as needed. We don't pass the post ID there, perhaps we should do that instead of introducing a new filter.

This ticket was mentioned in IRC in #wordpress-dev by SergeyBiryukov. View the logs.


10 years ago

#24 @SergeyBiryukov
10 years ago

  • Milestone changed from 4.0 to Future Release

#25 in reply to: ↑ 22 @boonebgorges
9 years ago

  • Keywords 2nd-opinion removed
  • Milestone changed from Future Release to 4.4

Replying to SergeyBiryukov:

  1. In 9227.4.diff, $categories should be an array rather than array|bool, because get_the_category() always returns an array. The docs need to be adjusted as well.
  2. get_the_categories filter was added in [16332]. Is there a use case that cannot be achieved using that filter? You can conditionally apply it on front-end as needed. We don't pass the post ID there, perhaps we should do that instead of introducing a new filter.

get_the_category() is also used in get_permalink(). This is a very different context from using get_the_category() to get categories for display. So I'd say that it's worth having a separate filter, though passing the post ID to 'get_the_categories' is also a dynamite idea.

#26 @boonebgorges
9 years ago

In 34624:

Pass the post ID to the get_the_categories filter.

Props SergeyBiryukov.
See #9227.

#27 @boonebgorges
9 years ago

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

In 34625:

Introduce 'the_category_list' filter.

Used to filter categories as structured data, before building markup in
get_the_category_list().

We use this filter in addition to upstream filters (such as
'get_the_categories'`) because those upstream filters are used in numerous
contexts, while 'the_category_list' is always used for generating markup
for display.

Props KevinB, ericlewis, SergeyBiryukov, DrewAPicture.
Fixes #9227.

Note: See TracTickets for help on using tickets.