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

[NFR] ORM. Ability to set custom Resultset class returned by find() #12166

Closed
ashpumpkin opened this issue Aug 25, 2016 · 10 comments
Closed

[NFR] ORM. Ability to set custom Resultset class returned by find() #12166

ashpumpkin opened this issue Aug 25, 2016 · 10 comments
Assignees
Milestone

Comments

@ashpumpkin
Copy link
Contributor

Hello. I think this feature is a good idea in cases when there`s some logic related to sets of models
Raugh but simple usecase example:
Day statistics table with columns (date, statistics_type, value). statistics_type can be 1,2 or 3.
You need to render a month graph with 3 types of stats on it, and inform user about min and max values

// you get Phalcon\Mvc\Model\Resultset\Simple
$stats = Stats::find([
    'date between "2016-06-01" AND "2016-06-30"',
    'order' => 'date'
]);

$graphData = [1 => [], 2 => [], 3 => []];
$maxValue  = $stats[0]->value;
$minValue  = $stats[0]->value;

foreach ($stats as $item) {
    $graphData[$item->statistics_type][] = [$item->date, $item->value];

    if ($item->value < $minValue) {
        $minValue = $item->value;
    }

    if ($item->value > $maxValue) {
        $maxValue = $item->value;
    }
}

$this->view->graphData = $graphData;
$this->view->maxValue  = $maxValue;
$this->view->minValue  = $minValue;

The view will take this data and render a page for current user. But the problem is that there are views and other objects where this data is fully or partially needed. And you need to always duplicate the code above or to seek for a place where this logic can be stored. I think Custom resultsets would be a good place for this.

namespace Models;
class Stats extends \Phalcon\Mvc\Model {
    /** CODE */
    public function getResultsetClass() {
        return 'Resultsets\Stats';
    }
    /** CODE */
}
namespace Resultsets;
class Stats extends \Phalcon\Mvc\Model\Resultset\Simple {
    public function getGraphData() {
        /** CODE */
    }

    public function getMaxValue() {
        /** CODE */
    }

    public function getMinValue() {
        /** CODE */
    }
}
// you get Resultsets\Stats
$stats = Stats::find([
    'date between "2016-06-01" AND "2016-06-30"',
    'order' => 'date'
]);

$this->view->graphData = $stats->getGraphData();
$this->view->minValue  = $stats->getMinValue();
$this->view->maxValue  = $stats->getMaxValue();

// OR $this->view->stats = $stats; and the view will iteract with $stats by itself

Also it will help contributors to create their own Resultset classes for incubator.

@sergeyklay sergeyklay added this to the 3.1.0 milestone Aug 25, 2016
@Jurigag
Copy link
Contributor

Jurigag commented Aug 25, 2016

Cool idea.

@tugrul
Copy link
Contributor

tugrul commented Aug 31, 2016

Also able to make batch operations like change status.

@sergeyklay sergeyklay self-assigned this Aug 31, 2016
@Jurigag
Copy link
Contributor

Jurigag commented Sep 19, 2016

I think you can already do some batch things with resultset like:

$resultset->update([
    'status' => 2
]);

Will update every row in resultset.

@tugrul
Copy link
Contributor

tugrul commented Sep 20, 2016

@Jurigag thanks. this is enough for atomic operations but this can't change easily ongoing revisions if you have this snippet many place in big codebase.

Change single function ease than multiple places.

@Jurigag
Copy link
Contributor

Jurigag commented Sep 20, 2016

@tugrul well i guess there will be no problem at all to add method like updateStatuses() then to this resultset class and call it :)

@tugrul
Copy link
Contributor

tugrul commented Sep 21, 2016

namespace App\Models\Content;

class Category extends \Phalcon\Mvc\Model
{
    public $id;
    public $name;
    public $parent;

    public function getSource()
    {
        return 'content_categories';
    }

    public function initialize()
    {
        $this->hasMany('id', 'App\Models\Content\Category', 'parent', ['alias' => 'Childs']);
        $this->belongsTo('parent', 'App\Models\Content\Category', 'id', ['alias' => 'Parent']);
    }

}

namespace Resultsets;
class Categories extends \Phalcon\Mvc\Model\Resultset\Simple {
    public function executeRecursive($callback, $level = 0) {

        if (!is_callable($callback)) {
            return;
        }

        foreach ($this as $category) {
            $callback($category, $level);
            $category->getChilds()->executeRecursive($callback, $level + 1);
        }

    }
}

echo '<select name="category">';
App\Models\Content\Category::find(['conditions' => 'parent IS NULL'])->executeRecursive(function($category, $level){
    echo '<option value="', $category->id ,'">', str_repeat('&ndash; ', $level), $category->name, '</option>';
});
echo '</select>';

@Fenikkusu
Copy link
Contributor

As a temporary solution to this, you are able to override the static find method on the model in question and force it to return the resultset you wish (return new Resultset('model', parent::find(...)); or something to that effect. I forget the exact logic I've used to acomplish this. It's hacky but gets the trick done.

@ashpumpkin
Copy link
Contributor Author

#12359

@ashpumpkin
Copy link
Contributor Author

Implemented in 3.1.x

@virgofx
Copy link
Contributor

virgofx commented Dec 7, 2016

@ashpumpkin Can this be revised to only go through the autoloader once -- instead of using class_implements() load the class instance and then check instanceof which will be faster as the autoloading() process only gets hit once.

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

No branches or pull requests

6 participants