Skip to content

feat(speakers): add unique activities count endpoints for speakers and submitters#543

Open
mulldug wants to merge 1 commit into
mainfrom
feature/speakers-submitters-activities-count
Open

feat(speakers): add unique activities count endpoints for speakers and submitters#543
mulldug wants to merge 1 commit into
mainfrom
feature/speakers-submitters-activities-count

Conversation

@mulldug
Copy link
Copy Markdown
Collaborator

@mulldug mulldug commented May 12, 2026

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

  • Add GET /summits/{id}/speakers/all/events/count and GET /summits/{id}/submitters/all/events/count controller methods with OpenAPI attributes and route registrations
  • Implement getUniqueActivitiesCountBySummit in DoctrineSpeakerRepository and DoctrineMemberRepository using two-stage DQL→raw SQL COUNT(DISTINCT)
  • Add interface declarations to ISpeakerRepository and IMemberRepository
  • Register both endpoints in ApiEndpointsSeeder with ReadSummitData scopes
  • Add PHPUnit tests for both HTTP endpoints and repository-level method
  • Fix null-guard bug in DoctrineMemberRepository::applyExtraJoins

Summary by CodeRabbit

  • New Features

    • Added two new API endpoints to retrieve unique activity counts for speakers and submitters with optional filtering.
  • Bug Fixes

    • OAuth2 origin validation now treats origins differing only by a trailing slash as equivalent.
    • Payment-profile initialization failures are now caught and logged to prevent service errors.
  • Tests

    • Added API and repository tests for the new endpoints and filtering; some tests now skip if required fixtures are absent.
  • Chores

    • Updated example env formatting, ignored local Docker override, added testing compose overrides, and evict cache during reseed.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 12, 2026

📝 Walkthrough

Walkthrough

Adds two OAuth2-protected endpoints to count unique summit activities for speakers and submitters with optional filter parsing and validation; implements repository methods to compute distinct presentation counts; wires routes and seeds endpoints; adds tests and several supporting infrastructure fixes.

Changes

Speakers and Submitters Activities Count API

Layer / File(s) Summary
Repository interface contracts
app/Models/Foundation/Main/Repositories/IMemberRepository.php, app/Models/Foundation/Summit/Repositories/ISpeakerRepository.php
New method getUniqueActivitiesCountBySummit(Summit, Filter = null): int declared on IMemberRepository and ISpeakerRepository with DBAL exception docblocks.
Speaker repository implementation
app/Repositories/Summit/DoctrineSpeakerRepository.php
Implements getUniqueActivitiesCountBySummit: runs speaker and moderator queries, collects and de-duplicates presentation IDs in PHP, and returns the distinct presentation count.
Member repository implementation
app/Repositories/Summit/DoctrineMemberRepository.php
Guards applyExtraJoins checks with !is_null($filter) and implements getUniqueActivitiesCountBySummit that builds a member-id subquery (with optional joins/filters) and returns COUNT(DISTINCT p.id) for presentations authored by those members.
API endpoint controllers
app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSpeakersApiController.php, app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSubmittersApiController.php
Add getSpeakersActivitiesCount and getSubmittersActivitiesCount methods with OpenAPI annotations; validate summit existence and optional filter, delegate to repository methods, and return JSON {"count": <int>}.
API routing and endpoint seeding
routes/api_v1.php, database/seeders/ApiEndpointsSeeder.php
Add authenticated GET events/count routes under summits/{id}/speakers/all and summits/{id}/submitters/all; register both endpoints in ApiEndpointsSeeder with ReadSummitData/ReadAllSummitData scopes and admin groups.
API endpoint tests
tests/oauth2/OAuth2SummitSpeakersApiTest.php, tests/oauth2/OAuth2SummitSubmittersApiTest.php
Add tests for speakers and submitters activities-count endpoints verifying HTTP 200 and numeric count values; speakers tests include selection-plan and accepted-presentations filters.
Repository method tests
tests/SubmitterRepositoryTest.php
Add testGetUniqueActivitiesCountBySummit() asserting unfiltered and filtered integer counts and that filtered ≤ unfiltered; add fixture-availability guards to two existing tests.
Infrastructure and supporting updates
.env.example, .gitignore, app/Http/Middleware/OAuth2BearerAccessTokenRequestValidator.php, app/ModelSerializers/Summit/SummitSerializer.php, database/seeders/ConfigSeeder.php, docker-compose.override.testing.yml
Quote CORS env values in .env.example; ignore docker-compose.override.yml; trim trailing / when validating JS client origin in OAuth2 middleware; wrap default payment profile serialization in try/catch and log warnings; evict Doctrine entity/query cache regions in ConfigSeeder before deletes; add docker-compose.override.testing.yml for local testing ports.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • OpenStackweb/summit-api#517: Modifies the same seeder files (ApiEndpointsSeeder and ConfigSeeder) to add/register endpoints and alter cache/clear behavior during seeding.

