Skip to content

WIP QueryBuilder pagination #11

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.

Sign up for GitHub

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

Draft
joelambert wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

WIP QueryBuilder pagination #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ef713c8
Remove redundant properties
joelambert Aug 25, 2020
ae4064d
Implement `paginate()` function
joelambert Aug 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 16 additions & 13 deletions src/Contracts/QueryBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,33 +2,36 @@

namespace Rareloop\Lumberjack\Contracts;

use Timber\PostQuery;
use Tightenco\Collect\Support\Collection;

interface QueryBuilder
{
public function getParameters() : array;
public function getParameters(): array;

public function wherePostType($postType) : QueryBuilder;
public function wherePostType($postType): QueryBuilder;

public function limit($limit) : QueryBuilder;
public function limit($limit): QueryBuilder;

public function offset($offset) : QueryBuilder;
public function offset($offset): QueryBuilder;

public function orderBy($orderBy, string $order = QueryBuilder::ASC) : QueryBuilder;
public function orderBy($orderBy, string $order = QueryBuilder::ASC): QueryBuilder;

public function orderByMeta($metaKey, string $order = QueryBuilder::ASC, string $type = null) : QueryBuilder;
public function orderByMeta($metaKey, string $order = QueryBuilder::ASC, string $type = null): QueryBuilder;

public function whereIdIn(array $ids) : QueryBuilder;
public function whereIdIn(array $ids): QueryBuilder;

public function whereIdNotIn(array $ids) : QueryBuilder;
public function whereIdNotIn(array $ids): QueryBuilder;

public function whereStatus() : QueryBuilder;
public function whereStatus(): QueryBuilder;

public function whereMeta($key, $value, $compare = '=', $type = null) : QueryBuilder;
public function whereMeta($key, $value, $compare = '=', $type = null): QueryBuilder;

public function whereMetaRelationshipIs(string $relation) : QueryBuilder;
public function whereMetaRelationshipIs(string $relation): QueryBuilder;

public function get() : Collection;
public function get(): Collection;

public function clone() : QueryBuilder;
public function paginate($perPage = 10, $page = 1): PostQuery;

public function clone(): QueryBuilder;
}
67 changes: 34 additions & 33 deletions src/QueryBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Rareloop\Lumberjack\Post;
use Spatie\Macroable\Macroable;
use Tightenco\Collect\Support\Collection;
use Timber\PostQuery;
use Timber\Timber;

class QueryBuilder implements QueryBuilderContract
Expand All @@ -15,24 +16,6 @@ class QueryBuilder implements QueryBuilderContract

protected $postClass = Post::class;

private $postType;

private $limit;
private $offset;

private $orderby;
private $order;

private $metaOrderBy;
private $metaOrder;
private $metaOrderNumeric;

private $whereIn;
private $whereNotIn;

private $metaRelationship;
private $metaQueries = [];

private $params = [];

// Order Directions
Expand All @@ -46,33 +29,33 @@ class QueryBuilder implements QueryBuilderContract
const OR = 'OR';
const AND = 'AND';

public function getParameters() : array
public function getParameters(): array
{
return $this->params;
}

public function wherePostType($postType) : QueryBuilderContract
public function wherePostType($postType): QueryBuilderContract
{
$this->params['post_type'] = $postType;

return $this;
}

public function limit($limit) : QueryBuilderContract
public function limit($limit): QueryBuilderContract
{
$this->params['posts_per_page'] = $limit;

return $this;
}

public function offset($offset) : QueryBuilderContract
public function offset($offset): QueryBuilderContract
{
$this->params['offset'] = $offset;

return $this;
}

public function orderBy($orderBy, string $order = QueryBuilder::ASC) : QueryBuilderContract
public function orderBy($orderBy, string $order = QueryBuilder::ASC): QueryBuilderContract
{
$order = strtoupper($order);

Expand All @@ -82,7 +65,7 @@ public function orderBy($orderBy, string $order = QueryBuilder::ASC) : QueryBuil
return $this;
}

