Repository navigation
Conversation
…e type The tickets per badge feature type queries joined SummitBadgeFeatureType with the same column on both sides of an OR and never referenced the SummitBadgeType_BadgeFeatures join, so features a badge inherits from its badge type were not counted in sold_qty and checkin_qty. Rewrite both queries as a UNION ALL of the badge level and the badge type level branches, counting distinct tickets per feature name, which matches SummitAttendeeBadge::getAllFeatures. The date filter is applied to each branch. Drop the try/catch that swallowed SQL errors and returned an empty result, so a failure no longer renders as zero sold.
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Shared query and coverage app/Models/Foundation/Summit/SummitRegistrationStats.php, tests/Repositories/RegistrationStatsBadgeFeatureTest.php |
A shared query counts distinct active, paid tickets by feature type across badge-level and badge-type features. Both public methods delegate to it. The checked-in query joins attendees and filters for checked-in status. Date filtering applies to both query branches. Tests cover direct and inherited features, checked-in counts, and date filtering. Query exceptions now propagate instead of returning an empty array. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix
Suggested reviewers: smarcet
Merge Risk: 
Minimal · up to 05f44
-
The change makes registration statistics count badge features inherited from the badge type, and it stops SQL errors from being reported as empty results. No merge-blocking risk was found, and the added tests cover the new counting and date-filter behavior.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed -
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Comment @coderabbitai help to get the list of available commands.
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-657/ This page is automatically updated on each push to this PR. |
ref https://app.clickup.com/t/9014802374/86bcjg5t1
Problem
getActiveTicketsPerBadgeFeatureTypeandgetAttendeesCheckinPerBadgeFeatureTypefeedsold_qtyandcheckin_qtyof the "tickets per badge feature type" block ofGET /summits/all/{id}/registration-stats.Their join to
SummitBadgeFeatureTypecompared the same column on both sides of anOR, and theLEFT JOINtoSummitBadgeType_BadgeFeatureswas never referenced. Features a badge inherits from its badge type were therefore never counted, unlikeSummitAttendeeBadge::getAllFeatures()and the badge serializer, which merge both levels. Both methods also wrapped everything in atry/catchthat returned[], so a SQL error rendered as "0 sold".Changes
UNION ALLof the badge-level branch (SummitAttendeeBadge_Features) and the badge-type-level branch (SummitBadgeType_BadgeFeaturesviaBadgeTypeID), then countsDISTINCTtickets per feature name. A feature granted at both levels counts once.SummitAttendee.SummitHallCheckedIn = 1join.addDatesFilteringWithTimeZoneis applied to each branch, since it concatenates onto theWHEREclause and would otherwise only filter the last one.try/catchthat swallowed exceptions.RegistrationStatsBadgeFeatureTest(runs in theRepositoriesCI job): badge-level, type-level and both-levels features forsold_qtyandcheckin_qty, plus a date filter test covering both branches. The three tests fail against the previous queries.Notes
SummitOrder.SummitID:SummitAttendeeTicket.SummitIDis not mapped onmain, it comes with the registration-stats performance work. Once that lands, the predicate can move in both branches.🤖 Generated with Claude Code
Summary by CodeRabbit