Skip to content
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
24f1e3d
feat(promo-codes): implement domain-authorized promo codes for early …
caseylocker Apr 8, 2026
05c889c
fix(promo-codes): address review follow-ups for Tasks 1–3
caseylocker Apr 8, 2026
a556412
docs(promo-codes): add Task 4 review follow-up note for no-op override
caseylocker Apr 8, 2026
fe32435
docs(promo-codes): add review follow-ups for Tasks 5 and 7
caseylocker Apr 9, 2026
6a12e47
fix(promo-codes): address Task 6 review follow-ups
caseylocker Apr 9, 2026
2967746
fix(promo-codes): address Task 7 review follow-ups
caseylocker Apr 9, 2026
5dd1ad7
fix(promo-codes): address review follow-ups for Tasks 8 and 9
caseylocker Apr 9, 2026
82a28c3
fix(promo-codes): address Task 10 review follow-ups — race-safe quant…
caseylocker Apr 9, 2026
a5809af
docs(promo-codes): add Task 11 review follow-up notes
caseylocker Apr 9, 2026
b38e434
fix(promo-codes): address Task 12 review follow-ups — tests for colli…
caseylocker Apr 9, 2026
a9ece25
fix(promo-codes): register discover endpoint in ApiEndpointsSeeder
caseylocker Apr 9, 2026
138c1f8
fix(promo-codes): use rate.limit instead of auth.user on discover route
caseylocker Apr 9, 2026
b87cefd
fix(promo-codes): guard WithPromoCode reservations and exclude exhaus…
caseylocker Apr 9, 2026
ed2064d
test(promo-codes): add mixed-payload and infinite-code regression tests
caseylocker Apr 9, 2026
19e5f53
fix(promo-codes): fix serializer tests and resolve D3 deviation
caseylocker Apr 10, 2026
ae261a7
fix(promo-codes): address CodeRabbit findings — CSV domain import and…
caseylocker Apr 10, 2026
c3f8df7
fix(promo-codes): harden CSV domain import and migration rollback safety
caseylocker Apr 10, 2026
c2719e1
docs(promo-codes): add D10/D11 deviations for CSV import and migratio…
caseylocker Apr 10, 2026
c4bcdef
fix(promo-codes): address smarcet review — saga compensation, discove…
caseylocker Apr 13, 2026
e2ca6b5
Merge remote-tracking branch 'origin/main' into pr-525
caseylocker Apr 13, 2026
d824974
test(saga): clear resolved facade instances in setUp for test isolation
caseylocker Apr 13, 2026
93bc180
fix(promo-codes): address CodeRabbit findings on saga reorder
caseylocker Apr 13, 2026
283f576
refactor(promo-codes): address romanetar's PR #525 review comments
caseylocker Apr 15, 2026
1ac95a0
refactor(migrations): use Builder/Table API in Version20260401150000
caseylocker Apr 15, 2026
b9f2b2a
fix(rules): accept multi-level TLD suffixes in AllowedEmailDomainsArray
caseylocker Apr 15, 2026
f464339
feat(promo-codes): add SummitPromoCodeMemberReservation entity (data …
caseylocker Apr 16, 2026
13e13a2
fix(promo-codes): harden reservation backfill per Codex review
caseylocker Apr 16, 2026
8ad56fc
feat(promo-codes): atomic per-member reserve in PreProcessReservation…
caseylocker Apr 16, 2026
a0a9ba4
fix(promo-codes): prevent partial-reservation leak on mid-loop failure
caseylocker Apr 16, 2026
ff81217
chore(unit-test): add unit test to demo the toctou bug
smarcet Apr 15, 2026
b909683
feat(promo-codes): finish TOCTOU fix — remove post-facto check, add t…
caseylocker Apr 16, 2026
310455a
fix(promo-codes): step-3 review follow-ups
caseylocker Apr 16, 2026
f12af1b
refactor(promo-codes): split discover query into targeted per-subtype…
caseylocker Apr 16, 2026
d2fbcb1
fix(promo-codes): address Codex review on discover query split
caseylocker Apr 16, 2026
39e452f
fix(promo-codes): close reservation counter leak on cancel + fix undo…
caseylocker Apr 16, 2026
a1bd4ea
fix(tests): add missing reservation repo mock to SummitOrderServiceTest
caseylocker Apr 16, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
use models\summit\SpeakersSummitRegistrationPromoCode;
use models\summit\SpeakerSummitRegistrationDiscountCode;
use models\summit\SpeakerSummitRegistrationPromoCode;
use models\summit\DomainAuthorizedSummitRegistrationDiscountCode;
use models\summit\DomainAuthorizedSummitRegistrationPromoCode;
use App\Rules\AllowedEmailDomainsArray;
use models\summit\SponsorSummitRegistrationDiscountCode;
use models\summit\SponsorSummitRegistrationPromoCode;
/**
Expand Down Expand Up @@ -72,19 +75,21 @@ public static function buildForAdd(array $payload = []): array
switch ($class_name){
case MemberSummitRegistrationPromoCode::ClassName:{
$specific_rules = [
'first_name' => 'required_without:owner_id|string',
'last_name' => 'required_without:owner_id|string',
'email' => 'required_without:owner_id|email|max:254',
'type' => 'required|string|in:'.join(",", PromoCodesConstants::MemberSummitRegistrationPromoCodeTypes),
'owner_id' => 'required_without:first_name,last_name,email|integer'
'first_name' => 'required_without:owner_id|string',
'last_name' => 'required_without:owner_id|string',
'email' => 'required_without:owner_id|email|max:254',
'type' => 'required|string|in:'.join(",", PromoCodesConstants::MemberSummitRegistrationPromoCodeTypes),
'owner_id' => 'required_without:first_name,last_name,email|integer',
'auto_apply' => 'sometimes|boolean',
];
}
break;
case SpeakerSummitRegistrationPromoCode::ClassName:
{
$specific_rules = [
'type' => 'required|string|in:'.join(",", PromoCodesConstants::SpeakerSummitRegistrationPromoCodeTypes),
'speaker_id' => 'sometimes|integer'
'speaker_id' => 'sometimes|integer',
'auto_apply' => 'sometimes|boolean',
];
}
break;
Expand All @@ -106,11 +111,12 @@ public static function buildForAdd(array $payload = []): array
case MemberSummitRegistrationDiscountCode::ClassName:
{
$specific_rules = array_merge([
'first_name' => 'required_without:owner_id|string',
'last_name' => 'required_without:owner_id|string',
'email' => 'required_without:owner_id|email|max:254',
'type' => 'required|string|in:'.join(",", PromoCodesConstants::MemberSummitRegistrationPromoCodeTypes),
'owner_id' => 'required_without:first_name,last_name,email|integer',
'first_name' => 'required_without:owner_id|string',
'last_name' => 'required_without:owner_id|string',
'email' => 'required_without:owner_id|email|max:254',
'type' => 'required|string|in:'.join(",", PromoCodesConstants::MemberSummitRegistrationPromoCodeTypes),
'owner_id' => 'required_without:first_name,last_name,email|integer',
'auto_apply' => 'sometimes|boolean',
], $discount_code_rules);
}
break;
Expand All @@ -119,6 +125,7 @@ public static function buildForAdd(array $payload = []): array
$specific_rules = array_merge([
'type' => 'required|string|in:'.join(",", PromoCodesConstants::SpeakerSummitRegistrationPromoCodeTypes),
'speaker_id' => 'sometimes|integer',
'auto_apply' => 'sometimes|boolean',
], $discount_code_rules);
}
break;
Expand All @@ -138,6 +145,24 @@ public static function buildForAdd(array $payload = []): array

}
break;
case DomainAuthorizedSummitRegistrationDiscountCode::ClassName:
{
$specific_rules = array_merge([
'allowed_email_domains' => ['sometimes', new AllowedEmailDomainsArray()],
'quantity_per_account' => 'sometimes|integer|min:0',
'auto_apply' => 'sometimes|boolean',
], $discount_code_rules);
}
break;
case DomainAuthorizedSummitRegistrationPromoCode::ClassName:
{
$specific_rules = [
'allowed_email_domains' => ['sometimes', new AllowedEmailDomainsArray()],
'quantity_per_account' => 'sometimes|integer|min:0',
'auto_apply' => 'sometimes|boolean',
];
}
break;
}

return array_merge($base_rules, $specific_rules);
Expand Down Expand Up @@ -188,19 +213,21 @@ public static function buildForUpdate(array $payload = []): array
switch ($class_name){
case MemberSummitRegistrationPromoCode::ClassName:{
$specific_rules = [
'first_name' => 'required_without:owner_id|string',
'last_name' => 'required_without:owner_id|string',
'email' => 'required_without:owner_id|email|max:254',
'type' => 'required|string|in:'.join(",", PromoCodesConstants::MemberSummitRegistrationPromoCodeTypes),
'owner_id' => 'required_without:first_name,last_name,email|integer'
'first_name' => 'required_without:owner_id|string',
'last_name' => 'required_without:owner_id|string',
'email' => 'required_without:owner_id|email|max:254',
'type' => 'required|string|in:'.join(",", PromoCodesConstants::MemberSummitRegistrationPromoCodeTypes),
'owner_id' => 'required_without:first_name,last_name,email|integer',
'auto_apply' => 'sometimes|boolean',
];
}
break;
case SpeakerSummitRegistrationPromoCode::ClassName:
{
$specific_rules = [
'type' => 'required|string|in:'.join(",", PromoCodesConstants::SpeakerSummitRegistrationPromoCodeTypes),
'speaker_id' => 'sometimes|integer'
'speaker_id' => 'sometimes|integer',
'auto_apply' => 'sometimes|boolean',
];
}
break;
Expand All @@ -222,11 +249,12 @@ public static function buildForUpdate(array $payload = []): array
case MemberSummitRegistrationDiscountCode::ClassName:
{
$specific_rules = array_merge([
'first_name' => 'required_without:owner_id|string',
'last_name' => 'required_without:owner_id|string',
'email' => 'required_without:owner_id|email|max:254',
'type' => 'required|string|in:'.join(",", PromoCodesConstants::MemberSummitRegistrationPromoCodeTypes),
'owner_id' => 'required_without:first_name,last_name,email|integer',
'first_name' => 'required_without:owner_id|string',
'last_name' => 'required_without:owner_id|string',
'email' => 'required_without:owner_id|email|max:254',
'type' => 'required|string|in:'.join(",", PromoCodesConstants::MemberSummitRegistrationPromoCodeTypes),
'owner_id' => 'required_without:first_name,last_name,email|integer',
'auto_apply' => 'sometimes|boolean',
], $discount_code_rules);
}
break;
Expand All @@ -235,6 +263,7 @@ public static function buildForUpdate(array $payload = []): array
$specific_rules = array_merge([
'type' => 'required|string|in:'.join(",", PromoCodesConstants::SpeakerSummitRegistrationPromoCodeTypes),
'speaker_id' => 'sometimes|integer',
'auto_apply' => 'sometimes|boolean',
], $discount_code_rules);
}
break;
Expand All @@ -254,6 +283,24 @@ public static function buildForUpdate(array $payload = []): array

}
break;
case DomainAuthorizedSummitRegistrationDiscountCode::ClassName:
{
$specific_rules = array_merge([
'allowed_email_domains' => ['sometimes', new AllowedEmailDomainsArray()],
'quantity_per_account' => 'sometimes|integer|min:0',
'auto_apply' => 'sometimes|boolean',
], $discount_code_rules);
}
break;
case DomainAuthorizedSummitRegistrationPromoCode::ClassName:
{
$specific_rules = [
'allowed_email_domains' => ['sometimes', new AllowedEmailDomainsArray()],
'quantity_per_account' => 'sometimes|integer|min:0',
'auto_apply' => 'sometimes|boolean',
];
}
break;
}

return array_merge($base_rules, $specific_rules);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1575,4 +1575,66 @@ public function sendSponsorPromoCodes($summit_id)
return $this->ok();
});
}

/**
* Discover qualifying promo codes for the current user.
* Returns domain-authorized codes (matched by email domain) and existing email-linked
* codes (member/speaker, matched by associated email) with auto_apply flag.
* Email is always derived from the authenticated principal — no email query parameter accepted.
*/
#[OA\Get(
path: "/api/v1/summits/{id}/promo-codes/all/discover",
summary: "Discover qualifying promo codes for the current user",
description: "Returns domain-authorized promo codes (matched by email domain) and existing email-linked promo codes (member/speaker, matched by associated email) for the current user",
operationId: "discoverPromoCodesBySummit",
tags: ["Promo Codes"],
security: [['summit_promo_codes_oauth2' => [SummitScopes::ReadSummitData]]],
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
parameters: [
new OA\Parameter(name: "id", in: "path", required: true, schema: new OA\Schema(type: "integer")),
new OA\Parameter(name: "expand", in: "query", required: false, schema: new OA\Schema(type: "string")),
],
responses: [
new OA\Response(response: Response::HTTP_OK, description: "OK"),
new OA\Response(response: Response::HTTP_UNAUTHORIZED, description: "Unauthorized"),
new OA\Response(response: Response::HTTP_FORBIDDEN, description: "Forbidden"),
new OA\Response(response: Response::HTTP_NOT_FOUND, description: "Summit not found"),
]
)]
public function discover($summit_id)
{
return $this->processRequest(function () use ($summit_id) {

$summit = SummitFinderStrategyFactory::build($this->summit_repository, $this->resource_server_context)->find(intval($summit_id));
if (is_null($summit))
return $this->error404();

$current_member = $this->resource_server_context->getCurrentUser();
if (is_null($current_member))
return $this->error403();

$codes = $this->promo_code_service->discoverPromoCodes($summit, $current_member);

$expand = Request::input('expand', '');
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@caseylocker this not follow api patterns please check other controllers to see how we are doing it

SerializerUtils::getExpand(),
SerializerUtils::getFields(),
SerializerUtils::getRelations()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated in c4bcdef. The discover() method now calls SerializerUtils::getExpand()/getFields()/getRelations() (controller L1617-1619), matching the pattern used elsewhere in this controller.

$fields = Request::input('fields', '');
$relations = Request::input('relations', '');

$relations = !empty($relations) ? explode(',', $relations) : ['allowed_ticket_types', 'badge_features', 'tags', 'ticket_types_rules'];
$fields = !empty($fields) ? explode(',', $fields) : [];

$data = [];
foreach ($codes as $code) {
$serializer = SerializerRegistry::getInstance()->getSerializer($code);
$data[] = $serializer->serialize($expand, $fields, $relations);
}

$total = count($data);
return $this->ok([
'total' => $total,
'per_page' => $total,
'current_page' => 1,
'last_page' => 1,
'data' => $data,
]);
});
}
}
12 changes: 12 additions & 0 deletions app/ModelSerializers/SerializerRegistry.php
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,18 @@ private function __construct()
self::SerializerType_PreValidation => SummitRegistrationPromoCodePreValidationSerializer::class,
];