Suggested reviewers

  • martinquiroga-exo
  • caseylocker
  • smarcet

Poem

🐰 I hopped through routes and queries bright,

Counting events by day and night,
Filters trimmed and caches cleared,
Controllers, repos all appeared,
A tiny rabbit logs with delight.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately summarizes the main change: adding unique activities count endpoints for speakers and submitters, which aligns with the substantial API additions across controllers, repositories, and tests in the changeset.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/speakers-submitters-activities-count

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 PHPStan (2.1.54)

Composer install failed: this project depends on private packages that require authentication (e.g. GitLab/GitHub, Laravel Nova, etc.).
CodeRabbit tooling environment cannot access private registries.
If your project requires private packages, disable the PHPStan tool in your coderabbit settings.

Instead, run PHPStan in a CI/CD pipeline where you can use custom packages — our pipeline remediation tool can use the PHPStan output from your CI/CD pipeline.


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

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

📘 OpenAPI / Swagger preview

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

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (4)
tests/oauth2/OAuth2SummitSubmittersApiTest.php (1)

264-301: ⚡ Quick win

Strengthen count assertions to catch regressions earlier.

These tests only assert count >= 0. Add an integer-type assertion and a filtered-vs-unfiltered comparison (filtered <= unfiltered) to validate behavior, not just shape.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/oauth2/OAuth2SummitSubmittersApiTest.php` around lines 264 - 301,
Update the assertions in
testGetCurrentSummitSubmittersActivitiesCountWithAcceptedPresentations (and the
sibling unfiltered test if present) to validate type and relational behavior:
assert the returned $data->count is an integer (use assertIsInt or
assertTrue(is_int(...))) and then perform an additional request to get the
unfiltered count (call
OAuth2SummitSubmittersApiController@getSubmittersActivitiesCount with the same
$params but without the 'filter' entry), decode that response and assert that
the filtered count is less than or equal to the unfiltered count ($filteredCount
<= $unfilteredCount); use the existing $response/$content/json_decode flow and
$this->action helper to obtain the unfiltered result.
app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSpeakersApiController.php (1)

432-497: ⚖️ Poor tradeoff

Consider extracting the duplicated filter definitions.

The filter field definitions (lines 442-464) and validation rules (lines 468-490) are duplicated across getSpeakers, getSpeakersCSV, send, and this new getSpeakersActivitiesCount method. Extracting these to private methods or class constants would improve maintainability and ensure consistency when filter fields are added or modified.

♻️ Example refactor
private function getSpeakerFilterDefinition(): array
{
    return [
        'id' => ['=='],
        'not_id' => ['=='],
        'first_name' => ['=@', '@@', '=='],
        'last_name' => ['=@', '@@', '=='],
        'email' => ['=@', '@@', '=='],
        'full_name' => ['=@', '@@', '=='],
        'member_id' => ['=='],
        'member_user_external_id' => ['=='],
        'has_accepted_presentations' => ['=='],
        'has_alternate_presentations' => ['=='],
        'has_rejected_presentations' => ['=='],
        'presentations_track_id' => ['=='],
        'presentations_track_group_id' => ['=='],
        'presentations_selection_plan_id' => ['=='],
        'presentations_type_id' => ['=='],
        'presentations_title' => ['=@', '@@', '=='],
        'presentations_abstract' => ['=@', '@@', '=='],
        'presentations_submitter_full_name' => ['=@', '@@', '=='],
        'presentations_submitter_email' => ['=@', '@@', '=='],
        'has_media_upload_with_type' => ['=='],
        'has_not_media_upload_with_type' => ['=='],
    ];
}

private function getSpeakerFilterValidationRules(): array
{
    return [
        'id' => 'sometimes|integer',
        'not_id' => 'sometimes|integer',
        'first_name' => 'sometimes|string',
        'last_name' => 'sometimes|string',
        'email' => 'sometimes|string',
        'full_name' => 'sometimes|string',
        'member_id' => 'sometimes|integer',
        'member_user_external_id' => 'sometimes|integer',
        'has_accepted_presentations' => 'sometimes|required|string|in:true,false',
        'has_alternate_presentations' => 'sometimes|required|string|in:true,false',
        'has_rejected_presentations' => 'sometimes|required|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',
        'presentations_abstract' => 'sometimes|string',
        'presentations_submitter_full_name' => 'sometimes|string',
        'presentations_submitter_email' => 'sometimes|string',
        'has_media_upload_with_type' => 'sometimes|integer',
        'has_not_media_upload_with_type' => 'sometimes|integer',
    ];
}

Then use:

$filter = FilterParser::parse(Request::input('filter'), $this->getSpeakerFilterDefinition());
// ...
if (!is_null($filter)) {
    $filter->validate($this->getSpeakerFilterValidationRules());
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSpeakersApiController.php`
around lines 432 - 497, The filter field definitions and validation rules
duplicated in getSpeakersActivitiesCount should be extracted into reusable
helpers (e.g., private methods or class constants) and reused across
getSpeakers, getSpeakersCSV, send and getSpeakersActivitiesCount; create methods
like getSpeakerFilterDefinition() and getSpeakerFilterValidationRules()
returning the arrays shown in the comment, then replace the inline arrays in
getSpeakersActivitiesCount (the FilterParser::parse call and the
$filter->validate call) and the same spots in getSpeakers, getSpeakersCSV and
send to call those helper methods instead.
tests/oauth2/OAuth2SummitSpeakersApiTest.php (1)

