fix(speakers/submitters): scope the activities count by the presentation-level filters

[romanetar]

ref https://app.clickup.com/t/86bbv67m1

Problem

GET /api/v1/summits/{id}/speakers/all/events/count and GET /api/v1/summits/{id}/submitters/all/events/count run in two phases (the temp-table structure from #563):

1. Phase 1 resolves which speakers/submitters match the request filter, in DQL.
2. Phase 2 counts the presentations of those people, in raw SQL over a temp table.

Phase 2 had E.SummitID = ? as its only predicate, so it counted every presentation of a matched person, whether or not the presentation itself satisfied the filter. A speaker with three presentations returned 3 for presentations_track_id, presentations_type_id, has_published_presentations, has_media_upload_with_type and for published+track combined — all five.

On dev summit 73 this reads as "768 Speakers | 707 Activities" for has_published_presentations==true while only 511 activities are published in total. The number on the summit-admin speakers/submitters page therefore did not describe the set of activities the email blast targets. This is what 86b9b1qrk asked for and what #543 / #563 never implemented.

Solution

Phase 2 derives its WHERE from the same Filter object through Filter::toRawSQL, the primitive that already walks the parsed AND/OR structure, dispatches per field mapping, handles multi-value elements (&& / ||) and binds the values as named parameters:

[$extra_filters, $bindings] = $this->buildActivitiesCountFilter($filter, $summit->getId());

Two FilterMapping implementations were missing for raw SQL and are added beside the existing SQLInFilterMapping / SQLNotInFilterMapping in app/Http/Utils/Filters/SQL/:

new class	counterpart of	what it does
SQLRawFilterMapping	DoctrineFilterMapping	renders a condition carrying :operator / :value, binding each value instead of interpolating it, honouring getSameFieldOp() for multi-value elements
SQLSwitchFilterMapping	DoctrineSwitchFilterMapping	picks the condition by filter value, OR-ing the conditions of every value the element carries

ActivitiesCountFilterMappingsTrait declares the fourteen presentation-level conditions, expressed against the physical presentation row rather than against the person, with the semantics copied from the phase-1 DQL (not re-derived), and exposes buildActivitiesCountFilter — the single method above, so neither repository repeats the wiring. Both use it, since the conditions correlate to the presentation and not to the role the person plays on it:

• DoctrineSpeakerRepository::getUniqueActivitiesCountBySummit — in both INSERT ... SELECT statements (speaker role via Presentation_Speakers, moderator role via Presentation.ModeratorID). They stay separate because MySQL error 1137 forbids referencing the same temporary table twice in one statement. The speaker-role statement gains INNER JOIN Presentation P, which it did not have, so P.SelectionPlanID is reachable; the join is non-restrictive (every Presentation_Speakers row points at a Presentation).
• DoctrineMemberRepository::getUniqueActivitiesCountBySummit — in the created_by statement.

The selection-status semantics:

filter	phase-2 condition
has_published_presentations==true	E.Published = 1
has_accepted_presentations==true	selected within PresentationCategory.SessionCount in a Group/Session list, OR E.Published = 1
has_alternate_presentations==true	selected beyond SessionCount in a Group/Session list
has_rejected_presentations==true	E.Published = 0 and absent from every Group/Session selected list (no order comparison, as in the mapping)
has_*_presentations==false	1 = 1

The == false side does not restrict because a person matched by it has no presentation with that status among the presentations that pass the remaining presentation-level filters — which holds only because those are applied here too. It is spelled out as 1 = 1 rather than left absent so that a multi-value element such as ==true||false still evaluates to true, the way the Doctrine switch mapping does.

Person-level filters, and one known limitation

Person-level filters (id, not_id, first_name, last_name, email, full_name, member_id, member_user_external_id, is_speaker) have no phase-2 mapping, so toRawSQL skips them and the count stays unrestricted for them. For a slot joined by AND that is exactly right — a slot that does not restrict contributes nothing to an AND chain.

Inside an OR group that same skip drops a branch instead of widening it: full_name==x,presentations_track_id==N counts only the track N presentations, even though phase 1 also matched people through full_name. Expressing the correct rule needs a per-person predicate, which no set-level condition can carry. This is the semantics every other toRawSQL caller in the codebase already lives with; the trait documents it and testActivitiesCountWithAnOredPersonLevelFilterKeepsThePresentationBranch pins the resulting count in both repository suites.

Filter itself is untouched. An earlier revision of this PR added accessors to it and re-implemented the traversal in a dedicated builder; that was 445 lines re-doing what toRawSQL already does, and it is gone.

Tests

186 tests / 915 assertions, all green. Run inside the container (docker compose exec app) — from the host the redis / db_model hostnames do not resolve.

Suite	Result
tests/ActivitiesCountFilterMappingsTest.php (new)	29 OK
tests/SpeakerRepositoryTest.php	38 OK (25 pre-existing + 13)
tests/SubmitterRepositoryTest.php	31 OK (17 pre-existing + 14)
tests/oauth2/OAuth2SummitSpeakersApiTest.php	76 OK
tests/oauth2/OAuth2SummitSubmittersApiTest.php	12 OK

The acceptance scenario — P1 (track A, type T1, published, media upload M), P2 (track B, T1, published), P3 (track A, T2, unpublished) — returns 2 / 1 / 2 / 1 / 1 at the repository layer and through both endpoints, for speaker and submitter. On main all five return 3.

Also covered: the == false side of every status filter returns the same count as before, the unfiltered count is unchanged, multi-value filters, the title filter, parameter numbering continuing across mappings, and that every returned binding has its placeholder in the statement.

tests/ActivitiesCountFilterMappingsTest.php also guards against drift in three directions: every presentation-level filter of phase 1 has a phase-2 mapping, every phase-2 mapping exists in phase 1, and no phase-2 mapping is person-level. It reads both mapping methods off an instance built without its constructor, so it needs no entity manager.

It is registered in the SpeakerSubmitterPublishedFilter shard in .github/workflows/push.yml — no job runs the tests/ root, so a file added there runs nowhere unless it is listed.

Two expectations from the original plan were wrong and were corrected against the real phase-1 behaviour rather than by changing the code: with has_not_media_upload_with_type==M the scenario speaker does not match at all (P1 carries media M), and with has_rejected_presentations==false they do not either (their unpublished presentations outside every selected list are rejected). Each became its own test with an appropriate subject.

Not verified

The performance criterion is unverified. The ticket asks for under 1 second on dev summit 73 with has_published_presentations==true (the #563 baseline). I have no access to that database, and the local fixtures are far too small for a timing to mean anything. The track / type / published / selection-plan conditions hit columns of SummitEvent and Presentation, both already joined; the media-upload and selected-list conditions add correlated subqueries, and that is where to look. This needs a measurement on dev before merging.

Release note

The activities figure shown in production for the existing selection-status filters goes down, because it was over-counted. The speaker/submitter count itself does not change. The CFP admins should be told.

No summit-admin change is needed: it already consumes the endpoint at src/actions/speaker-actions.js:946 and 1013 and src/actions/submitter-actions.js:65 and 131, so the displayed number corrects itself. The dashboard "Published Activities" figure (Summit::getPublishedEventsCount counts every published SummitEvent, not only presentations with speakers) is still not expected to equal this count.

Out of scope

Which speakers/submitters match a filter (phase 1) is unchanged.

🤖 Generated with Claude Code

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: d63eaf50-3a24-4bd7-a23b-dd45385a4bf5

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 364c5d8e-ec66-426c-ae79-57ffe81b521a

📥 Commits

Reviewing files that changed from the base of the PR and between 591b400 and 9ab18cf.

📒 Files selected for processing (11)

• .github/workflows/push.yml
• app/Http/Utils/Filters/SQL/SQLRawFilterMapping.php
• app/Http/Utils/Filters/SQL/SQLSwitchFilterMapping.php
• app/Repositories/Summit/DoctrineMemberRepository.php
• app/Repositories/Summit/DoctrineSpeakerRepository.php
• app/Repositories/Summit/Traits/ActivitiesCountFilterMappingsTrait.php
• tests/ActivitiesCountFilterMappingsTest.php
• tests/SpeakerRepositoryTest.php
• tests/SubmitterRepositoryTest.php
• tests/oauth2/OAuth2SummitSpeakersApiTest.php
• tests/oauth2/OAuth2SummitSubmittersApiTest.php

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

Activity count queries now apply presentation-level filters for speakers and submitters. New raw SQL mappings provide parameter binding and status handling. Repository, unit, integration, and OAuth2 tests cover filtered counts and combined filter behavior.

Changes

Activity count filtering

Layer / File(s)	Summary
Raw SQL filter mappings app/Http/Utils/Filters/SQL/*	New mappings render parameterized conditions and switch-based SQL for scalar and multi-value filters.
Repository count integration app/Repositories/Summit/Traits/ActivitiesCountFilterMappingsTrait.php, app/Repositories/Summit/DoctrineMemberRepository.php, app/Repositories/Summit/DoctrineSpeakerRepository.php	Shared presentation mappings now filter phase-2 activity counts. Queries use named bindings for summit and presentation parameters.
Mapping validation tests/ActivitiesCountFilterMappingsTest.php, .github/workflows/push.yml	Tests cover SQL rendering, bindings, status semantics, media uploads, mapping consistency, and CI execution.
Repository and API regression coverage tests/SpeakerRepositoryTest.php, tests/SubmitterRepositoryTest.php, tests/oauth2/OAuth2SummitSpeakersApiTest.php, tests/oauth2/OAuth2SummitSubmittersApiTest.php	Tests verify counts for track, type, publication, media upload, title, combined, and OR-ed filters.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant RequestFilter
  participant ActivityCountRepository
  participant PresentationDatabase
  RequestFilter->>ActivityCountRepository: Convert presentation filters to raw SQL
  ActivityCountRepository->>PresentationDatabase: Execute named-bound count query
  PresentationDatabase-->>ActivityCountRepository: Return filtered activity count

Loading

Suggested reviewers: mulldug, smarcet

Merge Risk: ⚪ Minimal · up to 9ab18

No actionable correctness or runtime issue remains. Performance measurement is still advisable but is not evidence of a current defect.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 30.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 72 functions across 10 files. (1 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: scoping speaker and submitter activity counts by presentation-level filters.

Full details: Docstring Coverage

Explanation

Docstring coverage is 30.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 72 functions across 10 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/speaker-submitter-presentations-count-filter

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-599/

This page is automatically updated on each push to this PR.

[github-actions]

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-599/

This page is automatically updated on each push to this PR.

[github-actions]

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-599/

This page is automatically updated on each push to this PR.

[github-actions]

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-599/

This page is automatically updated on each push to this PR.

[smarcet]

@romanetar please review

[Copilot]

🟡 Changes recommended

Filter::toRawSQL currently mishandles OR groups for FilterMapping mappings (overwriting instead of OR-ing), which can under-scope phase-2 counts for filters like presentations_track_id==X,presentations_type_id==Y.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR fixes over-counting in the speaker/submitter “activities count” endpoints by applying the same presentation-level filters to phase 2 (the raw-SQL counting step) as are used in phase 1 (the DQL “who matches” step), so the returned “Activities” number describes the same filtered presentation set targeted by the request.

Changes:

• Introduces raw-SQL filter mappings (SQLRawFilterMapping, SQLSwitchFilterMapping) and a shared ActivitiesCountFilterMappingsTrait to translate request filters into phase-2 SQL predicates + bindings.
• Updates DoctrineSpeakerRepository and DoctrineMemberRepository phase-2 queries to append the derived predicate fragment (and switch to named parameter bindings).
• Adds unit/integration/API test coverage for the corrected scoping behavior and wires the new unit test into the GitHub Actions test shard.

File summaries

File	Description
app/Repositories/Summit/Traits/ActivitiesCountFilterMappingsTrait.php	Defines shared phase-2 presentation-level filter mappings and builds the raw-SQL WHERE fragment + bindings.
app/Http/Utils/Filters/SQL/SQLRawFilterMapping.php	New FilterMapping that renders :operator / :value conditions and binds values as named params.
app/Http/Utils/Filters/SQL/SQLSwitchFilterMapping.php	New FilterMapping that selects literal SQL conditions by filter value and ORs multi-values.
app/Repositories/Summit/DoctrineSpeakerRepository.php	Applies phase-2 extra filters to both speaker-role and moderator-role inserts into the temp table.
app/Repositories/Summit/DoctrineMemberRepository.php	Applies phase-2 extra filters to the submitter(created_by)-based count query.
tests/ActivitiesCountFilterMappingsTest.php	Adds unit tests for the new SQL mappings and anti-drift checks between phase-1 and phase-2 mappings.
tests/SpeakerRepositoryTest.php	Adds repository-level acceptance scenario tests ensuring counts are scoped by presentation filters.
tests/SubmitterRepositoryTest.php	Adds repository-level acceptance scenario tests ensuring counts are scoped by presentation filters.
tests/oauth2/OAuth2SummitSpeakersApiTest.php	Adds API-level assertions that endpoint counts respect presentation-level filters.
tests/oauth2/OAuth2SummitSubmittersApiTest.php	Adds API-level assertions that endpoint counts respect presentation-level filters.
.github/workflows/push.yml	Registers the new unit test in the existing test matrix shard so it runs in CI.

Review details

• Files reviewed: 11/11 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-599/

This page is automatically updated on each push to this PR.

[github-actions]

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-599/

This page is automatically updated on each push to this PR.

[github-actions]

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-599/

This page is automatically updated on each push to this PR.