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

Wrong DataProvider->totalCount #17093

Closed
anditza opened this issue Feb 1, 2019 · 11 comments
Closed

Wrong DataProvider->totalCount #17093

anditza opened this issue Feb 1, 2019 · 11 comments

Comments

@anditza
Copy link

anditza commented Feb 1, 2019

Wrong DataProvider->totalCount

What steps will reproduce the problem?

$query = MyModel::find();
$dataProvider = new ActiveDataProvider(['query' => $query);
$dataProvider->pagination->pageSize = 30;
//    Now $dataProvider->totalCount is set because of BaseDataProvider::getPagination()
$query->andFilterWhere(['number' => $this->number]);
//    From now on totalCount is NOT updated to consider this condition and any other conditions added before feeding it to a GridView and rendering.

What is the expected result?

totalCount should be set only when DataProvider gets to be used to fetch and render data.
Up to version 12.0.15 DataProvider pagination could be invoked and set before setting all its query conditions and afterwards totalCount was calculated correctly as expected.

What do you get instead?

DataProvider totalCount is wrong - total number of rows in the table!! - doesn't care about query conditions added after invoking $dataProvider->pagination.

Additional info

Related issue: #16891

Q A
Yii version 2.0.16
PHP version 7.*
Operating system Windows/Linux
@samdark samdark added this to the 2.0.16.1 milestone Feb 1, 2019
@samdark
Copy link
Member

samdark commented Feb 1, 2019

What is the AR storage you are using?

@samdark samdark added status:to be verified Needs to be reproduced and validated. type:bug Bug severity:important labels Feb 1, 2019
@np25071984
Copy link
Contributor

Is here any reason you don't use config to set pageSize value?

I think we should block any changes in query or pagination after ActiveDataProvider was initialized. Otherwise we have to recalculate it parameters every time. Or introduce an events system, which to be able to catch such changes and adjust only needed parameters.

@anditza
Copy link
Author

anditza commented Feb 2, 2019

What is the AR storage you are using?

MySQL

Is here any reason you don't use config to set pageSize value?

Supposing you refer to app config, the reason is pageSize is not fixed but different for different DataProviders.

I think we should block any changes in query or pagination after ActiveDataProvider was initialized. Otherwise we have to recalculate it parameters every time. Or introduce an events system, which to be able to catch such changes and adjust only needed parameters.

...or delay calculating and setting totalCount until it is really needed. Why not?

@np25071984
Copy link
Contributor

Supposing you refer to app config, the reason is pageSize is not fixed but different for different DataProviders.

No proplem:

$dataProvider = new ActiveDataProvider([
     'query' => $query,
     'pagination' => [
            'pageSize' => $YOUR_DYNAMIC_VALUE_HERE,
     ],
);

...or delay calculating and setting totalCount until it is really needed. Why not?

Just let the framework take care about this. It makes sense but in current case you can get exactly what you want by making up right config before ActiveDataProvider initialization.

@samdark samdark removed the status:to be verified Needs to be reproduced and validated. label Feb 3, 2019
@samdark
Copy link
Member

samdark commented Feb 3, 2019

Discussed the case in private messages with @GHopperMSK. He has a good solution for it.

@anditza
Copy link
Author

anditza commented Feb 3, 2019

@GHopperMSK 's solution works, of course :-), and I'm refactoring this way.
It's refreshing to find myself still learning basing stuff after 3 yers of using Yii2. Thanks!
But if you find a (easy) way to have totalCount reflect query at final stage (get data and render) it would be great.

@np25071984
Copy link
Contributor

np25071984 commented Feb 3, 2019

Unfortunately, my solution isn't good at all. Here are a few cases:

  1. You can't anymore set totalCount by usual way:
$provider = new ActiveDataProvider([
    'query' => Order::find()->orderBy('id'),
    'totalCount' => 10, // it makes no sense anymore
    'pagination' => [
        'pageSize' => 1,
        // now 'totalCount' must be declared here
    ],
]);
  1. Strange behavior in some cases:
$provider = new ActiveDataProvider([
    'db' => $this->getConnection(),
    'query' => $query->from('order')->orderBy('id'),
]);
$perPage = $provider->getPagination()->getPageCount(); // null
// Pagination: getPageCount() => getPageSize() => setPageSize() => totalCount = null;
  1. Absurd cases:

Next code works fine:

$provider = new ActiveDataProvider([
    'query' => Order::find()->orderBy('id'),
    'pagination' => [
        'pageSize' => 1,
        'totalCount' => 100,
    ],
]);

But if we just swap two lines, it will break:

$provider = new ActiveDataProvider([
    'query' => Order::find()->orderBy('id'),
    'pagination' => [
        'totalCount' => 100,
        'pageSize' => 1,
    ],
]);
// the same reason as in 2

@samdark
Copy link
Member

samdark commented Feb 5, 2019

yiisoft/yii2-sphinx#109

@samdark
Copy link
Member

samdark commented Feb 5, 2019

Looks like we need to revert e162386#diff-3a9563dfdfd2448d1b7edfad3ec14522

@fcaldarelli
Copy link
Member

I tryed to fix in #17132

@sebathi
Copy link
Contributor

sebathi commented Feb 21, 2019

This is limiting us to move to 2.0.16 :( How can I help to fix this one?

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

No branches or pull requests

5 participants