Hotfix/attendee regenerate qr code#572
Merged
Merged
Conversation
…bling stacks otel-collector, elasticsearch, and rabbitmq_sponsor_services published the same host ports as openstackid's and purchases-api's local stacks (9200/9300, 1888/8888/8889/13133/4317/4318/55679, 5672/15672), which broke docker compose up when multiple FN local stacks ran side by side. Container-internal ports are unchanged; only host-side mappings moved.
…baxuvj3)
GET /summits/{id}/attendees/me?expand=tickets.badge now regenerates the
caller's badge QR code on every read (AttendeeService::regenerateAttendeeBadgesQRCodes),
so "My QR" renders even for never-printed badges.
Fixes badge QR staleness on ticket reassignment across all three writers:
AttendeeService::reassignAttendeeTicket, reassignAttendeeTicketByMember, and
SummitOrderService::updateTicket. A ticket fetched via an exclusive lock does
not populate Doctrine's inverse-side badge association, so hasBadge()/getBadge()
are unreliable afterward - resolved via a shared ResolvesLockedTicketBadge trait
that looks the badge up independently by ticket number and re-attaches it.
Also fixes a latent duplicate-badge-row risk in updateTicket's badge_type_id
branch, and adds try/catch guards around badge generation so one bad badge
(broken legacy ticket/order/summit chain) can't 500 an otherwise-successful
read or abort the rest of a generation batch.
Resolves fntechgit/ftn-docsnsklz#70.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughBadge QR codes are regenerated for expanded attendee reads and ticket reassignment flows, with persistence and ownership tests added. Docker Compose service host ports and the OpenTelemetry trace URL are also updated. ChangesBadge QR regeneration
Local service port mappings
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant OAuth2SummitAttendeesApiController
participant AttendeeService
participant SummitAttendeeBadge
Client->>OAuth2SummitAttendeesApiController: request attendee with tickets.badge expansion
OAuth2SummitAttendeesApiController->>AttendeeService: regenerate attendee badge QR codes
AttendeeService->>SummitAttendeeBadge: lock and generate QR code
SummitAttendeeBadge-->>OAuth2SummitAttendeesApiController: return persisted badge data
OAuth2SummitAttendeesApiController-->>Client: serialize attendee profile
sequenceDiagram
participant TicketOwner
participant TicketReassignmentService
participant SummitAttendeeBadge
TicketOwner->>TicketReassignmentService: reassign attendee ticket
TicketReassignmentService->>SummitAttendeeBadge: generate QR code for new owner
SummitAttendeeBadge-->>TicketReassignmentService: persist QR code
TicketReassignmentService-->>TicketOwner: complete reassignment
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-572/ This page is automatically updated on each push to this PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/Services/Model/AttendeeService.php`:
- Around line 583-598: Update AttendeeService::regenerateAttendeeBadgesQRCodes
to skip badges that already have QR codes before locking and recheck that
condition after getByIdExclusiveLock before generating. In
app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitAttendeesApiController.php
lines 187-187, invoke regeneration only when tickets.badge is expanded. In
tests/oauth2/OAuth2AttendeesApiTest.php lines 189-210, verify the QR generation
path is not invoked on the second read rather than relying only on matching
deterministic values.
🪄 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: b71a814c-2e57-4252-8608-65e1281fa1de
📒 Files selected for processing (10)
app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitAttendeesApiController.phpapp/Services/Model/AttendeeService.phpapp/Services/Model/IAttendeeService.phpapp/Services/Model/Imp/SummitOrderService.phpapp/Services/Model/Imp/Traits/ResolvesLockedTicketBadge.phpdocker-compose.ymlreadme.mdtests/AttendeeServiceTest.phptests/SummitOrderServiceTest.phptests/oauth2/OAuth2AttendeesApiTest.php
The previous commit shipped a trait working around a claimed Doctrine
behavior ("exclusive-lock fetch doesn't populate inverse-side to-one
associations") that does not actually exist. Verified with isolated,
instrumented tests (SQL capture, object-identity checks, UnitOfWork
state checks): $ticket->hasBadge()/getBadge() work correctly on a
ticket fetched via getByIdExclusiveLock(), with no workaround needed.
The earlier misleading result was caused by two unrelated confounds in
diagnostic testing: mixing two different EntityManager instances (the
LaravelDoctrine facade vs a test fixture's own self::$em), and a
pre-existing test fixture bug (InsertSummitTestData reuses one
SummitAttendeeBadge object across 5 tickets, so attendee->getTickets()
->first() is not reliably the ticket that badge's DB row points to).
Reverts all four call sites (AttendeeService::reassignAttendeeTicket,
reassignAttendeeTicketByMember, and both branches of
SummitOrderService::updateTicket) to plain hasBadge()/getBadge().
Keeps the try/catch guard around generateQRCode(), which is unrelated
and still valid. Deletes the ResolvesLockedTicketBadge trait.
Full targeted suite re-verified: 45/46 pass (same pre-existing,
unrelated failure as before).
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-572/ This page is automatically updated on each push to this PR. |
Per CodeRabbit review on PR #572 (discussion_r3587418919): getOwnAttendee was invoking regenerateAttendeeBadgesQRCodes() unconditionally, exclusively locking and regenerating every active badge even on requests that never expand tickets.badge and therefore never serialize badge data at all. Now only calls it when 'tickets.badge' is present in the expand param. Keeps the always-regenerate behavior (no skip-if-already-has-QR) per explicit prior decision - this only scopes *when* it runs, not whether existing QR codes are trusted. Added testGetOwnAttendeeDoesNotGenerateBadgeQRCodeWhenBadgeNotExpanded, verified RED (failed against the unconditional call) before GREEN. Full targeted suite re-verified: 46/47 pass (same pre-existing failure).
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-572/ This page is automatically updated on each push to this PR. |
romanetar
requested changes
Jul 15, 2026
Per PR #572 review (discussion_r3587772934, @romanetar): "que" was a Spanish word leaked into the English OpenAPI description. Also updated the wording to reflect the actual current behavior (regeneration only happens when tickets.badge is expanded).
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-572/ This page is automatically updated on each push to this PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ref: https://app.clickup.com/t/9014802374/86baxuvj3
Summary by CodeRabbit
New Features
Documentation
Chores
Tests