public function orderByMeta($metaKey, string $order = QueryBuilder::ASC, string $type = null) : QueryBuilderContract
public function orderByMeta($metaKey, string $order = QueryBuilder::ASC, string $type = null): QueryBuilderContract
{
$order = strtoupper($order);

Expand All @@ -93,21 +76,21 @@ public function orderByMeta($metaKey, string $order = QueryBuilder::ASC, string
return $this;
}

public function whereIdIn(array $ids) : QueryBuilderContract
public function whereIdIn(array $ids): QueryBuilderContract
{
$this->params['post__in'] = $ids;

return $this;
}

public function whereIdNotIn(array $ids) : QueryBuilderContract
public function whereIdNotIn(array $ids): QueryBuilderContract
{
$this->params['post__not_in'] = $ids;

return $this;
}

public function whereStatus() : QueryBuilderContract
public function whereStatus(): QueryBuilderContract
{
$args = func_get_args();

Expand All @@ -125,7 +108,7 @@ protected function initialiseMetaQuery()
$this->params['meta_query'] = $this->params['meta_query'] ?? [];
}

public function whereMeta($key, $value, $compare = '=', $type = null) : QueryBuilderContract
public function whereMeta($key, $value, $compare = '=', $type = null): QueryBuilderContract
{
$meta = [
'key' => $key,
Expand All @@ -143,7 +126,7 @@ public function whereMeta($key, $value, $compare = '=', $type = null) : QueryBui
return $this;
}

public function whereMetaRelationshipIs(string $relation) : QueryBuilderContract
public function whereMetaRelationshipIs(string $relation): QueryBuilderContract
{
$relation = strtoupper($relation);

Expand All @@ -159,14 +142,14 @@ public function whereMetaRelationshipIs(string $relation) : QueryBuilderContract
return $this;
}

public function as($postClass) : QueryBuilderContract
public function as($postClass): QueryBuilderContract
{
$this->postClass = $postClass;

return $this;
}

public function get() : Collection
public function get(): Collection
{
$posts = Timber::get_posts($this->getParameters(), $this->postClass);

Expand All @@ -177,12 +160,30 @@ public function get() : Collection
return collect($posts);
}

public function paginate($perPage = 10, $page = 1): PostQuery
{
global $paged;

if (isset($page)) {
Copy link

@idflood idflood Sep 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was missing this feature and stumbled upon this merge request. I quickly checked how it was implemented by curiosity and I might have found an issue here : )

Since the $page attribute has a default value in the function params it will always be set. This means that if the user doesn't pass the $page attribute this will always reset the $paged value to 1.

One solution is to set the $page = NULL in the params and check if $page !== NULL here instead of the isset. I still didn't test it in a real wordpress setup so I might be wrong.

Copy link
Member Author

@joelambert joelambert Sep 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback, I guess the if is redundant here. I can't see that it would cause any issue but you're right that it will always be set.

I think it's important to provide a default as I can't think of a scenario where when providing no $page param you wouldn't want the first page back.

$paged = $page;
}

if (!isset($paged) || !$paged) {
$paged = 1;
}

$this->limit($perPage);
$this->params['paged'] = $paged;

return new PostQuery($this->getParameters(), $this->postClass);
}

/**
* Get the first Post that matches the current query. If no Post matches then return `null`.
*
* @return \Rareloop\Lumberjack\Post|null
*/
public function first() : ?Post
public function first(): ?Post
{
$params = array_merge($this->getParameters(), [
'limit' => 1,
Expand All @@ -197,7 +198,7 @@ public function first() : ?Post
return collect($posts)->first();
}

public function clone() : QueryBuilderContract
public function clone(): QueryBuilderContract
{
$clone = clone $this;

Expand Down
84 changes: 79 additions & 5 deletions tests/Unit/QueryBuilderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,21 @@

namespace Rareloop\Lumberjack\Test;

use Illuminate\Support\Collection;
use Mockery;
use PHPUnit\Framework\TestCase;
use Timber\Timber;
use Timber\PostQuery;
use Timber\PostGetter;
use Timber\PostCollection;
use Brain\Monkey\Functions;
use Rareloop\Lumberjack\Post;
use PHPUnit\Framework\TestCase;
use Illuminate\Support\Collection;
use Rareloop\Lumberjack\QueryBuilder;
use Timber\Timber;
use Rareloop\Lumberjack\Test\Unit\BrainMonkeyPHPUnitIntegration;

class QueryBuilderTest extends TestCase
{
use \Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;
use BrainMonkeyPHPUnitIntegration;

/** @test */
public function correct_post_type_is_set()
Expand Down Expand Up @@ -400,6 +405,75 @@ public function get_retrieves_empty_collection_when_timber_returns_null()
$this->assertSame(0, $returnedPosts->count());
}

/**
* @test
* @runInSeparateProcess
* @preserveGlobalState disabled
*/
public function paginate_retrieves_PostQuery_object_with_correct_defaults()
{
$this->assertPostQueryParameters([
'post_status' => 'publish',
'posts_per_page' => 10,
'paged' => 1,
]);

$builder = new QueryBuilder();
$returnedPostQuery = $builder->whereStatus('publish')->paginate();

$this->assertInstanceOf(PostQuery::class, $returnedPostQuery);
}

/**
* @test
* @runInSeparateProcess
* @preserveGlobalState disabled
*/
public function paginate_retrieves_PostQuery_object_with_provided_arguments()
{
$this->assertPostQueryParameters([
'post_status' => 'publish',
'posts_per_page' => 20,
'paged' => 2,
]);

$builder = new QueryBuilder();
$returnedPostQuery = $builder->whereStatus('publish')->paginate(20, 2);

$this->assertInstanceOf(PostQuery::class, $returnedPostQuery);
}

/**
* We have to mock quite a bit more than is ideal for this but there is no other way to hook
* into this bit of Timber that I've found. It does leave us a little open to failing tests
* if Timber change their implementation but not much we can do about that.
*
* @param array $params The parameters to assert are passed in construction
* @return void
*/
protected function assertPostQueryParameters(array $params)
{
Functions\expect('get_post_type')
->andReturn('post');

$posts = [new Post(1, true), new Post(2, true)];

$postGetter = Mockery::mock('alias:' . PostGetter::class);

$postGetter
->shouldReceive('get_post_class')
->andReturn(Post::class);

$postGetter
->shouldReceive('query_posts')
->withArgs([
Mockery::subset($params),
Post::class,
])
->once()
->andReturn(new PostCollection($posts, Post::class));
}

/**
* @test
* @runInSeparateProcess
Expand Down Expand Up @@ -593,7 +667,7 @@ class QueryBuilderMixin
{
function testFunctionAddedByMixin()
{
return function() {
return function () {
$this->params['foo'] = 'bar';

return $this;
Expand Down