$this->registry['DomainAuthorizedSummitRegistrationDiscountCode'] = [
self::SerializerType_Public => DomainAuthorizedSummitRegistrationDiscountCodeSerializer::class,
self::SerializerType_CSV => DomainAuthorizedSummitRegistrationDiscountCodeSerializer::class,
self::SerializerType_PreValidation => SummitRegistrationPromoCodePreValidationSerializer::class,
];

$this->registry['DomainAuthorizedSummitRegistrationPromoCode'] = [
self::SerializerType_Public => DomainAuthorizedSummitRegistrationPromoCodeSerializer::class,
self::SerializerType_CSV => DomainAuthorizedSummitRegistrationPromoCodeSerializer::class,
self::SerializerType_PreValidation => SummitRegistrationPromoCodePreValidationSerializer::class,
];

$this->registry['PresentationSpeakerSummitAssistanceConfirmationRequest'] = PresentationSpeakerSummitAssistanceConfirmationRequestSerializer::class;
$this->registry['SummitRegistrationDiscountCodeTicketTypeRule'] = SummitRegistrationDiscountCodeTicketTypeRuleSerializer::class;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
<?php namespace ModelSerializers;
/**
* Copyright 2026 OpenStack Foundation
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
**/

use models\summit\DomainAuthorizedSummitRegistrationDiscountCode;