1938-1968: 💤 Low value

Consider comparing filtered vs unfiltered count for consistency.

The test testGetCurrentSummitSpeakersActivitiesCountFilteredBySelPlan (lines 1903-1936) establishes a useful pattern: it fetches both unfiltered and filtered counts, then asserts the filtered count is less than or equal to the unfiltered count. This test only fetches the filtered count and asserts it's greater than zero. For consistency and stronger validation, consider adding an unfiltered baseline call and comparing the results.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/oauth2/OAuth2SummitSpeakersApiTest.php` around lines 1938 - 1968, The
test method testGetCurrentSummitSpeakersActivitiesCountWithAcceptedPresentations
should also fetch an unfiltered baseline using the same controller action
getSpeakersActivitiesCount and same summit id (but without the filter param),
parse its JSON count, then assert the filtered count is > 0 and that
filtered_count <= unfiltered_count to mirror the pattern in
testGetCurrentSummitSpeakersActivitiesCountFilteredBySelPlan; locate and update
the test method
testGetCurrentSummitSpeakersActivitiesCountWithAcceptedPresentations to perform
the extra request, decode the response, and add the additional assertion
comparing counts.
app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSubmittersApiController.php (1)

539-604: ⚖️ Poor tradeoff

Consider extracting the duplicated filter definitions.

