Skip to content

Commit

Permalink
[5.x] Avoid querying status (#9317)
Browse files Browse the repository at this point in the history
  • Loading branch information
jasonvarga authored Apr 11, 2024
1 parent 7f34ead commit 32300df
Show file tree
Hide file tree
Showing 11 changed files with 329 additions and 23 deletions.
5 changes: 5 additions & 0 deletions src/Query/ItemQueryBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,9 @@ protected function getBaseItems()
{
return $this->items;
}

public function whereStatus($status)
{
return $this->where('status', $status);
}
}
2 changes: 1 addition & 1 deletion src/Query/Scopes/Filters/Status.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public function fieldItems()

public function apply($query, $values)
{
$query->where('status', $values['status']);
$query->whereStatus($values['status']);
}

public function badge($values)
Expand Down
4 changes: 2 additions & 2 deletions src/Query/StatusQueryBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public function __construct(Builder $builder, $status = 'published')
public function get($columns = ['*'])
{
if ($this->queryFallbackStatus) {
$this->builder->where('status', $this->fallbackStatus);
$this->builder->whereStatus($this->fallbackStatus);
}

return $this->builder->get($columns);
Expand All @@ -49,7 +49,7 @@ public function first()

public function __call($method, $parameters)
{
if (in_array($method, self::METHODS) && in_array(Arr::first($parameters), ['status', 'published'])) {
if ((in_array($method, self::METHODS) && in_array(Arr::first($parameters), ['status', 'published'])) || $method === 'whereStatus') {
$this->queryFallbackStatus = false;
}

Expand Down
76 changes: 75 additions & 1 deletion src/Stache/Query/EntryQueryBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,16 @@
use Statamic\Contracts\Entries\QueryBuilder;
use Statamic\Entries\EntryCollection;
use Statamic\Facades;
use Statamic\Facades\Collection;
use Statamic\Support\Arr;

class EntryQueryBuilder extends Builder implements QueryBuilder
{
use QueriesTaxonomizedEntries;

protected $collections;
private const STATUSES = ['published', 'draft', 'scheduled', 'expired'];

protected $collections = [];

public function where($column, $operator = null, $value = null, $boolean = 'and')
{
Expand All @@ -21,6 +24,10 @@ public function where($column, $operator = null, $value = null, $boolean = 'and'
return $this;
}

if ($column === 'status') {
trigger_error('Filtering by status is deprecated. Use whereStatus() instead.', E_USER_DEPRECATED);
}

return parent::where($column, $operator, $value, $boolean);
}

Expand All @@ -32,6 +39,10 @@ public function whereIn($column, $values, $boolean = 'and')
return $this;
}

if ($column === 'status') {
trigger_error('Filtering by status is deprecated. Use whereStatus() instead.', E_USER_DEPRECATED);
}

return parent::whereIn($column, $values, $boolean);
}

Expand Down Expand Up @@ -133,6 +144,69 @@ protected function getWhereColumnKeyValuesByIndex($column)
});
}

public function whereStatus(string $status)
{
if (! in_array($status, self::STATUSES)) {
throw new \Exception("Invalid status [$status]");
}

if ($status === 'draft') {
return $this->where('published', false);
}

$this->where('published', true);

return $this->where(fn ($query) => $this
->getCollectionsForStatus()
->each(fn ($collection) => $query->orWhere(fn ($q) => $this->addCollectionStatusLogicToQuery($q, $status, $collection))));
}

private function getCollectionsForStatus()
{
// Since we have to add nested queries for each collection, if collections have been provided,
// we'll use those to avoid the need for adding unnecessary query clauses.

if (empty($this->collections)) {
return Collection::all();
}

return collect($this->collections)->map(fn ($handle) => Collection::find($handle));
}

