We're using the workflow module with the publishing module to set up a publishing process a little more complicated than that built into the publishing module. It's working fine, but the client becomes confused when making a workflow state change by the fact that the available workflow states aren't in order. The workflow admin page with the workflow and all the states lists them in the proper order and the weight column in the workflow_states table has them in the appropriate order. But when the client goes to a workflow tab, the order might say:
approved
public
rework
review
board
I see in the module several calls to workflow states that do not include an Order by Weight clause. Would adding one solve the problem? Would it cause others?
This is a Drupal 4.7 site, php 5.1.6, Linux, MySQL 4.1.21-standard.
Comments
Comment #1
PanchoCan not imagine why ordering the items would cause any problems... If you know how, feel free to provide a patch, otherwise I will create one within the next week.
Comment #2
PanchoEnclosed is a small two-line patch against HEAD. Please check whether the problem occurs in the 4.7 version as well.
I'm being so bold and set this to RTBC, as I can't imagine there could be any negative side effects.
Comment #3
rickvug CreditAttribution: rickvug commentedI changed the lines of code and the ordering still does not work. Maybe this is a due to a difference between 4.7 and HEAD?
Comment #4
mfredrickson CreditAttribution: mfredrickson commentedDemoting to code needs work b/c of rickvug's comments.
Comment #5
rickvug CreditAttribution: rickvug commentedPancho,
Any news about this patch? Am I off base in thinking about what this does perhaps? The ordering bug is definitely a really glaring problem IMO.
Rick
Comment #6
mrsocks CreditAttribution: mrsocks commentedhas there been any update on this?
I am on drupal 5.1
php 5.2.1
Comment #7
rickvug CreditAttribution: rickvug commented@mrsocks - Have you tried the latest development build to see if this problem still exists? If it does we know that this patch still needs to make it in.
Comment #8
nessumsara CreditAttribution: nessumsara commentedThe problem also exists on the node edit tab. This is a glaring problem that my client is constantly bugging me about when it will be fixed. Please provide an update and ETA or an explanation as to why it is difficult to solve.
Comment #9
mrsocks CreditAttribution: mrsocks commentedI am using the 5.x-1.1 version and
replaced line
. "t.%s WHERE t.%s = %d", $field, $field, $field_where, $sid);
with
. "t.%s WHERE t.%s = %d ORDER BY s.weight ASC, s.state ASC",
$field, $field, $field_where, $sid);
and still nothing.
I also went through the entire module and added 'order by weight' and it didnt change.
It is showing in the order that the states are in the table, but not by weight.
Is there another function calling these same fields, mysql reserved word killing the ordering etc....?
Comment #10
mrsocks CreditAttribution: mrsocks commentedOK OK OK..... I am getting closer.
it has something to do with these two functions (and probably the allowable trasitions, but this is a start hopefully)
AND
The user I am viewing this through has access to send the node to any state from any state.
Is there a way (not toally familiar with drupal api yet) to gather all the available states in one call and then when the radio buttons redner, have it check the state that it is in...
It;s probably doing this or similar to this now, but just not catching why.
Let me know if this helps or you need more info from me.
Thanks.
Comment #11
mfredrickson CreditAttribution: mfredrickson commentedThis is a good bug report. I'll see if I can find time to use your findings and possibly shed some light.
I appreciate good bug reports (with apologies to the dude who submitted a screen cast of his bug. That was an historically great report, but I never touched the bug. :-) .).
Comment #12
mrsocks CreditAttribution: mrsocks commentedI see in the documentation for the workflow_allowable_transitions() function,
there is a note:
* @return
* Associative array of states (sid=>name pairs), excluding current state.
*
I originally thought this was intentional, but now realize that there is no choice in the table for that transition.... because when you choose transitions, there is no group when the X and Y are the same in the transition matrix.
This is what is causing the issue. when this array (which is properly ordered at this time) is combined with the $transitions array in: workflow_field_choices() , the only fields that are there to use for sorting are the state id / sid (key) and the state name (value).
so it sticks it in there at the end....
THEN
the ksort sorts the array by the key... the state id /sid.
.. i have an idea for now.... brb
Comment #13
mrsocks CreditAttribution: mrsocks commentedOK,
I have gotten this to work:
I created this function:
Then made these changes in the workflow_field_choices() function
then removed the ksort in the workflow_tab_form() AND workflow_form_alter() functions.
and now everything seems to look correct.
everything SHOULD work the same because all i really did was just resort the transitions array with the keys (state ids/ sid) from the allowed transations group.
Let me know if anyone seems any issues with it.
I dont think this is the best solution, but something to get it looking better for now.
hope this helps.
Comment #14
mrsocks CreditAttribution: mrsocks commentedchanged the title of the thread by accident.
so i changed it back here.
Comment #15
buchanae CreditAttribution: buchanae commentedmy patch, simple and seems to work for me
Comment #16
sammos CreditAttribution: sammos commentedI manually applied the patch and it worked for me in terms of addressing the problem. I haven't widely tested it yet. I hope removing the ksort doesn't change any other behavior but it doesn't seem like it would. Thank you!
Comment #17
wmostrey CreditAttribution: wmostrey commentedWorks perfectly, doesn't seem to affect any other processes.
Comment #18
shrop CreditAttribution: shrop commentedFixes sorting issues on both the edit node pages and workflow tabs for me.
Thanks!
Shrop
Comment #19
jeffomac CreditAttribution: jeffomac commentedi applied this patch.
i noticed the following.
on content that was created prior to the patch, the sorting was still out of order.
On new content, the sorting was correct.
But when i edit the node (on new and old content) and change the workflow state, the order gets updated but not to the correct order.
whatever state I choose goes to the top of the list.
Has this happened to anyone else?
This is a drupal 5.2 site.
Comment #20
JacobSingh CreditAttribution: JacobSingh commentedYes, I see the same bug.
Here is another patch which should resolve it. The issue was that not only was there no weight clause in the query, the current state was just tacked on. I added a UNION query to get it in there.
Comment #21
jbjaaz CreditAttribution: jbjaaz commentedapplied the patch and it cleared up the issue identified in comment #19.
good work
Comment #22
jbjaaz CreditAttribution: jbjaaz commentedJacobSingh,
I think I found something.
The patch correctly fixed issue from #19, but when I visited the "Workflow actions" page I noticed extra states listed.
It originally was
(creation) -> draft
draft -> review
review -> draft
review -> published
now, it looks like
(creation) -> (creation)
(creation) -> draft
draft -> draft
draft -> review
review -> draft
review -> review
review -> published
published-> published
Comment #23
jbjaaz CreditAttribution: jbjaaz commentedThe patch provided in comment #20 did fix the issue described in #19, but it broke other parts of the module. I observed one of the issues in comment #22.
I attached a patch that fixes the issue in comment #19 and seems to keep everything else intact. The fix was a one liner. Where ksorts were removed elsewhere, one is needed when the form is being built.
I hope this settles this bug once a for all :-)
Comment #24
JacobSingh CreditAttribution: JacobSingh commentedI like it, looks good.
Comment #25
JacobSingh CreditAttribution: JacobSingh commentedWould like to get a once over from someone else.
Comment #26
JacobSingh CreditAttribution: JacobSingh commentedSorry, take it back, that last patch doesn't work because it only orders the states on the workflow admin screen, it does nothing for the node form which still orders by sid. And adding the order clause there won't help because it tacks on the current state which is why I did the query above and killed the ksort.
I'll have to go back in and fix this.
Comment #27
JacobSingh CreditAttribution: JacobSingh commentedWhile not exactly a workflow bug, but an issue with actions integration, I've been able to fix the problem noted in #22:
Instead of adding the current state (as was being done before), I'm now removing same state transactions from the actions screen. I don't much like this, as I think same state transactions could in fact fire actions, but for now, this is what is happening to not fundamentally change too much.
Please review ASAP. I want to release a new version as soon as this critical (and old) bug is taken care of.
Comment #28
jvandyk CreditAttribution: jvandyk commentedThis seems to have been committed in 1.54.2.11 but the commit was not noted in this thread. This commit seems to have added a case as noted here that tests
$t->tid == $t->state_id
which makes no sense. Testing a transition ID against a state ID is absurd. Transition IDs are sequential and are just keys to from-to state IDs.
Comment #29
JacobSingh CreditAttribution: JacobSingh commentedHi John,
Indeed, this is messed up :(
Sorry I pushed it through, would have liked some more review. Here is a fix that might sort it.
Note: I also changed the syntax of the function slightly (switching to and from)
Please change back if you feel it has BC implications, but I found the previous way to be very unclear.
I also changed the statement in question which was a typo I think, but was part of allowing same state transitions.
Best,
Jacob
Comment #30
JacobSingh CreditAttribution: JacobSingh commentedd'oh! Left a debug statement in, sorry.
Comment #31
jvandyk CreditAttribution: jvandyk commentedApplied patch based on the above work by JacobSingh and others.
Comment #32
Anonymous (not verified) CreditAttribution: Anonymous commentedAutomatically closed -- issue fixed for two weeks with no activity.