Skip to content

fix: allow change of event type when registrations exist#4097

Draft
tindevw wants to merge 1 commit into
masterfrom
fix/event-type-change
Draft

fix: allow change of event type when registrations exist#4097
tindevw wants to merge 1 commit into
masterfrom
fix/event-type-change

Conversation

@tindevw
Copy link
Copy Markdown
Contributor

@tindevw tindevw commented Mar 17, 2026

Description

This change fixes the issue of not being able to update the event type to OPEN or TBA after registrations exist.
When registrations are referencing the pools they cannot be deleted, and the update failed for event types not using pools.

Testing

  • The code quality is at a minimum required level of quality, readability, and performance.
  • I have thoroughly tested my changes.

Tested locally by updating events with existing registrations to OPEN and TBA.
Resolves #4096

@tindevw tindevw force-pushed the fix/event-type-change branch from da90866 to d84a5bf Compare March 17, 2026 20:30
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 17, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 82.60%. Comparing base (f264a6a) to head (d84a5bf).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
lego/apps/events/serializers/events.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4097      +/-   ##
==========================================
- Coverage   82.61%   82.60%   -0.01%     
==========================================
  Files         359      359              
  Lines       12698    12700       +2     
==========================================
+ Hits        10490    10491       +1     
- Misses       2208     2209       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ch0rizo
Copy link
Copy Markdown
Contributor

ch0rizo commented Mar 17, 2026

Nice, this seems logically correct. But there are some business-logic cases we need to be careful of:

  1. It turns admitted registrations into waiting-list registrations. In the codebase, pool=None means “not admitted” and effectively “on the waiting list”. If accidentally changing to TBA or OPEN from a normal event with pools people would lose their seat.
  2. If accidentally changing to TBA or OPEN for a priced event, those registrations become non-admitted, so:
    • users can no longer pay
    • overdue-payment logic stops considering them
    • existing unpaid Stripe intents are not automatically canceled by the current business-logic
    • No refund for already-paid registrations after changing to TBA or OPEN

The second point is pretty critical and there is currently no business-logic implemented to handle this

@tindevw
Copy link
Copy Markdown
Contributor Author

tindevw commented Mar 17, 2026

Nice, this seems logically correct. But there are some business-logic cases we need to be careful of:

  1. It turns admitted registrations into waiting-list registrations. In the codebase, pool=None means “not admitted” and effectively “on the waiting list”. If accidentally changing to TBA or OPEN from a normal event with pools people would lose their seat.

  2. If accidentally changing to TBA or OPEN for a priced event, those registrations become non-admitted, so:

    • users can no longer pay
    • overdue-payment logic stops considering them
    • existing unpaid Stripe intents are not automatically canceled by the current business-logic
    • No refund for already-paid registrations after changing to TBA or OPEN

The second point is pretty critical and there is currently no business-logic implemented to handle this

Very good point. I was not aware of the waiting list issue. It would probably be safer to keep blocking the updates for now and let admins handle the registrations manually by admin-removing participants before changing the event type. I think it would be nice to work on a better fix for this but also event updates in general.

@tindevw tindevw marked this pull request as draft March 17, 2026 22:04
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.

Issue updating event type

2 participants