private function addCollectionStatusLogicToQuery($query, $status, $collection)
{
// Using collectionHandle instead of collection because we intercept collection
// and put it on a property. In this case we actually want the indexed value.
// We can probably refactor this elsewhere later.
$query->where('collectionHandle', $collection->handle());

if ($collection->futureDateBehavior() === 'public' && $collection->pastDateBehavior() === 'public') {
if ($status === 'scheduled' || $status === 'expired') {
$query->where('date', 'invalid'); // intentionally trigger no results.
}
}

if ($collection->futureDateBehavior() === 'private') {
$status === 'scheduled'
? $query->where('date', '>', now())
: $query->where('date', '<', now());

if ($status === 'expired') {
$query->where('date', 'invalid'); // intentionally trigger no results.
}
}

if ($collection->pastDateBehavior() === 'private') {
$status === 'expired'
? $query->where('date', '<', now())
: $query->where('date', '>', now());

if ($status === 'scheduled') {
$query->where('date', 'invalid'); // intentionally trigger no results.
}
}
}

public function prepareForFakeQuery(): array
{
$data = parent::prepareForFakeQuery();
Expand Down
6 changes: 3 additions & 3 deletions src/Tags/Collection/Entries.php
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ protected function query()

$this->querySelect($query);
$this->querySite($query);
$this->queryStatus($query);
$this->queryPublished($query);
$this->queryPastFuture($query);
$this->querySinceUntil($query);
$this->queryTaxonomies($query);
Expand Down Expand Up @@ -270,13 +270,13 @@ protected function querySite($query)
return $query->where('site', $site);
}

protected function queryStatus($query)
protected function queryPublished($query)
{
if ($this->isQueryingCondition('status') || $this->isQueryingCondition('published')) {
return;
}

return $query->where('status', 'published');
return $query->where('published', true);
}

protected function queryPastFuture($query)
Expand Down
4 changes: 4 additions & 0 deletions src/Tags/Concerns/QueriesConditions.php
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ protected function queryCondition($query, $field, $condition, $value)

protected function queryIsCondition($query, $field, $value)
{
if ($field === 'status') {
return $query->whereStatus($value);
}

return $query->where($field, $value);
}

Expand Down
40 changes: 40 additions & 0 deletions tests/API/APITest.php
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,46 @@ public function it_filters_published_entries_by_default()
$this->assertEndpointNotFound('/api/collections/pages/entries/nectar');
}

/** @test */
public function it_filters_out_future_entries_from_future_private_collection()
{
Facades\Config::set('statamic.api.resources.collections', true);

Facades\Collection::make('test')->dated(true)
->pastDateBehavior('public')
->futureDateBehavior('private')
->save();

Facades\Entry::make()->collection('test')->id('a')->published(true)->date(now()->addDay())->save();
Facades\Entry::make()->collection('test')->id('b')->published(false)->date(now()->addDay())->save();
Facades\Entry::make()->collection('test')->id('c')->published(true)->date(now()->subDay())->save();
Facades\Entry::make()->collection('test')->id('d')->published(false)->date(now()->subDay())->save();

$response = $this->get('/api/collections/test/entries')->assertSuccessful();
$this->assertCount(1, $response->getData()->data);
$response->assertJsonPath('data.0.id', 'c');
}

/** @test */
public function it_filters_out_past_entries_from_past_private_collection()
{
Facades\Config::set('statamic.api.resources.collections', true);

Facades\Collection::make('test')->dated(true)
->pastDateBehavior('private')
->futureDateBehavior('public')
->save();

Facades\Entry::make()->collection('test')->id('a')->published(true)->date(now()->addDay())->save();
Facades\Entry::make()->collection('test')->id('b')->published(false)->date(now()->addDay())->save();
Facades\Entry::make()->collection('test')->id('c')->published(true)->date(now()->subDay())->save();
Facades\Entry::make()->collection('test')->id('d')->published(false)->date(now()->subDay())->save();

$response = $this->get('/api/collections/test/entries')->assertSuccessful();
$this->assertCount(1, $response->getData()->data);
$response->assertJsonPath('data.0.id', 'a');
}