The filter field definitions (lines 549-571) and validation rules (lines 575-597) are duplicated across getAllBySummit, getAllBySummitCSV, send, and this new getSubmittersActivitiesCount method. Extracting these to a private method or class constant would improve maintainability and ensure consistency when filter fields are added or modified.

♻️ Example refactor
private function getSubmitterFilterDefinition(): array
{
    return [
        'id' => ['=='],
        'not_id' => ['=='],
        'first_name' => ['=@', '@@', '=='],
        'last_name' => ['=@', '@@', '=='],
        'email' => ['=@', '@@', '=='],
        'full_name' => ['=@', '@@', '=='],
        'member_id' => ['=='],
        'member_user_external_id' => ['=='],
        'has_accepted_presentations' => ['=='],
        'has_alternate_presentations' => ['=='],
        'has_rejected_presentations' => ['=='],
        'presentations_track_id' => ['=='],
        'presentations_selection_plan_id' => ['=='],
        'presentations_type_id' => ['=='],
        'presentations_title' => ['=@', '@@', '=='],
        'presentations_abstract' => ['=@', '@@', '=='],
        'presentations_submitter_full_name' => ['=@', '@@', '=='],
        'presentations_submitter_email' => ['=@', '@@', '=='],
        'is_speaker' => ['=='],
        'has_media_upload_with_type' => ['=='],
        'has_not_media_upload_with_type' => ['=='],
    ];
}

private function getSubmitterFilterValidationRules(): array
{
    return [
        'id' => 'sometimes|integer',
        'not_id' => 'sometimes|integer',
        'first_name' => 'sometimes|string',
        'last_name' => 'sometimes|string',
        'email' => 'sometimes|string',
        'full_name' => 'sometimes|string',
        'member_id' => 'sometimes|integer',
        'member_user_external_id' => 'sometimes|integer',
        'has_accepted_presentations' => 'sometimes|string|in:true,false',
        'has_alternate_presentations' => 'sometimes|string|in:true,false',
        'has_rejected_presentations' => 'sometimes|string|in:true,false',
        'presentations_track_id' => 'sometimes|integer',
        'presentations_selection_plan_id' => 'sometimes|integer',
        'presentations_type_id' => 'sometimes|integer',
        'presentations_title' => 'sometimes|string',
        'presentations_abstract' => 'sometimes|string',
        'presentations_submitter_full_name' => 'sometimes|string',
        'presentations_submitter_email' => 'sometimes|string',
        'is_speaker' => 'sometimes|string|in:true,false',
        'has_media_upload_with_type' => 'sometimes|integer',
        'has_not_media_upload_with_type' => 'sometimes|integer',
    ];
}

Then use:

$filter = FilterParser::parse(Request::input('filter'), $this->getSubmitterFilterDefinition());
// ...
$filter->validate($this->getSubmitterFilterValidationRules());
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSubmittersApiController.php`
around lines 539 - 604, The duplicated submitter filter definitions and
validation rules used in getSubmittersActivitiesCount (and also in
getAllBySummit, getAllBySummitCSV, send) should be extracted into reusable
methods or constants; add private methods like getSubmitterFilterDefinition()
and getSubmitterFilterValidationRules() that return the arrays shown in the
review, then replace the inline arrays in getSubmittersActivitiesCount with
FilterParser::parse(Request::input('filter'),
$this->getSubmitterFilterDefinition()) and
$filter->validate($this->getSubmitterFilterValidationRules()); update the other
methods (getAllBySummit, getAllBySummitCSV, send) to call these new methods to
remove duplication and keep behavior identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/Http/Middleware/OAuth2BearerAccessTokenRequestValidator.php`:
- Around line 175-177: The origin check in
OAuth2BearerAccessTokenRequestValidator uses str_contains on
token_info->getAllowedOrigins(), which allows partial matches; update the logic
to perform exact origin matching: normalize the incoming $origin (trim trailing
slash), split token_info->getAllowedOrigins() into discrete origins (e.g., by
comma) and check for strict equality (or use in_array) against the normalized
origin when getApplicationType() === 'JS_CLIENT', ensuring null/empty $origin
still fails.

