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

Selectable-Adapter->count() ignores Criteria #227

Closed
Blaimi opened this issue Apr 29, 2013 · 11 comments
Closed

Selectable-Adapter->count() ignores Criteria #227

Blaimi opened this issue Apr 29, 2013 · 11 comments
Assignees

Comments

@Blaimi
Copy link

Blaimi commented Apr 29, 2013

diff --git a/src/DoctrineModule/Paginator/Adapter/Selectable.php b/src/DoctrineModule/Paginator/Adapter/Selectable.php
index 5984449..1f35c7d 100644
--- a/src/DoctrineModule/Paginator/Adapter/Selectable.php
+++ b/src/DoctrineModule/Paginator/Adapter/Selectable.php
@@ -73,6 +73,6 @@
      */
     public function count()
     {
-        return count($this->selectable->matching(new Criteria()));
+        return count($this->selectable->matching($this->criteria));
     }
 }
@Ocramius
Copy link
Member

Ping @bakura10

@Ocramius
Copy link
Member

@Blaimi now that I think of it, it's:

$criteria = clone $this->criteria;
$criteria->setFirstResult(null);
$criteria->setMaxResults(null);
return $this->selectable->matching($this->criteria)->count();

@Blaimi
Copy link
Author

Blaimi commented Apr 29, 2013

If I’m using it with paginator, I’m getting the wrong amount of total Items

$adapter = new SelectableAdapter($repository, $criteria);
$paginator = new Paginator($adapter);
echo $paginator->getTotalItemCount();

@Ocramius
Copy link
Member

@Blaimi yes, was aware of that - any suggestions on the snippet I pasted?

@Blaimi
Copy link
Author

Blaimi commented Apr 29, 2013

Ah, sorry, I found the bug in my patch (and in my brain :D ).

diff --git a/src/DoctrineModule/Paginator/Adapter/Selectable.php b/src/DoctrineModule/Paginator/Adapter/Selectable.php
index 5984449..2f6b156 100644
--- a/src/DoctrineModule/Paginator/Adapter/Selectable.php
+++ b/src/DoctrineModule/Paginator/Adapter/Selectable.php
@@ -73,6 +73,9 @@
      */
     public function count()
     {
-        return count($this->selectable->matching(new Criteria()));
+        $criteria = clone $this->criteria;
+        $criteria->setFirstResult(null);
+        $criteria->setMaxResults(null);
+        return $this->selectable->matching($this->criteria)->count();
     }
 }

@Ocramius
Copy link
Member

@Blaimi any chance to make a PR with a test?

@Blaimi
Copy link
Author

Blaimi commented Apr 29, 2013

I’ll try it tonight. I’m not yet that firm with the git-hub-handling.

@Ocramius
Copy link
Member

@Blaimi ok. Otherwise ping me and I'll do it

@Ocramius
Copy link
Member

Ocramius commented May 1, 2013

News on this one?

@Blaimi
Copy link
Author

Blaimi commented May 15, 2013

@Ocramius sorry, I can’t find any time atm. Can you make the PR?

there’s another error on the return line

diff --git a/src/DoctrineModule/Paginator/Adapter/Selectable.php b/src/DoctrineModule/Paginator/Adapter/Selectable.php
index 5984449..2f6b156 100644
--- a/src/DoctrineModule/Paginator/Adapter/Selectable.php
+++ b/src/DoctrineModule/Paginator/Adapter/Selectable.php
@@ -73,6 +73,9 @@
      */
     public function count()
     {
-        return count($this->selectable->matching(new Criteria()));
+        $criteria = clone $this->criteria;
+        $criteria->setFirstResult(null);
+        $criteria->setMaxResults(null);
+        return $this->selectable->matching($criteria)->count();
     }
 }

@ghost ghost assigned Ocramius May 15, 2013
@Ocramius
Copy link
Member

Can do

Ocramius added a commit that referenced this issue May 17, 2013
…apter-count

Adding test that verifies #227 - criteria is ignored in `count`
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

2 participants