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

[2.0.x] The Model Paginator now iterates over the models. #10251

Merged
merged 1 commit into from
May 6, 2015
Merged

[2.0.x] The Model Paginator now iterates over the models. #10251

merged 1 commit into from
May 6, 2015

Conversation

SidRoberts
Copy link
Contributor

@KorsaR-ZN, could you check your code to see if this solves the problem?

Update: This does solve an issue in the Paginator, but see also #10253 for the real fix.

@SidRoberts SidRoberts mentioned this pull request May 5, 2015
@KorsaR-ZN
Copy link
Contributor

@SidRoberts I've tried it, it does not solve the problem.
But this code will solve the problem, after fix #10100

@patrick-zippenfenig
Copy link
Contributor

Looks good. Valid() previously moved the pointer to the next position which is a non-standard behaviour. Next() has to be called. I'm currently reviewing #10100 and look for other occurrences. I did not anticipate such problems...

@KorsaR-ZN
Copy link
Contributor

@SidRoberts

$robots = Robots::find();

$page  = isset($_GET['page']) ? $_GET['page'] : 1;
$limit = isset($_GET['limit']) ? $_GET['limit'] : 3;

$paginator = new Phalcon\Paginator\Adapter\Model([
    'data' => $robots,
    'limit' => $limit,
    'page' => $page
]);

$paginate = $paginator->getPaginate();

echo 'Items: ' . count($robots) . '<br>';
echo 'Page: ' . $page . ' / ' . $paginate->total_pages . '<br>';

echo 'Results: ';

foreach ($paginate->items as $item) {
    echo $item->id . ',';
}

2.0.x (fail)

Test 1 ($page = 1)

Items: 53
Page: 1 / 18
Results: 1,2,3,

Test 2 ($page = 2)

Items: 53
Page: 2 / 18
Results: 7,8,9,

Test 3 ($page = 5)

Items: 53
Page: 5 / 18
Results: 25,26,27,

Test 4 ($page = 17)

Items: 53
Page: 17 / 18
Results: // FAIL

2.0.0 (success)

Test 1 ($page = 1)

Items: 53
Page: 1 / 18
Results: 1,2,3,

Test 2 ($page = 2)

Items: 53
Page: 2 / 18
Results: 4,5,6,

Test 3 ($page = 5)

Items: 53
Page: 5 / 18
Results: 13,14,15,

Test 4 ($page = 17)

Items: 53
Page: 17 / 18
Results: 49,50,51,

@KorsaR-ZN
Copy link
Contributor

@patrick-zippenfenig
Error in the method \Phalcon\Mvc\Model\Resultset::seek()

Please use Paginator tests, https://gist.github.com/KorsaR-ZN/e649cf5a1325112472d3,
It is tests partly, but covers the error. When fix #10100, then I improve tests...

@patrick-zippenfenig
Copy link
Contributor

#10253 should fix the issues mentioned by @KorsaR-ZN. valid() is also used in volt.zep#L248 but correctly implemented.

@KorsaR-ZN
Copy link
Contributor

👍

andresgutierrez added a commit that referenced this pull request May 6, 2015
[2.0.x] The Model Paginator now iterates over the models.
@andresgutierrez andresgutierrez merged commit a012256 into phalcon:2.0.x May 6, 2015
@andresgutierrez
Copy link
Contributor

Thanks

@SidRoberts SidRoberts deleted the paginator-model-iterator branch May 6, 2015 13:17
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

Successfully merging this pull request may close these issues.

4 participants