From a61c8d7651935d048545c764b0c137c9d01a499c Mon Sep 17 00:00:00 2001 From: smarcet Date: Mon, 22 Jun 2026 15:37:24 -0300 Subject: [PATCH 01/13] refactor(DoctrineSpeakerRepository): two-phase getAllByPage + shared base query - Extract buildSpeakerBaseQuery() as the single place that defines the PresentationSpeaker/Member/RegistrationRequest join aliases used by all filter and order mappings. - Add getFastCount() and getAllIdsByPage() overrides following the same pattern as DoctrineSummitEventRepository. - Rewrite getAllByPage() with the two-phase approach: Phase 1 gets paged IDs via getAllIdsByPage(), Phase 2 fetches full entities WHERE id IN (:ids) with eager-loaded member and registration_request. Replaces ~130 lines of raw SQL + ResultSetMappingBuilder. - Simplify getUniqueActivitiesCountBySummit() to reuse buildSpeakerBaseQuery() instead of duplicating the from/leftJoin setup. - not_id filter was silently ignored by the old toRawSQL() mapping; now correctly applied via getFilterMappings(). - Add SpeakerRepositoryTest with 5 regression tests covering pagination, first_name filter, id filter, not_id exclusion, and first_name ordering. --- .../Summit/DoctrineSpeakerRepository.php | 257 ++++++++---------- tests/SpeakerRepositoryTest.php | 145 ++++++++++ 2 files changed, 252 insertions(+), 150 deletions(-) create mode 100644 tests/SpeakerRepositoryTest.php diff --git a/app/Repositories/Summit/DoctrineSpeakerRepository.php b/app/Repositories/Summit/DoctrineSpeakerRepository.php index 237042028..dd494dedc 100644 --- a/app/Repositories/Summit/DoctrineSpeakerRepository.php +++ b/app/Repositories/Summit/DoctrineSpeakerRepository.php @@ -603,20 +603,79 @@ protected function getOrderMappings() return [ 'id' => 'e.id', "first_name" => << << << <<getEntityManager()->createQueryBuilder() + ->from(PresentationSpeaker::class, 'e') + ->leftJoin('e.member', 'm') + ->leftJoin('e.registration_request', 'rr'); + } + + /** + * @param Filter|null $filter + * @param Order|null $order + * @return int + */ + public function getFastCount(Filter $filter = null, Order $order = null): int + { + $query = $this->buildSpeakerBaseQuery() + ->select('COUNT(DISTINCT e.id)'); + + if (!is_null($filter)) { + $filter->apply2Query($query, $this->getFilterMappings($filter)); + } + + return (int) $query->getQuery()->getSingleScalarResult(); + } + + /** + * @param PagingInfo $paging_info + * @param Filter|null $filter + * @param Order|null $order + * @return array + */ + public function getAllIdsByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null): array + { + $query = $this->buildSpeakerBaseQuery() + ->distinct(true) + ->select('e.id'); + + if (!is_null($filter)) { + $filter->apply2Query($query, $this->getFilterMappings($filter)); + } + + if (!is_null($order)) { + $order->apply2Query($query, $this->getOrderMappings()); + } else { + $query->addOrderBy('e.id', 'ASC'); + } + + $query->setFirstResult($paging_info->getOffset()) + ->setMaxResults($paging_info->getPerPage()); + + $res = $query->getQuery()->getArrayResult(); + return array_column($res, 'id'); + } + /** * @param Summit $summit * @param PagingInfo $paging_info @@ -705,17 +764,11 @@ function ($query) { */ public function getUniqueActivitiesCountBySummit(Summit $summit, Filter $filter = null): int { - // Inner query: distinct IDs of speakers who belong to this summit (via assignment - // or moderator role) and match any caller-supplied filter. Uses IDENTITY() with a - // scalar summit ID so no entity parameter is embedded in the getDQL() string — - // entity parameters copied into an outer QB via getDQL() are not correctly resolved - // to their primary key by Doctrine's type system. - $innerQb = $this->getEntityManager()->createQueryBuilder() - ->select('e.id') + // Phase 1: distinct speaker IDs scoped to this summit that match the caller filter. + // Uses IDENTITY() with a scalar so no entity param leaks into the embedded DQL string. + $speakerIdsQb = $this->buildSpeakerBaseQuery() ->distinct(true) - ->from('models\summit\PresentationSpeaker', 'e') - ->leftJoin('e.registration_request', 'rr') - ->leftJoin('e.member', 'm') + ->select('e.id') ->where( 'EXISTS (SELECT 1 FROM App\Models\Foundation\Summit\Speakers\PresentationSpeakerAssignment __a' . ' JOIN __a.presentation __ap WHERE IDENTITY(__ap.summit) = :summit_id AND __a.speaker = e)' @@ -724,14 +777,12 @@ public function getUniqueActivitiesCountBySummit(Summit $summit, Filter $filter ->setParameter('summit_id', $summit->getId()); if (!is_null($filter)) { - $filter->apply2Query($innerQb, $this->getFilterMappings($filter)); + $filter->apply2Query($speakerIdsQb, $this->getFilterMappings($filter)); } - $innerDql = $innerQb->getDQL(); + $innerDql = $speakerIdsQb->getDQL(); - // Outer query counts distinct presentations where at least one matched speaker is - // either an assigned speaker or the moderator. The inner DQL is embedded exactly - // once (inside a single wrapper EXISTS) to avoid Doctrine alias-conflict errors. + // Phase 2: count distinct presentations linked to any matched speaker. $outerQb = $this->getEntityManager()->createQueryBuilder() ->select('COUNT(DISTINCT p.id)') ->from('models\summit\Presentation', 'p') @@ -748,7 +799,7 @@ public function getUniqueActivitiesCountBySummit(Summit $summit, Filter $filter ) ->setParameter('summit', $summit); - foreach ($innerQb->getParameters() as $param) { + foreach ($speakerIdsQb->getParameters() as $param) { $outerQb->setParameter($param->getName(), $param->getValue()); } @@ -1047,141 +1098,47 @@ public function getSpeakersBySummitAndOnSchedule(Summit $summit, PagingInfo $pag */ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null) { - - $extra_filters = ''; - $extra_orders = ''; - $bindings = []; - - if (!is_null($filter)) { - $where_conditions = $filter->toRawSQL([ - 'first_name' => 'FirstName', - 'last_name' => 'LastName', - 'email' => [ - Filter::buildEmailField('Email'), - Filter::buildEmailField('Email2'), - Filter::buildEmailField('Email3'), - ], - 'id' => 'ID', - 'full_name' => "FullName", - 'member_id' => "MemberID", - 'member_user_external_id' => "MemberUserExternalID", - ]); - if (!empty($where_conditions)) { - $extra_filters = " WHERE {$where_conditions}"; - $bindings = array_merge($bindings, $filter->getSQLBindings()); - } + $total = $this->getFastCount($filter, $order); + $ids = $this->getAllIdsByPage($paging_info, $filter, $order); + + if (empty($ids)) { + return new PagingResponse( + $total, + $paging_info->getPerPage(), + $paging_info->getCurrentPage(), + $paging_info->getLastPage($total), + [] + ); } - if (!is_null($order)) { - $extra_orders = $order->toRawSQL(array - ( - 'first_name' => 'FirstName', - 'last_name' => 'LastName', - 'email' => 'Email', - 'id' => 'ID', - 'full_name' => "FullName", - )); + // Phase 2: fetch full entities for the paged IDs, eagerly loading member and + // registration_request to avoid N+1 on the fields used by the serializer. + $query = $this->buildSpeakerBaseQuery() + ->select('e') + ->addSelect('m') + ->addSelect('rr') + ->where('e.id IN (:ids)') + ->setParameter('ids', $ids); + + $speakers = $query->getQuery()->getResult(); + + // Restore Phase-1 order (IN (:ids) gives no ordering guarantee). + $byId = []; + foreach ($speakers as $s) { + $byId[$s->getId()] = $s; + } + $data = []; + foreach ($ids as $id) { + if (isset($byId[$id])) $data[] = $byId[$id]; } - $query_count = <<getEntityManager()->getConnection()->executeQuery($query_count, $bindings); - - $total = intval($stm->fetchOne()); - - $bindings = array_merge($bindings, array - ( - 'per_page' => $paging_info->getPerPage(), - 'offset' => $paging_info->getOffset(), - )); - - $query = <<addEntityResult(\models\summit\PresentationSpeaker::class, 's'); - $rsm->addJoinedEntityResult(\models\main\File::class,'p', 's', 'photo'); - $rsm->addJoinedEntityResult(\models\main\Member::class,'m', 's', 'member'); - - $rsm->addFieldResult('s', 'ID', 'id'); - $rsm->addFieldResult('s', 'FirstName', 'first_name'); - $rsm->addFieldResult('s', 'LastName', 'last_name'); - $rsm->addFieldResult('s', 'Bio', 'last_name'); - $rsm->addFieldResult('s', 'SpeakerTitle', 'title' ); - $rsm->addFieldResult('p', 'PhotoID', 'id'); - $rsm->addFieldResult('p', 'PhotoTitle', 'title'); - $rsm->addFieldResult('p', 'PhotoFileName', 'filename'); - $rsm->addFieldResult('p', 'PhotoName', 'name'); - $rsm->addFieldResult('m', 'MemberID', 'id');*/ - - $rsm = new ResultSetMappingBuilder($this->getEntityManager()); - $rsm->addRootEntityFromClassMetadata(\models\summit\PresentationSpeaker::class, 's', ['Title' => 'SpeakerTitle']); - - // build rsm here - $native_query = $this->getEntityManager()->createNativeQuery($query, $rsm); - - foreach ($bindings as $k => $v) - $native_query->setParameter($k, $v); - - $speakers = $native_query->getResult(); - - $last_page = (int)ceil($total / $paging_info->getPerPage()); - - return new PagingResponse($total, $paging_info->getPerPage(), $paging_info->getCurrentPage(), $last_page, $speakers); + return new PagingResponse( + $total, + $paging_info->getPerPage(), + $paging_info->getCurrentPage(), + $paging_info->getLastPage($total), + $data + ); } /** diff --git a/tests/SpeakerRepositoryTest.php b/tests/SpeakerRepositoryTest.php new file mode 100644 index 000000000..bab334bdc --- /dev/null +++ b/tests/SpeakerRepositoryTest.php @@ -0,0 +1,145 @@ +repo()->getAllByPage(new PagingInfo(1, 10)); + + $this->assertNotNull($page); + $this->assertGreaterThan(0, $page->getTotal()); + foreach ($page->getItems() as $speaker) { + $this->assertInstanceOf(PresentationSpeaker::class, $speaker); + // Entity must be hydrated enough for the serializer to call these. + $this->assertNotNull($speaker->getId()); + } + } + + // ----------------------------------------------------------------- + // getAllByPage - filter by first_name + // ----------------------------------------------------------------- + + public function testGetAllByPageFilterByFirstName(): void + { + // InsertSummitTestData seeds a speaker with first_name = "Sebastian". + $filter = FilterParser::parse( + ['filter' => 'first_name==Sebastian'], + ['first_name' => ['==']] + ); + + $page = $this->repo()->getAllByPage(new PagingInfo(1, 10), $filter); + + $this->assertGreaterThan(0, $page->getTotal()); + foreach ($page->getItems() as $speaker) { + $this->assertEquals('Sebastian', $speaker->getFirstName()); + } + } + + // ----------------------------------------------------------------- + // getAllByPage - filter by id + // ----------------------------------------------------------------- + + public function testGetAllByPageFilterById(): void + { + // Get any speaker to obtain a known ID. + $all = $this->repo()->getAllByPage(new PagingInfo(1, 1)); + $this->assertGreaterThan(0, $all->getTotal()); + $target = $all->getItems()[0]; + + $filter = FilterParser::parse( + ['filter' => 'id==' . $target->getId()], + ['id' => ['==']] + ); + + $page = $this->repo()->getAllByPage(new PagingInfo(1, 10), $filter); + + $this->assertEquals(1, $page->getTotal()); + $this->assertEquals($target->getId(), $page->getItems()[0]->getId()); + } + + // ----------------------------------------------------------------- + // getAllByPage - not_id filter (was silently ignored in raw-SQL version) + // ----------------------------------------------------------------- + + public function testGetAllByPageNotIdFilterExcludesSpeaker(): void + { + $all = $this->repo()->getAllByPage(new PagingInfo(1, 100)); + $this->assertGreaterThan(1, $all->getTotal(), 'Need at least 2 speakers for not_id test'); + + $excluded = $all->getItems()[0]->getId(); + + $filter = FilterParser::parse( + ['filter' => 'not_id==' . $excluded], + ['not_id' => ['==']] + ); + + $page = $this->repo()->getAllByPage(new PagingInfo(1, 100), $filter); + + $ids = array_map(fn($s) => $s->getId(), $page->getItems()); + $this->assertNotContains($excluded, $ids); + $this->assertEquals($all->getTotal() - 1, $page->getTotal()); + } + + // ----------------------------------------------------------------- + // getAllByPage - order + // ----------------------------------------------------------------- + + public function testGetAllByPageOrderByFirstNameAsc(): void + { + $order = new Order([OrderElement::buildAscFor('first_name')]); + $page = $this->repo()->getAllByPage(new PagingInfo(1, 50), null, $order); + + $names = array_map(fn($s) => strtolower((string) $s->getFirstName()), $page->getItems()); + $sorted = $names; + sort($sorted); + $this->assertEquals($sorted, $names); + } +} From 4da2767fd07eafb17561e9e01cb44c63171f20cb Mon Sep 17 00:00:00 2001 From: smarcet Date: Mon, 22 Jun 2026 16:08:12 -0300 Subject: [PATCH 02/13] refactor(DoctrineMemberRepository): two-phase submitter query pattern - Extract buildSubmitterBaseQuery(Summit): shared base QB scoping members to those with at least one presentation in the given summit - Add getSubmittersFastCount(Summit, Filter, Order): COUNT(DISTINCT e.id) - Add getSubmittersIdsByPage(Summit, PagingInfo, Filter, Order): Phase 1 returning a paginated, ordered array of member IDs - Refactor getSubmittersBySummit() to two-phase: Phase 1 IDs + Phase 2 entity fetch by IN (:ids), restoring original order after hydration - Simplify getSubmittersIdsBySummit() to delegate to getSubmittersIdsByPage() - Refactor getUniqueActivitiesCountBySummit() to two-phase: Phase 1 gets all matching member IDs (no pagination), Phase 2 counts presentations via IN (:member_ids) - eliminates alias collision risk vs. DQL embedding Existing SubmitterRepositoryTest (3 tests, 11 assertions) all pass. --- .../Summit/DoctrineMemberRepository.php | 167 +++++++++++------- 1 file changed, 106 insertions(+), 61 deletions(-) diff --git a/app/Repositories/Summit/DoctrineMemberRepository.php b/app/Repositories/Summit/DoctrineMemberRepository.php index 9ab2ce596..2679f5fae 100644 --- a/app/Repositories/Summit/DoctrineMemberRepository.php +++ b/app/Repositories/Summit/DoctrineMemberRepository.php @@ -575,40 +575,99 @@ public function getByEmailExclusiveLock($email): ?Member ->getOneOrNullResult(); } + /** + * Base QB shared by all per-summit submitter queries. + * Scopes to members who created at least one presentation in the given summit. + */ + private function buildSubmitterBaseQuery(Summit $summit): QueryBuilder + { + return $this->getEntityManager()->createQueryBuilder() + ->from($this->getBaseEntity(), 'e') + ->where( + 'EXISTS (SELECT __p.id FROM models\summit\Presentation __p ' . + 'JOIN __p.created_by __cb93 WITH __cb93 = e.id ' . + 'WHERE __p.summit = :summit)' + ) + ->setParameter('summit', $summit); + } + + /** + * COUNT(DISTINCT e.id) for per-summit submitters with optional filter. + */ + private function getSubmittersFastCount(Summit $summit, ?Filter $filter = null, ?Order $order = null): int + { + $qb = $this->buildSubmitterBaseQuery($summit)->select('COUNT(DISTINCT e.id)'); + $qb = $this->applyExtraJoins($qb, $filter, $order); + if (!is_null($filter)) $filter->apply2Query($qb, $this->getFilterMappings($filter)); + return (int) $qb->getQuery()->getSingleScalarResult(); + } + + /** + * Paginated, ordered array of member IDs for a summit. + * Phase 1 of the two-phase submitter query pattern. + */ + private function getSubmittersIdsByPage( + Summit $summit, + PagingInfo $paging_info, + ?Filter $filter = null, + ?Order $order = null + ): array { + $qb = $this->buildSubmitterBaseQuery($summit)->distinct(true)->select('e.id'); + $qb = $this->applyExtraJoins($qb, $filter, $order); + if (!is_null($filter)) $filter->apply2Query($qb, $this->getFilterMappings($filter)); + if (!is_null($order)) $order->apply2Query($qb, $this->getOrderMappings()); + else $qb->addOrderBy('e.id', 'ASC'); + $qb->setFirstResult($paging_info->getOffset())->setMaxResults($paging_info->getPerPage()); + return array_column($qb->getQuery()->getArrayResult(), 'id'); + } + /** * @inheritDoc */ public function getSubmittersBySummit(Summit $summit, PagingInfo $paging_info, Filter $filter = null, Order $order = null) { Log::debug(sprintf("DoctrineMemberRepository::getSubmittersBySummit summit %s", $summit->getId())); - $start = time(); - $res = $this->getParametrizedAllByPage(function () use ($summit) { - return $this->getEntityManager()->createQueryBuilder() - ->distinct(true) - ->select("e") - ->from($this->getBaseEntity(), "e") - ->where(" - EXISTS ( - SELECT __p.id FROM models\summit\Presentation __p - JOIN __p.created_by __cb93 WITH __cb93 = e.id - WHERE __p.summit = :summit - )") - ->setParameter("summit", $summit); - }, - $paging_info, - $filter, - $order, - function ($query) { - //default order - return $query->addOrderBy("e.id", 'ASC'); - }); + $start = time(); + + $total = $this->getSubmittersFastCount($summit, $filter, $order); + $ids = $this->getSubmittersIdsByPage($summit, $paging_info, $filter, $order); + + if (empty($ids)) { + return new PagingResponse( + $total, + $paging_info->getPerPage(), + $paging_info->getCurrentPage(), + $paging_info->getLastPage($total), + [] + ); + } + + $members = $this->getEntityManager()->createQueryBuilder() + ->select('e') + ->from($this->getBaseEntity(), 'e') + ->where('e.id IN (:ids)') + ->setParameter('ids', $ids) + ->getQuery() + ->getResult(); - $end = time(); - $delta = $end - $start; + $byId = []; + foreach ($members as $m) $byId[$m->getId()] = $m; + $data = []; + foreach ($ids as $id) if (isset($byId[$id])) $data[] = $byId[$id]; - Log::debug(sprintf("DoctrineMemberRepository::getSubmittersBySummit summit %s duration %s seconds.", $summit->getId(), $delta)); + Log::debug(sprintf( + "DoctrineMemberRepository::getSubmittersBySummit summit %s duration %s seconds.", + $summit->getId(), + time() - $start + )); - return $res; + return new PagingResponse( + $total, + $paging_info->getPerPage(), + $paging_info->getCurrentPage(), + $paging_info->getLastPage($total), + $data + ); } /** @@ -616,26 +675,7 @@ function ($query) { */ public function getSubmittersIdsBySummit(Summit $summit, PagingInfo $paging_info, Filter $filter = null, Order $order = null) { - return $this->getParametrizedAllIdsByPage(function () use ($summit) { - return $this->getEntityManager()->createQueryBuilder() - ->distinct(true) - ->select("e.id") - ->from($this->getBaseEntity(), "e") - ->where(" - EXISTS ( - SELECT __p.id FROM models\summit\Presentation __p - JOIN __p.created_by __cb93 WITH __cb93 = e.id - WHERE __p.summit = :summit - )") - ->setParameter("summit", $summit); - }, - $paging_info, - $filter, - $order, - function ($query) { - //default order - return $query->addOrderBy("e.id", 'ASC'); - }); + return $this->getSubmittersIdsByPage($summit, $paging_info, $filter, $order); } /** @@ -646,22 +686,27 @@ function ($query) { */ public function getUniqueActivitiesCountBySummit(Summit $summit, Filter $filter = null): int { - // Start from Presentation and JOIN to the submitter (created_by). - // Bounded by summit — no subquery needed, filter mappings apply to e unchanged. - $countQb = $this->getEntityManager()->createQueryBuilder() - ->select("COUNT(DISTINCT p.id)") - ->from('models\summit\Presentation', 'p') - ->join('p.created_by', 'e') - ->where('p.summit = :summit') - ->setParameter('summit', $summit); - - $countQb = $this->applyExtraJoins($countQb, $filter); - - if (!is_null($filter)) { - $filter->apply2Query($countQb, $this->getFilterMappings($filter)); - } - - return intval($countQb->getQuery()->getSingleScalarResult()); + // Phase 1: member IDs matching the summit scope + filter (no pagination). + $memberIdsQb = $this->buildSubmitterBaseQuery($summit)->distinct(true)->select('e.id'); + $memberIdsQb = $this->applyExtraJoins($memberIdsQb, $filter); + if (!is_null($filter)) $filter->apply2Query($memberIdsQb, $this->getFilterMappings($filter)); + $memberIds = array_column($memberIdsQb->getQuery()->getArrayResult(), 'id'); + + if (empty($memberIds)) return 0; + + // Phase 2: count distinct presentations whose author is one of those members. + return intval( + $this->getEntityManager()->createQueryBuilder() + ->select('COUNT(DISTINCT p.id)') + ->from('models\summit\Presentation', 'p') + ->join('p.created_by', 'cb') + ->where('p.summit = :summit') + ->andWhere('cb.id IN (:member_ids)') + ->setParameter('summit', $summit) + ->setParameter('member_ids', $memberIds) + ->getQuery() + ->getSingleScalarResult() + ); } /** From 66b0825c5434a998ef123e90c9cd37834ed23638 Mon Sep 17 00:00:00 2001 From: smarcet Date: Mon, 22 Jun 2026 16:41:18 -0300 Subject: [PATCH 03/13] refactor: replace nested IN(DQL)+EXISTS with temp-table chunk pattern in getUniqueActivitiesCountBySummit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both Speaker and Member repos were running a correlated EXISTS wrapping a nested IN(SELECT DISTINCT ...) subquery, resulting in O(presentations x speakers) execution — measured at 12-14s on dev data. New approach in both repos: - Phase 1: DQL QB (same filter system) executed in chunks of 1000 rows via LIMIT/OFFSET; each chunk is INSERT IGNOREd into a session-scoped MEMORY temp table (__tmp_spk_ids / __tmp_mbr_ids). PHP never holds the full ID set in memory; MySQL keeps IDs in an indexed MEMORY table. - Phase 2: raw SQL JOIN against the temp table — no correlated subqueries, no IN array passed over the wire, MySQL uses primary key index on the temp table. - try/finally guarantees DROP TEMPORARY TABLE even on exception. Speaker Phase 2: UNION of assignment-based and moderator-based counts, deduplicating across both roles in a single round-trip. Member Phase 2: single JOIN on SummitEvent.CreatedByID. 8 regression tests (5 Speaker + 3 Submitter) all pass. --- .../Summit/DoctrineMemberRepository.php | 66 +++++++++---- .../Summit/DoctrineSpeakerRepository.php | 96 ++++++++++++------- 2 files changed, 105 insertions(+), 57 deletions(-) diff --git a/app/Repositories/Summit/DoctrineMemberRepository.php b/app/Repositories/Summit/DoctrineMemberRepository.php index 2679f5fae..43d780c53 100644 --- a/app/Repositories/Summit/DoctrineMemberRepository.php +++ b/app/Repositories/Summit/DoctrineMemberRepository.php @@ -686,27 +686,53 @@ public function getSubmittersIdsBySummit(Summit $summit, PagingInfo $paging_info */ public function getUniqueActivitiesCountBySummit(Summit $summit, Filter $filter = null): int { - // Phase 1: member IDs matching the summit scope + filter (no pagination). - $memberIdsQb = $this->buildSubmitterBaseQuery($summit)->distinct(true)->select('e.id'); - $memberIdsQb = $this->applyExtraJoins($memberIdsQb, $filter); - if (!is_null($filter)) $filter->apply2Query($memberIdsQb, $this->getFilterMappings($filter)); - $memberIds = array_column($memberIdsQb->getQuery()->getArrayResult(), 'id'); - - if (empty($memberIds)) return 0; - - // Phase 2: count distinct presentations whose author is one of those members. - return intval( - $this->getEntityManager()->createQueryBuilder() - ->select('COUNT(DISTINCT p.id)') - ->from('models\summit\Presentation', 'p') - ->join('p.created_by', 'cb') - ->where('p.summit = :summit') - ->andWhere('cb.id IN (:member_ids)') - ->setParameter('summit', $summit) - ->setParameter('member_ids', $memberIds) - ->getQuery() - ->getSingleScalarResult() + $conn = $this->getEntityManager()->getConnection(); + $conn->executeStatement( + 'CREATE TEMPORARY TABLE IF NOT EXISTS `__tmp_mbr_ids` + (`id` INT UNSIGNED NOT NULL, PRIMARY KEY (`id`)) ENGINE=MEMORY' ); + $conn->executeStatement('TRUNCATE TABLE `__tmp_mbr_ids`'); + + try { + // Phase 1: stream member IDs (summit-scoped + filtered) into the + // temp table in chunks so PHP never holds the full set in memory. + $chunkSize = 1000; + $offset = 0; + + $qb = $this->buildSubmitterBaseQuery($summit) + ->distinct(true) + ->select('e.id') + ->addOrderBy('e.id', 'ASC'); + $qb = $this->applyExtraJoins($qb, $filter); + if (!is_null($filter)) { + $filter->apply2Query($qb, $this->getFilterMappings($filter)); + } + + do { + $chunk = $qb->setFirstResult($offset)->setMaxResults($chunkSize) + ->getQuery()->getArrayResult(); + + if (!empty($chunk)) { + $vals = implode(',', array_map(fn($r) => '(' . (int)$r['id'] . ')', $chunk)); + $conn->executeStatement("INSERT IGNORE INTO `__tmp_mbr_ids` (id) VALUES $vals"); + } + + $offset += $chunkSize; + } while (count($chunk) === $chunkSize); + + // Phase 2: count distinct presentations whose creator is in the matched set. + $sql = <<fetchOne($sql, [$summit->getId()]); + } finally { + $conn->executeStatement('DROP TEMPORARY TABLE IF EXISTS `__tmp_mbr_ids`'); + } } /** diff --git a/app/Repositories/Summit/DoctrineSpeakerRepository.php b/app/Repositories/Summit/DoctrineSpeakerRepository.php index dd494dedc..f594c6660 100644 --- a/app/Repositories/Summit/DoctrineSpeakerRepository.php +++ b/app/Repositories/Summit/DoctrineSpeakerRepository.php @@ -764,46 +764,68 @@ function ($query) { */ public function getUniqueActivitiesCountBySummit(Summit $summit, Filter $filter = null): int { - // Phase 1: distinct speaker IDs scoped to this summit that match the caller filter. - // Uses IDENTITY() with a scalar so no entity param leaks into the embedded DQL string. - $speakerIdsQb = $this->buildSpeakerBaseQuery() - ->distinct(true) - ->select('e.id') - ->where( - 'EXISTS (SELECT 1 FROM App\Models\Foundation\Summit\Speakers\PresentationSpeakerAssignment __a' - . ' JOIN __a.presentation __ap WHERE IDENTITY(__ap.summit) = :summit_id AND __a.speaker = e)' - . ' OR EXISTS (SELECT 1 FROM models\summit\Presentation __mp WHERE IDENTITY(__mp.summit) = :summit_id AND __mp.moderator = e)' - ) - ->setParameter('summit_id', $summit->getId()); + $conn = $this->getEntityManager()->getConnection(); + $conn->executeStatement( + 'CREATE TEMPORARY TABLE IF NOT EXISTS `__tmp_spk_ids` + (`id` INT UNSIGNED NOT NULL, PRIMARY KEY (`id`)) ENGINE=MEMORY' + ); + $conn->executeStatement('TRUNCATE TABLE `__tmp_spk_ids`'); - if (!is_null($filter)) { - $filter->apply2Query($speakerIdsQb, $this->getFilterMappings($filter)); - } + try { + // Phase 1: stream speaker IDs (summit-scoped + filtered) into the + // temp table in chunks so PHP never holds the full set in memory. + $chunkSize = 1000; + $offset = 0; - $innerDql = $speakerIdsQb->getDQL(); - - // Phase 2: count distinct presentations linked to any matched speaker. - $outerQb = $this->getEntityManager()->createQueryBuilder() - ->select('COUNT(DISTINCT p.id)') - ->from('models\summit\Presentation', 'p') - ->where('p.summit = :summit') - ->andWhere( - 'EXISTS (' - . 'SELECT 1 FROM models\summit\PresentationSpeaker __spk' - . ' WHERE __spk.id IN (' . $innerDql . ')' - . ' AND (' - . 'EXISTS (SELECT 1 FROM App\Models\Foundation\Summit\Speakers\PresentationSpeakerAssignment __cnt WHERE __cnt.presentation = p AND __cnt.speaker = __spk)' - . ' OR p.moderator = __spk' - . ')' - . ')' - ) - ->setParameter('summit', $summit); - - foreach ($speakerIdsQb->getParameters() as $param) { - $outerQb->setParameter($param->getName(), $param->getValue()); - } + $qb = $this->buildSpeakerBaseQuery() + ->distinct(true) + ->select('e.id') + ->addOrderBy('e.id', 'ASC') + ->where( + 'EXISTS (SELECT 1 FROM App\Models\Foundation\Summit\Speakers\PresentationSpeakerAssignment __a' + . ' JOIN __a.presentation __ap WHERE IDENTITY(__ap.summit) = :summit_id AND __a.speaker = e)' + . ' OR EXISTS (SELECT 1 FROM models\summit\Presentation __mp WHERE IDENTITY(__mp.summit) = :summit_id AND __mp.moderator = e)' + ) + ->setParameter('summit_id', $summit->getId()); + + if (!is_null($filter)) { + $filter->apply2Query($qb, $this->getFilterMappings($filter)); + } + + do { + $chunk = $qb->setFirstResult($offset)->setMaxResults($chunkSize) + ->getQuery()->getArrayResult(); + + if (!empty($chunk)) { + $vals = implode(',', array_map(fn($r) => '(' . (int)$r['id'] . ')', $chunk)); + $conn->executeStatement("INSERT IGNORE INTO `__tmp_spk_ids` (id) VALUES $vals"); + } - return intval($outerQb->getQuery()->getSingleScalarResult()); + $offset += $chunkSize; + } while (count($chunk) === $chunkSize); + + // Phase 2: count distinct presentations linked via assignment or moderator. + // UNION deduplicates across both roles in a single round-trip. + $sql = <<fetchOne($sql, [$summit->getId(), $summit->getId()]); + } finally { + $conn->executeStatement('DROP TEMPORARY TABLE IF EXISTS `__tmp_spk_ids`'); + } } /** From 73c169a0e9d79bbd52200b6fb29ab27a4522f17c Mon Sep 17 00:00:00 2001 From: smarcet Date: Mon, 22 Jun 2026 16:54:58 -0300 Subject: [PATCH 04/13] fix: MySQL 1137 in getUniqueActivitiesCountBySummit speaker repo MySQL forbids referencing a TEMPORARY TABLE more than once in a single SQL statement (error 1137 'Can't reopen table'). The previous UNION query touched __tmp_spk_ids in both branches. Fix: split Phase 2 into two separate INSERT IGNORE statements (one reference each) feeding a second MEMORY temp table __tmp_pres_ids, then count from that. Both temp tables are dropped in the finally block. --- .../Summit/DoctrineSpeakerRepository.php | 50 ++++++++++++------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/app/Repositories/Summit/DoctrineSpeakerRepository.php b/app/Repositories/Summit/DoctrineSpeakerRepository.php index f594c6660..be9aee209 100644 --- a/app/Repositories/Summit/DoctrineSpeakerRepository.php +++ b/app/Repositories/Summit/DoctrineSpeakerRepository.php @@ -804,26 +804,38 @@ public function getUniqueActivitiesCountBySummit(Summit $summit, Filter $filter $offset += $chunkSize; } while (count($chunk) === $chunkSize); - // Phase 2: count distinct presentations linked via assignment or moderator. - // UNION deduplicates across both roles in a single round-trip. - $sql = <<fetchOne($sql, [$summit->getId(), $summit->getId()]); + // Phase 2: collect distinct presentation IDs from both roles into a second + // temp table using two separate statements -- MySQL forbids referencing the + // same TEMPORARY TABLE more than once per query (error 1137). + $conn->executeStatement( + 'CREATE TEMPORARY TABLE IF NOT EXISTS `__tmp_pres_ids` + (`id` INT UNSIGNED NOT NULL, PRIMARY KEY (`id`)) ENGINE=MEMORY' + ); + $conn->executeStatement('TRUNCATE TABLE `__tmp_pres_ids`'); + + $conn->executeStatement( + 'INSERT IGNORE INTO `__tmp_pres_ids` (id) + SELECT DISTINCT E.ID + FROM SummitEvent E + INNER JOIN Presentation_Speakers PS ON PS.PresentationID = E.ID + INNER JOIN `__tmp_spk_ids` T ON T.id = PS.PresentationSpeakerID + WHERE E.SummitID = ?', + [$summit->getId()] + ); + + $conn->executeStatement( + 'INSERT IGNORE INTO `__tmp_pres_ids` (id) + SELECT DISTINCT E.ID + FROM SummitEvent E + INNER JOIN Presentation P ON P.ID = E.ID + INNER JOIN `__tmp_spk_ids` T ON T.id = P.ModeratorID + WHERE E.SummitID = ?', + [$summit->getId()] + ); + + return (int) $conn->fetchOne('SELECT COUNT(*) FROM `__tmp_pres_ids`'); } finally { + $conn->executeStatement('DROP TEMPORARY TABLE IF EXISTS `__tmp_pres_ids`'); $conn->executeStatement('DROP TEMPORARY TABLE IF EXISTS `__tmp_spk_ids`'); } } From cf4c5cd75566bf7467832c4ca476e7bc4ae21b1c Mon Sep 17 00:00:00 2001 From: smarcet Date: Mon, 22 Jun 2026 17:00:48 -0300 Subject: [PATCH 05/13] fix: bind :summit entity parameter in getUniqueActivitiesCountBySummit Filter mappings (e.g. presentations_selection_plan_id) reference the :summit parameter as a Doctrine entity in their EXISTS subqueries. Phase 1 was only binding :summit_id (integer), leaving :summit unbound when any presentation-scoped filter was applied -> HTTP 500. --- app/Repositories/Summit/DoctrineSpeakerRepository.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/Repositories/Summit/DoctrineSpeakerRepository.php b/app/Repositories/Summit/DoctrineSpeakerRepository.php index be9aee209..2e789c717 100644 --- a/app/Repositories/Summit/DoctrineSpeakerRepository.php +++ b/app/Repositories/Summit/DoctrineSpeakerRepository.php @@ -786,7 +786,9 @@ public function getUniqueActivitiesCountBySummit(Summit $summit, Filter $filter . ' JOIN __a.presentation __ap WHERE IDENTITY(__ap.summit) = :summit_id AND __a.speaker = e)' . ' OR EXISTS (SELECT 1 FROM models\summit\Presentation __mp WHERE IDENTITY(__mp.summit) = :summit_id AND __mp.moderator = e)' ) - ->setParameter('summit_id', $summit->getId()); + ->setParameter('summit_id', $summit->getId()) + // filter mappings reference :summit (entity) for EXISTS subqueries + ->setParameter('summit', $summit); if (!is_null($filter)) { $filter->apply2Query($qb, $this->getFilterMappings($filter)); From e4d293b4f69b0186020b789dedf5a769f39f59ac Mon Sep 17 00:00:00 2001 From: smarcet Date: Mon, 22 Jun 2026 17:01:00 -0300 Subject: [PATCH 06/13] test: add getUniqueActivitiesCountBySummit coverage in SpeakerRepositoryTest Regression tests for the MySQL 1137 fix and filter-with-no-match edge case. --- tests/SpeakerRepositoryTest.php | 36 +++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/SpeakerRepositoryTest.php b/tests/SpeakerRepositoryTest.php index bab334bdc..7bc2ed9b9 100644 --- a/tests/SpeakerRepositoryTest.php +++ b/tests/SpeakerRepositoryTest.php @@ -142,4 +142,40 @@ public function testGetAllByPageOrderByFirstNameAsc(): void sort($sorted); $this->assertEquals($sorted, $names); } + + // ----------------------------------------------------------------- + // getUniqueActivitiesCountBySummit - regression for MySQL 1137 + // The previous UNION query referenced __tmp_spk_ids twice in one + // SQL statement, triggering "Can't reopen table". + // ----------------------------------------------------------------- + + public function testGetUniqueActivitiesCountBySummitReturnsPositiveInt(): void + { + // InsertSummitTestData seeds speaker1 (first_name="Sebastian") on 40 presentations. + $count = $this->repo()->getUniqueActivitiesCountBySummit(self::$summit); + $this->assertIsInt($count); + $this->assertGreaterThan(0, $count); + } + + public function testGetUniqueActivitiesCountBySummitWithMatchingFilter(): void + { + $filter = FilterParser::parse( + ['filter' => 'first_name==Sebastian'], + ['first_name' => ['==']] + ); + $unfiltered = $this->repo()->getUniqueActivitiesCountBySummit(self::$summit); + $filtered = $this->repo()->getUniqueActivitiesCountBySummit(self::$summit, $filter); + // All seeded presentations use speaker1 (Sebastian), so the counts must be equal. + $this->assertEquals($unfiltered, $filtered); + } + + public function testGetUniqueActivitiesCountBySummitZeroForUnknownSpeaker(): void + { + $filter = FilterParser::parse( + ['filter' => 'first_name==NoSuchSpeakerXYZ'], + ['first_name' => ['==']] + ); + $count = $this->repo()->getUniqueActivitiesCountBySummit(self::$summit, $filter); + $this->assertEquals(0, $count); + } } From fcd2e6affe7075919eb6bd4693499dc3525c20d9 Mon Sep 17 00:00:00 2001 From: smarcet Date: Mon, 22 Jun 2026 17:06:06 -0300 Subject: [PATCH 07/13] fix: unify summit parameter to :summit entity in Phase 1 QB Using two summit params (:summit_id integer + :summit entity) caused DoctrineFilterMapping's counter-based :value_N naming to produce a parameter the query executor counted but couldn't match. Switch to a single :summit entity parameter matching the pattern all other speaker-repo methods use and that all filter mappings expect. --- app/Repositories/Summit/DoctrineSpeakerRepository.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/Repositories/Summit/DoctrineSpeakerRepository.php b/app/Repositories/Summit/DoctrineSpeakerRepository.php index 2e789c717..98c74ec4c 100644 --- a/app/Repositories/Summit/DoctrineSpeakerRepository.php +++ b/app/Repositories/Summit/DoctrineSpeakerRepository.php @@ -783,11 +783,9 @@ public function getUniqueActivitiesCountBySummit(Summit $summit, Filter $filter ->addOrderBy('e.id', 'ASC') ->where( 'EXISTS (SELECT 1 FROM App\Models\Foundation\Summit\Speakers\PresentationSpeakerAssignment __a' - . ' JOIN __a.presentation __ap WHERE IDENTITY(__ap.summit) = :summit_id AND __a.speaker = e)' - . ' OR EXISTS (SELECT 1 FROM models\summit\Presentation __mp WHERE IDENTITY(__mp.summit) = :summit_id AND __mp.moderator = e)' + . ' JOIN __a.presentation __ap WHERE __ap.summit = :summit AND __a.speaker = e)' + . ' OR EXISTS (SELECT 1 FROM models\summit\Presentation __mp WHERE __mp.summit = :summit AND __mp.moderator = e)' ) - ->setParameter('summit_id', $summit->getId()) - // filter mappings reference :summit (entity) for EXISTS subqueries ->setParameter('summit', $summit); if (!is_null($filter)) { From 09a0304682dd590f7038ee0cd4155196e47c0d8c Mon Sep 17 00:00:00 2001 From: smarcet Date: Mon, 22 Jun 2026 18:18:05 -0300 Subject: [PATCH 08/13] feat(submitters): add presentations_track_group_id filter support Port the presentations_track_group_id filter from speakers to submitters. - OAuth2SummitSubmittersApiController: add operator ['=='] and validation rule 'sometimes|integer' in all four public methods (getAllBySummit, getAllBySummitCSV, send, getSubmittersActivitiesCount) - DoctrineMemberRepository: add preamble block that injects a category-group subquery into $extraSelectionStatusFilter, $extraSelectionPlanFilter, and $extraMediaUploadFilter; add DoctrineFilterMapping entry with alias prefix 42_ - SubmitterRepositoryTest: testGetSubmittersByTrackGroupId asserts DQL filters correctly (member2 in, member out) - OAuth2SummitSubmittersApiTest: testGetSubmittersFilterByTrackGroupId asserts HTTP 200 instead of 422 --- .../OAuth2SummitSubmittersApiController.php | 8 +++ .../Summit/DoctrineMemberRepository.php | 29 +++++++-- tests/SubmitterRepositoryTest.php | 65 +++++++++++++++++++ .../oauth2/OAuth2SummitSubmittersApiTest.php | 35 ++++++++++ 4 files changed, 133 insertions(+), 4 deletions(-) diff --git a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSubmittersApiController.php b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSubmittersApiController.php index 86653a432..abf6da451 100644 --- a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSubmittersApiController.php +++ b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSubmittersApiController.php @@ -156,6 +156,7 @@ function () { 'has_alternate_presentations' => ['=='], 'has_rejected_presentations' => ['=='], 'presentations_track_id' => ['=='], + 'presentations_track_group_id' => ['=='], 'presentations_selection_plan_id' => ['=='], 'presentations_type_id' => ['=='], 'presentations_title' => ['=@', '@@', '=='], @@ -181,6 +182,7 @@ function () { 'has_alternate_presentations' => 'sometimes|string|in:true,false', 'has_rejected_presentations' => 'sometimes|string|in:true,false', 'presentations_track_id' => 'sometimes|integer', + 'presentations_track_group_id' => 'sometimes|integer', 'presentations_selection_plan_id' => 'sometimes|integer', 'presentations_type_id' => 'sometimes|integer', 'presentations_title' => 'sometimes|string', @@ -300,6 +302,7 @@ function () { 'has_alternate_presentations' => ['=='], 'has_rejected_presentations' => ['=='], 'presentations_track_id' => ['=='], + 'presentations_track_group_id' => ['=='], 'presentations_selection_plan_id' => ['=='], 'presentations_type_id' => ['=='], 'presentations_title' => ['=@', '@@', '=='], @@ -325,6 +328,7 @@ function () { 'has_alternate_presentations' => 'sometimes|string|in:true,false', 'has_rejected_presentations' => 'sometimes|string|in:true,false', 'presentations_track_id' => 'sometimes|integer', + 'presentations_track_group_id' => 'sometimes|integer', 'presentations_selection_plan_id' => 'sometimes|integer', 'presentations_type_id' => 'sometimes|integer', 'presentations_title' => 'sometimes|string', @@ -451,6 +455,7 @@ public function send($summit_id) 'has_alternate_presentations' => ['=='], 'has_rejected_presentations' => ['=='], 'presentations_track_id' => ['=='], + 'presentations_track_group_id' => ['=='], 'presentations_selection_plan_id' => ['=='], 'presentations_type_id' => ['=='], 'presentations_title' => ['=@', '@@', '=='], @@ -479,6 +484,7 @@ public function send($summit_id) 'has_alternate_presentations' => 'sometimes|string|in:true,false', 'has_rejected_presentations' => 'sometimes|string|in:true,false', 'presentations_track_id' => 'sometimes|integer', + 'presentations_track_group_id' => 'sometimes|integer', 'presentations_selection_plan_id' => 'sometimes|integer', 'presentations_type_id' => 'sometimes|integer', 'presentations_title' => 'sometimes|string', @@ -559,6 +565,7 @@ public function getSubmittersActivitiesCount($summit_id) 'has_alternate_presentations' => ['=='], 'has_rejected_presentations' => ['=='], 'presentations_track_id' => ['=='], + 'presentations_track_group_id' => ['=='], 'presentations_selection_plan_id' => ['=='], 'presentations_type_id' => ['=='], 'presentations_title' => ['=@', '@@', '=='], @@ -585,6 +592,7 @@ public function getSubmittersActivitiesCount($summit_id) 'has_alternate_presentations' => 'sometimes|string|in:true,false', 'has_rejected_presentations' => 'sometimes|string|in:true,false', 'presentations_track_id' => 'sometimes|integer', + 'presentations_track_group_id' => 'sometimes|integer', 'presentations_selection_plan_id' => 'sometimes|integer', 'presentations_type_id' => 'sometimes|integer', 'presentations_title' => 'sometimes|string', diff --git a/app/Repositories/Summit/DoctrineMemberRepository.php b/app/Repositories/Summit/DoctrineMemberRepository.php index 43d780c53..d2c23fd9c 100644 --- a/app/Repositories/Summit/DoctrineMemberRepository.php +++ b/app/Repositories/Summit/DoctrineMemberRepository.php @@ -97,6 +97,17 @@ protected function getFilterMappings() $extraSelectionPlanFilter .= ' AND __type%1$s_:i.id IN ('.implode(',', $v).')'; $extraMediaUploadFilter .= ' AND __type%1$s:i.id IN ('.implode(',', $v).')'; } + if($filter->hasFilter("presentations_track_group_id")){ + $v = $filter->getValue("presentations_track_group_id"); + $category_categorygroup_subquery = 'SELECT ___cat%1$s.id + FROM models\summit\PresentationCategory ___cat%1$s + JOIN ___cat%1$s.groups ___catg%1$s + WHERE ___catg%1$s.id IN ('.implode(',', $v).')'; + + $extraSelectionStatusFilter .= ' AND __cat%1$s.id IN ('.$category_categorygroup_subquery.')'; + $extraSelectionPlanFilter .= ' AND __tr%1$s_:i.id IN ('.$category_categorygroup_subquery.')'; + $extraMediaUploadFilter .= ' AND __tr%1$s:i.id IN ('.$category_categorygroup_subquery.')'; + } if($filter->hasFilter("has_media_upload_with_type")){ $v = $filter->getValue("has_media_upload_with_type"); @@ -158,14 +169,24 @@ protected function getFilterMappings() 'email_verified' => 'e.email_verified:json_int', 'active' => 'e.active:json_int', 'presentations_track_id' => new DoctrineFilterMapping( - "EXISTS ( - SELECT __p41_:i.id FROM models\summit\Presentation __p41_:i + "EXISTS ( + SELECT __p41_:i.id FROM models\summit\Presentation __p41_:i JOIN __p41_:i.created_by __c41_:i WITH __c41_:i = e.id - JOIN __p41_:i.category __tr41_:i - WHERE + JOIN __p41_:i.category __tr41_:i + WHERE __p41_:i.summit = :summit AND __tr41_:i.id :operator :value )" ), + 'presentations_track_group_id' => new DoctrineFilterMapping( + "EXISTS ( + SELECT __p42_:i.id FROM models\summit\Presentation __p42_:i + JOIN __p42_:i.created_by __c42_:i WITH __c42_:i = e.id + JOIN __p42_:i.category __tr42_:i + JOIN __tr42_:i.groups __trg42_:i + WHERE + __p42_:i.summit = :summit AND + __trg42_:i.id :operator :value )" + ), 'presentations_selection_plan_id' => new DoctrineFilterMapping( "EXISTS ( SELECT __p51_:i.id FROM models\summit\Presentation __p51_:i diff --git a/tests/SubmitterRepositoryTest.php b/tests/SubmitterRepositoryTest.php index 4f35be1ed..8f2523893 100644 --- a/tests/SubmitterRepositoryTest.php +++ b/tests/SubmitterRepositoryTest.php @@ -3,6 +3,7 @@ use LaravelDoctrine\ORM\Facades\EntityManager; use models\main\Member; use models\summit\Presentation; +use models\summit\PresentationCategory; use models\summit\PresentationSpeaker; use ModelSerializers\SerializerRegistry; use utils\FilterParser; @@ -179,4 +180,68 @@ public function testGetUniqueActivitiesCountBySummit(){ $filteredCount = $submitter_repository->getUniqueActivitiesCountBySummit(self::$summit, $filter); self::assertEquals(2, $filteredCount); } + + public function testGetSubmittersByTrackGroupId(): void + { + // P1: member2 submits in defaultTrack (belongs to defaultTrackGroup) -> must appear. + // P2: member submits in altTrack (not in any group) -> must be excluded. + // RED: before Task 2 the filter is silently ignored and both members are returned. + // GREEN: after Task 2 only member2 is returned. + $member = self::$em->find(Member::class, self::$member->getId()); + $member2 = self::$em->find(Member::class, self::$member2->getId()); + + $altTrack = new PresentationCategory(); + $altTrack->setTitle('Alt Track For Group Filter Test'); + $altTrack->setCode('ALTTST'); + $altTrack->setSessionCount(3); + $altTrack->setAlternateCount(3); + $altTrack->setLightningCount(3); + $altTrack->setChairVisible(false); + $altTrack->setVotingVisible(false); + self::$summit->addPresentationCategory($altTrack); + self::$em->persist($altTrack); + + $start = new \DateTime('now', new \DateTimeZone('UTC')); + $end = (clone $start)->add(new \DateInterval('PT2H')); + + $p1 = new Presentation(); + self::$summit->addEvent($p1); + $p1->setTitle('Track Group Filter Test - In Group'); + $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); + + $p2 = new Presentation(); + self::$summit->addEvent($p2); + $p2->setTitle('Track Group Filter Test - Not In Group'); + $p2->setAbstract('Abstract'); + $p2->setCategory($altTrack); + $p2->setType(self::$defaultPresentationType); + $p2->setProgress(Presentation::PHASE_COMPLETE); + $p2->setStatus(Presentation::STATUS_RECEIVED); + $p2->setStartDate($start); + $p2->setEndDate($end); + $p2->setCreatedBy($member); + + self::$em->flush(); + + $submitter_repository = EntityManager::getRepository(Member::class); + + $filter = FilterParser::parse( + ["filter" => sprintf("presentations_track_group_id==%s", self::$defaultTrackGroup->getId())], + ["presentations_track_group_id" => ['==']] + ); + + $page = $submitter_repository->getSubmittersBySummit(self::$summit, new PagingInfo(1, 10), $filter, null); + + self::assertNotNull($page); + $ids = array_map(fn($m) => $m->getId(), $page->getItems()); + self::assertContains($member2->getId(), $ids, 'member2 (defaultTrack in defaultTrackGroup) must be included'); + self::assertNotContains($member->getId(), $ids, 'member (altTrack, no group) must be excluded'); + } } diff --git a/tests/oauth2/OAuth2SummitSubmittersApiTest.php b/tests/oauth2/OAuth2SummitSubmittersApiTest.php index eb9c8a5eb..e487e9bf5 100644 --- a/tests/oauth2/OAuth2SummitSubmittersApiTest.php +++ b/tests/oauth2/OAuth2SummitSubmittersApiTest.php @@ -300,4 +300,39 @@ public function testGetCurrentSummitSubmittersActivitiesCountWithAcceptedPresent $this->assertTrue(isset($data->count)); $this->assertGreaterThanOrEqual(0, $data->count); } + + public function testGetSubmittersFilterByTrackGroupId() + { + // Smoke test: filter[]=presentations_track_group_id==N must return HTTP 200, not 422. + // Before Task 1 the controller rejects this field with "Filter by field ... is not allowed." + $params = [ + 'id' => self::$summit->getId(), + 'page' => 1, + 'per_page' => 10, + 'filter' => [ + sprintf('presentations_track_group_id==%s', self::$defaultTrackGroup->getId()), + ], + 'order' => '+id', + ]; + + $headers = [ + "HTTP_Authorization" => " Bearer " . $this->access_token, + "CONTENT_TYPE" => "application/json", + ]; + + $response = $this->action( + "GET", + "OAuth2SummitSubmittersApiController@getAllBySummit", + $params, + [], + [], + [], + $headers + ); + + $content = $response->getContent(); + $this->assertResponseStatus(200); + $submitters = json_decode($content); + $this->assertNotNull($submitters); + } } \ No newline at end of file From 456952ca3bc43a7d91b17b99d179de1fc55d2c22 Mon Sep 17 00:00:00 2001 From: smarcet Date: Mon, 22 Jun 2026 19:06:07 -0300 Subject: [PATCH 09/13] fix: correct @return annotation on IMemberRepository::getSubmittersIdsBySummit --- app/Models/Foundation/Main/Repositories/IMemberRepository.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Models/Foundation/Main/Repositories/IMemberRepository.php b/app/Models/Foundation/Main/Repositories/IMemberRepository.php index 324273c71..16cb06624 100644 --- a/app/Models/Foundation/Main/Repositories/IMemberRepository.php +++ b/app/Models/Foundation/Main/Repositories/IMemberRepository.php @@ -86,7 +86,7 @@ public function getSubmittersBySummit(Summit $summit, PagingInfo $paging_info, F * @param PagingInfo $paging_info * @param Filter|null $filter * @param Order|null $order - * @return PagingResponse + * @return array * @throws \Doctrine\DBAL\Exception */ public function getSubmittersIdsBySummit(Summit $summit, PagingInfo $paging_info, Filter $filter = null, Order $order = null); From 58a4f1a43f31042171d237540ec348a1bcf0e6b4 Mon Sep 17 00:00:00 2001 From: smarcet Date: Mon, 22 Jun 2026 19:29:37 -0300 Subject: [PATCH 10/13] fix: align COALESCE sort precedence in DoctrineSpeakerRepository to speaker entity first --- app/Repositories/Summit/DoctrineSpeakerRepository.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/Repositories/Summit/DoctrineSpeakerRepository.php b/app/Repositories/Summit/DoctrineSpeakerRepository.php index 98c74ec4c..d4ce2734c 100644 --- a/app/Repositories/Summit/DoctrineSpeakerRepository.php +++ b/app/Repositories/Summit/DoctrineSpeakerRepository.php @@ -603,10 +603,10 @@ protected function getOrderMappings() return [ 'id' => 'e.id', "first_name" => << << << Date: Mon, 22 Jun 2026 19:58:52 -0300 Subject: [PATCH 11/13] test: add coverage for critical filter gaps in speaker and submitter repos Adds 11 new regression tests covering: SpeakerRepositoryTest (getUniqueActivitiesCountBySummit): - presentations_track_group_id filter (new in this PR, was zero coverage) - unknown track group returns 0 - presentations_track_id filter - presentations_selection_plan_id filter - has_accepted_presentations==true - combined has_accepted + track_group_id (exercises extraSelectionStatusFilter injection) - combined has_accepted + unknown track_group_id returns 0 (injection not a no-op) SubmitterRepositoryTest (getSubmittersBySummit): - presentations_track_id filter - presentations_selection_plan_id filter - has_accepted_presentations==true - combined has_accepted + presentations_track_id (exercises extraSelectionStatusFilter injection) --- tests/SpeakerRepositoryTest.php | 111 ++++++++++++++++ tests/SubmitterRepositoryTest.php | 211 ++++++++++++++++++++++++++++++ 2 files changed, 322 insertions(+) diff --git a/tests/SpeakerRepositoryTest.php b/tests/SpeakerRepositoryTest.php index 7bc2ed9b9..8723875ad 100644 --- a/tests/SpeakerRepositoryTest.php +++ b/tests/SpeakerRepositoryTest.php @@ -178,4 +178,115 @@ public function testGetUniqueActivitiesCountBySummitZeroForUnknownSpeaker(): voi $count = $this->repo()->getUniqueActivitiesCountBySummit(self::$summit, $filter); $this->assertEquals(0, $count); } + + // ----------------------------------------------------------------- + // getUniqueActivitiesCountBySummit - presentations_track_group_id + // New filter added in this PR; zero coverage before. + // ----------------------------------------------------------------- + + public function testGetUniqueActivitiesCountBySummitFilterByPresentationsTrackGroupId(): void + { + // speaker1 (Sebastian) has 40 presentations in defaultTrack -> defaultTrackGroup + $filter = FilterParser::parse( + ['filter' => 'presentations_track_group_id==' . self::$defaultTrackGroup->getId()], + ['presentations_track_group_id' => ['==']] + ); + $count = $this->repo()->getUniqueActivitiesCountBySummit(self::$summit, $filter); + $this->assertGreaterThan(0, $count); + } + + public function testGetUniqueActivitiesCountBySummitFilterByUnknownTrackGroupIdReturnsZero(): void + { + $filter = FilterParser::parse( + ['filter' => 'presentations_track_group_id==999999'], + ['presentations_track_group_id' => ['==']] + ); + $count = $this->repo()->getUniqueActivitiesCountBySummit(self::$summit, $filter); + $this->assertEquals(0, $count); + } + + // ----------------------------------------------------------------- + // getUniqueActivitiesCountBySummit - presentations_track_id + // ----------------------------------------------------------------- + + public function testGetUniqueActivitiesCountBySummitFilterByPresentationsTrackId(): void + { + $filter = FilterParser::parse( + ['filter' => 'presentations_track_id==' . self::$defaultTrack->getId()], + ['presentations_track_id' => ['==']] + ); + $count = $this->repo()->getUniqueActivitiesCountBySummit(self::$summit, $filter); + $this->assertGreaterThan(0, $count); + } + + // ----------------------------------------------------------------- + // getUniqueActivitiesCountBySummit - presentations_selection_plan_id + // ----------------------------------------------------------------- + + public function testGetUniqueActivitiesCountBySummitFilterBySelectionPlanId(): void + { + // presentations 0-19 are in default_selection_plan; speaker1 is assigned to all of them + $filter = FilterParser::parse( + ['filter' => 'presentations_selection_plan_id==' . self::$default_selection_plan->getId()], + ['presentations_selection_plan_id' => ['==']] + ); + $count = $this->repo()->getUniqueActivitiesCountBySummit(self::$summit, $filter); + $this->assertGreaterThan(0, $count); + } + + // ----------------------------------------------------------------- + // getUniqueActivitiesCountBySummit - has_accepted_presentations + // ----------------------------------------------------------------- + + public function testGetUniqueActivitiesCountBySummitHasAcceptedPresentationsTrue(): void + { + // All seeded presentations are published (accepted); speaker1 satisfies the filter + $filter = FilterParser::parse( + ['filter' => 'has_accepted_presentations==true'], + ['has_accepted_presentations' => ['==']] + ); + $count = $this->repo()->getUniqueActivitiesCountBySummit(self::$summit, $filter); + $this->assertGreaterThan(0, $count); + } + + // ----------------------------------------------------------------- + // getUniqueActivitiesCountBySummit - combined filter + // Exercises the $extraSelectionStatusFilter injection path: + // when presentations_track_group_id is active alongside + // has_accepted_presentations the DQL for has_accepted is rewritten + // to also filter by track group. + // ----------------------------------------------------------------- + + public function testGetUniqueActivitiesCountBySummitHasAcceptedWithTrackGroupIdCombined(): void + { + // All seeded presentations are published (accepted) AND in defaultTrack (defaultTrackGroup), + // so the combined filter matches the same set as the unfiltered query. + $filter = FilterParser::parse( + ['filter' => [ + 'has_accepted_presentations==true', + 'presentations_track_group_id==' . self::$defaultTrackGroup->getId(), + ]], + ['has_accepted_presentations' => ['=='], 'presentations_track_group_id' => ['==']] + ); + + $unfiltered = $this->repo()->getUniqueActivitiesCountBySummit(self::$summit); + $combined = $this->repo()->getUniqueActivitiesCountBySummit(self::$summit, $filter); + + $this->assertEquals($unfiltered, $combined); + } + + public function testGetUniqueActivitiesCountBySummitHasAcceptedWithUnknownTrackGroupReturnsZero(): void + { + // The injected condition restricts has_accepted to group 999999, which has no presentations. + // This verifies the injection actually filters (not a no-op) and produces valid DQL. + $filter = FilterParser::parse( + ['filter' => [ + 'has_accepted_presentations==true', + 'presentations_track_group_id==999999', + ]], + ['has_accepted_presentations' => ['=='], 'presentations_track_group_id' => ['==']] + ); + $count = $this->repo()->getUniqueActivitiesCountBySummit(self::$summit, $filter); + $this->assertEquals(0, $count); + } } diff --git a/tests/SubmitterRepositoryTest.php b/tests/SubmitterRepositoryTest.php index 8f2523893..2caf7e07e 100644 --- a/tests/SubmitterRepositoryTest.php +++ b/tests/SubmitterRepositoryTest.php @@ -244,4 +244,215 @@ public function testGetSubmittersByTrackGroupId(): void self::assertContains($member2->getId(), $ids, 'member2 (defaultTrack in defaultTrackGroup) must be included'); self::assertNotContains($member->getId(), $ids, 'member (altTrack, no group) must be excluded'); } + + // ----------------------------------------------------------------- + // presentations_track_id filter + // ----------------------------------------------------------------- + + public function testGetSubmittersByPresentationsTrackId(): void + { + $member = self::$em->find(Member::class, self::$member->getId()); + $member2 = self::$em->find(Member::class, self::$member2->getId()); + + $altTrack = new PresentationCategory(); + $altTrack->setTitle('Alt Track For Track ID Filter Test'); + $altTrack->setCode('ALTTID'); + $altTrack->setSessionCount(3); + $altTrack->setAlternateCount(3); + $altTrack->setLightningCount(3); + $altTrack->setChairVisible(false); + $altTrack->setVotingVisible(false); + self::$summit->addPresentationCategory($altTrack); + self::$em->persist($altTrack); + + $start = new \DateTime('now', new \DateTimeZone('UTC')); + $end = (clone $start)->add(new \DateInterval('PT2H')); + + $p1 = new Presentation(); + self::$summit->addEvent($p1); + $p1->setTitle('Track ID Filter - Default Track'); + $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); + + $p2 = new Presentation(); + self::$summit->addEvent($p2); + $p2->setTitle('Track ID Filter - Alt Track'); + $p2->setAbstract('Abstract'); + $p2->setCategory($altTrack); + $p2->setType(self::$defaultPresentationType); + $p2->setProgress(Presentation::PHASE_COMPLETE); + $p2->setStatus(Presentation::STATUS_RECEIVED); + $p2->setStartDate($start); + $p2->setEndDate($end); + $p2->setCreatedBy($member); + + self::$em->flush(); + + $repo = EntityManager::getRepository(Member::class); + $filter = FilterParser::parse( + ['filter' => 'presentations_track_id==' . self::$defaultTrack->getId()], + ['presentations_track_id' => ['==']] + ); + $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 (defaultTrack) must be included'); + self::assertNotContains($member->getId(), $ids, 'member (altTrack) must be excluded'); + } + + // ----------------------------------------------------------------- + // presentations_selection_plan_id filter + // ----------------------------------------------------------------- + + public function testGetSubmittersBySelectionPlanId(): 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')); + + // p1: member2 in default_selection_plan + $p1 = new Presentation(); + self::$summit->addEvent($p1); + $p1->setTitle('Selection Plan Filter - In Plan'); + $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); + self::$default_selection_plan->addPresentation($p1); + + // p2: member not in any selection plan + $p2 = new Presentation(); + self::$summit->addEvent($p2); + $p2->setTitle('Selection Plan Filter - No Plan'); + $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); + + self::$em->flush(); + + $repo = EntityManager::getRepository(Member::class); + $filter = FilterParser::parse( + ['filter' => 'presentations_selection_plan_id==' . self::$default_selection_plan->getId()], + ['presentations_selection_plan_id' => ['==']] + ); + $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 (in selection plan) must be included'); + self::assertNotContains($member->getId(), $ids, 'member (no selection plan) must be excluded'); + } + + // ----------------------------------------------------------------- + // has_accepted_presentations filter + // ----------------------------------------------------------------- + + public function testGetSubmittersHasAcceptedPresentationsTrue(): 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 (accepted) presentation + $p1 = new Presentation(); + self::$summit->addEvent($p1); + $p1->setTitle('Accepted 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 (not accepted) + $p2 = new Presentation(); + self::$summit->addEvent($p2); + $p2->setTitle('Accepted 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); + + self::$em->flush(); + + $repo = EntityManager::getRepository(Member::class); + $filter = FilterParser::parse( + ['filter' => 'has_accepted_presentations==true'], + ['has_accepted_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) must be excluded'); + } + + // ----------------------------------------------------------------- + // combined has_accepted_presentations + presentations_track_id + // Exercises the $extraSelectionStatusFilter injection path: when + // presentations_track_id is active alongside has_accepted_presentations + // the accepted-check DQL is rewritten to also filter by track. + // ----------------------------------------------------------------- + + public function testGetSubmittersHasAcceptedWithTrackIdCombined(): 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('Combined Filter - Accepted + Track'); + $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_accepted_presentations==true', + 'presentations_track_id==' . self::$defaultTrack->getId(), + ]], + ['has_accepted_presentations' => ['=='], 'presentations_track_id' => ['==']] + ); + $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 must appear: published presentation in defaultTrack'); + } } From 0d5d4e4375af52b602ae832f1446d1bda9ed7498 Mon Sep 17 00:00:00 2001 From: smarcet Date: Mon, 22 Jun 2026 20:07:33 -0300 Subject: [PATCH 12/13] fix: use flat array for multi-filter FilterParser::parse calls FilterParser::parse expects a flat indexed array of filter strings when multiple filters are combined. Passing them nested inside a 'filter' key caused preg_match to receive an array instead of a string. --- tests/SpeakerRepositoryTest.php | 8 ++++---- tests/SubmitterRepositoryTest.php | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/SpeakerRepositoryTest.php b/tests/SpeakerRepositoryTest.php index 8723875ad..10c327a6a 100644 --- a/tests/SpeakerRepositoryTest.php +++ b/tests/SpeakerRepositoryTest.php @@ -262,10 +262,10 @@ public function testGetUniqueActivitiesCountBySummitHasAcceptedWithTrackGroupIdC // All seeded presentations are published (accepted) AND in defaultTrack (defaultTrackGroup), // so the combined filter matches the same set as the unfiltered query. $filter = FilterParser::parse( - ['filter' => [ + [ 'has_accepted_presentations==true', 'presentations_track_group_id==' . self::$defaultTrackGroup->getId(), - ]], + ], ['has_accepted_presentations' => ['=='], 'presentations_track_group_id' => ['==']] ); @@ -280,10 +280,10 @@ public function testGetUniqueActivitiesCountBySummitHasAcceptedWithUnknownTrackG // The injected condition restricts has_accepted to group 999999, which has no presentations. // This verifies the injection actually filters (not a no-op) and produces valid DQL. $filter = FilterParser::parse( - ['filter' => [ + [ 'has_accepted_presentations==true', 'presentations_track_group_id==999999', - ]], + ], ['has_accepted_presentations' => ['=='], 'presentations_track_group_id' => ['==']] ); $count = $this->repo()->getUniqueActivitiesCountBySummit(self::$summit, $filter); diff --git a/tests/SubmitterRepositoryTest.php b/tests/SubmitterRepositoryTest.php index 2caf7e07e..c776dd583 100644 --- a/tests/SubmitterRepositoryTest.php +++ b/tests/SubmitterRepositoryTest.php @@ -443,10 +443,10 @@ public function testGetSubmittersHasAcceptedWithTrackIdCombined(): void $repo = EntityManager::getRepository(Member::class); $filter = FilterParser::parse( - ['filter' => [ + [ 'has_accepted_presentations==true', 'presentations_track_id==' . self::$defaultTrack->getId(), - ]], + ], ['has_accepted_presentations' => ['=='], 'presentations_track_id' => ['==']] ); $page = $repo->getSubmittersBySummit(self::$summit, new PagingInfo(1, 10), $filter, null); From ee3b3bee486d141d4eba1e189b5db1d09b271073 Mon Sep 17 00:00:00 2001 From: smarcet Date: Mon, 22 Jun 2026 20:20:20 -0300 Subject: [PATCH 13/13] test: cover moderator path, multi-page pagination and track-group filter gaps - SpeakerRepositoryTest: add moderator-only presentation test to exercise the Presentation.ModeratorID INSERT in getUniqueActivitiesCountBySummit Phase 2 (previously untested code path) - SpeakerRepositoryTest: add multi-page pagination test for getAllByPage two-phase refactor (LIMIT/OFFSET for page > 1) - SubmitterRepositoryTest: add getUniqueActivitiesCountBySummit test for presentations_track_group_id filter (submitter path uses __tmp_mbr_ids; speaker path was already covered) - SubmitterRepositoryTest: add multi-page pagination test for getSubmittersBySummit - SubmitterRepositoryTest: add empty-result test covering the early-exit path when getSubmittersIdsByPage returns no IDs All 28 tests pass. --- tests/SpeakerRepositoryTest.php | 74 +++++++++++++++++++ tests/SubmitterRepositoryTest.php | 116 +++++++++++++++++++++++++++++- 2 files changed, 187 insertions(+), 3 deletions(-) diff --git a/tests/SpeakerRepositoryTest.php b/tests/SpeakerRepositoryTest.php index 10c327a6a..997d2d93d 100644 --- a/tests/SpeakerRepositoryTest.php +++ b/tests/SpeakerRepositoryTest.php @@ -1,6 +1,7 @@ repo()->getUniqueActivitiesCountBySummit(self::$summit, $filter); $this->assertEquals(0, $count); } + + // ----------------------------------------------------------------- + // getUniqueActivitiesCountBySummit - moderator role (Phase 2 path) + // Phase 2 has two INSERT statements: one via Presentation_Speakers + // (speaker role) and one via Presentation.ModeratorID (moderator role). + // All fixture speakers are added via addSpeaker(); this test is the only + // one that exercises the moderator INSERT path. + // ----------------------------------------------------------------- + + public function testGetUniqueActivitiesCountBySummitCountsModeratorPresentation(): void + { + $speaker = new PresentationSpeaker(); + $speaker->setFirstName('ModeratorOnlySpeaker'); + $speaker->setLastName('TestMod'); + self::$em->persist($speaker); + + $start = new \DateTime('now', new \DateTimeZone('UTC')); + $end = (clone $start)->add(new \DateInterval('PT2H')); + + $p = new Presentation(); + self::$summit->addEvent($p); + $p->setTitle('Moderator-Only Presentation'); + $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->setModerator($speaker); // moderator role only - NOT in Presentation_Speakers + + self::$em->flush(); + + // Filter to this speaker exclusively to isolate the moderator path. + $filter = FilterParser::parse( + ['filter' => 'first_name==ModeratorOnlySpeaker'], + ['first_name' => ['==']] + ); + $count = $this->repo()->getUniqueActivitiesCountBySummit(self::$summit, $filter); + + // The speaker moderates exactly 1 presentation; Phase 2 must find it via ModeratorID. + $this->assertEquals(1, $count); + } + + // ----------------------------------------------------------------- + // getAllByPage - multi-page pagination + // The two-phase approach uses LIMIT/OFFSET for page > 1. + // Verifies pages are disjoint and cover the full result set. + // ----------------------------------------------------------------- + + public function testGetAllByPagePaginatesCorrectlyAcrossPages(): void + { + $all = $this->repo()->getAllByPage(new PagingInfo(1, 100)); + $total = $all->getTotal(); + + if ($total < 2) { + $this->markTestSkipped('Need at least 2 speakers to test multi-page pagination.'); + } + + $perPage = (int) ceil($total / 2); + + $page1 = $this->repo()->getAllByPage(new PagingInfo(1, $perPage)); + $page2 = $this->repo()->getAllByPage(new PagingInfo(2, $perPage)); + + $ids1 = array_map(fn($s) => $s->getId(), $page1->getItems()); + $ids2 = array_map(fn($s) => $s->getId(), $page2->getItems()); + + $this->assertNotEmpty($ids1, 'Page 1 must not be empty'); + $this->assertNotEmpty($ids2, 'Page 2 must not be empty'); + $this->assertEmpty(array_intersect($ids1, $ids2), 'Pages must be disjoint'); + $this->assertEquals($total, $page1->getTotal(), 'Total must be consistent across pages'); + $this->assertEquals($total, $page2->getTotal()); + } } diff --git a/tests/SubmitterRepositoryTest.php b/tests/SubmitterRepositoryTest.php index c776dd583..ea44a8877 100644 --- a/tests/SubmitterRepositoryTest.php +++ b/tests/SubmitterRepositoryTest.php @@ -115,8 +115,8 @@ public function testGetUniqueActivitiesCountBySummit(){ // can be asserted. The trait fixture leaves created_by null on all presentations, // so without this seeding the method returns 0 for every call and the test is vacuous. // - // P1 + P2 — submitted by member2 (no PresentationSpeaker entity → is_speaker==false) - // P3 — submitted by member, who is also a speaker on that same presentation, + // P1 + P2 - submitted by member2 (no PresentationSpeaker entity -> is_speaker==false) + // P3 - submitted by member, who is also a speaker on that same presentation, // making them an is_speaker==true submitter. // // Re-fetch both members through the current EM. insertSummitTestData() resets the @@ -170,7 +170,7 @@ public function testGetUniqueActivitiesCountBySummit(){ $totalCount = $submitter_repository->getUniqueActivitiesCountBySummit(self::$summit, null); self::assertEquals(3, $totalCount); - // is_speaker==false: P1 and P2 only — member2 is never both creator and speaker + // is_speaker==false: P1 and P2 only - member2 is never both creator and speaker // on the same presentation. P3 is excluded because member is a speaker on their // own submission. $filter = FilterParser::parse( @@ -455,4 +455,114 @@ public function testGetSubmittersHasAcceptedWithTrackIdCombined(): void self::assertContains($member2->getId(), $ids, 'member2 must appear: published presentation in defaultTrack'); } + + // ----------------------------------------------------------------- + // getUniqueActivitiesCountBySummit - presentations_track_group_id + // The submitter repo and speaker repo share the filter name but use + // different DQL paths. The speaker repo has this covered; the + // submitter path (buildSubmitterBaseQuery + __tmp_mbr_ids) does not. + // ----------------------------------------------------------------- + + public function testGetUniqueActivitiesCountBySummitFilterByPresentationsTrackGroupId(): void + { + $member2 = self::$em->find(Member::class, self::$member2->getId()); + + $start = new \DateTime('now', new \DateTimeZone('UTC')); + $end = (clone $start)->add(new \DateInterval('PT2H')); + + // P1: member2 submits in defaultTrack, which belongs to defaultTrackGroup. + $p1 = new Presentation(); + self::$summit->addEvent($p1); + $p1->setTitle('Submitter Count - In Track Group'); + $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); + + self::$em->flush(); + + $repo = EntityManager::getRepository(Member::class); + + $filter = FilterParser::parse( + ['filter' => 'presentations_track_group_id==' . self::$defaultTrackGroup->getId()], + ['presentations_track_group_id' => ['==']] + ); + $count = $repo->getUniqueActivitiesCountBySummit(self::$summit, $filter); + $this->assertGreaterThan(0, $count, 'Must count presentations by submitters in defaultTrackGroup'); + + $filterNone = FilterParser::parse( + ['filter' => 'presentations_track_group_id==999999'], + ['presentations_track_group_id' => ['==']] + ); + $countNone = $repo->getUniqueActivitiesCountBySummit(self::$summit, $filterNone); + $this->assertEquals(0, $countNone, 'Non-existent track group must yield 0'); + } + + // ----------------------------------------------------------------- + // getSubmittersBySummit - multi-page pagination + // The two-phase refactor uses LIMIT/OFFSET for page > 1. + // Verifies page 2 is non-empty, disjoint from page 1, and reports + // the same total as page 1. + // ----------------------------------------------------------------- + + public function testGetSubmittersBySummitPaginatesCorrectlyAcrossPages(): 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')); + + foreach ([$member, $member2] as $creator) { + $p = new Presentation(); + self::$summit->addEvent($p); + $p->setTitle('Pagination Test Submission - ' . $creator->getId()); + $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($creator); + } + self::$em->flush(); + + $repo = EntityManager::getRepository(Member::class); + + $page1 = $repo->getSubmittersBySummit(self::$summit, new PagingInfo(1, 1), null, null); + $page2 = $repo->getSubmittersBySummit(self::$summit, new PagingInfo(2, 1), null, null); + + $ids1 = array_map(fn($m) => $m->getId(), $page1->getItems()); + $ids2 = array_map(fn($m) => $m->getId(), $page2->getItems()); + + $this->assertNotEmpty($ids1, 'Page 1 must not be empty'); + $this->assertNotEmpty($ids2, 'Page 2 must not be empty'); + $this->assertEmpty(array_intersect($ids1, $ids2), 'Page 1 and page 2 must not share submitters'); + $this->assertEquals($page1->getTotal(), $page2->getTotal(), 'Total must be consistent across pages'); + } + + // ----------------------------------------------------------------- + // getSubmittersBySummit - empty result early-exit path + // When no IDs match, the method returns early without a Phase-3 query. + // ----------------------------------------------------------------- + + public function testGetSubmittersBySummitReturnsEmptyPageForNonMatchingFilter(): void + { + $filter = FilterParser::parse( + ['filter' => 'first_name==__NONEXISTENT_9999__'], + ['first_name' => ['==']] + ); + + $repo = EntityManager::getRepository(Member::class); + $page = $repo->getSubmittersBySummit(self::$summit, new PagingInfo(1, 10), $filter, null); + + $this->assertNotNull($page); + $this->assertEquals(0, $page->getTotal()); + $this->assertEmpty($page->getItems()); + } }