add a feature to send a 404 HTTP status code if a taxonomy term does not contain any nodes, please.

CommentFileSizeAuthor
#12 taxonomy_cleanup_outdated_terms-d7.patch736 byteslilou
#9 taxonomy_cleanup_outdated_terms.patch777 byteshass
#5 taxonomy-HEAD.patch504 byteshass
#1 taxonomy-4.7.6.patch486 byteshass

Comments

hass’s picture

hass commented
Version: 5.x-dev » 4.7.x-dev
Status: Active » Needs review
StatusFileSize
new taxonomy-4.7.6.patch486 bytes

Attached a patch for 4.7.6

Crell’s picture

Crell commented
Version: 4.7.x-dev » 6.x-dev
Priority: Critical » Normal

Feature requests only go against the development version, not stable (5) or legacy (4.7).

hass’s picture

hass commented

hm, i need this fix in version 4.7.x and 5.x, too. I cannot wait for 6.x... should we change this into a bug for getting this critical problem fixed earlier?

Crell’s picture

Crell commented
Status: Needs review » Needs work

The behavior in 4.7 and 5 now is not a bug, but "by design". If you need it to behave differently for your site, you can modify it locally. Design decisions for stable versions of Drupal will not be revisited for that version. (Otherwise it ceases to be "stable", by definition.) Design decisions for the development version can be considered if there is agreement that the design change is a good one.

If you would like it considered for Drupal 6, provide a patch against the current HEAD version and include an explanation of why the change is warranted.

hass’s picture

hass commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new taxonomy-HEAD.patch504 bytes

ok, this is the patch against taxonomy.module CVS v1.340

This patch removes outdated term URLs from search engines and therefor stop search engines crawling non existing content. this cleans up outdated urls and make a site looking better to search engines.

Crell’s picture

Crell commented
Status: Reviewed & tested by the community » Needs review

Do not set your own patches to RTBC.

catch’s picture

catch
he/him
Primary language English
commented
Version: 6.x-dev » 7.x-dev
Status: Needs review » Needs work

Bumping to 7.x - patch isn't rolled from root, but if you specify the path it still applies with a 261(!) line offset. Nice idea though.

catch’s picture

catch
he/him
Primary language English
commented

This is related to: http://drupal.org/node/185893 in many ways. Would be good to have consistent behaviour for these empty containers between both taxonomy and blog modules.

hass’s picture

hass commented
Status: Needs work » Needs review
StatusFileSize
new taxonomy_cleanup_outdated_terms.patch777 bytes

Updated patch for latest HEAD with some minor code optimizations.

hass’s picture

hass commented
Title: HTTP 404 - if term does not contain any nodes » HTTP 404 - if category does not contain any content
birdmanx35’s picture

birdmanx35 commented

Still applies, with a bit of offset, to Drupal HEAD.

lilou’s picture

lilou commented
StatusFileSize
new taxonomy_cleanup_outdated_terms-d7.patch736 bytes

Reroll.

damien tournoud’s picture

damien tournoud commented

Simple and nice idea.

drawk’s picture

drawk commented
Status: Needs review » Reviewed & tested by the community

Patch applies and works as advertised.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Needs work

It is probably better to use drupal_not_found() so this gets logged, we can omit blocks to save resources, etc, etc.

hass’s picture

hass commented
Status: Needs work » Reviewed & tested by the community

@Dries: This wouldn't be a good idea. If we use drupal_not_found() we are no more able to show t('There are currently no posts in this category.'). This is the correct error message for the user. We only set this header for crawlers, but keep a good message for users. Sometimes a category is really empty, but a menu entry exists. This is why the patch looks like it is.

damien tournoud’s picture

damien tournoud commented

I'm with hass on this one.

catch’s picture

catch
he/him
Primary language English
commented

Me also, there's use cases for empty categories, you might still show parents, related terms, description and other stuff on that page.

dries’s picture

dries commented

Does that mean that drupal_not_found() should get a $message parameter to overwrite the default message? Let's think this through before making a decision.

hass’s picture

hass commented

Well, we could also do this, but it would require some more options like:

drupal_not_found(
  'title' => $title,
  'text' => 'There are currently no posts in this category.',
  'watchdog_msg' => 'no posts in category',
)

This way seems to be a bit overkill for me and we don't have $title here what gives us new problems. Adding another line for watchdog could be a good idea.

Or an keep_title option what looks awful to me:

drupal_not_found(
  'keep_title' => true,
  'text' => 'There are currently no posts in this category.',
  'watchdog_msg' => 'no posts in category',
)

Not sure what to do now.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs review

Putting back to needs review.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Post taxonomy sprint, I'm more convinced of #18 - you may want to show all kinds of things which don't require nodes being assigned to the term yet on these pages, so RTBC again.

dries’s picture

dries commented

@hass, re #20: I don't think we need custom watchdog messages. Using the standard 404 message would be sufficient. All we'd need is the ability to overwrite the title and the message.

@catch, Damien: on my personal blog, I use the features like "Top page not found errors" (?q=admin/reports/page-not-found). This patch would "break" that view in that it does not properly report 404s for use by other parts of the system.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Closed (won't fix)

@Dries, that's a good point, I'd partially forgotten what the original issue was about ;)

Per #18, I'm going to mark this won't fix - if a taxonomy term has a description, then it's not 'empty' as such - http://drupal.org/node/306224 will offer cleaner ways for modules like taxonomy_image etc. to interact with core terms, panels and views already allow for various things to be done on these pages which aren't simply nodes in the term.

I think we need much better administrative tools to help deal with merging terms, finding terms which aren't assigned to any nodes etc. - that's the real problem here.