Posted by ngmaloney on November 4, 2010 at 8:31pm
34 followers
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | taxonomy.module |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
| Issue tags: | needs backport to D7 |
Issue Summary
Is there a reason hook_field_update doesn't update the taxonomy index for unpublished entities? I have a use case where editors can manipulate the taxonomy terms of unpublished nodes but these updates aren't inserted into the taxonomy index table. This makes it a challenge to create a view that searches unpublished nodes by taxonomy term name (as opposed to tid, which can be search by taxonomy_field_name_instance)
This would be trivial to patch as it is just an if statement in line 501 of field.api.php.
Comments
#1
recategorizing
#2
The code for this indexing happens in /modules/field/field.api.php so one could argue it is belongs there. Regardless of where it lives, it would be helpful to give developers the ability to index non-published fields, particularly in views. The patch could be something as simple as:
(field.api.php:501)
if ($entity->status) {...to something like...
if($entity->status || variable_get('index_non_published',false)) {...#3
field.api.php is just documentation code that is never executed.
The behavior happens in taxonomy_field_update() in taxonomy.module.
#4
Thanks for the info yched. Attached is a patch that would allow developers to override the default behavior (allowing non-published entities to be indexed in taxonomy_index). It is similar to how taxonomy currently handles "taxonomy_maintain_index_table".
#5
catch and bangpound designed the behaviour of the taxonomy index table.
Changes around this would need their seal of approval.
#6
The last submitted patch, taxonomy.module.patch, failed testing.
#7
patch needs to be rolled with git diff --no-prefix :-)
#8
I'm going to need a little orientation with this issue, so please be patient with me.
The taxonomy_index table is used to reproduce the "taxonomy/term/%term" pages similar to how they were done in Drupal 6. Is Views also relying on it for some reason? Why can't it use the field data tables directly? The taxonomy_index table is mostly a bridge back to old functionality. You should still be able to create a view that can show unpublished nodes. These term vallues are stored in the field data tables.
#9
bangpound, you are correct, It is possible to query by field value in views. One of the drawbacks is, by default, you can only query by TID, not Term Name. Having the values in the taxonomy_index table gets around this issue. The other way is to write a php argument handler. The patched method seems like a "cleaner" implementation when doing look ups by Term Name.
I made the patch merely as a demo to spark some dialog. While it would be helpful I understand if it creates new issues and isn't worth going any further with.
#10
This is also necessary for #741914: Add a [node:term] to work reliably, which is a token that I'm getting a *ton* of requests for.
EDIT: Wrong issue link...fixed.
#11
The patch in #4 seems to fix this issue on nodes that are created, but upon saving a new (unpublished) node there still seems to be a problem. Any ideas on how to solve this?
#12
Subscribed.
Is there documentation for module maintainers on how to get taxonomy data for unpublished nodes? That is if this issue is not being followed through.
#13
Subbing
#14
Has this issue been abandoned? It is holding back #741914: Add a [node:term] and it would be really great to be able to have forums which have a proper naming structure again. If there is a work around could someone please post it?
#15
Subscribing, re-versioning, tagging, and linking to relevant Views issue: #1182924: Unpublished entities do not appear in views when using a taxonomy relationship
#16
I have written http://drupal.org/project/taxonomy_entity_index in the meantime to index all entities regardless of published or not. I've opened an issue to add views integration for it.
#17
This is the same sort of trap that D6 core had in node_access(): if a node was not published, all access grants were ignored. Game over.
There are two checks for
$entity->statusin the Taxonomy module, one when inserting entities and one when updating. In my opinion neither should be there.It should not be up to the Taxonomy module to do nothing simply because a node/entity is not published.
It's up to the (node) access modules to decide whether (fields of) a node/entity should be displayed to the logged-in user or not.
To give my D7.4 users Views that display taxonomy terms correctly whether the nodes in question are published or not, I ended up implementing in my module hook_node_insert() and hook_node_update() inserting in their bodies calls to a function that basically does what taxonomy_field_update() does, but unconditionally, regardless of publication status.
#18
Rerolled against 8.x, with the following changes:
taxonomy_index_unpublished, which is more standard terminologyFALSEso the default core behavior is maintained when the variable is not set.#19
The last submitted patch, taxonomy-index-unpublished-18.patch, failed testing.
#20
Erm. Let's try a valid patch.
#21
taxonomy_select_nodes() (which creates the term page and feed listings) selects all nodes by term from the
{taxonomy_index}table. This patch would introduce unpublished nodes into those listings. So, at a minimum, the patch needs to include a fix there as well.We would also need to identify any other instances where core might be making this assumption. There's no accounting for contrib. The upshot is that the setting would only ever be used by contrib or custom module developers who knew it existed, so for anyone else, no change has been made.
#22
From catch on IRC:
#23
So, to add a status column to the table, the following changes would need to be made:
WHERE status = 1condition to existing selects against this table where relevant.if ($entity->status)checks intaxonomy_field_insert()andtaxonomy_field_update().db_insert()queries intaxonomy_field_insert()andtaxonomy_field_update().#24
Shameless subscribe - been bitten hard by this one for creating adminisatration pages a client today.
#25
Subscribing
#26
subscribing, would love to see this in D7 asap :x thanks!
PS. Agreeing to what http://drupal.org/node/962664#comment-4692122 said, should this not be marked as a bug report? IMO it really is one as it makes it impossible to fetch unpublished nodes through their taxonomies :/
#27
Well, I think leaving out unpublished nodes was a design feature to reduce overhead, since this table can get crazy-big on a large site. Technically you could use the EFQ to look up unpublished nodes by taxonomy--fetch a list of all fields, look for taxonomy fields, look up which bundles have these fields, and loop to query against values in all these fields. But that's kinda crazy.
#962664-23: Taxonomy Index for unpublished entities has a pattern for a solution that one of the core taxonomy maintainers has tentatively okayed, so that's the best bet for fixing this. I can try a patch at some point when I have time, but I won't have time this week, so if someone else does have time this week... :)
#28
Here's a patch that does the following:
#29
The last submitted patch, add-unpublished-nodes-to-taxonomy-index-962664-28.patch, failed testing.
#30
Second try! This patch uses
$statusinstead of$node->statusand fixes some newline and whitespace issues.I have no idea why the hook_update is failing. Looks like testbot sets up a site with the new schema and then executes the update, instead of setting up a site with the old schema.
Does someone have any idea how to fix this?
#31
Setting status.
#32
The last submitted patch, add-unpublished-nodes-to-taxonomy-index-962664-30.patch, failed testing.
#33
Has anyone tried #30, really needing it for a project.
#34
@arcane: In the interim, you might consider:
http://drupal.org/project/taxonomy_entity_index
#35
Thanks, I did get taxonomy_entity_index working
#36
@xjm - re: taxonomy_select_nodes() update: the current patch includes that change.
#37
...friendly bump ;)
Kick to D9? :/
#38
Any reason it's a feature request and not a bug? I mean, really, index gets out of sync with nodes indexed in it... why? Was that some historic, pre-views design from times when adding filter->published for taxonomy listings was difficult? I was sure Drupal matured way past that.
#39
It's a performance optimization for a common query.
#40
I always believed that performance data was supposed to be kept in cache_* tables. Current approach simplify one query, but breaks other, particularly in views. And given views are going to be in core now, it's certainly a core bug that setting view filters to show unpublished nodes of taxonomy term always returns empty set, isn't it? Question is, is it a bug in views or in taxonomy?
#41
Adding "Views in Core" tag as this is quite significant incompatibility between views, new in core, and taxonomy. This incompatibility was forgiveable when views was a contrib module not meant to fully support everything out there. And please, consider if it's a views problem (they use table not meant for it in their filters) or taxonomy one (design flaw in table's routines).
#42
With Views in core, it is possible that the taxonomy-module page that relies on this table will be removed entirely and replaced with a view. Currently, you should use one or the other.
#43
It's not possible to build sane views with taxonomy filters now, as views are using this very table to join nodes to taxonomy, and it contains only part of the data. That's the very point. Do you suggest that using taxonomy_index in views code is views bug now?