diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 71c143955..74a832975 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -79,7 +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: "SpeakerSubmitterPublishedFilter", filter: "tests/ProcessSpeakersEmailRequestJobTest.php tests/ProcessSubmittersEmailRequestJobTest.php tests/SpeakerRepositoryTest.php tests/SubmitterRepositoryTest.php tests/SpeakerServiceOriginalFilterTest.php tests/SubmitterServiceOriginalFilterTest.php tests/ActivitiesCountFilterMappingsTest.php" } - { name: "Repositories", filter: "tests/Repositories/" } - { name: "Services", filter: "tests/Unit/Services/" } - { name: "Integration", filter: "tests/Integration/" } diff --git a/app/Http/Utils/Filters/SQL/SQLRawFilterMapping.php b/app/Http/Utils/Filters/SQL/SQLRawFilterMapping.php new file mode 100644 index 000000000..efeb8c7c2 --- /dev/null +++ b/app/Http/Utils/Filters/SQL/SQLRawFilterMapping.php @@ -0,0 +1,90 @@ +bindings = []; + $param_idx = count($bindings) + 1; + + $value = $filter->getValue(); + $operator = $filter->getOperator(); + + if (!is_array($value)) { + return $this->renderCondition( + $value, + is_array($operator) ? $operator[0] : $operator, + $param_idx + ); + } + + $conditions = []; + foreach ($value as $idx => $v) { + $conditions[] = $this->renderCondition( + $v, + is_array($operator) ? $operator[$idx] : $operator, + $param_idx++ + ); + } + + $same_field_op = $filter->getSameFieldOp() ?? Filter::MainOperatorOr; + + return '( ' . implode(sprintf(' %s ', $same_field_op), $conditions) . ' )'; + } + + /** + * @param mixed $value + * @param string $operator + * @param int $param_idx + * @return string + */ + private function renderCondition($value, string $operator, int $param_idx): string + { + $param = sprintf(Filter::ParamPrefix, $param_idx); + $this->bindings[$param] = $value; + + return str_replace( + [Filter::OperatorPlaceholder, Filter::ValuePlaceholder], + [$operator, ':' . $param], + $this->where + ); + } +} diff --git a/app/Http/Utils/Filters/SQL/SQLSwitchFilterMapping.php b/app/Http/Utils/Filters/SQL/SQLSwitchFilterMapping.php new file mode 100644 index 000000000..46c906fb1 --- /dev/null +++ b/app/Http/Utils/Filters/SQL/SQLSwitchFilterMapping.php @@ -0,0 +1,72 @@ + SQL condition + */ + private $case_statements; + + /** + * @param array $case_statements + */ + public function __construct(array $case_statements = []) + { + parent::__construct('', ''); + $this->case_statements = $case_statements; + } + + /** + * @param FilterElement $filter + * @param array $bindings + * @return string + */ + public function toRawSQL(FilterElement $filter, array $bindings = []): string + { + $this->bindings = []; + + $value = $filter->getValue(); + if (!is_array($value)) $value = [$value]; + + $conditions = []; + foreach ($value as $v) { + if (!isset($this->case_statements[$v])) continue; + $conditions[] = '( ' . $this->case_statements[$v] . ' )'; + } + + if (empty($conditions)) return ''; + + return implode(' OR ', $conditions); + } +} diff --git a/app/Repositories/Summit/DoctrineMemberRepository.php b/app/Repositories/Summit/DoctrineMemberRepository.php index e3fa54246..aa326e0d3 100644 --- a/app/Repositories/Summit/DoctrineMemberRepository.php +++ b/app/Repositories/Summit/DoctrineMemberRepository.php @@ -14,6 +14,7 @@ use App\Http\Utils\Filters\DoctrineInFilterMapping; use App\Http\Utils\Filters\DoctrineNotInFilterMapping; +use App\Repositories\Summit\Traits\ActivitiesCountFilterMappingsTrait; use App\Http\Utils\Filters\SQL\SQLInFilterMapping; use App\Http\Utils\Filters\SQL\SQLNotInFilterMapping; use App\libs\Utils\PunnyCodeHelper; @@ -43,6 +44,8 @@ final class DoctrineMemberRepository extends SilverStripeDoctrineRepository implements IMemberRepository { + use ActivitiesCountFilterMappingsTrait; + /** * @return string */ @@ -773,15 +776,20 @@ public function getUniqueActivitiesCountBySummit(Summit $summit, Filter $filter } while (count($chunk) === $chunkSize); // Phase 2: count distinct presentations whose creator is in the matched set. + // The presentation-level filters of the request scope phase 2 too: we count + // only the presentations that both belong to a matched submitter and satisfy + // the filter, so that "N Submitters | M Activities" describes one same set. + [$extra_filters, $bindings] = $this->buildActivitiesCountFilter($filter, $summit->getId()); + $sql = <<fetchOne($sql, [$summit->getId()]); + return (int) $conn->fetchOne($sql, $bindings); } 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 670a6f179..e716acc76 100644 --- a/app/Repositories/Summit/DoctrineSpeakerRepository.php +++ b/app/Repositories/Summit/DoctrineSpeakerRepository.php @@ -14,6 +14,7 @@ use App\Http\Utils\Filters\DoctrineInFilterMapping; use App\Http\Utils\Filters\DoctrineNotInFilterMapping; +use App\Repositories\Summit\Traits\ActivitiesCountFilterMappingsTrait; use App\libs\Utils\PunnyCodeHelper; use App\Repositories\SilverStripeDoctrineRepository; use Doctrine\ORM\Query\ResultSetMappingBuilder; @@ -41,6 +42,8 @@ final class DoctrineSpeakerRepository extends SilverStripeDoctrineRepository implements ISpeakerRepository { + use ActivitiesCountFilterMappingsTrait; + /** * @return array */ @@ -862,14 +865,20 @@ public function getUniqueActivitiesCountBySummit(Summit $summit, Filter $filter ); $conn->executeStatement('TRUNCATE TABLE `__tmp_pres_ids`'); + // The presentation-level filters of the request scope phase 2 too: we count + // only the presentations that both belong to a matched speaker and satisfy + // the filter, so that "N Speakers | M Activities" describes one same set. + [$extra_filters, $bindings] = $this->buildActivitiesCountFilter($filter, $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 Presentation_Speakers PS ON PS.PresentationID = E.ID INNER JOIN `__tmp_spk_ids` T ON T.id = PS.PresentationSpeakerID - WHERE E.SummitID = ?', - [$summit->getId()] + WHERE E.SummitID = :summit_id' . $extra_filters, + $bindings ); $conn->executeStatement( @@ -878,8 +887,8 @@ public function getUniqueActivitiesCountBySummit(Summit $summit, Filter $filter 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()] + WHERE E.SummitID = :summit_id' . $extra_filters, + $bindings ); return (int) $conn->fetchOne('SELECT COUNT(*) FROM `__tmp_pres_ids`'); diff --git a/app/Repositories/Summit/Traits/ActivitiesCountFilterMappingsTrait.php b/app/Repositories/Summit/Traits/ActivitiesCountFilterMappingsTrait.php new file mode 100644 index 000000000..9902cae0a --- /dev/null +++ b/app/Repositories/Summit/Traits/ActivitiesCountFilterMappingsTrait.php @@ -0,0 +1,219 @@ + $summit_id]; + + if (!is_null($filter)) { + $where = $filter->toRawSQL($this->getActivitiesCountFilterMappings()); + if (!empty($where)) { + $extra_filters = ' AND (' . $where . ')'; + $bindings = array_merge($bindings, $filter->getSQLBindings()); + } + } + + return [$extra_filters, $bindings]; + } + + /** + * Aliases the conditions correlate to: SummitEvent E, Presentation P. + * + * The selection-status semantics are copied from the phase-1 DQL mappings, not + * re-derived. PresentationMediaUpload is a JOINED subclass of PresentationMaterial, + * so its PresentationID column lives on the parent table and the type on the child. + * + * @return array + */ + private function getActivitiesCountFilterMappings(): array + { + return [ + 'presentations_track_id' => new SQLRawFilterMapping( + 'E.CategoryID :operator :value' + ), + 'presentations_track_group_id' => new SQLRawFilterMapping( + 'EXISTS ( + SELECT 1 + FROM PresentationCategoryGroup_Categories __cg + WHERE __cg.PresentationCategoryID = E.CategoryID + AND __cg.PresentationCategoryGroupID :operator :value + )' + ), + 'presentations_selection_plan_id' => new SQLRawFilterMapping( + 'P.SelectionPlanID :operator :value' + ), + 'presentations_type_id' => new SQLRawFilterMapping( + 'E.TypeID :operator :value' + ), + 'presentations_title' => new SQLRawFilterMapping( + 'LOWER(E.Title) :operator LOWER(:value)' + ), + 'presentations_abstract' => new SQLRawFilterMapping( + 'LOWER(E.Abstract) :operator LOWER(:value)' + ), + 'presentations_submitter_full_name' => new SQLRawFilterMapping( + "EXISTS ( + SELECT 1 + FROM `Member` __sub + WHERE __sub.ID = E.CreatedByID + AND CONCAT(LOWER(__sub.FirstName), ' ', LOWER(__sub.Surname)) :operator LOWER(:value) + )" + ), + 'presentations_submitter_email' => new SQLRawFilterMapping( + 'EXISTS ( + SELECT 1 + FROM `Member` __sub + WHERE __sub.ID = E.CreatedByID + AND LOWER(__sub.Email) :operator LOWER(:value) + )' + ), + 'has_media_upload_with_type' => new SQLRawFilterMapping( + 'EXISTS ( + SELECT 1 + FROM PresentationMediaUpload __mu + INNER JOIN PresentationMaterial __mat ON __mat.ID = __mu.ID + WHERE __mat.PresentationID = E.ID + AND __mu.SummitMediaUploadTypeID :operator :value + )' + ), + 'has_not_media_upload_with_type' => new SQLRawFilterMapping( + 'NOT EXISTS ( + SELECT 1 + FROM PresentationMediaUpload __mu + INNER JOIN PresentationMaterial __mat ON __mat.ID = __mu.ID + WHERE __mat.PresentationID = E.ID + AND __mu.SummitMediaUploadTypeID :operator :value + )' + ), + // The == false side of every status filter must not restrict the count: a + // person matched by it has no presentation with that status among the ones + // that pass the remaining presentation-level filters, so all of them qualify. + 'has_published_presentations' => new SQLSwitchFilterMapping([ + 'true' => 'E.Published = 1', + 'false' => SQLSwitchFilterMapping::NoRestriction, + ]), + 'has_accepted_presentations' => new SQLSwitchFilterMapping([ + // accepted = selected within the track session count, or published + 'true' => sprintf( + 'EXISTS ( + SELECT 1 + FROM SummitSelectedPresentation __sp + INNER JOIN SummitSelectedPresentationList __spl ON __spl.ID = __sp.SummitSelectedPresentationListID + INNER JOIN PresentationCategory __cat ON __cat.ID = E.CategoryID + WHERE __sp.PresentationID = E.ID + AND __sp.Collection = \'%1$s\' + AND __spl.ListType = \'%2$s\' + AND __spl.ListClass = \'%3$s\' + AND __sp.`Order` IS NOT NULL + AND __sp.`Order` <= __cat.SessionCount + ) OR E.Published = 1', + SummitSelectedPresentation::CollectionSelected, + SummitSelectedPresentationList::Group, + SummitSelectedPresentationList::Session + ), + 'false' => SQLSwitchFilterMapping::NoRestriction, + ]), + 'has_alternate_presentations' => new SQLSwitchFilterMapping([ + // alternate = selected beyond the track session count + 'true' => sprintf( + 'EXISTS ( + SELECT 1 + FROM SummitSelectedPresentation __sp + INNER JOIN SummitSelectedPresentationList __spl ON __spl.ID = __sp.SummitSelectedPresentationListID + INNER JOIN PresentationCategory __cat ON __cat.ID = E.CategoryID + WHERE __sp.PresentationID = E.ID + AND __sp.Collection = \'%1$s\' + AND __spl.ListType = \'%2$s\' + AND __spl.ListClass = \'%3$s\' + AND __sp.`Order` IS NOT NULL + AND __sp.`Order` > __cat.SessionCount + )', + SummitSelectedPresentation::CollectionSelected, + SummitSelectedPresentationList::Group, + SummitSelectedPresentationList::Session + ), + 'false' => SQLSwitchFilterMapping::NoRestriction, + ]), + 'has_rejected_presentations' => new SQLSwitchFilterMapping([ + // rejected = not published and absent from every Group/Session list, + // the order playing no part, as in the phase-1 mapping + 'true' => sprintf( + 'E.Published = 0 AND NOT EXISTS ( + SELECT 1 + FROM SummitSelectedPresentation __sp + INNER JOIN SummitSelectedPresentationList __spl ON __spl.ID = __sp.SummitSelectedPresentationListID + WHERE __sp.PresentationID = E.ID + AND __sp.Collection = \'%1$s\' + AND __spl.ListType = \'%2$s\' + AND __spl.ListClass = \'%3$s\' + )', + SummitSelectedPresentation::CollectionSelected, + SummitSelectedPresentationList::Group, + SummitSelectedPresentationList::Session + ), + 'false' => SQLSwitchFilterMapping::NoRestriction, + ]), + ]; + } +} diff --git a/tests/ActivitiesCountFilterMappingsTest.php b/tests/ActivitiesCountFilterMappingsTest.php new file mode 100644 index 000000000..35e83decd --- /dev/null +++ b/tests/ActivitiesCountFilterMappingsTest.php @@ -0,0 +1,435 @@ +toRawSQL(FilterElement::makeEqual('presentations_track_id', '5')); + + $this->assertEquals('E.CategoryID = :param_1', $sql); + $this->assertEquals(['param_1' => '5'], $mapping->getBindings()); + // the value must never be interpolated into the statement + $this->assertStringNotContainsString('= 5', $sql); + } + + public function testRawMappingContinuesTheCallersParameterNumbering(): void + { + $mapping = new SQLRawFilterMapping('E.TypeID :operator :value'); + + $sql = $mapping->toRawSQL( + FilterElement::makeEqual('presentations_type_id', '7'), + ['param_1' => 'already taken', 'param_2' => 'also taken'] + ); + + $this->assertEquals('E.TypeID = :param_3', $sql); + $this->assertEquals(['param_3' => '7'], $mapping->getBindings()); + } + + public function testRawMappingRendersEachValueOfAMultiValueElement(): void + { + $mapping = new SQLRawFilterMapping('E.CategoryID :operator :value'); + + $sql = $mapping->toRawSQL( + FilterElement::makeEqual('presentations_track_id', ['5', '6'], 'OR') + ); + + $this->assertEquals('( E.CategoryID = :param_1 OR E.CategoryID = :param_2 )', $sql); + $this->assertEquals(['param_1' => '5', 'param_2' => '6'], $mapping->getBindings()); + } + + public function testRawMappingHonoursTheAndSameFieldOperator(): void + { + $mapping = new SQLRawFilterMapping('X :operator :value'); + + $sql = $mapping->toRawSQL( + FilterElement::makeEqual('has_media_upload_with_type', ['1', '2'], 'AND') + ); + + $this->assertStringContainsString(' AND ', $sql); + $this->assertStringNotContainsString(' OR ', $sql); + } + + public function testRawMappingResetsItsBindingsBetweenUses(): void + { + $mapping = new SQLRawFilterMapping('E.CategoryID :operator :value'); + + $mapping->toRawSQL(FilterElement::makeEqual('presentations_track_id', '5')); + $mapping->toRawSQL(FilterElement::makeEqual('presentations_track_id', '9'), ['param_1' => '5']); + + $this->assertEquals(['param_2' => '9'], $mapping->getBindings()); + } + + public function testRawMappingLowersBothSidesOfATextCondition(): void + { + $mapping = new SQLRawFilterMapping('LOWER(E.Title) :operator LOWER(:value)'); + + $sql = $mapping->toRawSQL(FilterElement::makeLike('presentations_title', 'keynote')); + + $this->assertEquals('LOWER(E.Title) like LOWER(:param_1)', $sql); + // makeLike wraps the value in wildcards + $this->assertEquals(['param_1' => '%keynote%'], $mapping->getBindings()); + } + + // ----------------------------------------------------------------- + // SQLSwitchFilterMapping + // ----------------------------------------------------------------- + + public function testSwitchMappingPicksTheConditionOfTheValue(): void + { + $mapping = new SQLSwitchFilterMapping([ + 'true' => 'E.Published = 1', + 'false' => SQLSwitchFilterMapping::NoRestriction, + ]); + + $this->assertEquals( + '( E.Published = 1 )', + $mapping->toRawSQL(FilterElement::makeEqual('has_published_presentations', 'true')) + ); + $this->assertEquals( + '( 1 = 1 )', + $mapping->toRawSQL(FilterElement::makeEqual('has_published_presentations', 'false')) + ); + $this->assertEmpty($mapping->getBindings()); + } + + public function testSwitchMappingOrsEveryValueOfAMultiValueElement(): void + { + $mapping = new SQLSwitchFilterMapping([ + 'true' => 'E.Published = 1', + 'false' => SQLSwitchFilterMapping::NoRestriction, + ]); + + // true OR no-restriction evaluates to no restriction, as in the Doctrine mapping + $sql = $mapping->toRawSQL( + FilterElement::makeEqual('has_published_presentations', ['true', 'false'], 'OR') + ); + + $this->assertEquals('( E.Published = 1 ) OR ( 1 = 1 )', $sql); + } + + public function testSwitchMappingYieldsNothingForAnUnknownValue(): void + { + $mapping = new SQLSwitchFilterMapping(['true' => 'E.Published = 1']); + + $this->assertEquals( + '', + $mapping->toRawSQL(FilterElement::makeEqual('has_published_presentations', 'maybe')) + ); + } + + // ----------------------------------------------------------------- + // Filter::toRawSQL over the activities count mappings + // ----------------------------------------------------------------- + + private function activitiesCountSQL(Filter $filter, string $repository_class = DoctrineSpeakerRepository::class): array + { + $sql = $filter->toRawSQL($this->getMappings($repository_class, 'getActivitiesCountFilterMappings')); + + return [$sql, $filter->getSQLBindings()]; + } + + private function filterOf(...$conditions): Filter + { + $filter = new Filter(); + foreach ($conditions as $condition) { + $filter->addFilterCondition($condition); + } + return $filter; + } + + public function testAPersonLevelFilterYieldsNoPresentationCondition(): void + { + [$sql, $bindings] = $this->activitiesCountSQL( + $this->filterOf(FilterElement::makeEqual('id', '123')) + ); + + $this->assertEmpty($sql, 'id has no phase-2 mapping, so it must not restrict the count'); + $this->assertEmpty($bindings); + } + + public function testCombinedFiltersAreAnded(): void + { + [$sql, $bindings] = $this->activitiesCountSQL( + $this->filterOf( + FilterElement::makeEqual('has_published_presentations', 'true'), + FilterElement::makeEqual('presentations_track_id', '5') + ) + ); + + $this->assertStringContainsString('E.Published = 1', $sql); + $this->assertStringContainsString('E.CategoryID = :param_1', $sql); + $this->assertStringContainsString(') AND (', $sql); + $this->assertEquals(['param_1' => '5'], $bindings); + } + + public function testAPersonLevelFilterDoesNotStopTheOthersFromScoping(): void + { + [$sql] = $this->activitiesCountSQL( + $this->filterOf( + FilterElement::makeEqual('id', '123'), + FilterElement::makeEqual('presentations_track_id', '5') + ) + ); + + $this->assertStringContainsString('E.CategoryID = :param_1', $sql); + } + + public function testEveryReturnedBindingHasItsPlaceholderInTheStatement(): void + { + [$sql, $bindings] = $this->activitiesCountSQL( + $this->filterOf( + FilterElement::makeEqual( + 'presentations_track_id', + ['1', '2', '3', '4', '5', '6', '7', '8', '9', '10', '11'], + 'OR' + ), + FilterElement::makeEqual('presentations_type_id', '7') + ) + ); + + $this->assertCount(12, $bindings); + foreach (array_keys($bindings) as $name) { + $this->assertMatchesRegularExpression( + '/:' . preg_quote($name, '/') . '\b/', + $sql, + sprintf('binding "%s" has no placeholder in the statement', $name) + ); + } + } + + public function testSelectionStatusConditionsUseTheSelectedListSemantics(): void + { + [$accepted] = $this->activitiesCountSQL( + $this->filterOf(FilterElement::makeEqual('has_accepted_presentations', 'true')) + ); + $this->assertStringContainsString('__sp.`Order` <= __cat.SessionCount', $accepted); + $this->assertStringContainsString("__sp.Collection = 'selected'", $accepted); + $this->assertStringContainsString("__spl.ListType = 'Group'", $accepted); + $this->assertStringContainsString("__spl.ListClass = 'Session'", $accepted); + $this->assertStringContainsString('OR E.Published = 1', $accepted); + + [$alternate] = $this->activitiesCountSQL( + $this->filterOf(FilterElement::makeEqual('has_alternate_presentations', 'true')) + ); + $this->assertStringContainsString('__sp.`Order` > __cat.SessionCount', $alternate); + $this->assertStringNotContainsString('Published', $alternate); + + [$rejected] = $this->activitiesCountSQL( + $this->filterOf(FilterElement::makeEqual('has_rejected_presentations', 'true')) + ); + $this->assertStringContainsString('E.Published = 0', $rejected); + $this->assertStringContainsString('AND NOT EXISTS', $rejected); + // the rejected mapping ignores the order, it only asks for absence from the list + $this->assertStringNotContainsString('SessionCount', $rejected); + } + + public function testMediaUploadConditionJoinsTheParentMaterialTable(): void + { + [$sql] = $this->activitiesCountSQL( + $this->filterOf(FilterElement::makeEqual('has_media_upload_with_type', '11')) + ); + + // PresentationMediaUpload is a JOINED subclass: PresentationID lives on the parent + $this->assertStringContainsString('INNER JOIN PresentationMaterial __mat ON __mat.ID = __mu.ID', $sql); + $this->assertStringContainsString('__mat.PresentationID = E.ID', $sql); + $this->assertStringContainsString('__mu.SummitMediaUploadTypeID = :param_1', $sql); + } + + // ----------------------------------------------------------------- + // buildActivitiesCountFilter - the entry point both repositories share + // ----------------------------------------------------------------- + + private function buildFor(?Filter $filter, string $repository_class = DoctrineSpeakerRepository::class): array + { + return $this->invokeOnRepository($repository_class, 'buildActivitiesCountFilter', $filter, 73); + } + + #[DataProvider('repositoryProvider')] + public function testBuildWithoutAFilterOnlyBindsTheSummit(string $repository_class): void + { + [$extra_filters, $bindings] = $this->buildFor(null, $repository_class); + + $this->assertEmpty($extra_filters); + $this->assertEquals(['summit_id' => 73], $bindings); + } + + #[DataProvider('repositoryProvider')] + public function testBuildWithOnlyPersonLevelFiltersAddsNothing(string $repository_class): void + { + [$extra_filters, $bindings] = $this->buildFor( + $this->filterOf( + FilterElement::makeEqual('id', '123'), + FilterElement::makeEqual('first_name', 'Sebastian') + ), + $repository_class + ); + + $this->assertEmpty($extra_filters); + $this->assertEquals(['summit_id' => 73], $bindings); + } + + #[DataProvider('repositoryProvider')] + public function testBuildAppendsTheFragmentAndKeepsTheSummitBinding(string $repository_class): void + { + [$extra_filters, $bindings] = $this->buildFor( + $this->filterOf( + FilterElement::makeEqual('presentations_track_id', '5'), + FilterElement::makeEqual('has_published_presentations', 'true') + ), + $repository_class + ); + + $this->assertStringStartsWith(' AND (', $extra_filters); + $this->assertStringContainsString('E.CategoryID = :param_1', $extra_filters); + $this->assertStringContainsString('E.Published = 1', $extra_filters); + $this->assertEquals(['summit_id' => 73, 'param_1' => '5'], $bindings); + } + + #[DataProvider('repositoryProvider')] + public function testBuildProducesTheSameFragmentForBothRoles(string $repository_class): void + { + // the conditions correlate to the presentation, so both repositories must agree + [$speakers] = $this->buildFor( + $this->filterOf(FilterElement::makeEqual('presentations_type_id', '7')), + DoctrineSpeakerRepository::class + ); + [$submitters] = $this->buildFor( + $this->filterOf(FilterElement::makeEqual('presentations_type_id', '7')), + DoctrineMemberRepository::class + ); + + $this->assertEquals($speakers, $submitters); + } + + // ----------------------------------------------------------------- + // anti drift guard: phase 1 and phase 2 must know the same filters + // ----------------------------------------------------------------- + + public static function repositoryProvider(): array + { + return [ + 'speakers' => [DoctrineSpeakerRepository::class], + 'submitters' => [DoctrineMemberRepository::class], + ]; + } + + #[DataProvider('repositoryProvider')] + public function testPhaseTwoKnowsEveryPresentationLevelFilterOfPhaseOne(string $repository_class): void + { + $phase_one = array_keys($this->getMappings($repository_class, 'getFilterMappings')); + $phase_two = array_keys($this->getMappings($repository_class, 'getActivitiesCountFilterMappings')); + + $this->assertNotEmpty($phase_one); + + foreach ($phase_one as $key) { + if (!$this->looksPresentationLevel($key)) continue; + + $this->assertContains( + $key, + $phase_two, + sprintf( + '%s exposes the presentation-level filter "%s" in phase 1 but the ' . + 'activities count does not scope phase 2 with it, so the count would over-count.', + $repository_class, + $key + ) + ); + } + } + + #[DataProvider('repositoryProvider')] + public function testEveryPhaseTwoFilterExistsInPhaseOne(string $repository_class): void + { + $phase_one = array_keys($this->getMappings($repository_class, 'getFilterMappings')); + $phase_two = array_keys($this->getMappings($repository_class, 'getActivitiesCountFilterMappings')); + + foreach ($phase_two as $key) { + $this->assertContains( + $key, + $phase_one, + sprintf('%s has no phase-1 mapping for "%s".', $repository_class, $key) + ); + } + } + + #[DataProvider('repositoryProvider')] + public function testPhaseTwoCarriesNoPersonLevelFilter(string $repository_class): void + { + $phase_two = array_keys($this->getMappings($repository_class, 'getActivitiesCountFilterMappings')); + + foreach ($phase_two as $key) { + $this->assertTrue( + $this->looksPresentationLevel($key), + sprintf( + '"%s" is not a presentation-level filter: scoping phase 2 with it would ' . + 'count fewer presentations than the people phase 1 matched.', + $key + ) + ); + } + } + + /** + * A filter name that phase 1 resolves through a presentation, and that therefore has + * to scope phase 2 as well. + */ + private function looksPresentationLevel(string $key): bool + { + if (str_starts_with($key, 'presentations_')) return true; + if (str_starts_with($key, 'has_') && str_ends_with($key, '_presentations')) return true; + if (str_contains($key, 'media_upload')) return true; + return false; + } + + /** + * The trait's methods touch no $this beyond each other, so they can be called on an + * instance built without its constructor -- no entity manager needed. + */ + private function invokeOnRepository(string $repository_class, string $method, ...$args): array + { + $reflection = new \ReflectionClass($repository_class); + $instance = $reflection->newInstanceWithoutConstructor(); + $m = $reflection->getMethod($method); + $m->setAccessible(true); + + return $m->invoke($instance, ...$args); + } + + private function getMappings(string $repository_class, string $method): array + { + return $this->invokeOnRepository($repository_class, $method, null); + } +} diff --git a/tests/SpeakerRepositoryTest.php b/tests/SpeakerRepositoryTest.php index 1f9cd3133..0c4df87fa 100644 --- a/tests/SpeakerRepositoryTest.php +++ b/tests/SpeakerRepositoryTest.php @@ -2,6 +2,7 @@ use LaravelDoctrine\ORM\Facades\EntityManager; use models\summit\Presentation; +use models\summit\PresentationMediaUpload; use models\summit\PresentationSpeaker; use utils\FilterParser; use utils\Order; @@ -689,4 +690,272 @@ public function testGetAllByPagePaginatesCorrectlyAcrossPages(): void $this->assertEquals($total, $page1->getTotal(), 'Total must be consistent across pages'); $this->assertEquals($total, $page2->getTotal()); } + // ----------------------------------------------------------------- + // getUniqueActivitiesCountBySummit - the count is scoped by the + // presentation-level filters, not only by who matches them. + // + // Phase 1 resolves WHICH speakers match; phase 2 used to count every + // presentation of those speakers, so any presentation-level filter + // over-counted (a speaker with 3 presentations returned 3 for all of them). + // ----------------------------------------------------------------- + + /** + * Seeds the acceptance scenario: one speaker owning three presentations that + * differ in track, type, published state and media upload. + * + * P1 - defaultTrack, defaultPresentationType, published, media upload of type M + * P2 - secondaryTrack, defaultPresentationType, published, no media upload + * P3 - defaultTrack, allow2VotePresentationType, unpublished, no media upload + */ + private function seedActivitiesCountScenario(string $first_name): PresentationSpeaker + { + $speaker = new PresentationSpeaker(); + $speaker->setFirstName($first_name); + $speaker->setLastName('ActivitiesScenario'); + self::$em->persist($speaker); + + $p1 = $this->seedPresentation($speaker, self::$defaultTrack, 'P1 Published Default Track', true); + $media_upload = new PresentationMediaUpload(); + $media_upload->setName('P1 Media Upload'); + $media_upload->setDescription('P1 Media Upload Description'); + $media_upload->setFilename('p1.pdf'); + $media_upload->setMediaUploadType(self::$media_uploads_types[0]); + $p1->addMediaUpload($media_upload); + + $this->seedPresentation($speaker, self::$secondaryTrack, 'P2 Published Secondary Track', true); + + // allow2VotePresentationType does not allow a publishing period, so this one + // must not get start/end dates or SummitEvent rejects it. + $p3 = new Presentation(); + self::$summit->addEvent($p3); + $p3->setTitle('P3 Unpublished Default Track Other Type'); + $p3->setAbstract('Abstract'); + $p3->setCategory(self::$defaultTrack); + $p3->setType(self::$allow2VotePresentationType); + $p3->setProgress(Presentation::PHASE_COMPLETE); + $p3->setStatus(Presentation::STATUS_RECEIVED); + $p3->addSpeaker($speaker); + + self::$em->flush(); + + return $speaker; + } + + /** + * Counts the activities of one speaker under the given presentation-level conditions. + */ + private function countActivitiesOf(PresentationSpeaker $speaker, array $conditions = []): int + { + $expressions = ['id==' . $speaker->getId()]; + $rules = ['id' => ['==']]; + + foreach ($conditions as $field => $value) { + $expressions[] = $field . '==' . $value; + $rules[$field] = ['==']; + } + + return $this->repo()->getUniqueActivitiesCountBySummit( + self::$summit, + FilterParser::parse($expressions, $rules) + ); + } + + public function testActivitiesCountWithoutPresentationFilterCountsEveryPresentationOfTheSpeaker(): void + { + $speaker = $this->seedActivitiesCountScenario('ScenarioAll'); + + $this->assertEquals(3, $this->countActivitiesOf($speaker)); + } + + public function testActivitiesCountIsScopedByTrackFilter(): void + { + $speaker = $this->seedActivitiesCountScenario('ScenarioTrack'); + + // P1 and P3 are in defaultTrack, P2 is in secondaryTrack. + $this->assertEquals(2, $this->countActivitiesOf($speaker, [ + 'presentations_track_id' => self::$defaultTrack->getId(), + ])); + $this->assertEquals(1, $this->countActivitiesOf($speaker, [ + 'presentations_track_id' => self::$secondaryTrack->getId(), + ])); + } + + public function testActivitiesCountIsScopedByTrackGroupFilter(): void + { + $speaker = $this->seedActivitiesCountScenario('ScenarioTrackGroup'); + + // defaultTrackGroup contains defaultTrack only: P1 and P3. + $this->assertEquals(2, $this->countActivitiesOf($speaker, [ + 'presentations_track_group_id' => self::$defaultTrackGroup->getId(), + ])); + } + + public function testActivitiesCountIsScopedByTypeFilter(): void + { + $speaker = $this->seedActivitiesCountScenario('ScenarioType'); + + // only P3 uses allow2VotePresentationType + $this->assertEquals(1, $this->countActivitiesOf($speaker, [ + 'presentations_type_id' => self::$allow2VotePresentationType->getId(), + ])); + $this->assertEquals(2, $this->countActivitiesOf($speaker, [ + 'presentations_type_id' => self::$defaultPresentationType->getId(), + ])); + } + + public function testActivitiesCountIsScopedByPublishedFilter(): void + { + $speaker = $this->seedActivitiesCountScenario('ScenarioPublished'); + + // P1 and P2 are published, P3 is not. + $this->assertEquals(2, $this->countActivitiesOf($speaker, [ + 'has_published_presentations' => 'true', + ])); + } + + public function testActivitiesCountIsScopedByMediaUploadFilter(): void + { + $speaker = $this->seedActivitiesCountScenario('ScenarioMediaUpload'); + + // only P1 carries a media upload of that type + $this->assertEquals(1, $this->countActivitiesOf($speaker, [ + 'has_media_upload_with_type' => self::$media_uploads_types[0]->getId(), + ])); + } + + public function testActivitiesCountForNotMediaUploadFilterCountsEveryPresentationOfTheMatchedSpeaker(): void + { + // has_not_media_upload_with_type matches a speaker only when NONE of their + // presentations carries a media upload of that type, so every presentation + // phase 2 reaches already qualifies. + $speaker = new PresentationSpeaker(); + $speaker->setFirstName('ScenarioNoMediaUpload'); + $speaker->setLastName('ActivitiesScenario'); + self::$em->persist($speaker); + + $this->seedPresentation($speaker, self::$defaultTrack, 'No Media A', true); + $this->seedPresentation($speaker, self::$secondaryTrack, 'No Media B', true); + self::$em->flush(); + + $this->assertEquals(2, $this->countActivitiesOf($speaker, [ + 'has_not_media_upload_with_type' => self::$media_uploads_types[0]->getId(), + ])); + + // the scenario speaker owns P1 with that media upload, so they do not match at all + $with_media = $this->seedActivitiesCountScenario('ScenarioHasMediaUpload'); + $this->assertEquals(0, $this->countActivitiesOf($with_media, [ + 'has_not_media_upload_with_type' => self::$media_uploads_types[0]->getId(), + ])); + } + + public function testActivitiesCountIsScopedByPublishedAndTrackCombined(): void + { + $speaker = $this->seedActivitiesCountScenario('ScenarioCombined'); + + // only P1 is both published and in defaultTrack + $this->assertEquals(1, $this->countActivitiesOf($speaker, [ + 'has_published_presentations' => 'true', + 'presentations_track_id' => self::$defaultTrack->getId(), + ])); + + // P2 is published but in secondaryTrack + $this->assertEquals(1, $this->countActivitiesOf($speaker, [ + 'has_published_presentations' => 'true', + 'presentations_track_id' => self::$secondaryTrack->getId(), + ])); + + // P3 is in defaultTrack with that type but unpublished + $this->assertEquals(0, $this->countActivitiesOf($speaker, [ + 'has_published_presentations' => 'true', + 'presentations_type_id' => self::$allow2VotePresentationType->getId(), + ])); + } + + public function testActivitiesCountIsScopedByTitleFilter(): void + { + $speaker = $this->seedActivitiesCountScenario('ScenarioTitle'); + + $expressions = [ + 'id==' . $speaker->getId(), + 'presentations_title=@P1 Published', + ]; + $count = $this->repo()->getUniqueActivitiesCountBySummit( + self::$summit, + FilterParser::parse($expressions, ['id' => ['=='], 'presentations_title' => ['=@']]) + ); + + $this->assertEquals(1, $count); + } + + public function testActivitiesCountForPublishedFalseCountsEveryPresentationOfTheMatchedSpeaker(): void + { + // A speaker with no published and no selected presentation at all: the == false + // side adds no presentation predicate, so every presentation of theirs is counted. + $speaker = new PresentationSpeaker(); + $speaker->setFirstName('ScenarioUnpublishedOnly'); + $speaker->setLastName('ActivitiesScenario'); + self::$em->persist($speaker); + + $this->seedPresentation($speaker, self::$defaultTrack, 'Unpublished A', false); + $this->seedPresentation($speaker, self::$secondaryTrack, 'Unpublished B', false); + self::$em->flush(); + + foreach (['has_published_presentations', 'has_accepted_presentations', 'has_alternate_presentations'] as $field) { + $this->assertEquals( + 2, + $this->countActivitiesOf($speaker, [$field => 'false']), + $field . '==false must not restrict which presentations are counted' + ); + } + } + + public function testActivitiesCountForRejectedFalseCountsEveryPresentationOfTheMatchedSpeaker(): void + { + // rejected == false needs a speaker with no rejected presentation: an unpublished + // presentation outside every selected list is rejected, so this one publishes both. + $speaker = new PresentationSpeaker(); + $speaker->setFirstName('ScenarioNoRejected'); + $speaker->setLastName('ActivitiesScenario'); + self::$em->persist($speaker); + + $this->seedPresentation($speaker, self::$defaultTrack, 'Published A', true); + $this->seedPresentation($speaker, self::$secondaryTrack, 'Published B', true); + self::$em->flush(); + + $this->assertEquals(2, $this->countActivitiesOf($speaker, [ + 'has_rejected_presentations' => 'false', + ])); + } + + public function testActivitiesCountWithNoFilterIsUnaffectedByTheScoping(): void + { + $before = $this->repo()->getUniqueActivitiesCountBySummit(self::$summit); + + $this->seedActivitiesCountScenario('ScenarioUnfiltered'); + + $after = $this->repo()->getUniqueActivitiesCountBySummit(self::$summit); + + $this->assertEquals($before + 3, $after); + } + + public function testActivitiesCountWithAnOredPersonLevelFilterKeepsThePresentationBranch(): void + { + $speaker = $this->seedActivitiesCountScenario('ScenarioOrGroup'); + + // "id== OR presentations_track_id==". Phase 1 matches + // the speaker through either branch, but phase 2 sees only the presentation-level + // branch: Filter::toRawSQL skips the fields it has no mapping for, which is the + // same semantics every other toRawSQL caller lives with. The count is therefore + // the secondaryTrack presentations of the matched speakers -- P2 alone -- and not + // all three. Pinned here because it is the one case where phase 2 ends up + // narrower than the set phase 1 matched. + $filter = FilterParser::parse( + ['id==' . $speaker->getId() . ',presentations_track_id==' . self::$secondaryTrack->getId()], + ['id' => ['=='], 'presentations_track_id' => ['==']] + ); + + $count = $this->repo()->getUniqueActivitiesCountBySummit(self::$summit, $filter); + + $this->assertEquals(1, $count); + } } diff --git a/tests/SubmitterRepositoryTest.php b/tests/SubmitterRepositoryTest.php index cc3fbc9b8..dea3ce3e0 100644 --- a/tests/SubmitterRepositoryTest.php +++ b/tests/SubmitterRepositoryTest.php @@ -4,6 +4,7 @@ use models\main\Member; use models\summit\Presentation; use models\summit\PresentationCategory; +use models\summit\PresentationMediaUpload; use models\summit\PresentationSpeaker; use ModelSerializers\SerializerRegistry; use utils\FilterParser; @@ -833,4 +834,264 @@ public function testGetSubmittersBySummitReturnsEmptyPageForNonMatchingFilter(): $this->assertEquals(0, $page->getTotal()); $this->assertEmpty($page->getItems()); } + // ----------------------------------------------------------------- + // getUniqueActivitiesCountBySummit - the count is scoped by the + // presentation-level filters, not only by who matches them. + // + // Phase 1 resolves WHICH submitters match; phase 2 used to count every + // presentation of those submitters, so any presentation-level filter + // over-counted (a submitter with 3 presentations returned 3 for all of them). + // ----------------------------------------------------------------- + + /** + * Seeds the acceptance scenario: one submitter owning three presentations that + * differ in track, type, published state and media upload. InsertSummitTestData + * never sets created_by, so these are the only presentations of this member. + * + * P1 - defaultTrack, defaultPresentationType, published, media upload of type M + * P2 - secondaryTrack, defaultPresentationType, published, no media upload + * P3 - defaultTrack, allow2VotePresentationType, unpublished, no media upload + */ + private function seedActivitiesCountScenario(): Member + { + $submitter = self::$em->find(Member::class, self::$member2->getId()); + + $p1 = $this->seedPresentation($submitter, self::$defaultTrack, 'P1 Published Default Track', true); + $media_upload = new PresentationMediaUpload(); + $media_upload->setName('P1 Media Upload'); + $media_upload->setDescription('P1 Media Upload Description'); + $media_upload->setFilename('p1.pdf'); + $media_upload->setMediaUploadType(self::$media_uploads_types[0]); + $p1->addMediaUpload($media_upload); + + $this->seedPresentation($submitter, self::$secondaryTrack, 'P2 Published Secondary Track', true); + + // allow2VotePresentationType does not allow a publishing period, so this one + // must not get start/end dates or SummitEvent rejects it. + $p3 = new Presentation(); + self::$summit->addEvent($p3); + $p3->setTitle('P3 Unpublished Default Track Other Type'); + $p3->setAbstract('Abstract'); + $p3->setCategory(self::$defaultTrack); + $p3->setType(self::$allow2VotePresentationType); + $p3->setProgress(Presentation::PHASE_COMPLETE); + $p3->setStatus(Presentation::STATUS_RECEIVED); + $p3->setCreatedBy($submitter); + + self::$em->flush(); + + return $submitter; + } + + /** + * Counts the activities of one submitter under the given presentation-level conditions. + */ + private function countActivitiesOf(Member $submitter, array $conditions = []): int + { + $expressions = ['id==' . $submitter->getId()]; + $rules = ['id' => ['==']]; + + foreach ($conditions as $field => $value) { + $expressions[] = $field . '==' . $value; + $rules[$field] = ['==']; + } + + return EntityManager::getRepository(Member::class)->getUniqueActivitiesCountBySummit( + self::$summit, + FilterParser::parse($expressions, $rules) + ); + } + + public function testActivitiesCountWithoutPresentationFilterCountsEveryPresentationOfTheSubmitter(): void + { + $submitter = $this->seedActivitiesCountScenario(); + + $this->assertEquals(3, $this->countActivitiesOf($submitter)); + } + + public function testActivitiesCountIsScopedByTrackFilter(): void + { + $submitter = $this->seedActivitiesCountScenario(); + + // P1 and P3 are in defaultTrack, P2 is in secondaryTrack. + $this->assertEquals(2, $this->countActivitiesOf($submitter, [ + 'presentations_track_id' => self::$defaultTrack->getId(), + ])); + $this->assertEquals(1, $this->countActivitiesOf($submitter, [ + 'presentations_track_id' => self::$secondaryTrack->getId(), + ])); + } + + public function testActivitiesCountIsScopedByTrackGroupFilter(): void + { + $submitter = $this->seedActivitiesCountScenario(); + + // defaultTrackGroup contains defaultTrack only: P1 and P3. + $this->assertEquals(2, $this->countActivitiesOf($submitter, [ + 'presentations_track_group_id' => self::$defaultTrackGroup->getId(), + ])); + } + + public function testActivitiesCountIsScopedByTypeFilter(): void + { + $submitter = $this->seedActivitiesCountScenario(); + + // only P3 uses allow2VotePresentationType + $this->assertEquals(1, $this->countActivitiesOf($submitter, [ + 'presentations_type_id' => self::$allow2VotePresentationType->getId(), + ])); + $this->assertEquals(2, $this->countActivitiesOf($submitter, [ + 'presentations_type_id' => self::$defaultPresentationType->getId(), + ])); + } + + public function testActivitiesCountIsScopedByPublishedFilter(): void + { + $submitter = $this->seedActivitiesCountScenario(); + + // P1 and P2 are published, P3 is not. + $this->assertEquals(2, $this->countActivitiesOf($submitter, [ + 'has_published_presentations' => 'true', + ])); + } + + public function testActivitiesCountIsScopedByMediaUploadFilter(): void + { + $submitter = $this->seedActivitiesCountScenario(); + + // only P1 carries a media upload of that type + $this->assertEquals(1, $this->countActivitiesOf($submitter, [ + 'has_media_upload_with_type' => self::$media_uploads_types[0]->getId(), + ])); + } + + public function testActivitiesCountForNotMediaUploadFilterCountsEveryPresentationOfTheMatchedSubmitter(): void + { + // has_not_media_upload_with_type matches a submitter only when NONE of their + // presentations carries a media upload of that type, so every presentation + // phase 2 reaches already qualifies. + $submitter = self::$em->find(Member::class, self::$member2->getId()); + + $this->seedPresentation($submitter, self::$defaultTrack, 'No Media A', true); + $this->seedPresentation($submitter, self::$secondaryTrack, 'No Media B', true); + self::$em->flush(); + + $this->assertEquals(2, $this->countActivitiesOf($submitter, [ + 'has_not_media_upload_with_type' => self::$media_uploads_types[0]->getId(), + ])); + } + + public function testActivitiesCountIsZeroWhenTheSubmitterOwnsAMediaUploadOfThatType(): void + { + // the scenario submitter owns P1 with that media upload, so they do not match at all + $submitter = $this->seedActivitiesCountScenario(); + + $this->assertEquals(0, $this->countActivitiesOf($submitter, [ + 'has_not_media_upload_with_type' => self::$media_uploads_types[0]->getId(), + ])); + } + + public function testActivitiesCountIsScopedByPublishedAndTrackCombined(): void + { + $submitter = $this->seedActivitiesCountScenario(); + + // only P1 is both published and in defaultTrack + $this->assertEquals(1, $this->countActivitiesOf($submitter, [ + 'has_published_presentations' => 'true', + 'presentations_track_id' => self::$defaultTrack->getId(), + ])); + + // P2 is published but in secondaryTrack + $this->assertEquals(1, $this->countActivitiesOf($submitter, [ + 'has_published_presentations' => 'true', + 'presentations_track_id' => self::$secondaryTrack->getId(), + ])); + + // P3 is in defaultTrack with that type but unpublished + $this->assertEquals(0, $this->countActivitiesOf($submitter, [ + 'has_published_presentations' => 'true', + 'presentations_type_id' => self::$allow2VotePresentationType->getId(), + ])); + } + + public function testActivitiesCountIsScopedByTitleFilter(): void + { + $submitter = $this->seedActivitiesCountScenario(); + + $count = EntityManager::getRepository(Member::class)->getUniqueActivitiesCountBySummit( + self::$summit, + FilterParser::parse( + ['id==' . $submitter->getId(), 'presentations_title=@P1 Published'], + ['id' => ['=='], 'presentations_title' => ['=@']] + ) + ); + + $this->assertEquals(1, $count); + } + + public function testActivitiesCountForPublishedFalseCountsEveryPresentationOfTheMatchedSubmitter(): void + { + // A submitter with no published and no selected presentation at all: the == false + // side adds no presentation predicate, so every presentation of theirs is counted. + $submitter = self::$em->find(Member::class, self::$member2->getId()); + + $this->seedPresentation($submitter, self::$defaultTrack, 'Unpublished A', false); + $this->seedPresentation($submitter, self::$secondaryTrack, 'Unpublished B', false); + self::$em->flush(); + + foreach (['has_published_presentations', 'has_accepted_presentations', 'has_alternate_presentations'] as $field) { + $this->assertEquals( + 2, + $this->countActivitiesOf($submitter, [$field => 'false']), + $field . '==false must not restrict which presentations are counted' + ); + } + } + + public function testActivitiesCountForRejectedFalseCountsEveryPresentationOfTheMatchedSubmitter(): void + { + // rejected == false needs a submitter with no rejected presentation: an unpublished + // presentation outside every selected list is rejected, so this one publishes both. + $submitter = self::$em->find(Member::class, self::$member2->getId()); + + $this->seedPresentation($submitter, self::$defaultTrack, 'Published A', true); + $this->seedPresentation($submitter, self::$secondaryTrack, 'Published B', true); + self::$em->flush(); + + $this->assertEquals(2, $this->countActivitiesOf($submitter, [ + 'has_rejected_presentations' => 'false', + ])); + } + + public function testActivitiesCountWithNoFilterIsUnaffectedByTheScoping(): void + { + $repo = EntityManager::getRepository(Member::class); + $before = $repo->getUniqueActivitiesCountBySummit(self::$summit); + + $this->seedActivitiesCountScenario(); + + $this->assertEquals($before + 3, $repo->getUniqueActivitiesCountBySummit(self::$summit)); + } + + public function testActivitiesCountWithAnOredPersonLevelFilterKeepsThePresentationBranch(): void + { + $submitter = $this->seedActivitiesCountScenario(); + + // "id== OR presentations_track_id==". Phase 1 matches + // the submitter through either branch, but phase 2 sees only the + // presentation-level branch: Filter::toRawSQL skips the fields it has no mapping + // for, which is the same semantics every other toRawSQL caller lives with. The + // count is therefore the secondaryTrack presentations of the matched submitters -- + // P2 alone -- and not all three. Pinned here because it is the one case where + // phase 2 ends up narrower than the set phase 1 matched. + $filter = FilterParser::parse( + ['id==' . $submitter->getId() . ',presentations_track_id==' . self::$secondaryTrack->getId()], + ['id' => ['=='], 'presentations_track_id' => ['==']] + ); + + $count = EntityManager::getRepository(Member::class) + ->getUniqueActivitiesCountBySummit(self::$summit, $filter); + + $this->assertEquals(1, $count); + } } diff --git a/tests/oauth2/OAuth2SummitSpeakersApiTest.php b/tests/oauth2/OAuth2SummitSpeakersApiTest.php index 05e5b5ff8..521660829 100644 --- a/tests/oauth2/OAuth2SummitSpeakersApiTest.php +++ b/tests/oauth2/OAuth2SummitSpeakersApiTest.php @@ -21,6 +21,7 @@ use Illuminate\Support\Facades\Queue; use LaravelDoctrine\ORM\Facades\EntityManager; use models\summit\Presentation; +use models\summit\PresentationMediaUpload; use models\summit\PresentationSpeaker; use utils\FilterParser; use models\summit\SpeakersSummitRegistrationPromoCode; @@ -2907,4 +2908,133 @@ public function testGetCurrentSummitSpeakersActivitiesCountWithPublishedPresenta $this->assertGreaterThan(0, $data->count); } + // ----------------------------------------------------------------- + // GET /api/v1/summits/{id}/speakers/all/events/count + // The count must describe the presentations that satisfy the request + // filter, not every presentation of the matched speakers. + // ----------------------------------------------------------------- + + /** + * P1 - defaultTrack, defaultPresentationType, published, media upload of type M + * P2 - secondaryTrack, defaultPresentationType, published, no media upload + * P3 - defaultTrack, allow2VotePresentationType, unpublished, no media upload + * + * The fixture speaker owns 40 other presentations, so the assertions below filter + * by this speaker's id to get exact counts. + */ + private function seedActivitiesCountScenario(): PresentationSpeaker + { + $speaker = new PresentationSpeaker(); + $speaker->setFirstName('ApiActivitiesScenario'); + $speaker->setLastName('TestSpeaker'); + self::$em->persist($speaker); + + $start = new \DateTime('now', new \DateTimeZone('UTC')); + + $p1 = new Presentation(); + self::$summit->addEvent($p1); + $p1->setTitle('Api Count P1 Published 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((clone $start)->add(new \DateInterval('PT2H'))); + $p1->addSpeaker($speaker); + $p1->publish(); + + $media_upload = new PresentationMediaUpload(); + $media_upload->setName('Api Count P1 Media Upload'); + $media_upload->setDescription('Api Count P1 Media Upload Description'); + $media_upload->setFilename('p1.pdf'); + $media_upload->setMediaUploadType(self::$media_uploads_types[0]); + $p1->addMediaUpload($media_upload); + + $p2 = new Presentation(); + self::$summit->addEvent($p2); + $p2->setTitle('Api Count P2 Published Secondary Track'); + $p2->setAbstract('Abstract'); + $p2->setCategory(self::$secondaryTrack); + $p2->setType(self::$defaultPresentationType); + $p2->setProgress(Presentation::PHASE_COMPLETE); + $p2->setStatus(Presentation::STATUS_RECEIVED); + $p2->setStartDate($start); + $p2->setEndDate((clone $start)->add(new \DateInterval('PT2H'))); + $p2->addSpeaker($speaker); + $p2->publish(); + + // allow2VotePresentationType does not allow a publishing period: no start/end dates. + $p3 = new Presentation(); + self::$summit->addEvent($p3); + $p3->setTitle('Api Count P3 Unpublished Default Track Other Type'); + $p3->setAbstract('Abstract'); + $p3->setCategory(self::$defaultTrack); + $p3->setType(self::$allow2VotePresentationType); + $p3->setProgress(Presentation::PHASE_COMPLETE); + $p3->setStatus(Presentation::STATUS_RECEIVED); + $p3->addSpeaker($speaker); + + self::$em->flush(); + + return $speaker; + } + + private function getActivitiesCount(array $filter): int + { + $headers = [ + "HTTP_Authorization" => " Bearer " . $this->access_token, + "CONTENT_TYPE" => "application/json", + ]; + + $response = $this->action( + "GET", + "OAuth2SummitSpeakersApiController@getSpeakersActivitiesCount", + ['id' => self::$summit->getId(), 'filter' => $filter], + [], [], [], $headers + ); + + $this->assertResponseStatus(200); + $data = json_decode($response->getContent()); + $this->assertNotNull($data); + $this->assertTrue(isset($data->count)); + + return (int) $data->count; + } + + public function testGetSpeakersActivitiesCountIsScopedByThePresentationFilters() + { + $speaker = $this->seedActivitiesCountScenario(); + $id = 'id==' . $speaker->getId(); + + // no presentation-level filter: every presentation of the speaker + $this->assertEquals(3, $this->getActivitiesCount([$id])); + + // P1 and P3 are in defaultTrack + $this->assertEquals(2, $this->getActivitiesCount([ + $id, 'presentations_track_id==' . self::$defaultTrack->getId(), + ])); + + // only P3 uses allow2VotePresentationType + $this->assertEquals(1, $this->getActivitiesCount([ + $id, 'presentations_type_id==' . self::$allow2VotePresentationType->getId(), + ])); + + // P1 and P2 are published + $this->assertEquals(2, $this->getActivitiesCount([ + $id, 'has_published_presentations==true', + ])); + + // only P1 carries a media upload of that type + $this->assertEquals(1, $this->getActivitiesCount([ + $id, 'has_media_upload_with_type==' . self::$media_uploads_types[0]->getId(), + ])); + + // only P1 is both published and in defaultTrack + $this->assertEquals(1, $this->getActivitiesCount([ + $id, + 'has_published_presentations==true', + 'presentations_track_id==' . self::$defaultTrack->getId(), + ])); + } } diff --git a/tests/oauth2/OAuth2SummitSubmittersApiTest.php b/tests/oauth2/OAuth2SummitSubmittersApiTest.php index a7dc6a98e..cd97755fa 100644 --- a/tests/oauth2/OAuth2SummitSubmittersApiTest.php +++ b/tests/oauth2/OAuth2SummitSubmittersApiTest.php @@ -2,6 +2,7 @@ use App\Models\Foundation\Main\IGroup; use models\main\Member; use models\summit\Presentation; +use models\summit\PresentationMediaUpload; /** * Copyright 2023 OpenStack Foundation @@ -444,4 +445,129 @@ public function testGetCurrentSummitSubmittersActivitiesCountWithPublishedPresen $this->assertEquals(1, $data->count, 'exactly one published presentation was seeded; count must be 1'); } -} \ No newline at end of file + // ----------------------------------------------------------------- + // GET /api/v1/summits/{id}/submitters/all/events/count + // The count must describe the presentations that satisfy the request + // filter, not every presentation of the matched submitters. + // ----------------------------------------------------------------- + + /** + * P1 - defaultTrack, defaultPresentationType, published, media upload of type M + * P2 - secondaryTrack, defaultPresentationType, published, no media upload + * P3 - defaultTrack, allow2VotePresentationType, unpublished, no media upload + * + * InsertSummitTestData never sets created_by, so these are the only presentations + * of this submitter in the summit. + */ + private function seedActivitiesCountScenario(): Member + { + $submitter = self::$em->find(Member::class, self::$member2->getId()); + $start = new \DateTime('now', new \DateTimeZone('UTC')); + + $p1 = new Presentation(); + self::$summit->addEvent($p1); + $p1->setTitle('Api Count P1 Published 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((clone $start)->add(new \DateInterval('PT2H'))); + $p1->setCreatedBy($submitter); + $p1->publish(); + + $media_upload = new PresentationMediaUpload(); + $media_upload->setName('Api Count P1 Media Upload'); + $media_upload->setDescription('Api Count P1 Media Upload Description'); + $media_upload->setFilename('p1.pdf'); + $media_upload->setMediaUploadType(self::$media_uploads_types[0]); + $p1->addMediaUpload($media_upload); + + $p2 = new Presentation(); + self::$summit->addEvent($p2); + $p2->setTitle('Api Count P2 Published Secondary Track'); + $p2->setAbstract('Abstract'); + $p2->setCategory(self::$secondaryTrack); + $p2->setType(self::$defaultPresentationType); + $p2->setProgress(Presentation::PHASE_COMPLETE); + $p2->setStatus(Presentation::STATUS_RECEIVED); + $p2->setStartDate($start); + $p2->setEndDate((clone $start)->add(new \DateInterval('PT2H'))); + $p2->setCreatedBy($submitter); + $p2->publish(); + + // allow2VotePresentationType does not allow a publishing period: no start/end dates. + $p3 = new Presentation(); + self::$summit->addEvent($p3); + $p3->setTitle('Api Count P3 Unpublished Default Track Other Type'); + $p3->setAbstract('Abstract'); + $p3->setCategory(self::$defaultTrack); + $p3->setType(self::$allow2VotePresentationType); + $p3->setProgress(Presentation::PHASE_COMPLETE); + $p3->setStatus(Presentation::STATUS_RECEIVED); + $p3->setCreatedBy($submitter); + + self::$em->flush(); + + return $submitter; + } + + private function getActivitiesCount(array $filter): int + { + $headers = [ + "HTTP_Authorization" => " Bearer " . $this->access_token, + "CONTENT_TYPE" => "application/json", + ]; + + $response = $this->action( + "GET", + "OAuth2SummitSubmittersApiController@getSubmittersActivitiesCount", + ['id' => self::$summit->getId(), 'filter' => $filter], + [], [], [], $headers + ); + + $this->assertResponseStatus(200); + $data = json_decode($response->getContent()); + $this->assertNotNull($data); + $this->assertTrue(isset($data->count)); + + return (int) $data->count; + } + + public function testGetSubmittersActivitiesCountIsScopedByThePresentationFilters() + { + $submitter = $this->seedActivitiesCountScenario(); + $id = 'id==' . $submitter->getId(); + + // no presentation-level filter: every presentation of the submitter + $this->assertEquals(3, $this->getActivitiesCount([$id])); + + // P1 and P3 are in defaultTrack + $this->assertEquals(2, $this->getActivitiesCount([ + $id, 'presentations_track_id==' . self::$defaultTrack->getId(), + ])); + + // only P3 uses allow2VotePresentationType + $this->assertEquals(1, $this->getActivitiesCount([ + $id, 'presentations_type_id==' . self::$allow2VotePresentationType->getId(), + ])); + + // P1 and P2 are published + $this->assertEquals(2, $this->getActivitiesCount([ + $id, 'has_published_presentations==true', + ])); + + // only P1 carries a media upload of that type + $this->assertEquals(1, $this->getActivitiesCount([ + $id, 'has_media_upload_with_type==' . self::$media_uploads_types[0]->getId(), + ])); + + // only P1 is both published and in defaultTrack + $this->assertEquals(1, $this->getActivitiesCount([ + $id, + 'has_published_presentations==true', + 'presentations_track_id==' . self::$defaultTrack->getId(), + ])); + } +}