/** @test */
public function it_can_filter_collection_entries_when_configuration_allows_for_it()
{
Expand Down
91 changes: 91 additions & 0 deletions tests/Data/Entries/EntryQueryBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,97 @@ public function entries_are_found_using_lazy()
}

/** @test */
public function filtering_using_where_status_column_writes_deprecation_log()
{
$this->withoutDeprecationHandling();
$this->expectException(\ErrorException::class);
$this->expectExceptionMessage('Filtering by status is deprecated. Use whereStatus() instead.');

$this->createDummyCollectionAndEntries();

Entry::query()->where('collection', 'posts')->where('status', 'published')->get();
}

/** @test */
public function filtering_using_whereIn_status_column_writes_deprecation_log()
{
$this->withoutDeprecationHandling();
$this->expectException(\ErrorException::class);
$this->expectExceptionMessage('Filtering by status is deprecated. Use whereStatus() instead.');

$this->createDummyCollectionAndEntries();

Entry::query()->where('collection', 'posts')->whereIn('status', ['published'])->get();
}

/** @test */
public function filtering_by_unexpected_status_throws_exception()
{
$this->expectExceptionMessage('Invalid status [foo]');

Entry::query()->whereStatus('foo')->get();
}

/**
* @test
*
* @dataProvider filterByStatusProvider
*/
public function it_filters_by_status($status, $expected)
{
Collection::make('pages')->dated(false)->save();
EntryFactory::collection('pages')->id('page')->published(true)->create();
EntryFactory::collection('pages')->id('page-draft')->published(false)->create();

Collection::make('blog')->dated(true)->futureDateBehavior('private')->pastDateBehavior('public')->save();
EntryFactory::collection('blog')->id('blog-future')->published(true)->date(now()->addDay())->create();
EntryFactory::collection('blog')->id('blog-future-draft')->published(false)->date(now()->addDay())->create();
EntryFactory::collection('blog')->id('blog-past')->published(true)->date(now()->subDay())->create();
EntryFactory::collection('blog')->id('blog-past-draft')->published(false)->date(now()->subDay())->create();

Collection::make('events')->dated(true)->futureDateBehavior('public')->pastDateBehavior('private')->save();
EntryFactory::collection('events')->id('event-future')->published(true)->date(now()->addDay())->create();
EntryFactory::collection('events')->id('event-future-draft')->published(false)->date(now()->addDay())->create();
EntryFactory::collection('events')->id('event-past')->published(true)->date(now()->subDay())->create();
EntryFactory::collection('events')->id('event-past-draft')->published(false)->date(now()->subDay())->create();

Collection::make('calendar')->dated(true)->futureDateBehavior('public')->pastDateBehavior('public')->save();
EntryFactory::collection('calendar')->id('calendar-future')->published(true)->date(now()->addDay())->create();
EntryFactory::collection('calendar')->id('calendar-future-draft')->published(false)->date(now()->addDay())->create();
EntryFactory::collection('calendar')->id('calendar-past')->published(true)->date(now()->subDay())->create();
EntryFactory::collection('calendar')->id('calendar-past-draft')->published(false)->date(now()->subDay())->create();

$this->assertEquals($expected, Entry::query()->whereStatus($status)->get()->map->id->all());
}

public static function filterByStatusProvider()
{
return [
'draft' => ['draft', [
'page-draft',
'blog-future-draft',
'blog-past-draft',
'event-future-draft',
'event-past-draft',
'calendar-future-draft',
'calendar-past-draft',
]],
'published' => ['published', [
'page',
'blog-past',
'event-future',
'calendar-future',
'calendar-past',
]],
'scheduled' => ['scheduled', [
'blog-future',
]],
'expired' => ['expired', [
'event-past',
]],
];
}

public function values_can_be_plucked()
{
$this->createDummyCollectionAndEntries();
Expand Down
Loading

0 comments on commit 32300df

Please sign in to comment.