In `@app/Repositories/Summit/DoctrineMemberRepository.php`:
- Around line 669-686: The current code in DoctrineMemberRepository builds
$memberIds by materializing $qb->getQuery()->getArrayResult() and then passes
that array into the COUNT query (member_ids), which can blow up for large
summits; replace this two-step approach with a single SQL that counts distinct
presentations for submitters from the summit using a subquery or JOIN instead of
an IN-list (e.g. COUNT(DISTINCT p.ID) FROM Presentation p INNER JOIN SummitEvent
se ON se.ID = p.ID WHERE se.SummitID = :summit_id AND p.CreatedByID IN (SELECT
DISTINCT m.ID FROM Member m INNER JOIN <relevant_table> ... WHERE SummitID =
:summit_id) or use EXISTS: WHERE EXISTS (SELECT 1 FROM <submitter_relation> s
WHERE s.MemberID = p.CreatedByID AND s.SummitID = :summit_id)); update the call
to executeQuery to only bind the single summit_id parameter (remove member_ids
and ArrayParameterType usage) and keep the query execution within
DoctrineMemberRepository (target symbols: $qb, getArrayResult, $memberIds,
Presentation p, SummitEvent se, executeQuery).

In `@app/Repositories/Summit/DoctrineSpeakerRepository.php`:
- Around line 731-755: The code in DoctrineSpeakerRepository materializes all
speaker IDs via $qb->getQuery()->getArrayResult() into $speakerIds and then uses
IN (:speaker_ids) in a raw SQL count, which is brittle for large sets; instead
modify the count SQL to embed the speaker selection as a SQL subquery (or reuse
the QueryBuilder as a subselect) so you never pass a large PHP array — replace
the IN (:speaker_ids) clauses with EXISTS/subselects that reference the same
speaker-selection logic (derived from $qb) and execute the count with a single
query using bound scalar parameters (e.g., :summit_id) rather than an
ArrayParameterType for speaker ids.

In `@database/seeders/ConfigSeeder.php`:
- Around line 38-42: The seeder currently calls non-existent methods
evictEntityRegions() and evictQueryRegions() on $l2Cache; replace them with the
proper Doctrine Cache API calls by invoking $l2Cache->evictQueryCache() for
query cache, and for entities either call
$l2Cache->evictEntityRegion($entityClass) in a loop over your entity class names
(e.g., from metadata) or call $l2Cache->evictEntityRegion(null) to evict all
entity regions; update the code in ConfigSeeder where $l2Cache is used to
perform these correct calls.

---

Nitpick comments:
In
`@app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSpeakersApiController.php`:
- Around line 432-497: The filter field definitions and validation rules
duplicated in getSpeakersActivitiesCount should be extracted into reusable
helpers (e.g., private methods or class constants) and reused across
getSpeakers, getSpeakersCSV, send and getSpeakersActivitiesCount; create methods
like getSpeakerFilterDefinition() and getSpeakerFilterValidationRules()
returning the arrays shown in the comment, then replace the inline arrays in
getSpeakersActivitiesCount (the FilterParser::parse call and the
$filter->validate call) and the same spots in getSpeakers, getSpeakersCSV and
send to call those helper methods instead.

In
`@app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSubmittersApiController.php`:
- Around line 539-604: The duplicated submitter filter definitions and
validation rules used in getSubmittersActivitiesCount (and also in
getAllBySummit, getAllBySummitCSV, send) should be extracted into reusable
methods or constants; add private methods like getSubmitterFilterDefinition()
and getSubmitterFilterValidationRules() that return the arrays shown in the
review, then replace the inline arrays in getSubmittersActivitiesCount with
FilterParser::parse(Request::input('filter'),
$this->getSubmitterFilterDefinition()) and
$filter->validate($this->getSubmitterFilterValidationRules()); update the other
methods (getAllBySummit, getAllBySummitCSV, send) to call these new methods to
remove duplication and keep behavior identical.

