Show total count #13
Show total count #13
Conversation
src/Controller.php
Outdated
| @@ -249,6 +249,10 @@ protected function getDefaultReadResponse( | |||
|
|
|||
| $model->setLimit($this->getRequest()->getParam('limit')); | |||
|
|
|||
| if ($this->getRequest()->getParam('include_count') == 'true') { | |||
There was a problem hiding this comment.
Might want to do strict checking ===
…delSet. Clearly assign boolean value to .
…SelectStatement::count() aggregation feature, avoids returning full data set which can potentially crash API endpoint.
|
@kfriedman, ready for review 🙂 |
src/Model/ModelTrait/DBReadTrait.php
Outdated
| */ | ||
| $baseModel = $this->getBaseModel(); | ||
|
|
||
| $selectStatement = DB::getDatabase()->select() |
There was a problem hiding this comment.
Investigate using count statement: https://github.com/FaaPz/Slim-PDO/blob/master/docs/Statement/SELECT.md#countcolumn---as--null-distinct--false
src/Model/ModelTrait/DBReadTrait.php
Outdated
| @@ -289,6 +290,10 @@ protected function setSet($ignoreNoRecord = false) | |||
| } | |||
|
|
|||
| if ($selectStatement->rowCount()) { | |||
| if ($this->isIncludeTotalCount() == true) { | |||
There was a problem hiding this comment.
Try to use strict checking
| @@ -289,6 +290,10 @@ protected function setSet($ignoreNoRecord = false) | |||
| } | |||
|
|
|||
| if ($selectStatement->rowCount()) { | |||
| if ($this->isIncludeTotalCount() === true) { | |||
src/Controller.php
Outdated
| @@ -249,6 +249,12 @@ protected function getDefaultReadResponse( | |||
|
|
|||
| $model->setLimit($this->getRequest()->getParam('limit')); | |||
|
|
|||
| $includeTotalCount = $this->getRequest()->getParam('includeTotalCount') === 'true' ? true : false ; | |||
|
|
|||
| if ($includeTotalCount === true) { | |||
There was a problem hiding this comment.
Can remove checking for true here if you're sure it's going to be boolean
| */ | ||
| public function initializeResponse($model) | ||
| { | ||
| if (is_array($model)) { |
There was a problem hiding this comment.
Refactor out elses; you can just use return statements
| { | ||
| if (is_array($model)) { | ||
| $this->initializeArrayOfModels($model); | ||
| } else if ($model instanceof ModelSet) { |
There was a problem hiding this comment.
Maybe move ModelSet to end to indicate it's the newest functionality?
src/ModelSet.php
Outdated
| */ | ||
| public function setIncludeTotalCount($includeTotalCount) | ||
| { | ||
| $this->includeTotalCount = $includeTotalCount; |
There was a problem hiding this comment.
Maybe typecast the $includeTotalCount to (bool)?
src/ModelSet.php
Outdated
| */ | ||
| public function setTotalCount($totalCount = 0) | ||
| { | ||
| $this->totalCount = $totalCount; |
There was a problem hiding this comment.
Maybe typecast $totalCount to (int)?
…n instead. Checking for ModelSet first to avoid returning baseModel in response.
Let's find a time to talk about this? Thanks!