From 1124df813846542157de09e2cfc874d06d312560 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Fri, 12 Jan 2024 16:03:46 -0500 Subject: [PATCH 01/10] query published instead of status --- src/Tags/Collection/Entries.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tags/Collection/Entries.php b/src/Tags/Collection/Entries.php index d18a9f55aa9..6acf3722c2e 100644 --- a/src/Tags/Collection/Entries.php +++ b/src/Tags/Collection/Entries.php @@ -276,7 +276,7 @@ protected function queryStatus($query) return; } - return $query->where('status', 'published'); + return $query->where('published', true); } protected function queryPastFuture($query) From ebe4a2f94a3ed3db12206e3ffb0ad58df24375ff Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Fri, 12 Jan 2024 17:31:18 -0500 Subject: [PATCH 02/10] rename --- src/Tags/Collection/Entries.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Tags/Collection/Entries.php b/src/Tags/Collection/Entries.php index 6acf3722c2e..d3a9a6b0db8 100644 --- a/src/Tags/Collection/Entries.php +++ b/src/Tags/Collection/Entries.php @@ -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); @@ -270,7 +270,7 @@ 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; From 31b344fe2cf724054818dd6ebae149a29893a649 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Fri, 12 Jan 2024 17:31:51 -0500 Subject: [PATCH 03/10] adjust status filter to query by date appropriately, and add tests --- src/Query/Scopes/Filters/Status.php | 22 +++- .../Query/Scopes/Filters/StatusFilterTest.php | 114 ++++++++++++++++++ 2 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 tests/Query/Scopes/Filters/StatusFilterTest.php diff --git a/src/Query/Scopes/Filters/Status.php b/src/Query/Scopes/Filters/Status.php index d0ab3c540aa..de153901914 100644 --- a/src/Query/Scopes/Filters/Status.php +++ b/src/Query/Scopes/Filters/Status.php @@ -26,7 +26,27 @@ public function fieldItems() public function apply($query, $values) { - $query->where('status', $values['status']); + $status = $values['status']; + + if ($status === 'draft') { + return $query->where('published', false); + } + + $query->where('published', true); + + $collection = $this->collection(); + + if ($collection->futureDateBehavior() === 'private') { + $status === 'scheduled' + ? $query->where('date', '>', now()) + : $query->where('date', '<', now()); + } + + if ($collection->pastDateBehavior() === 'private') { + $status === 'expired' + ? $query->where('date', '<', now()) + : $query->where('date', '>', now()); + } } public function badge($values) diff --git a/tests/Query/Scopes/Filters/StatusFilterTest.php b/tests/Query/Scopes/Filters/StatusFilterTest.php new file mode 100644 index 00000000000..292992cffd0 --- /dev/null +++ b/tests/Query/Scopes/Filters/StatusFilterTest.php @@ -0,0 +1,114 @@ +context(['collection' => 'test']) + ->apply($query, ['status' => $status]); + } + + /** @test */ + public function filters_by_draft() + { + $query = Mockery::mock(EntryQueryBuilder::class); + $query->shouldReceive('where')->with('published', false)->once(); + + $this->filter($query, 'draft'); + } + + /** @test */ + public function non_dated_collection_filters_by_published() + { + Collection::make('test')->save(); + + $query = Mockery::mock(EntryQueryBuilder::class); + $query->shouldReceive('where')->with('published', true)->once(); + + $this->filter($query, 'published'); + } + + /** @test */ + public function future_private_dated_collection_filters_by_published() + { + Collection::make('test')->dated(true)->futureDateBehavior('private')->save(); + + $query = Mockery::mock(EntryQueryBuilder::class); + $query->shouldReceive('where')->with('published', true)->once(); + $query->shouldReceive('where')->withArgs(function ($arg1, $arg2, $arg3) { + return $arg1 === 'date' + && $arg2 === '<' + && $arg3->eq(now()); + })->once(); + + $this->filter($query, 'published'); + } + + /** @test */ + public function future_private_dated_collection_filters_by_scheduled() + { + Collection::make('test')->dated(true)->futureDateBehavior('private')->save(); + + $query = Mockery::mock(EntryQueryBuilder::class); + $query->shouldReceive('where')->with('published', true)->once(); + $query->shouldReceive('where')->withArgs(function ($arg1, $arg2, $arg3) { + return $arg1 === 'date' + && $arg2 === '>' + && $arg3->eq(now()); + })->once(); + + $this->filter($query, 'scheduled'); + } + + /** @test */ + public function past_private_dated_collection_filters_by_published() + { + Collection::make('test')->dated(true)->pastDateBehavior('private')->save(); + + $query = Mockery::mock(EntryQueryBuilder::class); + $query->shouldReceive('where')->with('published', true)->once(); + $query->shouldReceive('where')->withArgs(function ($arg1, $arg2, $arg3) { + return $arg1 === 'date' + && $arg2 === '>' + && $arg3->eq(now()); + })->once(); + + $this->filter($query, 'published'); + } + + /** @test */ + public function past_private_dated_collection_filters_by_expired() + { + Collection::make('test')->dated(true)->pastDateBehavior('private')->save(); + + $query = Mockery::mock(EntryQueryBuilder::class); + $query->shouldReceive('where')->with('published', true)->once(); + $query->shouldReceive('where')->withArgs(function ($arg1, $arg2, $arg3) { + return $arg1 === 'date' + && $arg2 === '<' + && $arg3->eq(now()); + })->once(); + + $this->filter($query, 'expired'); + } +} From d1dfcc5feeb0fb86c4f0397ec5355e0364c58b02 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Wed, 17 Jan 2024 11:12:46 -0500 Subject: [PATCH 04/10] wip --- src/Query/Scopes/Filters/Status.php | 22 +--- src/Query/StatusQueryBuilder.php | 4 +- src/Stache/Query/EntryQueryBuilder.php | 79 +++++++++++- tests/API/APITest.php | 40 ++++++ tests/Data/Entries/EntryQueryBuilderTest.php | 79 ++++++++++++ tests/Feature/GraphQL/EntriesTest.php | 79 +++++++++++- tests/Fieldtypes/EntriesTest.php | 27 +++-- .../Query/Scopes/Filters/StatusFilterTest.php | 114 ------------------ tests/Query/StatusQueryBuilderTest.php | 18 ++- 9 files changed, 308 insertions(+), 154 deletions(-) delete mode 100644 tests/Query/Scopes/Filters/StatusFilterTest.php diff --git a/src/Query/Scopes/Filters/Status.php b/src/Query/Scopes/Filters/Status.php index de153901914..733e9bde76a 100644 --- a/src/Query/Scopes/Filters/Status.php +++ b/src/Query/Scopes/Filters/Status.php @@ -26,27 +26,7 @@ public function fieldItems() public function apply($query, $values) { - $status = $values['status']; - - if ($status === 'draft') { - return $query->where('published', false); - } - - $query->where('published', true); - - $collection = $this->collection(); - - if ($collection->futureDateBehavior() === 'private') { - $status === 'scheduled' - ? $query->where('date', '>', now()) - : $query->where('date', '<', now()); - } - - if ($collection->pastDateBehavior() === 'private') { - $status === 'expired' - ? $query->where('date', '<', now()) - : $query->where('date', '>', now()); - } + $query->whereStatus($values['status']); } public function badge($values) diff --git a/src/Query/StatusQueryBuilder.php b/src/Query/StatusQueryBuilder.php index 1ee60ee1d21..d5ee98d3cdb 100644 --- a/src/Query/StatusQueryBuilder.php +++ b/src/Query/StatusQueryBuilder.php @@ -35,7 +35,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); @@ -48,7 +48,7 @@ public function first() public function __call($method, $parameters) { - if (in_array($method, self::METHODS) && in_array(array_first($parameters), ['status', 'published'])) { + if ((in_array($method, self::METHODS) && in_array(array_first($parameters), ['status', 'published'])) || $method === 'whereStatus') { $this->queryFallbackStatus = false; } diff --git a/src/Stache/Query/EntryQueryBuilder.php b/src/Stache/Query/EntryQueryBuilder.php index e43fe045b52..aa4533a3cc2 100644 --- a/src/Stache/Query/EntryQueryBuilder.php +++ b/src/Stache/Query/EntryQueryBuilder.php @@ -2,15 +2,19 @@ namespace Statamic\Stache\Query; +use Illuminate\Support\Facades\Log; use Statamic\Contracts\Entries\QueryBuilder; use Statamic\Entries\EntryCollection; use Statamic\Facades; +use Statamic\Facades\Collection; 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') { @@ -20,6 +24,10 @@ public function where($column, $operator = null, $value = null, $boolean = 'and' return $this; } + if ($column === 'status') { + Log::debug('Filtering by status is deprecated. Use whereStatus() instead.'); + } + return parent::where($column, $operator, $value, $boolean); } @@ -131,4 +139,73 @@ protected function getWhereColumnKeyValuesByIndex($column) return $this->getWhereColumnKeysFromStore($collection, ['column' => $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); + + $this->where(function ($query) use ($status) { + $this->getCollectionsForStatus()->each(function ($collection) use ($query, $status) { + $query->orWhere(function ($q) use ($collection, $status) { + $this->addCollectionStatusLogicToQuery($q, $status, $collection); + }); + }); + }); + + return $this; + } + + 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. + } + } + } } diff --git a/tests/API/APITest.php b/tests/API/APITest.php index 2e3ad65265d..72efd04dc47 100644 --- a/tests/API/APITest.php +++ b/tests/API/APITest.php @@ -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() { diff --git a/tests/Data/Entries/EntryQueryBuilderTest.php b/tests/Data/Entries/EntryQueryBuilderTest.php index ed85feb3566..5456c0528ec 100644 --- a/tests/Data/Entries/EntryQueryBuilderTest.php +++ b/tests/Data/Entries/EntryQueryBuilderTest.php @@ -3,6 +3,7 @@ namespace Tests\Data\Entries; use Facades\Tests\Factories\EntryFactory; +use Illuminate\Support\Facades\Log; use Statamic\Facades\Blueprint; use Statamic\Facades\Collection; use Statamic\Facades\Entry; @@ -759,4 +760,82 @@ public function entries_are_found_using_lazy() $this->assertInstanceOf(\Illuminate\Support\LazyCollection::class, $entries); $this->assertCount(3, $entries); } + + /** @test */ + public function filtering_by_status_column_writes_deprecation_log() + { + $this->createDummyCollectionAndEntries(); + + Log::shouldReceive('debug')->with('Filtering by status is deprecated. Use whereStatus() instead.')->once(); + + Entry::query()->where('collection', 'posts')->where('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 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', + ]], + ]; + } } diff --git a/tests/Feature/GraphQL/EntriesTest.php b/tests/Feature/GraphQL/EntriesTest.php index 64771fc86bd..06841217491 100644 --- a/tests/Feature/GraphQL/EntriesTest.php +++ b/tests/Feature/GraphQL/EntriesTest.php @@ -761,7 +761,7 @@ public function it_sorts_entries_on_multiple_fields() } /** @test */ - public function it_only_shows_published_entries_by_default() + public function it_filters_out_drafts_by_default() { FilterAuthorizer::shouldReceive('allowedForSubResources') ->andReturn(['published', 'status']); @@ -871,4 +871,81 @@ public function it_only_shows_published_entries_by_default() ['id' => '1', 'title' => 'Standard Blog Post'], ]]]]); } + + /** @test */ + public function it_filters_out_future_entries_from_future_private_collection() + { + $default = Blueprint::makeFromFields([]); + BlueprintRepository::shouldReceive('find')->with('default')->andReturn($default); + + FilterAuthorizer::shouldReceive('allowedForSubResources') + ->andReturn(['published', 'status']); + + Collection::make('test')->dated(true) + ->pastDateBehavior('public') + ->futureDateBehavior('private') + ->save(); + + Entry::make()->collection('test')->id('a')->published(true)->date(now()->addDay())->save(); + Entry::make()->collection('test')->id('b')->published(false)->date(now()->addDay())->save(); + Entry::make()->collection('test')->id('c')->published(true)->date(now()->subDay())->save(); + Entry::make()->collection('test')->id('d')->published(false)->date(now()->subDay())->save(); + + $query = <<<'GQL' +{ + entries { + data { + id + } + } +} +GQL; + + $this + ->withoutExceptionHandling() + ->post('/graphql', ['query' => $query]) + ->assertGqlOk() + ->assertExactJson(['data' => ['entries' => ['data' => [ + ['id' => 'c'], + ]]]]); + } + + /** @test */ + public function it_filters_out_past_entries_from_past_private_collection() + { + + $default = Blueprint::makeFromFields([]); + BlueprintRepository::shouldReceive('find')->with('default')->andReturn($default); + + FilterAuthorizer::shouldReceive('allowedForSubResources') + ->andReturn(['published', 'status']); + + Collection::make('test')->dated(true) + ->pastDateBehavior('private') + ->futureDateBehavior('public') + ->save(); + + Entry::make()->collection('test')->id('a')->published(true)->date(now()->addDay())->save(); + Entry::make()->collection('test')->id('b')->published(false)->date(now()->addDay())->save(); + Entry::make()->collection('test')->id('c')->published(true)->date(now()->subDay())->save(); + Entry::make()->collection('test')->id('d')->published(false)->date(now()->subDay())->save(); + + $query = <<<'GQL' +{ + entries { + data { + id + } + } +} +GQL; + + $this + ->withoutExceptionHandling() + ->post('/graphql', ['query' => $query]) + ->assertGqlOk() + ->assertExactJson(['data' => ['entries' => ['data' => [ + ['id' => 'a'], + ]]]]); + } } diff --git a/tests/Fieldtypes/EntriesTest.php b/tests/Fieldtypes/EntriesTest.php index 2d3890657ff..6f494cc4474 100644 --- a/tests/Fieldtypes/EntriesTest.php +++ b/tests/Fieldtypes/EntriesTest.php @@ -26,22 +26,23 @@ public function setUp(): void { parent::setUp(); - Carbon::setTestNow(Carbon::parse('2021-01-02')); + Carbon::setTestNow(Carbon::parse('2021-01-03')); Site::setConfig(['sites' => [ 'en' => ['url' => 'http://localhost/', 'locale' => 'en'], 'fr' => ['url' => 'http://localhost/fr/', 'locale' => 'fr'], ]]); - $collection = tap(Facades\Collection::make('blog')->routes('blog/{slug}'))->sites(['en', 'fr'])->dated(true)->pastDateBehavior('private')->futureDateBehavior('private')->save(); + $blog = tap(Facades\Collection::make('blog')->routes('blog/{slug}'))->sites(['en', 'fr'])->dated(true)->pastDateBehavior('public')->futureDateBehavior('private')->save(); + $events = Facades\Collection::make('events')->sites(['en', 'fr'])->dated(true)->pastDateBehavior('private')->futureDateBehavior('public')->save(); - EntryFactory::id('123')->collection($collection)->slug('one')->data(['title' => 'One'])->date('2021-01-02')->create(); - EntryFactory::id('456')->collection($collection)->slug('two')->data(['title' => 'Two'])->date('2021-01-02')->create(); - EntryFactory::id('789')->collection($collection)->slug('three')->data(['title' => 'Three'])->date('2021-01-02')->create(); - EntryFactory::id('910')->collection($collection)->slug('four')->data(['title' => 'Four'])->date('2021-01-02')->create(); - EntryFactory::id('draft')->collection($collection)->slug('draft')->data(['title' => 'Draft'])->published(false)->create(); - EntryFactory::id('scheduled')->collection($collection)->slug('scheduled')->data(['title' => 'Scheduled'])->date('2021-01-03')->create(); - EntryFactory::id('expired')->collection($collection)->slug('expired')->data(['title' => 'Expired'])->date('2021-01-01')->create(); + EntryFactory::id('123')->collection($blog)->slug('one')->data(['title' => 'One'])->date('2021-01-02')->create(); + EntryFactory::id('456')->collection($blog)->slug('two')->data(['title' => 'Two'])->date('2021-01-02')->create(); + EntryFactory::id('789')->collection($blog)->slug('three')->data(['title' => 'Three'])->date('2021-01-02')->create(); + EntryFactory::id('910')->collection($blog)->slug('four')->data(['title' => 'Four'])->date('2021-01-02')->create(); + EntryFactory::id('draft')->collection($blog)->slug('draft')->data(['title' => 'Draft'])->published(false)->date('2021-01-02')->create(); + EntryFactory::id('scheduled')->collection($blog)->slug('scheduled')->data(['title' => 'Scheduled'])->date('2021-01-04')->create(); + EntryFactory::id('expired')->collection($events)->slug('expired')->data(['title' => 'Expired'])->date('2021-01-01')->create(); } /** @@ -65,10 +66,10 @@ public function augmentQueryBuilderProvider() { return [ 'published (default, no where clause)' => [['456', '123'], fn ($q) => null], - 'status published (explicit where status clause)' => [['456', '123'], fn ($q) => $q->where('status', 'published')], - 'status draft' => [['draft'], fn ($q) => $q->where('status', 'draft')], - 'status scheduled' => [['scheduled'], fn ($q) => $q->where('status', 'scheduled')], - 'status expired' => [['expired'], fn ($q) => $q->where('status', 'expired')], + 'status published (explicit where status clause)' => [['456', '123'], fn ($q) => $q->whereStatus('published')], + 'status draft' => [['draft'], fn ($q) => $q->whereStatus('draft')], + 'status scheduled' => [['scheduled'], fn ($q) => $q->whereStatus('scheduled')], + 'status expired' => [['expired'], fn ($q) => $q->whereStatus('expired')], 'any status' => [['456', '123', 'draft', 'scheduled', 'expired'], fn ($q) => $q->whereAnyStatus()], 'published true' => [['456', '123', 'scheduled', 'expired'], fn ($q) => $q->where('published', true)], 'published false' => [['draft'], fn ($q) => $q->where('published', false)], diff --git a/tests/Query/Scopes/Filters/StatusFilterTest.php b/tests/Query/Scopes/Filters/StatusFilterTest.php deleted file mode 100644 index 292992cffd0..00000000000 --- a/tests/Query/Scopes/Filters/StatusFilterTest.php +++ /dev/null @@ -1,114 +0,0 @@ -context(['collection' => 'test']) - ->apply($query, ['status' => $status]); - } - - /** @test */ - public function filters_by_draft() - { - $query = Mockery::mock(EntryQueryBuilder::class); - $query->shouldReceive('where')->with('published', false)->once(); - - $this->filter($query, 'draft'); - } - - /** @test */ - public function non_dated_collection_filters_by_published() - { - Collection::make('test')->save(); - - $query = Mockery::mock(EntryQueryBuilder::class); - $query->shouldReceive('where')->with('published', true)->once(); - - $this->filter($query, 'published'); - } - - /** @test */ - public function future_private_dated_collection_filters_by_published() - { - Collection::make('test')->dated(true)->futureDateBehavior('private')->save(); - - $query = Mockery::mock(EntryQueryBuilder::class); - $query->shouldReceive('where')->with('published', true)->once(); - $query->shouldReceive('where')->withArgs(function ($arg1, $arg2, $arg3) { - return $arg1 === 'date' - && $arg2 === '<' - && $arg3->eq(now()); - })->once(); - - $this->filter($query, 'published'); - } - - /** @test */ - public function future_private_dated_collection_filters_by_scheduled() - { - Collection::make('test')->dated(true)->futureDateBehavior('private')->save(); - - $query = Mockery::mock(EntryQueryBuilder::class); - $query->shouldReceive('where')->with('published', true)->once(); - $query->shouldReceive('where')->withArgs(function ($arg1, $arg2, $arg3) { - return $arg1 === 'date' - && $arg2 === '>' - && $arg3->eq(now()); - })->once(); - - $this->filter($query, 'scheduled'); - } - - /** @test */ - public function past_private_dated_collection_filters_by_published() - { - Collection::make('test')->dated(true)->pastDateBehavior('private')->save(); - - $query = Mockery::mock(EntryQueryBuilder::class); - $query->shouldReceive('where')->with('published', true)->once(); - $query->shouldReceive('where')->withArgs(function ($arg1, $arg2, $arg3) { - return $arg1 === 'date' - && $arg2 === '>' - && $arg3->eq(now()); - })->once(); - - $this->filter($query, 'published'); - } - - /** @test */ - public function past_private_dated_collection_filters_by_expired() - { - Collection::make('test')->dated(true)->pastDateBehavior('private')->save(); - - $query = Mockery::mock(EntryQueryBuilder::class); - $query->shouldReceive('where')->with('published', true)->once(); - $query->shouldReceive('where')->withArgs(function ($arg1, $arg2, $arg3) { - return $arg1 === 'date' - && $arg2 === '<' - && $arg3->eq(now()); - })->once(); - - $this->filter($query, 'expired'); - } -} diff --git a/tests/Query/StatusQueryBuilderTest.php b/tests/Query/StatusQueryBuilderTest.php index c7b9667f7d8..e3d92c6351a 100644 --- a/tests/Query/StatusQueryBuilderTest.php +++ b/tests/Query/StatusQueryBuilderTest.php @@ -40,7 +40,7 @@ public function it_proxies_methods_onto_the_builder() public function it_queries_status_by_default() { $builder = $this->mock(Builder::class); - $builder->shouldReceive('where')->with('status', 'published')->once()->andReturnSelf(); + $builder->shouldReceive('whereStatus')->with('published')->once()->andReturnSelf(); $builder->shouldReceive('get')->once()->andReturn('results'); $results = (new StatusQueryBuilder($builder))->get(); @@ -52,7 +52,7 @@ public function it_queries_status_by_default() public function the_fallback_query_status_value_can_be_set_in_the_constructor() { $builder = $this->mock(Builder::class); - $builder->shouldReceive('where')->with('status', 'potato')->once()->andReturnSelf(); + $builder->shouldReceive('whereStatus')->with('potato')->once()->andReturnSelf(); $builder->shouldReceive('get')->once()->andReturn('results'); $results = (new StatusQueryBuilder($builder, 'potato'))->get(); @@ -78,6 +78,20 @@ public function it_doesnt_perform_fallback_status_query_when_status_is_explicitl $this->assertEquals('results', $query->get()); } + /** @test */ + public function it_doesnt_perform_fallback_status_query_when_wherestatus_is_explicitly_queried() + { + $builder = $this->mock(Builder::class); + $builder->shouldReceive('whereStatus')->with('foo')->once()->andReturnSelf(); + $builder->shouldReceive('get')->once()->andReturn('results'); + + $query = (new StatusQueryBuilder($builder)); + + $query->whereStatus('foo'); + + $this->assertEquals('results', $query->get()); + } + /** * @test * From 0c7687b666aef466a00ecc081f8c468549def138 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Wed, 17 Jan 2024 14:31:13 -0500 Subject: [PATCH 05/10] wip --- src/Stache/Query/EntryQueryBuilder.php | 4 ++++ tests/Data/Entries/EntryQueryBuilderTest.php | 12 +++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/Stache/Query/EntryQueryBuilder.php b/src/Stache/Query/EntryQueryBuilder.php index aa4533a3cc2..2e696053fea 100644 --- a/src/Stache/Query/EntryQueryBuilder.php +++ b/src/Stache/Query/EntryQueryBuilder.php @@ -39,6 +39,10 @@ public function whereIn($column, $values, $boolean = 'and') return $this; } + if ($column === 'status') { + Log::debug('Filtering by status is deprecated. Use whereStatus() instead.'); + } + return parent::whereIn($column, $values, $boolean); } diff --git a/tests/Data/Entries/EntryQueryBuilderTest.php b/tests/Data/Entries/EntryQueryBuilderTest.php index 5456c0528ec..dd56ed544e7 100644 --- a/tests/Data/Entries/EntryQueryBuilderTest.php +++ b/tests/Data/Entries/EntryQueryBuilderTest.php @@ -762,7 +762,7 @@ public function entries_are_found_using_lazy() } /** @test */ - public function filtering_by_status_column_writes_deprecation_log() + public function filtering_using_where_status_column_writes_deprecation_log() { $this->createDummyCollectionAndEntries(); @@ -771,6 +771,16 @@ public function filtering_by_status_column_writes_deprecation_log() Entry::query()->where('collection', 'posts')->where('status', 'published')->get(); } + /** @test */ + public function filtering_using_whereIn_status_column_writes_deprecation_log() + { + $this->createDummyCollectionAndEntries(); + + Log::shouldReceive('debug')->with('Filtering by status is deprecated. Use whereStatus() instead.')->once(); + + Entry::query()->where('collection', 'posts')->whereIn('status', ['published'])->get(); + } + /** @test */ public function filtering_by_unexpected_status_throws_exception() { From 8fd4934c63dba69789cd72625c1c6ade51b0c04d Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Wed, 17 Jan 2024 14:31:39 -0500 Subject: [PATCH 06/10] item query builder (e.g. pages) can continue to kick it old school --- src/Query/ItemQueryBuilder.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Query/ItemQueryBuilder.php b/src/Query/ItemQueryBuilder.php index d8137617ada..3ad11bdc770 100644 --- a/src/Query/ItemQueryBuilder.php +++ b/src/Query/ItemQueryBuilder.php @@ -19,4 +19,9 @@ protected function getBaseItems() { return $this->items; } + + public function whereStatus($status) + { + return $this->where('status', $status); + } } From 76ec005c65a2c4d0ced73dd0ae60a1879c0a460a Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Wed, 17 Jan 2024 14:32:20 -0500 Subject: [PATCH 07/10] let the most basic status equals filter (used in apis) continue to hum along. --- src/Tags/Concerns/QueriesConditions.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Tags/Concerns/QueriesConditions.php b/src/Tags/Concerns/QueriesConditions.php index df755389a8b..79617a699df 100644 --- a/src/Tags/Concerns/QueriesConditions.php +++ b/src/Tags/Concerns/QueriesConditions.php @@ -127,6 +127,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); } From 4c5fd591445f83b03c9f021ab0e0cbb9cd2e72c2 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Wed, 17 Jan 2024 15:00:34 -0500 Subject: [PATCH 08/10] short closures --- src/Stache/Query/EntryQueryBuilder.php | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/Stache/Query/EntryQueryBuilder.php b/src/Stache/Query/EntryQueryBuilder.php index 2e696053fea..abf86c7eeb5 100644 --- a/src/Stache/Query/EntryQueryBuilder.php +++ b/src/Stache/Query/EntryQueryBuilder.php @@ -156,15 +156,9 @@ public function whereStatus(string $status) $this->where('published', true); - $this->where(function ($query) use ($status) { - $this->getCollectionsForStatus()->each(function ($collection) use ($query, $status) { - $query->orWhere(function ($q) use ($collection, $status) { - $this->addCollectionStatusLogicToQuery($q, $status, $collection); - }); - }); - }); - - return $this; + return $this->where(fn ($query) => $this + ->getCollectionsForStatus() + ->each(fn ($collection) => $query->orWhere(fn ($q) => $this->addCollectionStatusLogicToQuery($q, $status, $collection)))); } private function getCollectionsForStatus() From 27a803dc2e4cf12df18aaad508499bd2923221c0 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Thu, 11 Apr 2024 15:44:55 -0400 Subject: [PATCH 09/10] use actual deprecations --- src/Stache/Query/EntryQueryBuilder.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Stache/Query/EntryQueryBuilder.php b/src/Stache/Query/EntryQueryBuilder.php index 98ddabe5ae5..89d421252cb 100644 --- a/src/Stache/Query/EntryQueryBuilder.php +++ b/src/Stache/Query/EntryQueryBuilder.php @@ -2,7 +2,6 @@ namespace Statamic\Stache\Query; -use Illuminate\Support\Facades\Log; use Statamic\Contracts\Entries\QueryBuilder; use Statamic\Entries\EntryCollection; use Statamic\Facades; @@ -26,7 +25,7 @@ public function where($column, $operator = null, $value = null, $boolean = 'and' } if ($column === 'status') { - Log::debug('Filtering by status is deprecated. Use whereStatus() instead.'); + trigger_error('Filtering by status is deprecated. Use whereStatus() instead.', E_USER_DEPRECATED); } return parent::where($column, $operator, $value, $boolean); @@ -41,7 +40,7 @@ public function whereIn($column, $values, $boolean = 'and') } if ($column === 'status') { - Log::debug('Filtering by status is deprecated. Use whereStatus() instead.'); + trigger_error('Filtering by status is deprecated. Use whereStatus() instead.', E_USER_DEPRECATED); } return parent::whereIn($column, $values, $boolean); From af1dfd821acc45c8cefa48e237719d7b095ed93d Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Thu, 11 Apr 2024 16:29:29 -0400 Subject: [PATCH 10/10] fix tests --- tests/Data/Entries/EntryQueryBuilderTest.php | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/Data/Entries/EntryQueryBuilderTest.php b/tests/Data/Entries/EntryQueryBuilderTest.php index c6ab661f472..06244c03412 100644 --- a/tests/Data/Entries/EntryQueryBuilderTest.php +++ b/tests/Data/Entries/EntryQueryBuilderTest.php @@ -4,7 +4,6 @@ use Facades\Tests\Factories\EntryFactory; use Illuminate\Support\Carbon; -use Illuminate\Support\Facades\Log; use Statamic\Facades\Blueprint; use Statamic\Facades\Collection; use Statamic\Facades\Entry; @@ -773,9 +772,11 @@ public function entries_are_found_using_lazy() /** @test */ public function filtering_using_where_status_column_writes_deprecation_log() { - $this->createDummyCollectionAndEntries(); + $this->withoutDeprecationHandling(); + $this->expectException(\ErrorException::class); + $this->expectExceptionMessage('Filtering by status is deprecated. Use whereStatus() instead.'); - Log::shouldReceive('debug')->with('Filtering by status is deprecated. Use whereStatus() instead.')->once(); + $this->createDummyCollectionAndEntries(); Entry::query()->where('collection', 'posts')->where('status', 'published')->get(); } @@ -783,9 +784,11 @@ public function filtering_using_where_status_column_writes_deprecation_log() /** @test */ public function filtering_using_whereIn_status_column_writes_deprecation_log() { - $this->createDummyCollectionAndEntries(); + $this->withoutDeprecationHandling(); + $this->expectException(\ErrorException::class); + $this->expectExceptionMessage('Filtering by status is deprecated. Use whereStatus() instead.'); - Log::shouldReceive('debug')->with('Filtering by status is deprecated. Use whereStatus() instead.')->once(); + $this->createDummyCollectionAndEntries(); Entry::query()->where('collection', 'posts')->whereIn('status', ['published'])->get(); } @@ -830,7 +833,7 @@ public function it_filters_by_status($status, $expected) $this->assertEquals($expected, Entry::query()->whereStatus($status)->get()->map->id->all()); } - public function filterByStatusProvider() + public static function filterByStatusProvider() { return [ 'draft' => ['draft', [