Is it an idea to make the overview tables of rules and rulesets sortable?

Further I think they should be sorted first on their weight and secondly on their label.

Issue fork rules-372328

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

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
StatusFileSize
new1.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
StatusFileSize
new2.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

StatusFileSize
new9.65 KB

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
StatusFileSize
new2.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
StatusFileSize
new2.33 KB

after this i'm giving up, testbot

hargobind’s picture

StatusFileSize
new3.6 KB

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
StatusFileSize
new11.33 KB
new10.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

StatusFileSize
new846 bytes

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

StatusFileSize
new11.52 KB
new2.41 KB

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).

hargobind’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new11.51 KB

Similar to #35, I experienced the same error when running on PHP 8.x.

Attached is an updated patch to fix this.

hargobind’s picture

StatusFileSize
new946 bytes

Attaching interdiff from #33 to #36.

tr’s picture

@hargobind: Can you convert your patch into a MR? Drupal.org does not run testing on patches any more - all contributions are expected to be through merge releases.

hargobind’s picture

@TR: There you go.

Thanks to pipeline tests, I found a PHP 8.2 bug in the new code. All tests are passing now.

This was my first time using gitlab's MR. (I didn't realize the community switched away from the patch-based approach in the last few years.) Let me know if I missed anything.

hargobind’s picture

Question: on the MR, there is an option "Delete source branch when merge request is accepted."

I want to make sure I'm correct in understanding that source branch = MR/feature branch, and target branch = the 7.x-2.x branch... right?

Am I correct that checking it means that once the MR is merged, the feature branch I created for this MR will be deleted?

jacob.embree’s picture

That is correct, @hargobind, the feature branch you created would be deleted when the MR is accepted if you checked that box.

tr’s picture

Question: on the MR, there is an option "Delete source branch when merge request is accepted."

I want to make sure I'm correct in understanding that source branch = MR/feature branch, and target branch = the 7.x-2.x branch... right?

Am I correct that checking it means that once the MR is merged, the feature branch I created for this MR will be deleted?

Yes, that's what it means. But you shouldn't usually have to worry about that - that box is not checked, by default, and it's OK to leave it like that.

I'm not sure why the Drupal administrators set it up that way, and what they have in mind for cleaning up all the old feature branches accumulating on gitlab, but for now I am just going with the default settings.