Elements in table are not in correct order after saving rule with more than 10 elements

I am seeing this same issue Rules 7.x-2.1.

#1045916-15: Add ability to sort rules by weight appears to fix this.

I also had this issue when building #1252860-4: Action: Differences between lists.

Hi,
I have the same problem with sorting when more than 10 actions take place. I think that, basically, rules has its own method for sorting and it wasn't taking in account max default weight values for child elements in the RulesContainerPluginUI.
The patch will look for the maximun weight of every element in the rules container, then it will keep that weight +10 more "points".
Finally the patch will set the weight delta value of every rule's element to the weight that we calculate before.

Reviews and comments will be very welcome.

Marked #1430526: Actions' Weighting Values will not Save as a duplicate.

Anyone reviewed #5 yet?

#5 reviewed, seems ok and works fine.

I also experienced the same issue.
#5 fixed this for me too.

@klausi / @fago: Reports confirm that this works (I also tested this back in [#1045916-?] but didn't report, whoops!! Edit: see #4), and the code quality looks good to me.

Marked #1706184: If a rule has more than 12 actions (and possibly Events and conditions), then all subsequent lines will have a weight of -20. as a duplicate. Anyone else have a better title? There are a couple of duplicates to consider.
--

I think this is also, partially, an "advisable configuration" issue, (that we should address in the docs.. maybe even within the UI, or other cool ways... see #1706500: Advise users on recommended configuration approaches).
Edit: Discussion moved to referenced issue.
---

Side note: This seems odd for Rules alone to have this. Shouldn't this somehow be a default behavior of tabledrag.js connected code? I don't think that's germane, but if it comes up in D8 and especially if its then targeted for backporting to D7, adding a reference to this issue would be useful in combination with a new change request for Rules.

This will come out as frustratingly indifferent to many.

I know the limited weight #delta is an issue, but I can't seem to reproduce the problem in the screenshot #1. I've tried to hit the issue by bulk creating a rule with 50+ actions. I've also tried the imports from #1252860-4: Action: Differences between lists.

Could everyone who's run into the problem report the browser + version + OS used to test?

I've tried it on Chrome 21.0.1180.60 m, Firefox 14.01, Pale Moon 12.3, and IE 9. OS is Windows 7 64-bit.

> Reproducibility (blocker)
Like I said in #4, I ran into this problem with that, but I also remember seeing a few people's Drupal installs having rules that stretched for pages. So, there must be another factor used for setting element weights that can somehow prevent problems from being introduced (maybe those were also defined in code)...

Ahh.. @zhangtaihao: did you try dragging any of the elements around? I think if all the elements are created sequentially, then the problem won't occur. I think it might be that you have to do some reordering between adding new elements to mess up the weight values (which will make multiple 20's??).

Yes, I had a rule with 15 conditions and 12 actions.
When I just sort them and hit save, the either the conditions are in good order but not the actions or vice versa.
(Chrome 20.0.1132.57 OS is Windows 7 64-bit, but almost sure this has nothing to do with the browser, because there were just to little weights even with javascript off.)

Patch #5 actually worked perfectly adding only more weights if they are needed.

Not sure I understand why this needs more work, what kind of new problems will it introduce?

Ah, sorry, I just discovered the key step is "Save".

The patch in #5 doesn't account for the workaround introduced in f207391.

This patch undoes the increment by 2 (which presumably was originally added to force weights downward since the #delta wasn't adapted to element sibling count) and correctly adds the sibling count to #delta.

Works for me.

+++ b/ui/ui.core.incundefined
@@ -989,7 +989,7 @@ class RulesContainerPluginUI extends RulesPluginUI {
-        '#delta' => 20,
+        '#delta' => $child->parentElement()->getIterator()->count(),

this is inefficient, as you have 3 extra function calls on every iteration of the foreach loop. Better retrieve this once before the loop starts. E.g. $children_count = $iterator->count(); or similar.

Oops.

Premature optimization leads to unoptimization.

Here's the cached version. One non-impacting caveat: in a rule, an action's parent is the rule itself, not its action container, meaning the child count for the parent includes both actions and conditions.

Thanks. However I don't think it's necessary to calculate the #delta on the fly - that looks like overkill and complicates code unnecessary. Let's just raise the limit.

50 should be fine. Everything else is insanty anyway :P
Please test the patch attached.

Fair enough. This raised limit should be documented somewhere, though.

I though this was due to the module Rules Conditional and I was wrong !
The problem was happening to me on a rule with conditionals + on an actions set + with the order of the parameters of the actions set.
Current patch solved it for me for the Rule and the Actions set, thanks!
However the problem still persists on the parameters of the action set with Rules 7.x-2.2 (not so important for my use case but I figured you might want to correct it all).

So does #19 solve the problem?

I believe this issue can just be fixed.

And if 50 isn't enough, then further support/fix effort will be needed at a later time.

EDIT: Besides, this effectively raises the limit to 5 times the current value (since it will no longer be $child->weight = $i * 2;).

@fago: What I was trying to say is that patch #19 almost solved it for me but not completely. Order of conditionals and actions does not get messed up anymore when there are too many of them. However, order of parameters of a component still gets changed when there are too many of them.

I believe that's a separate issue. The '#delta' in the variable table does not increase with the number of parameters, causing tabledrag to start skipping to negative weights, thus messing up the order. I don't believe that particular problem affects functionality; it's only annoying that the parameter order keeps changing.

You can create another issue to resolve that.

I don't particularly need it to be solved. As you say, it is only annoying that the parameters appear in a weird order when using the component inside a rule... Just thought you guys would like the module to work 100%... But maybe you're not even a maintainer of rules and just helping out here for this particular issue. Anyway, I'm not going to open a new issue for this. Thanks for solving the ordering of actions and conditionals that really was critical!

ok, thanks - I committed #19.