/**
* Class DomainAuthorizedSummitRegistrationDiscountCodeSerializer
* @package ModelSerializers
*/
class DomainAuthorizedSummitRegistrationDiscountCodeSerializer
extends SummitRegistrationDiscountCodeSerializer
{
protected static $array_mappings = [
'AllowedEmailDomains' => 'allowed_email_domains:json_string_array',
'QuantityPerAccount' => 'quantity_per_account:json_int',
'AutoApply' => 'auto_apply:json_boolean',
];

protected static $allowed_relations = [
'allowed_ticket_types',
];

/**
* @param null $expand
* @param array $fields
* @param array $relations
* @param array $params
* @return array
*/
public function serialize($expand = null, array $fields = [], array $relations = [], array $params = [])
{
$code = $this->object;
if (!$code instanceof DomainAuthorizedSummitRegistrationDiscountCode) return [];
$values = parent::serialize($expand, $fields, $relations, $params);

// RE-ADD allowed_ticket_types (parent discount serializer unsets it).
// Check both relations (default serialization) and expand (explicit ?expand= request).
$needs_allowed_ticket_types = in_array('allowed_ticket_types', $relations)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@caseylocker This silently compensates for a parent behavior that's undocumented. If the parent serializer changes, this will break without any error. Consider either overriding the parent behaviour more explicitly (resetting the parent's unset) or documenting why the parent SummitRegistrationDiscountCodeSerializer removes allowed_ticket_types and whether that should be changed there instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fair flag. I dug into this — the parent's unset($values['allowed_ticket_types']) is actually deliberate: generic discount codes express ticket-type coverage via ticket_types_rules (which carries rate/amount per type), so a flat allowed_ticket_types would be redundant and ambiguous for a generic discount code. Fully removing the unset at the root would re-introduce that redundancy across all 7 discount-code subclasses — wrong direction.

What was sloppy was that three subclasses (DomainAuthorized…, Member…, Speaker…) each needed the flat list and were independently re-adding it via 8-line duplicates. In 283f576:

  • Added a block comment on the parent's unset explaining why it's deliberate and pointing subclasses at the helper.
  • Added protected function restoreAllowedTicketTypes(array &$values, $expand, array $relations): void on the parent.
  • Replaced the three duplicated re-add blocks with a single $this->restoreAllowedTicketTypes(...) call each.

Wire output is byte-identical; the contract is now explicit so a future edit to the parent can't silently break the children.

|| (!empty($expand) && str_contains($expand, 'allowed_ticket_types'));
if ($needs_allowed_ticket_types && !isset($values['allowed_ticket_types'])) {
$ticket_types = [];
foreach ($code->getAllowedTicketTypes() as $ticket_type) {
$ticket_types[] = $ticket_type->getId();
}
$values['allowed_ticket_types'] = $ticket_types;
}

// Transient remaining_quantity_per_account (set by service layer)
$values['remaining_quantity_per_account'] = $code->getRemainingQuantityPerAccount();

return $values;
}

