Posted by jdleonard on April 7, 2009 at 8:59pm
| Project: | Workflow |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | reviewed & tested by the community |
Issue Summary
I'm running into problems with the "Workflow: Previous time" views field. When enabled, the field results in duplicate nodes in the view, each duplicate having a different time (each corresponding to one time when the workflow state was changed). I feel like this field should just result in one of each node and that it should have just the most recent time that the state changed.
Comments
#1
Yes, I run into this too. I already tried several ways to get rid of this (as a grouping the query by nids) but with current implementation of views and WF it's impossible to solve this issue. We need to link up workflow_node and workflow_node_history tables by adding hid column to workflow_node. After this, proper joins can be build in views.
#2
Im having same issue. Is there a fix???
#3
subscribing.
#4
So, I'm a Drupal newb, but not a stranger to coding. I've written a couple patches, but never worked with the Views2 API. I'm getting bitten by this bug, and here's my thoughts -- I'd love for someone to comment on them.
I think the core problem here is that the 'previous time' and 'previous comment' fields are doing what they're supposed to. The scheduled fields work too, but if we have previous and scheduled, what's missing? Current, of course -- we need a 'current comment' and a 'current time' option. We can use table aliasing in views2 for this I think:
<?php
$data['workflow_node_current']['table']['group'] = t('Workflow');
// Explain how this table joins to others.
$data['workflow_node_current']['table']['join'] = array(
'node' => array(
'table' => 'workflow_node_history',
'field' => 'nid',
'left_table' => 'node',
'left_field' => 'nid',
),
);
// time
$data['workflow_node_current']['stamp'] = array(
'title' => t('current time'), // appears in views ui.
'help' => t('the time at which the node moved from the last state to the current state.'),
'field' => array(
'handler' => 'workflow_views_handler_field_current_date',
'click sortable' => true,
),
'filter' => array(
'handler' => 'workflow_views_handler_filter_current_date',
'numeric' => true,
),
'sort' => array(
'handler' => 'workflow_views_handler_sort_current_date',
),
);
// comment
$data['workflow_node_current']['comment'] = array(
'title' => t('current comment'), // Appears in views UI.
'help' => t('The comment describing why the node was moved from the last state to the current state.'),
'field' => array(
'handler' => 'workflow_views_handler_field_current_xss',
'click sortable' => TRUE,
),
'filter' => array(
'handler' => 'workflow_views_handler_filter_current_string',
'numeric' => TRUE,
),
);
?>
Where I'm stuck at, is the use of a custom handler to add a 'MAX(stamp)' to the query. I've seen references to doing a
<?php$this->query->add_field(NULL, "MAX(SOME_FIELD)");
?>
If no one else can help, I can muddle through it with dpr() and get it figured out, but I'd love a little guidance here.
#5
Well, here's my first crack at it, and it works for me.
I created a table alias, and use a sub select (yuck) on the join to ensure only the most recent entry is returned. This adds two fields/filters/sort options: "Current time" and "Current comment".
I'm still very open to better ways to do this, but this method does appear to work.
#6
#7
Thanks for patching, justintime. I tested it out and it seems to work fine.
I'm also of the opinion that the code could be a bit cleaner; that subquery raises some red flags for me too. I poked at it for a while and couldn't come up with anything better though.
Also, your patch includes some extraneous code at the top. It changes some strings and Boolean values to lowercase, which does not comply with Drupal coding standards. So you may want to have a look at that.
#8
I have tested the patch in #5 and it didn't work for me.
I'm trying to use it in sort criteria. As soon as I add it to my view it gives me all kinds of duplicates. One for each time the state was ever changed in the existence of the node. In some cases this might be nice to see every time the state changed but not in my case. So it would be nice if in whatever solution is found here that we can have a checkbox or something to decide if all we want to see was the most recent "previous time" or all previous time changes.
#9
Patch #5 worked for me. It even worked with the relationship I built for the owner of the latest state change.
#10
I'm having this issue as well. I will test the patch out and report back.
#11
Patch #5 is working for me. This is an important bug that needs to be addressed in the next release.
#12
Patch #5 is also working for me. Nice work justintime.
#13
Patch #5 working. Capitalization fixes and RTBC.
#14
Comments suggest this is RTBC for the most common case and behavior not worse for #8.
#15
The database prefix not implemented.
Origi patch:
+ $data['workflow_node_current']['table']['group'] = t('Workflow');
+ // Explain how this table joins to others.
+ $data['workflow_node_current']['table']['join'] = array(
+ 'node' => array(
+ 'table' => 'workflow_node_history',
+ 'field' => 'nid',
+ 'left_table' => 'node',
+ 'left_field' => 'nid',
+ 'extra' => 'workflow_node_current.stamp = (SELECT max(stamp) from workflow_node_history where nid = node.nid)',
+ ),
+ );
I think, we need to modify, for this:
+ global $db_prefix;
+ $data['workflow_node_current']['table']['group'] = t('Workflow');
+ // Explain how this table joins to others.
+ $data['workflow_node_current']['table']['join'] = array(
+ 'node' => array(
+ 'table' => 'workflow_node_history',
+ 'field' => 'nid',
+ 'left_table' => 'node',
+ 'left_field' => 'nid',
+ 'extra' => 'workflow_node_current.stamp = (SELECT max(stamp) from '.$db_prefix.'workflow_node_history where nid = node.nid)'
+ ),
+ );
#16
That's handled in the SQL by surrounding the table name in curly-braces: {workflow_node_history}.
#17
Removed cruft at the beginning, and surrounded the table name in query with braces. I'm going to just move this RTBC because it was in that state before this little mistake was found, and now it's fixed.
#18
When I tried applying the patch Im getting the following error message
user warning: Unknown column 'workflow_node_current.stamp' in 'on clause' query:
im using View 6.x-2.11, and workflow 6.x-1.4 in Drupal 6
#19
Also whenever a node is taken for editing and saved again another entry will be made into the workflow_node_history table (even though I didnt change the workflow state).
So the current time we are getting is the latest time, but we should be able to get the time when it reached this state.
#20
The ability to get the time that the node was moved into the current state was added in http://drupal.org/node/309918. I think it makes more sense to get that out of the workflow_node table. So that leaves the "comment that was made when this node was moved into the current state". I think for that the following 'extra' makes sense:
'extra' => 'workflow_node.stamp = workflow_node_history.stamp AND workflow_node.nid = workflow_node_history.nid'#21
Closing #964240: 'Previouse time' field in views does not work when there are multiple state changes before as a duplicate of this one.
#22
Wow - blast from the past. It's amazing the difference a year makes - when I first wrote that patch, I really had no clue what I was doing wrt Drupal. I'm no expert now, but at least I know not to kill kittens and to uppercase TRUE and FALSE :)
Anyway, I came upon a need for this patch again, and realized I hadn't subscribed to the thread. I rerolled it against current 6.x-1.x-dev branch and implemented the suggestions suggested by @jvandyk. It also fixes the bug reported in #18.
It works for me, and as a bonus, the nested subselect is eliminated. Please review and set RTBC if it works for you. I'm subscribed now, and will follow through so we can get this committed.
#23
I tried your patch Justintime. I am sorry to say it does not work for me. There are still many duplications when using "Workflow: Previous time" as a field.
Regards,
Tilo
#24
@tilo - that's the way that particular field is supposed to work. Try using "Workflow: current time" and "Workflow: current comment" instead.
#25
subscribe
#26
Hi Justintime
Sorry, I didn't realize I was using Workflow 1.4 where the field "Workflow: current time" was not present. Now with 1.5 it works excellent. Many thanks great job!
Tilo
#27
Confirming the patch works for me too. Thanks.
#28
I would argue that this should be marked as "fixed" because the "Workflow: Current time" introduced in Workflow 6.x-1.5 should be the feature that most were looking for in this case.
#29
@dandaman - in #20, @jvandyk pointed to that new feature, and in #22 I reworked the patch to leverage the new table provided by that feature to get at a "Workflow: Current comment" field exposed to views. That particular feature isn't present in 1.5, and is the feature that at least some (namely me :) ) of the subscribers on this thread needed.
I argue it stays RTBC.
#30
bump!