In `@tests/oauth2/OAuth2SummitSpeakersApiTest.php`:
- Around line 1938-1968: The test method
testGetCurrentSummitSpeakersActivitiesCountWithAcceptedPresentations should also
fetch an unfiltered baseline using the same controller action
getSpeakersActivitiesCount and same summit id (but without the filter param),
parse its JSON count, then assert the filtered count is > 0 and that
filtered_count <= unfiltered_count to mirror the pattern in
testGetCurrentSummitSpeakersActivitiesCountFilteredBySelPlan; locate and update
the test method
testGetCurrentSummitSpeakersActivitiesCountWithAcceptedPresentations to perform
the extra request, decode the response, and add the additional assertion
comparing counts.

In `@tests/oauth2/OAuth2SummitSubmittersApiTest.php`:
- Around line 264-301: Update the assertions in
testGetCurrentSummitSubmittersActivitiesCountWithAcceptedPresentations (and the
sibling unfiltered test if present) to validate type and relational behavior:
assert the returned $data->count is an integer (use assertIsInt or
assertTrue(is_int(...))) and then perform an additional request to get the
unfiltered count (call
OAuth2SummitSubmittersApiController@getSubmittersActivitiesCount with the same
$params but without the 'filter' entry), decode that response and assert that
the filtered count is less than or equal to the unfiltered count ($filteredCount
<= $unfilteredCount); use the existing $response/$content/json_decode flow and
$this->action helper to obtain the unfiltered result.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 36745d1c-f076-4af9-b8a0-83be313aee9a

📥 Commits

Reviewing files that changed from the base of the PR and between 34e47f9 and ca6dbae.

📒 Files selected for processing (17)
  • .env.example
  • .gitignore
  • app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSpeakersApiController.php
  • app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSubmittersApiController.php
  • app/Http/Middleware/OAuth2BearerAccessTokenRequestValidator.php
  • app/ModelSerializers/Summit/SummitSerializer.php
  • app/Models/Foundation/Main/Repositories/IMemberRepository.php
  • app/Models/Foundation/Summit/Repositories/ISpeakerRepository.php
  • app/Repositories/Summit/DoctrineMemberRepository.php
  • app/Repositories/Summit/DoctrineSpeakerRepository.php
  • database/seeders/ApiEndpointsSeeder.php
  • database/seeders/ConfigSeeder.php
  • docker-compose.override.testing.yml
  • routes/api_v1.php
  • tests/SubmitterRepositoryTest.php
  • tests/oauth2/OAuth2SummitSpeakersApiTest.php
  • tests/oauth2/OAuth2SummitSubmittersApiTest.php

Comment thread app/Http/Middleware/OAuth2BearerAccessTokenRequestValidator.php
Comment thread app/Repositories/Summit/DoctrineMemberRepository.php Outdated
Comment thread app/Repositories/Summit/DoctrineSpeakerRepository.php Outdated
Comment thread database/seeders/ConfigSeeder.php
…d submitters

- Add GET /summits/{id}/speakers/all/events/count and GET /summits/{id}/submitters/all/events/count controller methods with OpenAPI attributes and route registrations
- Implement getUniqueActivitiesCountBySummit in DoctrineSpeakerRepository and DoctrineMemberRepository using two-stage DQL→raw SQL COUNT(DISTINCT)
- Add interface declarations to ISpeakerRepository and IMemberRepository
- Register both endpoints in ApiEndpointsSeeder with ReadSummitData scopes
- Add PHPUnit tests for both HTTP endpoints and repository-level method
- Fix null-guard bug in DoctrineMemberRepository::applyExtraJoins
@mulldug mulldug force-pushed the feature/speakers-submitters-activities-count branch from ca6dbae to bd000e6 Compare May 14, 2026 06:37
@github-actions
Copy link
Copy Markdown