protected static $expand_mappings = [
'allowed_ticket_types' => [
'type' => \Libs\ModelSerializers\Many2OneExpandSerializer::class,
'getter' => 'getAllowedTicketTypes',
],
];
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php namespace ModelSerializers;
/**
* Copyright 2026 OpenStack Foundation
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
**/

use models\summit\DomainAuthorizedSummitRegistrationPromoCode;

/**
* Class DomainAuthorizedSummitRegistrationPromoCodeSerializer
* @package ModelSerializers
*/
class DomainAuthorizedSummitRegistrationPromoCodeSerializer
extends SummitRegistrationPromoCodeSerializer
{
protected static $array_mappings = [
'AllowedEmailDomains' => 'allowed_email_domains:json_string_array',
'QuantityPerAccount' => 'quantity_per_account:json_int',
'AutoApply' => 'auto_apply:json_boolean',
];

/**
* @param null $expand
* @param array $fields
* @param array $relations
* @param array $params
* @return array
*/
public function serialize($expand = null, array $fields = [], array $relations = [], array $params = [])
{
$code = $this->object;
if (!$code instanceof DomainAuthorizedSummitRegistrationPromoCode) return [];
$values = parent::serialize($expand, $fields, $relations, $params);

// Transient remaining_quantity_per_account (set by service layer)
$values['remaining_quantity_per_account'] = $code->getRemainingQuantityPerAccount();

return $values;
}
}
Loading
Loading