diff --git a/src/Query/ItemQueryBuilder.php b/src/Query/ItemQueryBuilder.php index d8137617ad..3ad11bdc77 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); + } } diff --git a/src/Query/Scopes/Filters/Status.php b/src/Query/Scopes/Filters/Status.php index d0ab3c540a..733e9bde76 100644 --- a/src/Query/Scopes/Filters/Status.php +++ b/src/Query/Scopes/Filters/Status.php @@ -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) diff --git a/src/Query/StatusQueryBuilder.php b/src/Query/StatusQueryBuilder.php index 8da4c2d018..366f4029f9 100644 --- a/src/Query/StatusQueryBuilder.php +++ b/src/Query/StatusQueryBuilder.php @@ -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); @@ -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; } diff --git a/src/Stache/Query/EntryQueryBuilder.php b/src/Stache/Query/EntryQueryBuilder.php index fbf92cd17e..89d421252c 100644 --- a/src/Stache/Query/EntryQueryBuilder.php +++ b/src/Stache/Query/EntryQueryBuilder.php @@ -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') { @@ -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); } @@ -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); } @@ -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(); diff --git a/src/Tags/Collection/Entries.php b/src/Tags/Collection/Entries.php index fbe3f2ce6a..dc910752d6 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,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) diff --git a/src/Tags/Concerns/QueriesConditions.php b/src/Tags/Concerns/QueriesConditions.php index c844c8b77e..201aaa2841 100644 --- a/src/Tags/Concerns/QueriesConditions.php +++ b/src/Tags/Concerns/QueriesConditions.php @@ -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); } diff --git a/tests/API/APITest.php b/tests/API/APITest.php index 97344e5eb4..b9577ea360 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 189e80cc0d..06244c0341 100644 --- a/tests/Data/Entries/EntryQueryBuilderTest.php +++ b/tests/Data/Entries/EntryQueryBuilderTest.php @@ -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(); diff --git a/tests/Feature/GraphQL/EntriesTest.php b/tests/Feature/GraphQL/EntriesTest.php index cb202a95d8..ce1daf5d93 100644 --- a/tests/Feature/GraphQL/EntriesTest.php +++ b/tests/Feature/GraphQL/EntriesTest.php @@ -760,7 +760,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']); @@ -870,4 +870,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 5034ed2402..169b34c3aa 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')); $this->setSites([ '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 static 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/StatusQueryBuilderTest.php b/tests/Query/StatusQueryBuilderTest.php index 371a99b178..0b216651dd 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 *