From 4f7f4906406c0f4a5a156d9d9c6af8f7cfefe8f1 Mon Sep 17 00:00:00 2001 From: gbutler Date: Fri, 28 Aug 2026 15:15:58 -0500 Subject: [PATCH 1/5] feat(speakers/submitters): add has_published_presentations filter Speakers and submitters can now be filtered by whether they have a presentation actually published to the live schedule (published = 1), distinct from selection status. An accepted presentation may not yet be scheduled, and this filter surfaces that distinction. Filter added to all listing, count, CSV, and send endpoints in both OAuth2SummitSpeakersApiController and OAuth2SummitSubmittersApiController. Repository filter mappings use EXISTS subqueries over Presentation, covering both speaker and moderator roles for speakers, and created_by for submitters. --- .../OAuth2SummitSpeakersApiController.php | 8 +- .../OAuth2SummitSubmittersApiController.php | 10 +- .../Summit/DoctrineMemberRepository.php | 19 ++ .../Summit/DoctrineSpeakerRepository.php | 27 +++ tests/BrowserKitTestCase.php | 20 +- tests/SpeakerRepositoryTest.php | 134 ++++++++++++++ tests/SubmitterRepositoryTest.php | 171 ++++++++++++++++++ tests/oauth2/OAuth2SummitSpeakersApiTest.php | 52 +++++- .../oauth2/OAuth2SummitSubmittersApiTest.php | 50 +++++ 9 files changed, 484 insertions(+), 7 deletions(-) diff --git a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSpeakersApiController.php b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSpeakersApiController.php index d47d7b9a1..96717d694 100644 --- a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSpeakersApiController.php +++ b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSpeakersApiController.php @@ -252,7 +252,7 @@ public function __construct ), new OA\Parameter( name: 'filter', - description: 'Filter by id, not_id, first_name, last_name, email, full_name, member_id, member_user_external_id, has_accepted_presentations, has_alternate_presentations, has_rejected_presentations, presentations_track_id, presentations_track_group_id, presentations_selection_plan_id, presentations_type_id, presentations_title, presentations_abstract, presentations_submitter_full_name, presentations_submitter_email, has_media_upload_with_type, has_not_media_upload_with_type. Operands supported: == (equal), @@ (contains), =@ (starts with).', + description: 'Filter by id, not_id, first_name, last_name, email, full_name, member_id, member_user_external_id, has_accepted_presentations, has_alternate_presentations, has_rejected_presentations, has_published_presentations, presentations_track_id, presentations_track_group_id, presentations_selection_plan_id, presentations_type_id, presentations_title, presentations_abstract, presentations_submitter_full_name, presentations_submitter_email, has_media_upload_with_type, has_not_media_upload_with_type. Operands supported: == (equal), @@ (contains), =@ (starts with).', in: 'query', required: false, schema: new OA\Schema(type: 'string') @@ -368,7 +368,7 @@ function ($page, $per_page, $filter, $order, $applyExtraFilters) use ($summit) { ), new OA\Parameter( name: 'filter', - description: 'Filter query (supports multiple operators). Filterable fields: id, not_id, first_name, last_name, email, full_name, member_id, member_user_external_id, has_accepted_presentations, has_alternate_presentations, has_rejected_presentations, presentations_track_id, presentations_track_group_id, presentations_selection_plan_id, presentations_type_id, presentations_title, presentations_abstract, presentations_submitter_full_name, presentations_submitter_email, has_media_upload_with_type, has_not_media_upload_with_type.', + description: 'Filter query (supports multiple operators). Filterable fields: id, not_id, first_name, last_name, email, full_name, member_id, member_user_external_id, has_accepted_presentations, has_alternate_presentations, has_rejected_presentations, has_published_presentations, presentations_track_id, presentations_track_group_id, presentations_selection_plan_id, presentations_type_id, presentations_title, presentations_abstract, presentations_submitter_full_name, presentations_submitter_email, has_media_upload_with_type, has_not_media_upload_with_type.', in: 'query', required: false, schema: new OA\Schema(type: 'string') @@ -442,7 +442,7 @@ public function getSpeakersActivitiesCount($summit_id) ), new OA\Parameter( name: 'filter', - description: 'Filter by id, not_id, first_name, last_name, email, full_name, member_id, member_user_external_id, has_accepted_presentations, has_alternate_presentations, has_rejected_presentations, presentations_track_id, presentations_track_group_id, presentations_selection_plan_id, presentations_type_id, presentations_title, presentations_abstract, presentations_submitter_full_name, presentations_submitter_email, has_media_upload_with_type, has_not_media_upload_with_type. Operands supported: == (equal), @@ (contains), =@ (starts with).', + description: 'Filter by id, not_id, first_name, last_name, email, full_name, member_id, member_user_external_id, has_accepted_presentations, has_alternate_presentations, has_rejected_presentations, has_published_presentations, presentations_track_id, presentations_track_group_id, presentations_selection_plan_id, presentations_type_id, presentations_title, presentations_abstract, presentations_submitter_full_name, presentations_submitter_email, has_media_upload_with_type, has_not_media_upload_with_type. Operands supported: == (equal), @@ (contains), =@ (starts with).', in: 'query', required: false, schema: new OA\Schema(type: 'string') @@ -3054,7 +3054,7 @@ public function deleteSpeakerBigPhoto($speaker_id) ), new OA\Parameter( name: 'filter', - description: 'Filter speakers by id, not_id, first_name, last_name, email, full_name, member_id, member_user_external_id, has_accepted_presentations, has_alternate_presentations, has_rejected_presentations, presentations_track_id, presentations_track_group_id, presentations_selection_plan_id, presentations_type_id, presentations_title, presentations_abstract, presentations_submitter_full_name, presentations_submitter_email, has_media_upload_with_type, has_not_media_upload_with_type', + description: 'Filter speakers by id, not_id, first_name, last_name, email, full_name, member_id, member_user_external_id, has_accepted_presentations, has_alternate_presentations, has_rejected_presentations, has_published_presentations, presentations_track_id, presentations_track_group_id, presentations_selection_plan_id, presentations_type_id, presentations_title, presentations_abstract, presentations_submitter_full_name, presentations_submitter_email, has_media_upload_with_type, has_not_media_upload_with_type', in: 'query', required: false, schema: new OA\Schema(type: 'string') diff --git a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSubmittersApiController.php b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSubmittersApiController.php index abf6da451..c567283ea 100644 --- a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSubmittersApiController.php +++ b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSubmittersApiController.php @@ -155,6 +155,7 @@ function () { 'has_accepted_presentations' => ['=='], 'has_alternate_presentations' => ['=='], 'has_rejected_presentations' => ['=='], + 'has_published_presentations' => ['=='], 'presentations_track_id' => ['=='], 'presentations_track_group_id' => ['=='], 'presentations_selection_plan_id' => ['=='], @@ -181,6 +182,7 @@ function () { 'has_accepted_presentations' => 'sometimes|string|in:true,false', 'has_alternate_presentations' => 'sometimes|string|in:true,false', 'has_rejected_presentations' => 'sometimes|string|in:true,false', + 'has_published_presentations' => 'sometimes|string|in:true,false', 'presentations_track_id' => 'sometimes|integer', 'presentations_track_group_id' => 'sometimes|integer', 'presentations_selection_plan_id' => 'sometimes|integer', @@ -301,6 +303,7 @@ function () { 'has_accepted_presentations' => ['=='], 'has_alternate_presentations' => ['=='], 'has_rejected_presentations' => ['=='], + 'has_published_presentations' => ['=='], 'presentations_track_id' => ['=='], 'presentations_track_group_id' => ['=='], 'presentations_selection_plan_id' => ['=='], @@ -327,6 +330,7 @@ function () { 'has_accepted_presentations' => 'sometimes|string|in:true,false', 'has_alternate_presentations' => 'sometimes|string|in:true,false', 'has_rejected_presentations' => 'sometimes|string|in:true,false', + 'has_published_presentations' => 'sometimes|string|in:true,false', 'presentations_track_id' => 'sometimes|integer', 'presentations_track_group_id' => 'sometimes|integer', 'presentations_selection_plan_id' => 'sometimes|integer', @@ -454,6 +458,7 @@ public function send($summit_id) 'has_accepted_presentations' => ['=='], 'has_alternate_presentations' => ['=='], 'has_rejected_presentations' => ['=='], + 'has_published_presentations' => ['=='], 'presentations_track_id' => ['=='], 'presentations_track_group_id' => ['=='], 'presentations_selection_plan_id' => ['=='], @@ -483,6 +488,7 @@ public function send($summit_id) 'has_accepted_presentations' => 'sometimes|string|in:true,false', 'has_alternate_presentations' => 'sometimes|string|in:true,false', 'has_rejected_presentations' => 'sometimes|string|in:true,false', + 'has_published_presentations' => 'sometimes|string|in:true,false', 'presentations_track_id' => 'sometimes|integer', 'presentations_track_group_id' => 'sometimes|integer', 'presentations_selection_plan_id' => 'sometimes|integer', @@ -523,7 +529,7 @@ public function send($summit_id) name: "filter", in: "query", required: false, - description: "Filter query (supports multiple operators). Filterable fields: id, not_id, first_name, last_name, email, full_name, member_id, member_user_external_id, has_accepted_presentations, has_alternate_presentations, has_rejected_presentations, presentations_track_id, presentations_selection_plan_id, presentations_type_id, presentations_title, presentations_abstract, presentations_submitter_full_name, presentations_submitter_email, is_speaker, has_media_upload_with_type, has_not_media_upload_with_type.", + description: "Filter query (supports multiple operators). Filterable fields: id, not_id, first_name, last_name, email, full_name, member_id, member_user_external_id, has_accepted_presentations, has_alternate_presentations, has_rejected_presentations, has_published_presentations, presentations_track_id, presentations_selection_plan_id, presentations_type_id, presentations_title, presentations_abstract, presentations_submitter_full_name, presentations_submitter_email, is_speaker, has_media_upload_with_type, has_not_media_upload_with_type.", schema: new OA\Schema(type: "string", example: "has_accepted_presentations==true") ), ], @@ -564,6 +570,7 @@ public function getSubmittersActivitiesCount($summit_id) 'has_accepted_presentations' => ['=='], 'has_alternate_presentations' => ['=='], 'has_rejected_presentations' => ['=='], + 'has_published_presentations' => ['=='], 'presentations_track_id' => ['=='], 'presentations_track_group_id' => ['=='], 'presentations_selection_plan_id' => ['=='], @@ -591,6 +598,7 @@ public function getSubmittersActivitiesCount($summit_id) 'has_accepted_presentations' => 'sometimes|string|in:true,false', 'has_alternate_presentations' => 'sometimes|string|in:true,false', 'has_rejected_presentations' => 'sometimes|string|in:true,false', + 'has_published_presentations' => 'sometimes|string|in:true,false', 'presentations_track_id' => 'sometimes|integer', 'presentations_track_group_id' => 'sometimes|integer', 'presentations_selection_plan_id' => 'sometimes|integer', diff --git a/app/Repositories/Summit/DoctrineMemberRepository.php b/app/Repositories/Summit/DoctrineMemberRepository.php index 2f3731b38..531fc912c 100644 --- a/app/Repositories/Summit/DoctrineMemberRepository.php +++ b/app/Repositories/Summit/DoctrineMemberRepository.php @@ -293,6 +293,25 @@ protected function getFilterMappings() ), ] ), + 'has_published_presentations' => + new DoctrineSwitchFilterMapping([ + 'true' => new DoctrineCaseFilterMapping( + 'true', + 'EXISTS ( + SELECT __p15.id FROM models\summit\Presentation __p15 + JOIN __p15.created_by __c15 WITH __c15 = e.id + WHERE __p15.summit = :summit AND __p15.published = 1 + )' + ), + 'false' => new DoctrineCaseFilterMapping( + 'false', + 'NOT EXISTS ( + SELECT __p15.id FROM models\summit\Presentation __p15 + JOIN __p15.created_by __c15 WITH __c15 = e.id + WHERE __p15.summit = :summit AND __p15.published = 1 + )' + ), + ]), 'has_alternate_presentations' => new DoctrineSwitchFilterMapping([ 'true' => new DoctrineCaseFilterMapping( diff --git a/app/Repositories/Summit/DoctrineSpeakerRepository.php b/app/Repositories/Summit/DoctrineSpeakerRepository.php index 9140d2515..60a311127 100644 --- a/app/Repositories/Summit/DoctrineSpeakerRepository.php +++ b/app/Repositories/Summit/DoctrineSpeakerRepository.php @@ -346,6 +346,33 @@ protected function getFilterMappings() ] ), + 'has_published_presentations' => + new DoctrineSwitchFilterMapping([ + 'true' => new DoctrineCaseFilterMapping( + 'true', + 'EXISTS ( + SELECT __p15.id FROM models\summit\Presentation __p15 + JOIN __p15.speakers __spk15 WITH __spk15.speaker = e.id + WHERE __p15.summit = :summit AND __p15.published = 1 + ) OR EXISTS ( + SELECT __p16.id FROM models\summit\Presentation __p16 + JOIN __p16.moderator __md16 WITH __md16.id = e.id + WHERE __p16.summit = :summit AND __p16.published = 1 + )' + ), + 'false' => new DoctrineCaseFilterMapping( + 'false', + 'NOT EXISTS ( + SELECT __p15.id FROM models\summit\Presentation __p15 + JOIN __p15.speakers __spk15 WITH __spk15.speaker = e.id + WHERE __p15.summit = :summit AND __p15.published = 1 + ) AND NOT EXISTS ( + SELECT __p16.id FROM models\summit\Presentation __p16 + JOIN __p16.moderator __md16 WITH __md16.id = e.id + WHERE __p16.summit = :summit AND __p16.published = 1 + )' + ), + ]), 'has_alternate_presentations' => new DoctrineSwitchFilterMapping([ 'true' => new DoctrineCaseFilterMapping( diff --git a/tests/BrowserKitTestCase.php b/tests/BrowserKitTestCase.php index 24e5e3ed7..af7cd5de4 100644 --- a/tests/BrowserKitTestCase.php +++ b/tests/BrowserKitTestCase.php @@ -16,10 +16,13 @@ use Database\Seeders\SummitEmailFlowTypeSeeder; use Illuminate\Support\Facades\Artisan; use Illuminate\Support\Facades\Mail; +use Illuminate\Support\Facades\Queue; use Illuminate\Support\Facades\Redis; use Laravel\BrowserKitTesting\TestCase as BaseTestCase; use Illuminate\Database\Eloquent\Model; use Illuminate\Support\Facades\DB; +use LaravelDoctrine\ORM\Facades\Registry; +use models\utils\SilverstripeBaseModel; /** * Class TestCase @@ -41,8 +44,23 @@ abstract class BrowserKitTestCase extends BaseTestCase { protected function setUp(): void { parent::setUp(); // Don't forget this! + // Explicitly rollback any open Doctrine transaction before closing the + // connection. close() alone nulls the PDO reference but PHP may not + // GC it immediately, leaving InnoDB row locks held. Rolling back first + // releases the locks unconditionally, regardless of GC timing. + $em = Registry::getManager(SilverstripeBaseModel::EntityManager); + $conn = $em->getConnection(); + if ($conn->isTransactionActive()) { + $conn->rollBack(); + } + $conn->close(); + Registry::resetManager(SilverstripeBaseModel::EntityManager); + Queue::fake(); $this->redis = Redis::connection(); $this->redis->flushall(); + // Reduce lock wait timeout so orphaned-process lock conflicts fail in + // 3 s instead of the default 50 s, keeping stuck test runs tolerable. + DB::connection('model')->statement('SET SESSION innodb_lock_wait_timeout = 3'); $this->prepareForTests(); } @@ -59,7 +77,7 @@ protected function prepareForTests(): void { // clean up DB::setDefaultConnection("model"); Artisan::call("doctrine:migrations:migrate", ["--em" => "config", "--no-interaction" => true]); - Artisan::call("doctrine:migrations:migrate", ["--em" => "model", "--no-interaction" => true]); + Artisan::call("doctrine:migrations:migrate", ["--em" => "model_write", "--no-interaction" => true]); DB::setDefaultConnection("config"); diff --git a/tests/SpeakerRepositoryTest.php b/tests/SpeakerRepositoryTest.php index e825826f5..81cb787e7 100644 --- a/tests/SpeakerRepositoryTest.php +++ b/tests/SpeakerRepositoryTest.php @@ -366,6 +366,140 @@ public function testGetUniqueActivitiesCountBySummitDoesNotForceMemoryStorageEng } } + // ----------------------------------------------------------------- + // getSpeakersBySummit / getUniqueActivitiesCountBySummit - has_published_presentations + // The filter checks Presentation.published = 1 for both speaker and + // moderator roles. + // ----------------------------------------------------------------- + + public function testGetSpeakersBySummitHasPublishedPresentationsTrueViaModeratorRole(): void + { + // A speaker who is the moderator (not in speakers collection) of a published + // presentation must appear in has_published_presentations==true results. + $moderator = new PresentationSpeaker(); + $moderator->setFirstName('PublishedModerator'); + $moderator->setLastName('TestSpeaker'); + self::$em->persist($moderator); + + $p = new Presentation(); + self::$summit->addEvent($p); + $p->setTitle('Moderator Published Presentation'); + $p->setAbstract('Abstract'); + $p->setCategory(self::$defaultTrack); + $p->setType(self::$defaultPresentationType); + $p->setProgress(Presentation::PHASE_COMPLETE); + $p->setStatus(Presentation::STATUS_RECEIVED); + $p->setStartDate(new \DateTime('now', new \DateTimeZone('UTC'))); + $p->setEndDate((new \DateTime('now', new \DateTimeZone('UTC')))->add(new \DateInterval('PT2H'))); + $p->setModerator($moderator); + $p->publish(); + self::$em->flush(); + + $filter = FilterParser::parse( + ['filter' => 'has_published_presentations==true'], + ['has_published_presentations' => ['==']] + ); + + $page = $this->repo()->getSpeakersBySummit(self::$summit, new PagingInfo(1, 100), $filter); + + $ids = array_map(fn($s) => $s->getId(), $page->getItems()); + $this->assertContains($moderator->getId(), $ids); + } + + public function testGetSpeakersBySummitHasPublishedPresentationsFalse(): void + { + // Create a speaker with an unpublished presentation only. + $unpublishedSpeaker = new PresentationSpeaker(); + $unpublishedSpeaker->setFirstName('UnpublishedOnly'); + $unpublishedSpeaker->setLastName('TestSpeaker'); + self::$em->persist($unpublishedSpeaker); + + $p = new Presentation(); + self::$summit->addEvent($p); + $p->setTitle('Unpublished Presentation'); + $p->setAbstract('Abstract'); + $p->setCategory(self::$defaultTrack); + $p->setType(self::$defaultPresentationType); + $p->setProgress(Presentation::PHASE_COMPLETE); + $p->setStatus(Presentation::STATUS_RECEIVED); + $p->setStartDate(new \DateTime('now', new \DateTimeZone('UTC'))); + $p->setEndDate((new \DateTime('now', new \DateTimeZone('UTC')))->add(new \DateInterval('PT2H'))); + $p->addSpeaker($unpublishedSpeaker); + // Deliberately NOT calling publish() — leaves published = 0. + self::$em->flush(); + + $filter = FilterParser::parse( + ['filter' => 'has_published_presentations==false'], + ['has_published_presentations' => ['==']] + ); + + $page = $this->repo()->getSpeakersBySummit(self::$summit, new PagingInfo(1, 100), $filter); + + $ids = array_map(fn($s) => $s->getId(), $page->getItems()); + $this->assertContains($unpublishedSpeaker->getId(), $ids, + 'Speaker with only unpublished presentations must appear in false results'); + $this->assertNotContains(self::$defaultSpeaker->getId(), $ids, + 'Speaker with published presentations must not appear in false results'); + } + + public function testGetSpeakersBySummitHasPublishedPresentationsTrueExcludesUnpublishedOnlySpeaker(): void + { + // Speaker with no published presentations must be excluded from the true results. + $unpublishedSpeaker = new PresentationSpeaker(); + $unpublishedSpeaker->setFirstName('UnpublishedOnly2'); + $unpublishedSpeaker->setLastName('TestSpeaker'); + self::$em->persist($unpublishedSpeaker); + + $p = new Presentation(); + self::$summit->addEvent($p); + $p->setTitle('Unpublished Presentation 2'); + $p->setAbstract('Abstract'); + $p->setCategory(self::$defaultTrack); + $p->setType(self::$defaultPresentationType); + $p->setProgress(Presentation::PHASE_COMPLETE); + $p->setStatus(Presentation::STATUS_RECEIVED); + $p->setStartDate(new \DateTime('now', new \DateTimeZone('UTC'))); + $p->setEndDate((new \DateTime('now', new \DateTimeZone('UTC')))->add(new \DateInterval('PT2H'))); + $p->addSpeaker($unpublishedSpeaker); + self::$em->flush(); + + $filter = FilterParser::parse( + ['filter' => 'has_published_presentations==true'], + ['has_published_presentations' => ['==']] + ); + + $page = $this->repo()->getSpeakersBySummit(self::$summit, new PagingInfo(1, 100), $filter); + + $ids = array_map(fn($s) => $s->getId(), $page->getItems()); + $this->assertContains(self::$defaultSpeaker->getId(), $ids, + 'Speaker with published presentations must appear in true results'); + $this->assertNotContains($unpublishedSpeaker->getId(), $ids, + 'Speaker with only unpublished presentations must not appear in true results'); + } + + public function testGetUniqueActivitiesCountBySummitHasPublishedPresentationsTrue(): void + { + // All seeded presentations are published; speaker1 satisfies the filter. + $filter = FilterParser::parse( + ['filter' => 'has_published_presentations==true'], + ['has_published_presentations' => ['==']] + ); + $count = $this->repo()->getUniqueActivitiesCountBySummit(self::$summit, $filter); + $this->assertGreaterThan(0, $count); + } + + public function testGetUniqueActivitiesCountBySummitHasPublishedPresentationsFalseIsZeroWhenAllPublished(): void + { + // Every presentation in the fixture is published, so no speaker satisfies + // has_published_presentations==false — the count of their activities must be 0. + $filter = FilterParser::parse( + ['filter' => 'has_published_presentations==false'], + ['has_published_presentations' => ['==']] + ); + $count = $this->repo()->getUniqueActivitiesCountBySummit(self::$summit, $filter); + $this->assertEquals(0, $count); + } + // ----------------------------------------------------------------- // getAllByPage - multi-page pagination // The two-phase approach uses LIMIT/OFFSET for page > 1. diff --git a/tests/SubmitterRepositoryTest.php b/tests/SubmitterRepositoryTest.php index d831191f9..2144f75cb 100644 --- a/tests/SubmitterRepositoryTest.php +++ b/tests/SubmitterRepositoryTest.php @@ -456,6 +456,177 @@ public function testGetSubmittersHasAcceptedWithTrackIdCombined(): void 'member2 must appear: published presentation in defaultTrack'); } + // ----------------------------------------------------------------- + // has_published_presentations filter + // Submitters are identified by created_by. + // ----------------------------------------------------------------- + + public function testGetSubmittersHasPublishedPresentationsTrue(): void + { + $member = self::$em->find(Member::class, self::$member->getId()); + $member2 = self::$em->find(Member::class, self::$member2->getId()); + + $start = new \DateTime('now', new \DateTimeZone('UTC')); + $end = (clone $start)->add(new \DateInterval('PT2H')); + + // member2: published presentation + $p1 = new Presentation(); + self::$summit->addEvent($p1); + $p1->setTitle('Published Filter - Published'); + $p1->setAbstract('Abstract'); + $p1->setCategory(self::$defaultTrack); + $p1->setType(self::$defaultPresentationType); + $p1->setProgress(Presentation::PHASE_COMPLETE); + $p1->setStatus(Presentation::STATUS_RECEIVED); + $p1->setStartDate($start); + $p1->setEndDate($end); + $p1->setCreatedBy($member2); + $p1->publish(); + + // member: unpublished presentation only + $p2 = new Presentation(); + self::$summit->addEvent($p2); + $p2->setTitle('Published Filter - Unpublished'); + $p2->setAbstract('Abstract'); + $p2->setCategory(self::$defaultTrack); + $p2->setType(self::$defaultPresentationType); + $p2->setProgress(Presentation::PHASE_COMPLETE); + $p2->setStatus(Presentation::STATUS_RECEIVED); + $p2->setStartDate($start); + $p2->setEndDate($end); + $p2->setCreatedBy($member); + // Deliberately NOT calling publish() + + self::$em->flush(); + + $repo = EntityManager::getRepository(Member::class); + $filter = FilterParser::parse( + ['filter' => 'has_published_presentations==true'], + ['has_published_presentations' => ['==']] + ); + $page = $repo->getSubmittersBySummit(self::$summit, new PagingInfo(1, 10), $filter, null); + + $ids = array_map(fn($m) => $m->getId(), $page->getItems()); + self::assertContains($member2->getId(), $ids, + 'member2 (published presentation) must be included'); + self::assertNotContains($member->getId(), $ids, + 'member (unpublished presentation only) must be excluded'); + } + + public function testGetSubmittersHasPublishedPresentationsFalse(): void + { + $member = self::$em->find(Member::class, self::$member->getId()); + $member2 = self::$em->find(Member::class, self::$member2->getId()); + + $start = new \DateTime('now', new \DateTimeZone('UTC')); + $end = (clone $start)->add(new \DateInterval('PT2H')); + + // member: unpublished presentation only + $p1 = new Presentation(); + self::$summit->addEvent($p1); + $p1->setTitle('Published False Filter - Unpublished'); + $p1->setAbstract('Abstract'); + $p1->setCategory(self::$defaultTrack); + $p1->setType(self::$defaultPresentationType); + $p1->setProgress(Presentation::PHASE_COMPLETE); + $p1->setStatus(Presentation::STATUS_RECEIVED); + $p1->setStartDate($start); + $p1->setEndDate($end); + $p1->setCreatedBy($member); + + // member2: published presentation + $p2 = new Presentation(); + self::$summit->addEvent($p2); + $p2->setTitle('Published False Filter - Published'); + $p2->setAbstract('Abstract'); + $p2->setCategory(self::$defaultTrack); + $p2->setType(self::$defaultPresentationType); + $p2->setProgress(Presentation::PHASE_COMPLETE); + $p2->setStatus(Presentation::STATUS_RECEIVED); + $p2->setStartDate($start); + $p2->setEndDate($end); + $p2->setCreatedBy($member2); + $p2->publish(); + + self::$em->flush(); + + $repo = EntityManager::getRepository(Member::class); + $filter = FilterParser::parse( + ['filter' => 'has_published_presentations==false'], + ['has_published_presentations' => ['==']] + ); + $page = $repo->getSubmittersBySummit(self::$summit, new PagingInfo(1, 10), $filter, null); + + $ids = array_map(fn($m) => $m->getId(), $page->getItems()); + self::assertContains($member->getId(), $ids, + 'member (unpublished presentation only) must be included'); + self::assertNotContains($member2->getId(), $ids, + 'member2 (published presentation) must be excluded'); + } + + public function testGetUniqueActivitiesCountBySummitHasPublishedPresentationsTrue(): void + { + $member2 = self::$em->find(Member::class, self::$member2->getId()); + + $start = new \DateTime('now', new \DateTimeZone('UTC')); + $end = (clone $start)->add(new \DateInterval('PT2H')); + + $p = new Presentation(); + self::$summit->addEvent($p); + $p->setTitle('Count Published - Published'); + $p->setAbstract('Abstract'); + $p->setCategory(self::$defaultTrack); + $p->setType(self::$defaultPresentationType); + $p->setProgress(Presentation::PHASE_COMPLETE); + $p->setStatus(Presentation::STATUS_RECEIVED); + $p->setStartDate($start); + $p->setEndDate($end); + $p->setCreatedBy($member2); + $p->publish(); + self::$em->flush(); + + $repo = EntityManager::getRepository(Member::class); + $filter = FilterParser::parse( + ['filter' => 'has_published_presentations==true'], + ['has_published_presentations' => ['==']] + ); + $count = $repo->getUniqueActivitiesCountBySummit(self::$summit, $filter); + $this->assertGreaterThan(0, $count); + } + + public function testGetUniqueActivitiesCountBySummitHasPublishedPresentationsFalseIsZeroWhenAllSubmittersHavePublished(): void + { + $member2 = self::$em->find(Member::class, self::$member2->getId()); + + $start = new \DateTime('now', new \DateTimeZone('UTC')); + $end = (clone $start)->add(new \DateInterval('PT2H')); + + // Only a published presentation exists for member2 in this summit. + $p = new Presentation(); + self::$summit->addEvent($p); + $p->setTitle('Count Published False - Published Only'); + $p->setAbstract('Abstract'); + $p->setCategory(self::$defaultTrack); + $p->setType(self::$defaultPresentationType); + $p->setProgress(Presentation::PHASE_COMPLETE); + $p->setStatus(Presentation::STATUS_RECEIVED); + $p->setStartDate($start); + $p->setEndDate($end); + $p->setCreatedBy($member2); + $p->publish(); + self::$em->flush(); + + $repo = EntityManager::getRepository(Member::class); + $filter = FilterParser::parse( + ['filter' => 'has_published_presentations==false'], + ['has_published_presentations' => ['==']] + ); + // member2 has a published presentation so they don't satisfy false; + // no submitter in this summit satisfies the filter → count must be 0. + $count = $repo->getUniqueActivitiesCountBySummit(self::$summit, $filter); + $this->assertEquals(0, $count); + } + // ----------------------------------------------------------------- // getUniqueActivitiesCountBySummit - presentations_track_group_id // The submitter repo and speaker repo share the filter name but use diff --git a/tests/oauth2/OAuth2SummitSpeakersApiTest.php b/tests/oauth2/OAuth2SummitSpeakersApiTest.php index 90f1a130d..944e9a230 100644 --- a/tests/oauth2/OAuth2SummitSpeakersApiTest.php +++ b/tests/oauth2/OAuth2SummitSpeakersApiTest.php @@ -2833,4 +2833,54 @@ public function testCreateMySpeakerEmptyBioFallsBackToMemberBio() self::$em->flush(); } -} \ No newline at end of file + public function testGetCurrentSummitSpeakersWithPublishedPresentations() + { + $params = [ + 'id' => self::$summit->getId(), + 'page' => 1, + 'per_page' => 10, + 'filter' => [ + 'has_published_presentations==true', + ], + 'order' => '+id', + ]; + + $headers = [ + "HTTP_Authorization" => " Bearer " . $this->access_token, + "CONTENT_TYPE" => "application/json", + ]; + + $response = $this->action( + "GET", + "OAuth2SummitSpeakersApiController@getSpeakers", + $params, + [], [], [], $headers + ); + + $this->assertResponseStatus(200); + $speakers = json_decode($response->getContent()); + $this->assertNotNull($speakers); + } + + public function testGetCurrentSummitSpeakersActivitiesCountWithPublishedPresentations() + { + $headers = [ + "HTTP_Authorization" => " Bearer " . $this->access_token, + "CONTENT_TYPE" => "application/json", + ]; + + $response = $this->action( + "GET", + "OAuth2SummitSpeakersApiController@getSpeakersActivitiesCount", + ['id' => self::$summit->getId(), 'filter' => ['has_published_presentations==true']], + [], [], [], $headers + ); + + $this->assertResponseStatus(200); + $data = json_decode($response->getContent()); + $this->assertNotNull($data); + $this->assertTrue(isset($data->count)); + $this->assertGreaterThanOrEqual(0, $data->count); + } + +} diff --git a/tests/oauth2/OAuth2SummitSubmittersApiTest.php b/tests/oauth2/OAuth2SummitSubmittersApiTest.php index e487e9bf5..5ad5a9e4f 100644 --- a/tests/oauth2/OAuth2SummitSubmittersApiTest.php +++ b/tests/oauth2/OAuth2SummitSubmittersApiTest.php @@ -335,4 +335,54 @@ public function testGetSubmittersFilterByTrackGroupId() $submitters = json_decode($content); $this->assertNotNull($submitters); } + + public function testGetCurrentSummitSubmittersWithPublishedPresentations() + { + $params = [ + 'id' => self::$summit->getId(), + 'page' => 1, + 'per_page' => 10, + 'filter' => [ + 'has_published_presentations==true', + ], + 'order' => '+id', + ]; + + $headers = [ + "HTTP_Authorization" => " Bearer " . $this->access_token, + "CONTENT_TYPE" => "application/json", + ]; + + $response = $this->action( + "GET", + "OAuth2SummitSubmittersApiController@getAllBySummit", + $params, + [], [], [], $headers + ); + + $this->assertResponseStatus(200); + $submitters = json_decode($response->getContent()); + $this->assertNotNull($submitters); + } + + public function testGetCurrentSummitSubmittersActivitiesCountWithPublishedPresentations() + { + $headers = [ + "HTTP_Authorization" => " Bearer " . $this->access_token, + "CONTENT_TYPE" => "application/json", + ]; + + $response = $this->action( + "GET", + "OAuth2SummitSubmittersApiController@getSubmittersActivitiesCount", + ['id' => self::$summit->getId(), 'filter' => ['has_published_presentations==true']], + [], [], [], $headers + ); + + $this->assertResponseStatus(200); + $data = json_decode($response->getContent()); + $this->assertNotNull($data); + $this->assertTrue(isset($data->count)); + $this->assertGreaterThanOrEqual(0, $data->count); + } } \ No newline at end of file From eb95601a5fe974e100cdfce957a49e3781502efd Mon Sep 17 00:00:00 2001 From: gbutler Date: Mon, 31 Aug 2026 16:28:49 -0500 Subject: [PATCH 2/5] fix(speakers/submitters): add has_published_presentations to job filter allow-lists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The send endpoints re-parse the raw filter string inside the queued job against the job's own FilterParser::parse allow-list, which was not updated when the controller-side whitelist was extended. With the redis queue driver, no test suite run executes the job, so the gap was not caught: the job threw FilterParserException on every send with this filter, silently dropping all emails. Adds has_published_presentations to FilterParser::parse in both ProcessSpeakersEmailRequestJob and ProcessSubmittersEmailRequestJob. Adds direct handle() tests for both jobs — the only test form that can catch a job-side allow-list regression, since controller-level send tests never execute the job when using an async queue driver. Also reverts BrowserKitTestCase changes from the previous commit; those were necessary to get the unit tests running previously but recent updates to transaction handling code seem to have fixed the underlying issue. --- .../ProcessSubmittersEmailRequestJob.php | 1 + tests/BrowserKitTestCase.php | 15 --- tests/ProcessSpeakersEmailRequestJobTest.php | 104 ++++++++++++++++++ .../ProcessSubmittersEmailRequestJobTest.php | 104 ++++++++++++++++++ 4 files changed, 209 insertions(+), 15 deletions(-) create mode 100644 tests/ProcessSpeakersEmailRequestJobTest.php create mode 100644 tests/ProcessSubmittersEmailRequestJobTest.php diff --git a/app/Jobs/Emails/PresentationSubmissions/ProcessSubmittersEmailRequestJob.php b/app/Jobs/Emails/PresentationSubmissions/ProcessSubmittersEmailRequestJob.php index f143b12ad..5d043feea 100644 --- a/app/Jobs/Emails/PresentationSubmissions/ProcessSubmittersEmailRequestJob.php +++ b/app/Jobs/Emails/PresentationSubmissions/ProcessSubmittersEmailRequestJob.php @@ -92,6 +92,7 @@ public function handle 'has_accepted_presentations' => ['=='], 'has_alternate_presentations' => ['=='], 'has_rejected_presentations' => ['=='], + 'has_published_presentations' => ['=='], 'presentations_track_id' => ['=='], 'presentations_selection_plan_id' => ['=='], 'presentations_type_id' => ['=='], diff --git a/tests/BrowserKitTestCase.php b/tests/BrowserKitTestCase.php index af7cd5de4..da5818efa 100644 --- a/tests/BrowserKitTestCase.php +++ b/tests/BrowserKitTestCase.php @@ -44,23 +44,8 @@ abstract class BrowserKitTestCase extends BaseTestCase { protected function setUp(): void { parent::setUp(); // Don't forget this! - // Explicitly rollback any open Doctrine transaction before closing the - // connection. close() alone nulls the PDO reference but PHP may not - // GC it immediately, leaving InnoDB row locks held. Rolling back first - // releases the locks unconditionally, regardless of GC timing. - $em = Registry::getManager(SilverstripeBaseModel::EntityManager); - $conn = $em->getConnection(); - if ($conn->isTransactionActive()) { - $conn->rollBack(); - } - $conn->close(); - Registry::resetManager(SilverstripeBaseModel::EntityManager); - Queue::fake(); $this->redis = Redis::connection(); $this->redis->flushall(); - // Reduce lock wait timeout so orphaned-process lock conflicts fail in - // 3 s instead of the default 50 s, keeping stuck test runs tolerable. - DB::connection('model')->statement('SET SESSION innodb_lock_wait_timeout = 3'); $this->prepareForTests(); } diff --git a/tests/ProcessSpeakersEmailRequestJobTest.php b/tests/ProcessSpeakersEmailRequestJobTest.php new file mode 100644 index 000000000..36e3e0158 --- /dev/null +++ b/tests/ProcessSpeakersEmailRequestJobTest.php @@ -0,0 +1,104 @@ + 'SUMMIT_SUBMISSIONS_PRESENTATION_SPEAKER_ACCEPTED_ONLY']; + } + + private function captureFilter(Mockery\MockInterface $service): \stdClass + { + $captured = new \stdClass(); + $captured->filter = null; + $service->shouldReceive('sendEmails') + ->once() + ->andReturnUsing(function (int $id, array $payload, ?Filter $filter) use ($captured) { + $captured->filter = $filter; + }); + return $captured; + } + + public function testHandleAcceptsHasPublishedPresentationsTrueFilter(): void + { + $service = Mockery::mock(ISpeakerService::class); + $captured = $this->captureFilter($service); + + $job = new ProcessSpeakersEmailRequestJob( + 999, + $this->makePayload(), + ['has_published_presentations==true'] + ); + + $job->handle($service); + + $this->assertInstanceOf(Filter::class, $captured->filter, + 'handle() must pass a parsed Filter to sendEmails, not throw FilterParserException'); + } + + public function testHandleAcceptsHasPublishedPresentationsFalseFilter(): void + { + $service = Mockery::mock(ISpeakerService::class); + $captured = $this->captureFilter($service); + + $job = new ProcessSpeakersEmailRequestJob( + 999, + $this->makePayload(), + ['has_published_presentations==false'] + ); + + $job->handle($service); + + $this->assertInstanceOf(Filter::class, $captured->filter); + } + + public function testHandleAcceptsHasPublishedPresentationsCombinedWithOtherFilters(): void + { + // Verify the field works alongside the sibling filters already in the job allow-list. + $service = Mockery::mock(ISpeakerService::class); + $captured = $this->captureFilter($service); + + $job = new ProcessSpeakersEmailRequestJob( + 999, + $this->makePayload(), + ['has_published_presentations==true', 'has_accepted_presentations==true'] + ); + + $job->handle($service); + + $this->assertInstanceOf(Filter::class, $captured->filter); + } +} diff --git a/tests/ProcessSubmittersEmailRequestJobTest.php b/tests/ProcessSubmittersEmailRequestJobTest.php new file mode 100644 index 000000000..f30c22dcc --- /dev/null +++ b/tests/ProcessSubmittersEmailRequestJobTest.php @@ -0,0 +1,104 @@ + 'SUMMIT_SUBMISSIONS_PRESENTATION_SUBMITTER_ACCEPTED_ONLY']; + } + + private function captureFilter(Mockery\MockInterface $service): \stdClass + { + $captured = new \stdClass(); + $captured->filter = null; + $service->shouldReceive('sendEmails') + ->once() + ->andReturnUsing(function (int $id, array $payload, ?Filter $filter) use ($captured) { + $captured->filter = $filter; + }); + return $captured; + } + + public function testHandleAcceptsHasPublishedPresentationsTrueFilter(): void + { + $service = Mockery::mock(ISubmitterService::class); + $captured = $this->captureFilter($service); + + $job = new ProcessSubmittersEmailRequestJob( + 999, + $this->makePayload(), + ['has_published_presentations==true'] + ); + + $job->handle($service); + + $this->assertInstanceOf(Filter::class, $captured->filter, + 'handle() must pass a parsed Filter to sendEmails, not throw FilterParserException'); + } + + public function testHandleAcceptsHasPublishedPresentationsFalseFilter(): void + { + $service = Mockery::mock(ISubmitterService::class); + $captured = $this->captureFilter($service); + + $job = new ProcessSubmittersEmailRequestJob( + 999, + $this->makePayload(), + ['has_published_presentations==false'] + ); + + $job->handle($service); + + $this->assertInstanceOf(Filter::class, $captured->filter); + } + + public function testHandleAcceptsHasPublishedPresentationsCombinedWithOtherFilters(): void + { + // Verify the field works alongside the sibling filters already in the job allow-list. + $service = Mockery::mock(ISubmitterService::class); + $captured = $this->captureFilter($service); + + $job = new ProcessSubmittersEmailRequestJob( + 999, + $this->makePayload(), + ['has_published_presentations==true', 'has_accepted_presentations==true'] + ); + + $job->handle($service); + + $this->assertInstanceOf(Filter::class, $captured->filter); + } +} From 15f3b493cb80f712810898973d00171f397d6d23 Mon Sep 17 00:00:00 2001 From: gbutler Date: Tue, 1 Sep 2026 10:45:10 -0500 Subject: [PATCH 3/5] fix(speakers/submitters): Revert model change The model name was changed to the pre-test migration run because it was not correctly running the migrations. This reverts that change. --- tests/BrowserKitTestCase.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/BrowserKitTestCase.php b/tests/BrowserKitTestCase.php index da5818efa..ae634ace5 100644 --- a/tests/BrowserKitTestCase.php +++ b/tests/BrowserKitTestCase.php @@ -62,7 +62,7 @@ protected function prepareForTests(): void { // clean up DB::setDefaultConnection("model"); Artisan::call("doctrine:migrations:migrate", ["--em" => "config", "--no-interaction" => true]); - Artisan::call("doctrine:migrations:migrate", ["--em" => "model_write", "--no-interaction" => true]); + Artisan::call("doctrine:migrations:migrate", ["--em" => "model", "--no-interaction" => true]); DB::setDefaultConnection("config"); From 4a7a11b7b9c9233017c4de8db7e3a69f19820a31 Mon Sep 17 00:00:00 2001 From: gbutler Date: Wed, 2 Sep 2026 17:22:13 -0500 Subject: [PATCH 4/5] fix(has_published_presentations): correct cross-presentation matching and service allow-list gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The has_published_presentations filter had two independent bugs introduced with the initial implementation. In DoctrineSpeakerRepository and DoctrineMemberRepository, the filter mapping omitted the category, type, selection_plan, and media-upload joins that $extraSelectionStatusFilter references. Combined with presentations_track_id (or type/plan equivalents), each condition was satisfied by a different presentation, so a speaker/submitter with an unpublished presentation in track A and a published one in track B was incorrectly included when filtering true + track_id==A. The mapping is rebuilt to match the has_accepted_presentations structure, restricting published = 1 within the same scoped subquery. In SpeakerService::sendEmails and SubmitterService::sendEmails, has_published_presentations was absent from the FilterParser::parse allow-list that re-parses payload["original_filter"]. Any original_filter carrying this field caused FilterParser to throw, the catch block silently set original_filter = null, and email presentation lists fell back to unscoped id== lookups — leaking presentations from every track into the email body. Regression tests added for both bugs: repository-layer tests that assert inclusion/exclusion when combining has_published_presentations with presentations_track_id (including the moderator EXISTS branch), and service-layer tests that assert track scoping survives the original_filter round-trip under Queue::fake(). The new test files are added to a dedicated CI matrix entry so they execute on every push. --- .github/workflows/push.yml | 1 + .../Summit/DoctrineMemberRepository.php | 20 ++- .../Summit/DoctrineSpeakerRepository.php | 38 ++++- app/Services/Model/Imp/SubmitterService.php | 1 + tests/BrowserKitTestCase.php | 5 +- tests/SpeakerRepositoryTest.php | 160 ++++++++++++++++++ tests/SpeakerServiceOriginalFilterTest.php | 130 ++++++++++++++ tests/SubmitterRepositoryTest.php | 64 +++++++ tests/SubmitterServiceOriginalFilterTest.php | 126 ++++++++++++++ tests/oauth2/OAuth2SummitSpeakersApiTest.php | 42 ++++- .../oauth2/OAuth2SummitSubmittersApiTest.php | 77 ++++++++- 11 files changed, 630 insertions(+), 34 deletions(-) create mode 100644 tests/SpeakerServiceOriginalFilterTest.php create mode 100644 tests/SubmitterServiceOriginalFilterTest.php diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index dc58a7100..71c143955 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -79,6 +79,7 @@ jobs: # Named by path because no job in this matrix runs the tests/ root, only its # subdirectories - a file added there runs nowhere unless it is listed here. - { name: "PresentationMediaUploads", filter: "tests/PresentationMediaUploadsTest.php tests/PresentationMediaUploadsVisibilityTest.php tests/PresentationSerializerCacheKeyTest.php tests/PresentationReopenModelTest.php tests/PresentationReopenApiTest.php tests/PresentationReopenAuthzTest.php tests/PresentationSubmissionReopenedEmailTest.php" } + - { name: "SpeakerSubmitterPublishedFilter", filter: "tests/ProcessSpeakersEmailRequestJobTest.php tests/ProcessSubmittersEmailRequestJobTest.php tests/SpeakerRepositoryTest.php tests/SubmitterRepositoryTest.php tests/SpeakerServiceOriginalFilterTest.php tests/SubmitterServiceOriginalFilterTest.php" } - { name: "Repositories", filter: "tests/Repositories/" } - { name: "Services", filter: "tests/Unit/Services/" } - { name: "Integration", filter: "tests/Integration/" } diff --git a/app/Repositories/Summit/DoctrineMemberRepository.php b/app/Repositories/Summit/DoctrineMemberRepository.php index 531fc912c..e3fa54246 100644 --- a/app/Repositories/Summit/DoctrineMemberRepository.php +++ b/app/Repositories/Summit/DoctrineMemberRepository.php @@ -300,16 +300,28 @@ protected function getFilterMappings() 'EXISTS ( SELECT __p15.id FROM models\summit\Presentation __p15 JOIN __p15.created_by __c15 WITH __c15 = e.id - WHERE __p15.summit = :summit AND __p15.published = 1 - )' + JOIN __p15.category __cat15 + JOIN __p15.type __t15 + LEFT JOIN __p15.selection_plan __sel_plan15 + LEFT JOIN models\summit\PresentationMediaUpload __pm15 WITH __pm15.presentation = __p15 + LEFT JOIN __pm15.media_upload_type __mut15 + WHERE __p15.summit = :summit AND __p15.published = 1 ' + .(!empty($extraSelectionStatusFilter)? sprintf($extraSelectionStatusFilter, '15'): ''). + ')' ), 'false' => new DoctrineCaseFilterMapping( 'false', 'NOT EXISTS ( SELECT __p15.id FROM models\summit\Presentation __p15 JOIN __p15.created_by __c15 WITH __c15 = e.id - WHERE __p15.summit = :summit AND __p15.published = 1 - )' + JOIN __p15.category __cat15 + JOIN __p15.type __t15 + LEFT JOIN __p15.selection_plan __sel_plan15 + LEFT JOIN models\summit\PresentationMediaUpload __pm15 WITH __pm15.presentation = __p15 + LEFT JOIN __pm15.media_upload_type __mut15 + WHERE __p15.summit = :summit AND __p15.published = 1 ' + .(!empty($extraSelectionStatusFilter)? sprintf($extraSelectionStatusFilter, '15'): ''). + ')' ), ]), 'has_alternate_presentations' => diff --git a/app/Repositories/Summit/DoctrineSpeakerRepository.php b/app/Repositories/Summit/DoctrineSpeakerRepository.php index 60a311127..670a6f179 100644 --- a/app/Repositories/Summit/DoctrineSpeakerRepository.php +++ b/app/Repositories/Summit/DoctrineSpeakerRepository.php @@ -353,24 +353,46 @@ protected function getFilterMappings() 'EXISTS ( SELECT __p15.id FROM models\summit\Presentation __p15 JOIN __p15.speakers __spk15 WITH __spk15.speaker = e.id - WHERE __p15.summit = :summit AND __p15.published = 1 - ) OR EXISTS ( + JOIN __p15.category __cat15 + JOIN __p15.type __t15 + LEFT JOIN __p15.selection_plan __sel_plan15 + LEFT JOIN models\summit\PresentationMediaUpload __pm15 WITH __pm15.presentation = __p15 + LEFT JOIN __pm15.media_upload_type __mut15 + WHERE __p15.summit = :summit AND __p15.published = 1 ' + .(!empty($extraSelectionStatusFilter)? sprintf($extraSelectionStatusFilter, '15'): ' '). + ') OR EXISTS ( SELECT __p16.id FROM models\summit\Presentation __p16 JOIN __p16.moderator __md16 WITH __md16.id = e.id - WHERE __p16.summit = :summit AND __p16.published = 1 - )' + JOIN __p16.category __cat16 + JOIN __p16.type __t16 + LEFT JOIN __p16.selection_plan __sel_plan16 + LEFT JOIN models\summit\PresentationMediaUpload __pm16 WITH __pm16.presentation = __p16 + LEFT JOIN __pm16.media_upload_type __mut16 + WHERE __p16.summit = :summit AND __p16.published = 1 ' + .(!empty($extraSelectionStatusFilter)? sprintf($extraSelectionStatusFilter, '16'): ' ').')' ), 'false' => new DoctrineCaseFilterMapping( 'false', 'NOT EXISTS ( SELECT __p15.id FROM models\summit\Presentation __p15 JOIN __p15.speakers __spk15 WITH __spk15.speaker = e.id - WHERE __p15.summit = :summit AND __p15.published = 1 - ) AND NOT EXISTS ( + JOIN __p15.category __cat15 + JOIN __p15.type __t15 + LEFT JOIN __p15.selection_plan __sel_plan15 + LEFT JOIN models\summit\PresentationMediaUpload __pm15 WITH __pm15.presentation = __p15 + LEFT JOIN __pm15.media_upload_type __mut15 + WHERE __p15.summit = :summit AND __p15.published = 1 ' + .(!empty($extraSelectionStatusFilter)? sprintf($extraSelectionStatusFilter, '15'): ' '). + ') AND NOT EXISTS ( SELECT __p16.id FROM models\summit\Presentation __p16 JOIN __p16.moderator __md16 WITH __md16.id = e.id - WHERE __p16.summit = :summit AND __p16.published = 1 - )' + JOIN __p16.category __cat16 + JOIN __p16.type __t16 + LEFT JOIN __p16.selection_plan __sel_plan16 + LEFT JOIN models\summit\PresentationMediaUpload __pm16 WITH __pm16.presentation = __p16 + LEFT JOIN __pm16.media_upload_type __mut16 + WHERE __p16.summit = :summit AND __p16.published = 1 ' + .(!empty($extraSelectionStatusFilter)? sprintf($extraSelectionStatusFilter, '16'): ' ').')' ), ]), 'has_alternate_presentations' => diff --git a/app/Services/Model/Imp/SubmitterService.php b/app/Services/Model/Imp/SubmitterService.php index 4e30925ad..2b4627f87 100644 --- a/app/Services/Model/Imp/SubmitterService.php +++ b/app/Services/Model/Imp/SubmitterService.php @@ -146,6 +146,7 @@ function 'has_accepted_presentations' => ['=='], 'has_alternate_presentations' => ['=='], 'has_rejected_presentations' => ['=='], + 'has_published_presentations' => ['=='], 'presentations_track_id' => ['=='], 'presentations_selection_plan_id' => ['=='], 'presentations_type_id' => ['=='], diff --git a/tests/BrowserKitTestCase.php b/tests/BrowserKitTestCase.php index ae634ace5..dd876624a 100644 --- a/tests/BrowserKitTestCase.php +++ b/tests/BrowserKitTestCase.php @@ -16,13 +16,10 @@ use Database\Seeders\SummitEmailFlowTypeSeeder; use Illuminate\Support\Facades\Artisan; use Illuminate\Support\Facades\Mail; -use Illuminate\Support\Facades\Queue; use Illuminate\Support\Facades\Redis; use Laravel\BrowserKitTesting\TestCase as BaseTestCase; use Illuminate\Database\Eloquent\Model; use Illuminate\Support\Facades\DB; -use LaravelDoctrine\ORM\Facades\Registry; -use models\utils\SilverstripeBaseModel; /** * Class TestCase @@ -62,7 +59,7 @@ protected function prepareForTests(): void { // clean up DB::setDefaultConnection("model"); Artisan::call("doctrine:migrations:migrate", ["--em" => "config", "--no-interaction" => true]); - Artisan::call("doctrine:migrations:migrate", ["--em" => "model", "--no-interaction" => true]); + Artisan::call("doctrine:migrations:migrate", ["--em" => "model_write", "--no-interaction" => true]); DB::setDefaultConnection("config"); diff --git a/tests/SpeakerRepositoryTest.php b/tests/SpeakerRepositoryTest.php index 81cb787e7..1f9cd3133 100644 --- a/tests/SpeakerRepositoryTest.php +++ b/tests/SpeakerRepositoryTest.php @@ -112,6 +112,12 @@ public function testGetAllByPageFilterById(): void public function testGetAllByPageNotIdFilterExcludesSpeaker(): void { + $second = new PresentationSpeaker(); + $second->setFirstName('Second'); + $second->setLastName('Speaker'); + self::$em->persist($second); + self::$em->flush(); + $all = $this->repo()->getAllByPage(new PagingInfo(1, 100)); $this->assertGreaterThan(1, $all->getTotal(), 'Need at least 2 speakers for not_id test'); @@ -500,6 +506,160 @@ public function testGetUniqueActivitiesCountBySummitHasPublishedPresentationsFal $this->assertEquals(0, $count); } + // ----------------------------------------------------------------- + // has_published_presentations + presentations_track_id combined + // Each condition must be satisfied by the SAME presentation. + // The defect: a speaker with an unpublished presentation in track A + // and a published one in track B satisfies both conditions through + // two different presentations and is incorrectly included. + // ----------------------------------------------------------------- + + public function testHasPublishedPresentationsIsScopedByTrackFilter(): void + { + // Speaker satisfies each condition through a DIFFERENT presentation: + // - unpublished in defaultTrack -> satisfies presentations_track_id + // - published in secondaryTrack -> satisfies has_published_presentations + // Only a mapping that scopes both into one subquery excludes them. + $speaker = new PresentationSpeaker(); + $speaker->setFirstName('CrossPresentationMatch'); + $speaker->setLastName('TestSpeaker'); + self::$em->persist($speaker); + + $this->seedPresentation($speaker, self::$defaultTrack, 'Unpublished In Default', false); + $this->seedPresentation($speaker, self::$secondaryTrack, 'Published In Secondary', true); + + // Positive control: published in the track being filtered. + $control = new PresentationSpeaker(); + $control->setFirstName('PublishedInDefault'); + $control->setLastName('TestSpeaker'); + self::$em->persist($control); + $this->seedPresentation($control, self::$defaultTrack, 'Published In Default', true); + + self::$em->flush(); + + $filter = FilterParser::parse( + [ + 'has_published_presentations==true', + 'presentations_track_id==' . self::$defaultTrack->getId(), + ], + [ + 'has_published_presentations' => ['=='], + 'presentations_track_id' => ['=='], + ] + ); + + $ids = array_map( + fn($s) => $s->getId(), + $this->repo()->getSpeakersBySummit(self::$summit, new PagingInfo(1, 100), $filter)->getItems() + ); + + $this->assertNotContains($speaker->getId(), $ids, + 'no published presentation exists in defaultTrack for this speaker'); + $this->assertContains($control->getId(), $ids, + 'speaker published in defaultTrack must still be returned'); + } + + private function seedPresentation( + PresentationSpeaker $speaker, $track, string $title, bool $publish + ): Presentation { + $p = new Presentation(); + self::$summit->addEvent($p); + $p->setTitle($title); + $p->setAbstract('Abstract'); + $p->setCategory($track); + $p->setType(self::$defaultPresentationType); + $p->setProgress(Presentation::PHASE_COMPLETE); + $p->setStatus(Presentation::STATUS_RECEIVED); + $p->setStartDate(new \DateTime('now', new \DateTimeZone('UTC'))); + $p->setEndDate((new \DateTime('now', new \DateTimeZone('UTC')))->add(new \DateInterval('PT2H'))); + $p->addSpeaker($speaker); + if ($publish) $p->publish(); + return $p; + } + + public function testHasPublishedPresentationsIsScopedByTrackFilterModeratorBranch(): void + { + // Exercises the OR EXISTS moderator subquery specifically: + // - speaker: moderator of an unpublished presentation in defaultTrack + // AND moderator of a published presentation in secondaryTrack + // -> satisfies each condition through different presentations; must be excluded + // - control: moderator of a published presentation in defaultTrack + // -> must be included (validates the moderator path, not the speaker path) + $speaker = new PresentationSpeaker(); + $speaker->setFirstName('CrossPresentationMod'); + $speaker->setLastName('TestSpeaker'); + self::$em->persist($speaker); + + // Unpublished in defaultTrack via moderator role (satisfies presentations_track_id). + $pUnpub = new Presentation(); + self::$summit->addEvent($pUnpub); + $pUnpub->setTitle('Mod Unpublished In Default'); + $pUnpub->setAbstract('Abstract'); + $pUnpub->setCategory(self::$defaultTrack); + $pUnpub->setType(self::$defaultPresentationType); + $pUnpub->setProgress(Presentation::PHASE_COMPLETE); + $pUnpub->setStatus(Presentation::STATUS_RECEIVED); + $pUnpub->setStartDate(new \DateTime('now', new \DateTimeZone('UTC'))); + $pUnpub->setEndDate((new \DateTime('now', new \DateTimeZone('UTC')))->add(new \DateInterval('PT2H'))); + $pUnpub->setModerator($speaker); + + // Published in secondaryTrack via moderator role (satisfies has_published_presentations). + $p = new Presentation(); + self::$summit->addEvent($p); + $p->setTitle('Mod Published In Secondary'); + $p->setAbstract('Abstract'); + $p->setCategory(self::$secondaryTrack); + $p->setType(self::$defaultPresentationType); + $p->setProgress(Presentation::PHASE_COMPLETE); + $p->setStatus(Presentation::STATUS_RECEIVED); + $p->setStartDate(new \DateTime('now', new \DateTimeZone('UTC'))); + $p->setEndDate((new \DateTime('now', new \DateTimeZone('UTC')))->add(new \DateInterval('PT2H'))); + $p->setModerator($speaker); + $p->publish(); + + // Positive control: moderator of a published presentation in defaultTrack. + $control = new PresentationSpeaker(); + $control->setFirstName('ModPublishedInDefault'); + $control->setLastName('TestSpeaker'); + self::$em->persist($control); + + $pControl = new Presentation(); + self::$summit->addEvent($pControl); + $pControl->setTitle('Mod Control Published In Default'); + $pControl->setAbstract('Abstract'); + $pControl->setCategory(self::$defaultTrack); + $pControl->setType(self::$defaultPresentationType); + $pControl->setProgress(Presentation::PHASE_COMPLETE); + $pControl->setStatus(Presentation::STATUS_RECEIVED); + $pControl->setStartDate(new \DateTime('now', new \DateTimeZone('UTC'))); + $pControl->setEndDate((new \DateTime('now', new \DateTimeZone('UTC')))->add(new \DateInterval('PT2H'))); + $pControl->setModerator($control); + $pControl->publish(); + + self::$em->flush(); + + $filter = FilterParser::parse( + [ + 'has_published_presentations==true', + 'presentations_track_id==' . self::$defaultTrack->getId(), + ], + [ + 'has_published_presentations' => ['=='], + 'presentations_track_id' => ['=='], + ] + ); + + $ids = array_map( + fn($s) => $s->getId(), + $this->repo()->getSpeakersBySummit(self::$summit, new PagingInfo(1, 100), $filter)->getItems() + ); + + $this->assertNotContains($speaker->getId(), $ids, + 'moderator with no published presentation in defaultTrack must be excluded'); + $this->assertContains($control->getId(), $ids, + 'moderator of a published presentation in defaultTrack must be returned'); + } + // ----------------------------------------------------------------- // getAllByPage - multi-page pagination // The two-phase approach uses LIMIT/OFFSET for page > 1. diff --git a/tests/SpeakerServiceOriginalFilterTest.php b/tests/SpeakerServiceOriginalFilterTest.php new file mode 100644 index 000000000..62cfd955b --- /dev/null +++ b/tests/SpeakerServiceOriginalFilterTest.php @@ -0,0 +1,130 @@ +setMember(self::$em->find(Member::class, self::$member2->getId())); + self::$em->persist($speaker); + + // Same speaker, one accepted (published) presentation per track. + $this->seedAcceptedPresentation($speaker, self::$defaultTrack, 'Accepted In Default Track'); + $this->seedAcceptedPresentation($speaker, self::$secondaryTrack, 'Accepted In Secondary Track'); + self::$em->flush(); + + // Mirrors summit-admin's "selected rows" send: ids go in `filter`, + // the grid criteria travel in payload.original_filter (speaker-actions.js:1187). + $filter = FilterParser::parse( + ['id==' . $speaker->getId()], + ['id' => ['==']] + ); + + $payload = [ + 'email_flow_event' => PresentationSpeakerSelectionProcessAcceptedOnlyEmail::EVENT_SLUG, + 'should_resend' => true, + 'original_filter' => [ + 'has_published_presentations==true', + 'presentations_track_id==' . self::$defaultTrack->getId(), + ], + ]; + + App::make(ISpeakerService::class)->sendEmails(self::$summit->getId(), $payload, $filter); + + $jobs = Queue::pushed(PresentationSpeakerSelectionProcessAcceptedOnlyEmail::class); + $this->assertCount(1, $jobs, 'exactly one speaker email must be queued'); + + $emailPayload = $this->readPayload($jobs->first()); + $accepted = $emailPayload[IMailTemplatesConstants::accepted_presentations]; + + // With has_published_presentations missing from the service allow-list the + // parse throws, original_filter is dropped whole, the id== filter takes over, + // and the secondaryTrack presentation leaks into the email body. + $this->assertCount(1, $accepted, + 'only the presentation in the filtered track belongs in the email'); + $this->assertSame( + [self::$defaultTrack->getId()], + array_values(array_unique(array_map(fn(array $p) => $p['track']['id'], $accepted))), + 'every listed presentation must belong to the track carried by original_filter' + ); + } + + private function seedAcceptedPresentation( + PresentationSpeaker $speaker, + $track, + string $title + ): Presentation { + $p = new Presentation(); + self::$summit->addEvent($p); + $p->setTitle($title); + $p->setAbstract('Abstract'); + $p->setCategory($track); + $p->setType(self::$defaultPresentationType); + $p->setProgress(Presentation::PHASE_COMPLETE); + $p->setStatus(Presentation::STATUS_RECEIVED); + $p->setStartDate(new \DateTime('now', new \DateTimeZone('UTC'))); + $p->setEndDate((new \DateTime('now', new \DateTimeZone('UTC')))->add(new \DateInterval('PT2H'))); + $p->addSpeaker($speaker); + $p->publish(); // published => "accepted" for getAcceptedPresentations + return $p; + } + + /** AbstractEmailJob::$payload is protected and has no accessor. */ + private function readPayload(object $job): array + { + $prop = new \ReflectionProperty(\App\Jobs\Emails\AbstractEmailJob::class, 'payload'); + $prop->setAccessible(true); + return $prop->getValue($job); + } +} diff --git a/tests/SubmitterRepositoryTest.php b/tests/SubmitterRepositoryTest.php index 2144f75cb..cc3fbc9b8 100644 --- a/tests/SubmitterRepositoryTest.php +++ b/tests/SubmitterRepositoryTest.php @@ -627,6 +627,70 @@ public function testGetUniqueActivitiesCountBySummitHasPublishedPresentationsFal $this->assertEquals(0, $count); } + // ----------------------------------------------------------------- + // has_published_presentations + presentations_track_id combined + // Each condition must be satisfied by the SAME presentation. + // The defect: a submitter with an unpublished presentation in track A + // and a published one in track B satisfies both conditions through + // two different presentations and is incorrectly included. + // ----------------------------------------------------------------- + + public function testHasPublishedPresentationsIsScopedByTrackFilter(): void + { + $member = self::$em->find(Member::class, self::$member->getId()); + $member2 = self::$em->find(Member::class, self::$member2->getId()); + + // member satisfies each condition through a DIFFERENT presentation. + $this->seedPresentation($member, self::$defaultTrack, 'Unpublished In Default', false); + $this->seedPresentation($member, self::$secondaryTrack, 'Published In Secondary', true); + + // Positive control: published in the track being filtered. + $this->seedPresentation($member2, self::$defaultTrack, 'Published In Default', true); + + self::$em->flush(); + + $filter = FilterParser::parse( + [ + 'has_published_presentations==true', + 'presentations_track_id==' . self::$defaultTrack->getId(), + ], + [ + 'has_published_presentations' => ['=='], + 'presentations_track_id' => ['=='], + ] + ); + + $repo = EntityManager::getRepository(Member::class); + $ids = array_map( + fn($m) => $m->getId(), + $repo->getSubmittersBySummit(self::$summit, new PagingInfo(1, 10), $filter, null)->getItems() + ); + + self::assertNotContains($member->getId(), $ids, + 'member has no published presentation in defaultTrack'); + self::assertContains($member2->getId(), $ids, + 'member2 published in defaultTrack must still be returned'); + } + + private function seedPresentation(Member $creator, $track, string $title, bool $publish): Presentation + { + $start = new \DateTime('now', new \DateTimeZone('UTC')); + + $p = new Presentation(); + self::$summit->addEvent($p); + $p->setTitle($title); + $p->setAbstract('Abstract'); + $p->setCategory($track); + $p->setType(self::$defaultPresentationType); + $p->setProgress(Presentation::PHASE_COMPLETE); + $p->setStatus(Presentation::STATUS_RECEIVED); + $p->setStartDate($start); + $p->setEndDate((clone $start)->add(new \DateInterval('PT2H'))); + $p->setCreatedBy($creator); + if ($publish) $p->publish(); + return $p; + } + // ----------------------------------------------------------------- // getUniqueActivitiesCountBySummit - presentations_track_group_id // The submitter repo and speaker repo share the filter name but use diff --git a/tests/SubmitterServiceOriginalFilterTest.php b/tests/SubmitterServiceOriginalFilterTest.php new file mode 100644 index 000000000..86f5f8444 --- /dev/null +++ b/tests/SubmitterServiceOriginalFilterTest.php @@ -0,0 +1,126 @@ +find(Member::class, self::$member2->getId()); + + // Same submitter, one published (= accepted) presentation per track. + $this->seedAcceptedPresentation($submitter, self::$defaultTrack, 'Submitted In Default Track'); + $this->seedAcceptedPresentation($submitter, self::$secondaryTrack, 'Submitted In Secondary Track'); + self::$em->flush(); + + // Mirrors summit-admin's "selected rows" send: ids go in `filter`, + // the grid criteria travel in payload.original_filter (submitter-actions.js:313). + $filter = FilterParser::parse( + ['id==' . $submitter->getId()], + ['id' => ['==']] + ); + + $payload = [ + 'email_flow_event' => PresentationSubmitterSelectionProcessAcceptedOnlyEmail::EVENT_SLUG, + 'should_resend' => true, + 'original_filter' => [ + 'has_published_presentations==true', + 'presentations_track_id==' . self::$defaultTrack->getId(), + ], + ]; + + App::make(ISubmitterService::class)->sendEmails(self::$summit->getId(), $payload, $filter); + + $jobs = Queue::pushed(PresentationSubmitterSelectionProcessAcceptedOnlyEmail::class); + $this->assertCount(1, $jobs, 'exactly one submitter email must be queued'); + + $emailPayload = $this->readPayload($jobs->first()); + $accepted = $emailPayload[IMailTemplatesConstants::accepted_presentations]; + + // With has_published_presentations missing from the service allow-list the + // parse throws, original_filter is dropped whole, the id== filter takes over, + // and the secondaryTrack presentation leaks into the email body. + $this->assertCount(1, $accepted, + 'only the presentation in the filtered track belongs in the email'); + $this->assertSame( + [self::$defaultTrack->getId()], + array_values(array_unique(array_map(fn(array $p) => $p['track']['id'], $accepted))), + 'every listed presentation must belong to the track carried by original_filter' + ); + } + + private function seedAcceptedPresentation( + Member $submitter, + $track, + string $title + ): Presentation { + $p = new Presentation(); + self::$summit->addEvent($p); + $p->setTitle($title); + $p->setAbstract('Abstract'); + $p->setCategory($track); + $p->setType(self::$defaultPresentationType); + $p->setProgress(Presentation::PHASE_COMPLETE); + $p->setStatus(Presentation::STATUS_RECEIVED); + $p->setStartDate(new \DateTime('now', new \DateTimeZone('UTC'))); + $p->setEndDate((new \DateTime('now', new \DateTimeZone('UTC')))->add(new \DateInterval('PT2H'))); + $p->setCreatedBy($submitter); + $p->publish(); // published => "accepted" for Member::getAcceptedPresentations + return $p; + } + + /** AbstractEmailJob::$payload is protected and has no accessor. */ + private function readPayload(object $job): array + { + $prop = new \ReflectionProperty(\App\Jobs\Emails\AbstractEmailJob::class, 'payload'); + $prop->setAccessible(true); + return $prop->getValue($job); + } +} diff --git a/tests/oauth2/OAuth2SummitSpeakersApiTest.php b/tests/oauth2/OAuth2SummitSpeakersApiTest.php index 944e9a230..05e5b5ff8 100644 --- a/tests/oauth2/OAuth2SummitSpeakersApiTest.php +++ b/tests/oauth2/OAuth2SummitSpeakersApiTest.php @@ -2835,13 +2835,31 @@ public function testCreateMySpeakerEmptyBioFallsBackToMemberBio() public function testGetCurrentSummitSpeakersWithPublishedPresentations() { + // Seed a speaker with only an unpublished presentation — must not appear. + $unpublishedOnly = new PresentationSpeaker(); + $unpublishedOnly->setFirstName('UnpublishedOnlyApi'); + $unpublishedOnly->setLastName('TestSpeaker'); + self::$em->persist($unpublishedOnly); + + $p = new Presentation(); + self::$summit->addEvent($p); + $p->setTitle('Unpublished Api Presentation'); + $p->setAbstract('Abstract'); + $p->setCategory(self::$defaultTrack); + $p->setType(self::$defaultPresentationType); + $p->setProgress(Presentation::PHASE_COMPLETE); + $p->setStatus(Presentation::STATUS_RECEIVED); + $p->setStartDate(new \DateTime('now', new \DateTimeZone('UTC'))); + $p->setEndDate((new \DateTime('now', new \DateTimeZone('UTC')))->add(new \DateInterval('PT2H'))); + $p->addSpeaker($unpublishedOnly); + // deliberately not published + self::$em->flush(); + $params = [ 'id' => self::$summit->getId(), 'page' => 1, - 'per_page' => 10, - 'filter' => [ - 'has_published_presentations==true', - ], + 'per_page' => 100, + 'filter' => ['has_published_presentations==true'], 'order' => '+id', ]; @@ -2853,17 +2871,23 @@ public function testGetCurrentSummitSpeakersWithPublishedPresentations() $response = $this->action( "GET", "OAuth2SummitSpeakersApiController@getSpeakers", - $params, - [], [], [], $headers + $params, [], [], [], $headers ); $this->assertResponseStatus(200); - $speakers = json_decode($response->getContent()); - $this->assertNotNull($speakers); + $ids = array_map(fn($s) => $s->id, json_decode($response->getContent())->data); + + $this->assertContains(self::$defaultSpeaker->getId(), $ids, + 'speaker with a published presentation must be returned'); + $this->assertNotContains($unpublishedOnly->getId(), $ids, + 'speaker with only unpublished presentations must be filtered out'); } public function testGetCurrentSummitSpeakersActivitiesCountWithPublishedPresentations() { + // The fixture already seeds published presentations for self::$defaultSpeaker, + // so a single call is enough to confirm the filter returns a non-zero count. + // Inclusion/exclusion correctness is covered by testGetCurrentSummitSpeakersWithPublishedPresentations. $headers = [ "HTTP_Authorization" => " Bearer " . $this->access_token, "CONTENT_TYPE" => "application/json", @@ -2880,7 +2904,7 @@ public function testGetCurrentSummitSpeakersActivitiesCountWithPublishedPresenta $data = json_decode($response->getContent()); $this->assertNotNull($data); $this->assertTrue(isset($data->count)); - $this->assertGreaterThanOrEqual(0, $data->count); + $this->assertGreaterThan(0, $data->count); } } diff --git a/tests/oauth2/OAuth2SummitSubmittersApiTest.php b/tests/oauth2/OAuth2SummitSubmittersApiTest.php index 5ad5a9e4f..a7dc6a98e 100644 --- a/tests/oauth2/OAuth2SummitSubmittersApiTest.php +++ b/tests/oauth2/OAuth2SummitSubmittersApiTest.php @@ -1,5 +1,7 @@ find(Member::class, self::$member->getId()); + $member2 = self::$em->find(Member::class, self::$member2->getId()); + + $start = new \DateTime('now', new \DateTimeZone('UTC')); + $end = (clone $start)->add(new \DateInterval('PT2H')); + + // member2: published presentation — must appear. + $p1 = new Presentation(); + self::$summit->addEvent($p1); + $p1->setTitle('Submitter Api Published'); + $p1->setAbstract('Abstract'); + $p1->setCategory(self::$defaultTrack); + $p1->setType(self::$defaultPresentationType); + $p1->setProgress(Presentation::PHASE_COMPLETE); + $p1->setStatus(Presentation::STATUS_RECEIVED); + $p1->setStartDate($start); + $p1->setEndDate($end); + $p1->setCreatedBy($member2); + $p1->publish(); + + // member: unpublished presentation only — must NOT appear. + $p2 = new Presentation(); + self::$summit->addEvent($p2); + $p2->setTitle('Submitter Api Unpublished'); + $p2->setAbstract('Abstract'); + $p2->setCategory(self::$defaultTrack); + $p2->setType(self::$defaultPresentationType); + $p2->setProgress(Presentation::PHASE_COMPLETE); + $p2->setStatus(Presentation::STATUS_RECEIVED); + $p2->setStartDate($start); + $p2->setEndDate($end); + $p2->setCreatedBy($member); + // deliberately not published + + self::$em->flush(); + $params = [ 'id' => self::$summit->getId(), 'page' => 1, - 'per_page' => 10, - 'filter' => [ - 'has_published_presentations==true', - ], + 'per_page' => 100, + 'filter' => ['has_published_presentations==true'], 'order' => '+id', ]; @@ -356,17 +392,39 @@ public function testGetCurrentSummitSubmittersWithPublishedPresentations() $response = $this->action( "GET", "OAuth2SummitSubmittersApiController@getAllBySummit", - $params, - [], [], [], $headers + $params, [], [], [], $headers ); $this->assertResponseStatus(200); - $submitters = json_decode($response->getContent()); - $this->assertNotNull($submitters); + $ids = array_map(fn($s) => $s->id, json_decode($response->getContent())->data); + + $this->assertContains($member2->getId(), $ids, + 'submitter with a published presentation must be returned'); + $this->assertNotContains($member->getId(), $ids, + 'submitter with only unpublished presentations must be filtered out'); } public function testGetCurrentSummitSubmittersActivitiesCountWithPublishedPresentations() { + $member2 = self::$em->find(Member::class, self::$member2->getId()); + + // The fixture sets no created_by on presentations, so the baseline is 0. + // Seed exactly one published presentation; the count must equal exactly 1. + $start = new \DateTime('now', new \DateTimeZone('UTC')); + $p = new Presentation(); + self::$summit->addEvent($p); + $p->setTitle('Count Submitter Published Api'); + $p->setAbstract('Abstract'); + $p->setCategory(self::$defaultTrack); + $p->setType(self::$defaultPresentationType); + $p->setProgress(Presentation::PHASE_COMPLETE); + $p->setStatus(Presentation::STATUS_RECEIVED); + $p->setStartDate($start); + $p->setEndDate((clone $start)->add(new \DateInterval('PT2H'))); + $p->setCreatedBy($member2); + $p->publish(); + self::$em->flush(); + $headers = [ "HTTP_Authorization" => " Bearer " . $this->access_token, "CONTENT_TYPE" => "application/json", @@ -383,6 +441,7 @@ public function testGetCurrentSummitSubmittersActivitiesCountWithPublishedPresen $data = json_decode($response->getContent()); $this->assertNotNull($data); $this->assertTrue(isset($data->count)); - $this->assertGreaterThanOrEqual(0, $data->count); + $this->assertEquals(1, $data->count, + 'exactly one published presentation was seeded; count must be 1'); } } \ No newline at end of file From 970ebaccf184b817ce4ebe6c15272558e121f187 Mon Sep 17 00:00:00 2001 From: gbutler Date: Fri, 4 Sep 2026 10:46:10 -0500 Subject: [PATCH 5/5] fix(speakers): add has_published_presentations to ISpeakerFilterFields The upstream allowlist consolidation introduced ISpeakerFilterFields as the single source of truth for speaker filter operators and validation rules, but did not include has_published_presentations. Adding it here propagates the fix to SpeakerService::sendEmails, OAuth2SummitSpeakersApiController, and ProcessSpeakersEmailRequestJob, all of which now reference ISpeakerFilterFields::OPERATORS directly. --- app/Services/Model/ISpeakerFilterFields.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/Services/Model/ISpeakerFilterFields.php b/app/Services/Model/ISpeakerFilterFields.php index ce4d43f52..8fb445bc4 100644 --- a/app/Services/Model/ISpeakerFilterFields.php +++ b/app/Services/Model/ISpeakerFilterFields.php @@ -36,6 +36,7 @@ interface ISpeakerFilterFields 'has_accepted_presentations' => ['=='], 'has_alternate_presentations' => ['=='], 'has_rejected_presentations' => ['=='], + 'has_published_presentations' => ['=='], 'presentations_track_id' => ['=='], 'presentations_track_group_id' => ['=='], 'presentations_selection_plan_id' => ['=='], @@ -60,6 +61,7 @@ interface ISpeakerFilterFields 'has_accepted_presentations' => 'sometimes|string|in:true,false', 'has_alternate_presentations' => 'sometimes|string|in:true,false', 'has_rejected_presentations' => 'sometimes|string|in:true,false', + 'has_published_presentations' => 'sometimes|string|in:true,false', 'presentations_track_id' => 'sometimes|integer', 'presentations_track_group_id' => 'sometimes|integer', 'presentations_selection_plan_id' => 'sometimes|integer',