Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fago’s picture

Component: User Interface » User interface
Status: Active » Postponed

>Further I think they should be sorted first on their weight and secondly on their label.
That's the case. Isn't it?

>Is it an idea to make the overview tables of rules and rulesets sortable?
Yep it is, however this won't happen for 1.0. But probably for a later version..

mitchell’s picture

Version: 6.x-1.x-dev » 7.x-2.x-dev
Status: Postponed » Active

Bumping to 7.x.

fago’s picture

Status: Active » Closed (won't fix)

We cannot rely on the DB for this, we would have to manually code this. Thus for now, won't fix.

videographics’s picture

Is this worth revisiting? It just seems weird to see all my rules with a weight of -10 sitting at the bottom of the list and all the higher values above them. I wouldn't expect live updates with handles and all that, but if we can set the weight for each rule and show the weight for each rule on each line of the overview page, surely that value can be used to affect the sorting on that page.

mitchell’s picture

Status: Closed (won't fix) » Postponed (maintainer needs more info)
Issue tags: +ux

>>>>Is this worth revisiting?
>>Yep it is, however this won't happen for 1.0.
Reopening for 2.x development.

>>We cannot rely on the DB for this, we would have to manually code this.
How might this be coded? What are the steps ahead?

videographics’s picture

Just to be clear, I'm only suggesting we sort the list by the weight value. I understanding setting up handles to facilitate interactive reordering would be a bigger deal and might need to be put off, but If the weight value is right there to be displayed, why is it a challenge to use it for sorting? What am I missing?

AaronBauman’s picture

Component: User interface » User Interface
Status: Postponed (maintainer needs more info) » Needs review
FileSize
1.11 KB

I haven't read through all the other threads and comments about this issue.
I'm not sure why it's so complicated to sort the rows in a table.
Here's a quick and simple patch that accomplishes the request without compromising any functionality.

I'm sure someone will let me know if i'm being naive.

Status: Needs review » Needs work

The last submitted patch, rules_sort_weight-372328.patch, failed testing.

timmarwick’s picture

Any update on this?

AaronBauman’s picture

The patch in #7 does what it says, even though testbot doesn't like it.
May need a re-roll against latest dev.

As If’s picture

Re #7: Found a problem on version 7.x-2.3+1-dev. While the patch does indeed sort rules by weight, it fails to make the corresponding changes in the edit/clone/etc links on the right-hand side. Reverting the patch restored my links to their proper destinations.

sbrattla’s picture

I don't see why rules should be sorted by weight only?

I'd say they should be sorted by event and then weight. The weight does after all only apply within an event.

sbrattla’s picture

I realize that a rule can be triggered by multiple events; so I see that it does not really make sense to sort by event.

As If’s picture

Not to make extra work for anybody, but it seems to me the way to do this right would be a sortable table:

Name | Event | Weight | Status | Operations

The first 4 columns could be sortable by clicking column headers. The default might be alpha by name (hell, what is the default now?)

Even cooler would be if a second click remembered the previous sort and used it as its secondary sort (i.e., if I click Weight then Event I get all rules sorted by weights within events).

The trickiest part is rules that fall under more than one event, as sbrattla pointed out. Those might appear more than once. But since both links would lead to the same place anyway, I don't really see that as a problem.

cjoy’s picture

The patch from #7 sorts by weight alright, but also breaks the operations links correlation.

I ran into this issue coming from a Drupal Commerce use case:
Many rules reacting to a single event where execution order is relevant.

Problems encountered with the default Rules UI:

- it is hard to tell what logic will be performed when event X is triggered.
- it provides no overview of what events are actually tied to active rules

I ended up throwing a little javascript at the problem.
The script first groups the rules by event (alphabetical) and then sorts by weight within each group. Rules that are tied to multiple events are listed in each respective group and easily identified as "multiple event" rules. In this context, being able to dynamically sort the rules is of lesser importance, so I did not include it.

If you'd like to try it out, just go to 'admin/config/workflow/rules' and paste/run the script below in your browser console.

var $activeRulesTable=jQuery(".rules-overview-table").first();var $segmentedTable=$activeRulesTable.clone();$segmentedTable.find("tbody").html(" ");var $cssSample=jQuery("#rules-admin-reaction-overview").find("th");var bgColor=$cssSample.css("background");var color=$cssSample.css("color");var $ruleRows=$activeRulesTable.find("tbody tr");var allEvents=[];$ruleRows.each(function(){var e=jQuery(this);var t=e.find(".rules-element-content-group").first().html();t=t.match(/[.]*Weight: ([-0-9]*)/);e.attr("data-weight",t[1]);var n=e.find("td:nth-child(2)").html();if(n){n=n.split(", ");jQuery.each(n,function(t,r){r=r.replace(/ /g,"-").toLowerCase();if(jQuery.inArray(r,allEvents)==-1){allEvents.push(r)}e.addClass(r);e.removeClass("even odd");if(n.length>1){e.addClass("multiple-events");e.find("td:nth-child(2)").html(e.find("td:nth-child(2)").html().replace(/, /g,"<br>"))}else{e.find("td:nth-child(2)").html("")}})}});allEvents.sort();jQuery.each(allEvents,function(e,t){var n="<tr>"+'<td colspan="8" style="text-transform:capitalize;color:'+color+";background:"+bgColor+';font-weight:bold;">'+t.replace(/-/g," ")+"</td>"+"</tr>";jQuery(n).appendTo($segmentedTable.find("tbody"));var r=$ruleRows.filter("."+t).clone();r.sort(function(e,t){var n=parseInt(jQuery(e).attr("data-weight"));var r=parseInt(jQuery(t).attr("data-weight"));return n<r?-1:n>r?1:0});r.filter(function(e){return(e+1)%2==0}).addClass("even");$segmentedTable.find("tbody").append(r)});$activeRulesTable.replaceWith($segmentedTable)

minified to save space, pretty syntax at: https://www.dropbox.com/s/eyirg2dyzldpqww/rules_bettertableformat.js
preview at: https://www.dropbox.com/s/m8y43e16di1hyht/rules_bettertableformat.jpg

P.S.: If this issue is the wrong place for this comment, feel free to move and/or delete it.

reszli’s picture

Issue summary: View changes
FileSize
2.95 KB

I converted cjoy's comment into a patch for ease of use if someone else also needs it - until a final solution is implemented.

dema501’s picture

#16 works for me

cjoy’s picture

thank you @reszli - happy to see that the little hack is of use to others :)
Just a minor glitch in that patch: the behaviors scope.
I guess "behaviors.quantityWidgetSpinner" is a copy/paste leftover and should probably be renamed to avoid conflicts.

Imaaxa-Cory’s picture

Until this functionality is incorporated into rules, I am using a view to give me a list of rules that is sortable by Label, Plugin, Active, Weight and grouped by Plugin. It also has exposed filters in a block to add to the page. The Labels are links to edit the the rules.

AaronBauman’s picture

Status: Needs work » Needs review
FileSize
2.39 KB

2 years later... rules overview table is sorted by entity id, which really makes no sense.
Here's a reroll.

Status: Needs review » Needs work

The last submitted patch, 20: rules-sort_weight-372328.patch, failed testing.

AaronBauman’s picture

Status: Needs work » Needs review
FileSize
2.33 KB

after this i'm giving up, testbot

hargobind’s picture

Thanks @aaronbauman for your work on this patch.

I see a typo/discrepancy with the code uasort($entities, array('self', 'sortObjectsByWeight')); and the actual function name sortByWeight(). I fixed this in the attached patch.

Since I make use of the Event column in the rules config, I have found it useful to sort on that column. I also made it possible to specify multiple sorts.

Thoughts?

NWOM’s picture

Status: Needs review » Needs work

#23 applied cleanly, but the following notices/warnings are shown on the "workflow/rules" page:

This notice is shown multiple times, dependent on the amount of rules that exist:
Notice: Undefined offset: 0 in RulesUIController->sortObjectsByEvent() (line 283 of /var/aegir/platforms/panopoly-7.x-1.35/sites/{SITE}/modules/rules/ui/ui.controller.inc).

This warning is shown once at the end of all of the notices:
Warning: uasort(): Array was modified by the user comparison function in RulesUIController->overviewTable() (line 228 of /var/aegir/platforms/panopoly-7.x-1.35/sites/{SITE}/modules/rules/ui/ui.controller.inc).

Running cron and clearing all cache does not remove the notices/warnings. Sorting was also not possible.

upunkt’s picture

+1 @Imaaxa-Cory #19 for this approach! I added a column Module for a commerce site also. This helped a lot.

ThePiano.SG’s picture

@NWOM, there's a slight error in the patch in #23.

rules > ui > ui.controller.inc > sortObjectsByEvent()

Before:

    // Get the first event and use its label for sorting.
    $a_event = is_object($a) ? $a->events() : '';
    $a_event = is_array($a_event) && isset($events_list[$a_event[0]]) ? $events_list[$a_event[0]]['label'] : '';
    $b_event = is_object($b) ? $b->events() : '';
    $b_event = is_array($b_event) && isset($events_list[$b_event[0]]) ? $events_list[$b_event[0]]['label'] : '';

After:

    // Get the first event and use its label for sorting.
    $a_event = is_object($a) ? $a->events() : '';
    $a_event = is_array($a_event) && isset($a_event[0]) && isset($events_list[$a_event[0]]) ? $events_list[$a_event[0]]['label'] : '';
    $b_event = is_object($b) ? $b->events() : '';
    $b_event = is_array($b_event) && isset($b_event[0]) && isset($events_list[$b_event[0]]) ? $events_list[$b_event[0]]['label'] : '';

3 very important things to note:

  1. The priority of the sorting is hard-coded. If you need to prioritise a specific sorting criteria, you have to change it in code.
  2. If you want to sort the rules by weight, and then by event, your sorting option will be hard-coded to -- 'sort' => 'event,weight',
  3. Lastly, the sorting is not a stable sort. If you sort by 'event' first, then by 'weight', your sequence order after 'event' sort is not respected after 'weight' sort. You need to write another function to perform a stable sort.

Taking into account stable sort:

    $sort_options = explode(',', $options['sort']);
    foreach ($sort_options as $sort) {
      if ($sort == 'id') {
        ksort($entities);
      }
      elseif ($sort == 'label') {
        self::rulesStableSort($entities, 'sortObjectsByLabel');
        //uasort($entities, array('self', 'sortObjectsByLabel'));
      }
      elseif ($sort == 'weight') {
        self::rulesStableSort($entities, 'sortObjectsByWeight');
        //uasort($entities, array('self', 'sortObjectsByWeight'));
      }
      elseif ($sort == 'event' && $options['show events']) {
        self::rulesStableSort($entities, 'sortObjectsByEvent');
        //uasort($entities, array('self', 'sortObjectsByEvent'));
      }
    }

  /**
   * Helper function to stable sort the array
   */
  public static function rulesStableSort(&$array, $cmp_function) {
    // Arrays of size < 2 require no action.
    if (count($array) < 2) return;
    
    // Split the array in half
    $halfway = count($array) / 2;
    $array1 = array_slice($array, 0, $halfway);
    $array2 = array_slice($array, $halfway);
    
    // Recurse to sort the two halves
    self::rulesStableSort($array1, $cmp_function);
    self::rulesStableSort($array2, $cmp_function);
    
    // If all of $array1 is <= all of $array2, just append them.
    if (call_user_func(array('self', $cmp_function), end($array1), $array2[0]) < 1) {
      $array = array_merge($array1, $array2);
      return;
    }
    
    // Merge the two sorted arrays into a single sorted array
    $array = array();
    $ptr1 = $ptr2 = 0;
    
    while ($ptr1 < count($array1) && $ptr2 < count($array2)) {
      if (call_user_func(array('self', $cmp_function), $array1[$ptr1], $array2[$ptr2]) < 1) {
        $array[] = $array1[$ptr1++];
      }
      else {
        $array[] = $array2[$ptr2++];
      }
    }
    
    // Merge the remainder
    while ($ptr1 < count($array1)) $array[] = $array1[$ptr1++];
    while ($ptr2 < count($array2)) $array[] = $array2[$ptr2++];
    
    return;
  }

hargobind’s picture

Status: Needs work » Needs review
FileSize
11.33 KB
10.27 KB

@ThePiano.SG great idea to add a stable-sort!

Here's another take which incorporates the bug fix and stable-sort code in #24.

Another big improvement was to add a draggable table on the Rules Settings page for users to choose sort order. I also added Status sorting, and Plugin sorting on the Components page.

Module maintainers: this new code makes some big modifications to rules_admin.inc, namely it adds a theme_FORM_ID() wrapper to the settings form, and a submit handler to process the draggable form weights. I based that approach on the tabledrag_example_simple_form code example.

I think this is finally ready for mainstream. Please test!!

TR’s picture

hargobind’s picture

@TR yes, it will. I just closed that issue as a duplicate of this one.

TR’s picture

Carine23’s picture

Just tested patch #27 and the issue found by #11 is still relevant:

While the patch does indeed sort rules by weight, it fails to make the corresponding changes in the edit/clone/etc links.

So the links go to another rule basically.

Carine23’s picture

I have created a patch for the links, as the solution looks very simple (but let me know if it's not OK).
Instead of sending the entity id to overviewTableRow() from overviewTable(), I send the machine name of the rule, as intended (overviewTableRow takes $name as second parameter, not id).

jacob.embree’s picture

I'm not sure what to make of #32.

Keys need to be preserved for the operation links. This patch is based on #27 and preserves the keys.

hargobind’s picture

Status: Needs review » Reviewed & tested by the community

Thanks Jacob! This patch works great and the links are finally working again. (I'm guessing my patch in #27 broke with the release of 2.10 or 2.11.)

Did we get all the possible sort options? It's now possible to sort on: Name, Event (on the Rules page), Weight, Plugin (on the Components page), Status

Marking this as RTBC since I tested it and couldn't find any problems. But would love to get a few other people to test this since Rules is such a highly used module and we don't want to introduce any new bugs.

delacosta456’s picture

hi apply the patch #33 which apply correctly But as soon as i change the table's behavior in settings i always get this messages

Notice: Undefined offset: 0 in RulesUIController::sortObjectsByEvent() (line 287 of /Applications/MAMP/htdocs/webdev/rbbdoc/rbbdev/wroot/sites/all/modules/dev/rules/ui/ui.controller.inc).