📘 OpenAPI / Swagger preview

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

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
app/Repositories/Summit/DoctrineSpeakerRepository.php (1)

711-713: ⚡ Quick win

Push presentation de-duplication into SQL for the speaker branch.

speakerQb can return duplicate p.id rows due to the p.speakers join; deduping earlier reduces result size and PHP work with no behavior change.

Proposed change
-        $speakerQb = $this->getEntityManager()->createQueryBuilder()
-            ->select("p.id")
+        $speakerQb = $this->getEntityManager()->createQueryBuilder()
+            ->select("DISTINCT p.id")
             ->from('models\summit\Presentation', 'p')
             ->join('p.speakers', '__cnt_assign')
             ->join('__cnt_assign.speaker', 'e')

Also applies to: 741-744

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/Repositories/Summit/DoctrineSpeakerRepository.php` around lines 711 -
713, The query builder in DoctrineSpeakerRepository producing $speakerQb can
return duplicate p.id rows because of the p.speakers join; modify the query to
deduplicate at the SQL level (e.g., change the select to request unique ids or
add a GROUP BY) so it returns distinct presentation ids (use DISTINCT p.id or
group by p.id) to reduce result size and PHP-side work; apply the same fix to
the other occurrence of $speakerQb-like code around the later block (the similar
select at lines ~741-744).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@app/Repositories/Summit/DoctrineSpeakerRepository.php`:
- Around line 711-713: The query builder in DoctrineSpeakerRepository producing
$speakerQb can return duplicate p.id rows because of the p.speakers join; modify
the query to deduplicate at the SQL level (e.g., change the select to request
unique ids or add a GROUP BY) so it returns distinct presentation ids (use
DISTINCT p.id or group by p.id) to reduce result size and PHP-side work; apply
the same fix to the other occurrence of $speakerQb-like code around the later
block (the similar select at lines ~741-744).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6fac9124-d47b-4e92-be67-726afb96df68

📥 Commits

Reviewing files that changed from the base of the PR and between ca6dbae and bd000e6.

📒 Files selected for processing (17)
  • .env.example
  • .gitignore
  • app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSpeakersApiController.php
  • app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSubmittersApiController.php
  • app/Http/Middleware/OAuth2BearerAccessTokenRequestValidator.php
  • app/ModelSerializers/Summit/SummitSerializer.php
  • app/Models/Foundation/Main/Repositories/IMemberRepository.php
  • app/Models/Foundation/Summit/Repositories/ISpeakerRepository.php
  • app/Repositories/Summit/DoctrineMemberRepository.php
  • app/Repositories/Summit/DoctrineSpeakerRepository.php
  • database/seeders/ApiEndpointsSeeder.php
  • database/seeders/ConfigSeeder.php
  • docker-compose.override.testing.yml
  • routes/api_v1.php
  • tests/SubmitterRepositoryTest.php
  • tests/oauth2/OAuth2SummitSpeakersApiTest.php
  • tests/oauth2/OAuth2SummitSubmittersApiTest.php
✅ Files skipped from review due to trivial changes (4)
  • .env.example
  • docker-compose.override.testing.yml
  • .gitignore
  • app/Models/Foundation/Main/Repositories/IMemberRepository.php
🚧 Files skipped from review as they are similar to previous changes (10)
  • app/Http/Middleware/OAuth2BearerAccessTokenRequestValidator.php
  • database/seeders/ConfigSeeder.php
  • app/ModelSerializers/Summit/SummitSerializer.php
  • routes/api_v1.php
  • tests/oauth2/OAuth2SummitSubmittersApiTest.php
  • app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSubmittersApiController.php
  • app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSpeakersApiController.php
  • tests/oauth2/OAuth2SummitSpeakersApiTest.php
  • database/seeders/ApiEndpointsSeeder.php
  • tests/SubmitterRepositoryTest.php

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant