Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "Reset Orders" to Sortable pages #1923

Closed
dotgreg opened this issue Apr 11, 2016 · 14 comments
Closed

Add "Reset Orders" to Sortable pages #1923

dotgreg opened this issue Apr 11, 2016 · 14 comments

Comments

@dotgreg
Copy link

dotgreg commented Apr 11, 2016

Hello, Thanks first for that awesome CMS, lifechanger!

I stumbled upon a problem on the Reorder/sorting list system in the backend. I followed the documentation https://octobercms.com/docs/backend/reorder and successfully created the backend pages and all the pieces looks working fine. (Btw, it would be interesting to add the fact the dev needs to add a column on the database on the selected model called "sort_order", surely integer and nullable to make it working)

config_reorder.yaml

# ===================================
#  Reorder Behavior Config
# ===================================

# Reorder Title
title: Reorder Budgets

# Attribute name
nameFrom: name

# Model Class name
modelClass: Ten31\Programmes\Models\Budget

# Toolbar widget configuration
#toolbar:
    # Partial for toolbar buttons
    #buttons: reorder_toolbar

Controller file

<?php namespace Ten31\Programmes\Controllers;

use Backend\Classes\Controller;
use BackendMenu;

class Budgets extends Controller
{
    public $implement = [
    'Backend\Behaviors\ListController',
    'Backend\Behaviors\FormController',
    'Backend\Behaviors\ReorderController'
    ];

    public $listConfig = 'config_list.yaml';
    public $formConfig = 'config_form.yaml';
    public $reorderConfig = 'config_reorder.yaml';

    public $requiredPermissions = [
        'ten31.programmes.manage_programmes' 
    ];

    public function __construct()
    {
        parent::__construct();
        BackendMenu::setContext('Ten31.Programmes', 'programmes', 'budget');
    }
}

Model file

<?php namespace Ten31\Programmes\Models;

use Model;

/**
 * Model
 */
class Budget extends Model
{
    use \October\Rain\Database\Traits\Validation;
    use \October\Rain\Database\Traits\Sortable;

    /*
     * Validation
     */
    public $rules = [
    ];

    /*
     * Disable timestamps by default.
     * Remove this line if timestamps are defined in the database table.
     */
    public $timestamps = false;

    /**
     * @var string The database table used by the model.
     */
    public $table = 'ten31_programmes_budgets';
}

reorder.htm

<?= $this->reorderRender() ?>

There is only one big problem : The order is not taken in account. I digged abit inside the code and saw that the order set is always 0. Here is an extract of the db queries operations on the admin page admin/ten31/programmes/budgets/reorder

update `ten31_programmes_budgets` set `sort_order` = '0' where `id` = '3'
2.41ms
pvi
update `ten31_programmes_budgets` set `sort_order` = '0' where `id` = '6'
2.05ms
pvi
update `ten31_programmes_budgets` set `sort_order` = '0' where `id` = '4'
2.15ms
pvi
update `ten31_programmes_budgets` set `sort_order` = '0' where `id` = '5'
2.18ms

When I output the values of the Trait \October\Rain\Database\Traits\Sortable with debugbar like :

 public function setSortableOrder($itemIds, $itemOrders = null)
    {
        Debugbar::info("itemIds",$itemIds);
        Debugbar::info("itemOrders",$itemOrders);
        if (!is_array($itemIds))
            $itemIds = [$itemIds];

        if ($itemOrders === null)
            $itemOrders = $itemIds;

        if (count($itemIds) != count($itemOrders)) {
            throw new Exception('Invalid setSortableOrder call - count of itemIds do not match count of itemOrders');
        }

        foreach ($itemIds as $index => $id) {
            $order = $itemOrders[$index];
            $this->newQuery()->where('id', $id)->update([$this->getSortOrderColumn() => $order]);
        }
    }

I get the following result, looks like a bug, no?

itemIds
array:4 [ 0 => "3" 1 => "6" 2 => "4" 3 => "5" ]

itemOrders
array:4 [ 0 => "0" 1 => "0" 2 => "0" 3 => "0" ]
@tobias-kuendig
Copy link
Member

Have you tried setting the position in the database manually or recreating the records all together? If you implemented the sort_order column after creating the records all the entries have the same position and maybe won't be sorted correctly.

@dotgreg
Copy link
Author

dotgreg commented Apr 11, 2016

@tobias-kuendig interesting, gonna try that, I was indeed thinking it could be coming from that. I was also exploring the JS to see if it was coming from there and created a very nifty dirty hack to make the system working in

\modules\backend\behaviors\reordercontroller\assets\jsoctober.reorder.js

this.processReorder = function(ev, sortData){
            var postData

            var newOrder = [];
            for (var i = 0; i <= this.simpleSortOrders.length - 1; i++) {
                newOrder[i] = i;
            };

            if (this.sortMode == 'simple') {
                postData = { sort_orders: newOrder }
            }
            else if (this.sortMode == 'nested') {
                postData = this.getNestedMoveData(sortData)
            }


            console.log(postData);

            $('#reorderTreeList').request('onReorder', {
                data: postData
            })
        }

@dotgreg
Copy link
Author

dotgreg commented Apr 11, 2016

@tobias-kuendig ok you are right, once the sort order column filled in the DB, the system is working as expected.

So to sum up

  • the ordering system is not working if implemented when the model already have some entries in the database. You will then need to manually:
  1. Create a new nullable integer field in the Model
  2. Switch their values from Null to actual numbers

I guess it is not ideal, I have right now 4 entries but what if someone need to implement the reorder system on a model with hundred of entries? The best would be to have an initialization that provide a first ordering if it detects the whole column "sort_order" is Null.
I guess that case should be included in the expected behavior "If the orders is undefined, the record identifier is used." defined in the comment of the setSortableOrder in October\Rain\Database\Traits\Sortable

I can do a pull request on that point

@daftspunk
Copy link
Member

I think we need a "Reset Order Positions" button or something like this. It can be an issue for the NestedTree behavior too when this becomes corrupted.

Refs: rainlab/blog-plugin#195

@daftspunk daftspunk self-assigned this Apr 15, 2016
@daftspunk daftspunk changed the title Reorder List Broken Add "Reset Orders" to Sortable pages Aug 16, 2016
@austinderrick
Copy link
Contributor

austinderrick commented Nov 1, 2016

I'm not seeing this being set correctly on new items either. (Always defaults to "0")

I ended up adding something like this to get it working for now:

public function beforeSave()
{
if (empty($this->sort_order)) {
$this->sort_order = DB::table('orders')->max('sort_order') + 1;
}
}

@jakobfdev
Copy link
Contributor

It seems to me at least if the Trait is set in the model upon creation values are filled in automatically.
But adding this to a later point always leads to buggy results...

@swt83
Copy link

swt83 commented Nov 6, 2016

When I create new records, the order_id is set properly. When I attempt to reorder the records, they all are set to 0. I'm going to look and see if the const SORT_ORDER = 'order_id'; is working properly.

EDIT: I can confirm that the sorting function only works when the field name is sort_order and will not work if you give it a custom field name. I will open a ticket for this.

@daftspunk
Copy link
Member

daftspunk commented Nov 8, 2016

Adding the Sortable trait to a model that has existing records will require a migration script to set the order values (a good idea is to copy them from the record IDs). Providing a "Reset Orders" button would also work as a solution.

LukeTowers added a commit to octobercms/docs that referenced this issue Jul 6, 2017
Documents how to prepare existing data to be used with the Sortable trait. Related: octobercms/october#1923
@LukeTowers
Copy link
Contributor

The support demand for October is increasing, while the available time the founders have to give remains the same. These issues will be handled in time, however there are ways you can help:

@mohsin
Copy link
Contributor

mohsin commented Jul 27, 2018

This bug still persists, correct? I'm using sorting on a relation and have managed to get it working great but the sorting doesn't happen since the post('sort_orders') value is always zeroes and literally the database call updates the sorting column to all zeroes.

array (
  0 => '0',
  1 => '0',
  2 => '0',
  3 => '0',
  4 => '0',
)

@LukeTowers
Copy link
Contributor

Yes, you need to follow the instructions in octobercms/docs@e07d603 if you are adding the trait to a table that already has data.

@mohsin
Copy link
Contributor

mohsin commented Jul 28, 2018

Ah, so this bug occurs because the initial sort order doesn't exist! Thanks for the comment man... I thought this was due to something else and wrote a crude workaround to make it work in my case...:

File modules/backend/behaviors/reordercontroller/assets/js/october.reorder.js
+            var newOrder = [];
+            $('#reorderTreeList li').each(function(){
+                newOrder[$(this).attr('data-record-id')] = $(this).index()
+            });
+
             if (this.sortMode == 'simple') {
-                postData = { sort_orders: this.simpleSortOrders }
+                postData = { sort_orders: Object.values(newOrder) }

And in my model I had added:

File plugins/mohsin/startup/models/Module.php
+
+    public function setSortableOrder($itemIds, $itemOrders = null)
+    {
+        if (!is_array($itemIds)) {
+            $itemIds = [$itemIds];
+        }
+        if ($itemOrders === null) {
+            $itemOrders = $itemIds;
+        }
+        if (count($itemIds) != count($itemOrders)) {
+            throw new \Exception('Invalid setSortableOrder call - count of itemIds do not match count of itemOrders');
+        }
+        sort($itemIds);
+        foreach ($itemIds as $index => $id) {
+            $order = $itemOrders[$index];
+            $this->newQuery()->where($this->getKeyName(), $id)->update([$this->getSortOrderColumn() => $order]);
+        }
+    }

Setting the sort_order to copy it's own record ID as suggested by to docs wouldn't work in my case since I'm using this against a relation. So now each set of related model elements have a sort order spanning from 0..n related to the parent model they belong to. In other words, the sort_order column is unique numbers only for a given parent_id rather than the entire related model itself.

@LukeTowers
Copy link
Contributor

@SaifurRahmanMohsin Regarding using sort order on related records, the ID method still works since the relation will load only the relevant records and they will be in the appropriate order even if their sort orders are 5,7,13 instead of 1,2,3

@github-actions
Copy link

This issue will be closed and archived in 3 days, as there has been no activity in the last 30 days. If this issue is still relevant or you would like to see action on it, please respond and we will get the ball rolling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

8 participants