Skip to content

Commit 6a12e47

Browse files
caseylockerclaude
caseylocker
and
claude
committed
fix(promo-codes): address Task 6 review follow-ups
- Replace 'sometimes|json' with custom AllowedEmailDomainsArray rule for allowed_email_domains validation — accepts pre-decoded PHP array and validates each entry against @domain.com/.tld/user@email formats - Remove json_decode() from factory populate for both domain-authorized types — value is already a PHP array after request decoding - Fix expand=allowed_ticket_types silently dropping field on DomainAuthorizedSummitRegistrationDiscountCodeSerializer — extend re-add guard to check both $relations and $expand - Rename json_array → json_string_array in both new serializers Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent fe32435 commit 6a12e47

6 files changed

Lines changed: 91 additions & 11 deletions

File tree

‎app/Http/Controllers/Apis/Protected/Summit/Factories/Registration/PromoCodesValidationRulesFactory.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use models\summit\SpeakerSummitRegistrationPromoCode;
2323
use models\summit\DomainAuthorizedSummitRegistrationDiscountCode;
2424
use models\summit\DomainAuthorizedSummitRegistrationPromoCode;
25+
use App\Rules\AllowedEmailDomainsArray;
2526
use models\summit\SponsorSummitRegistrationDiscountCode;
2627
use models\summit\SponsorSummitRegistrationPromoCode;
2728
/**
@@ -147,7 +148,7 @@ public static function buildForAdd(array $payload = []): array
147148
case DomainAuthorizedSummitRegistrationDiscountCode::ClassName:
148149
{
149150
$specific_rules = array_merge([
150-
'allowed_email_domains' => 'sometimes|json',
151+
'allowed_email_domains' => ['sometimes', new AllowedEmailDomainsArray()],
151152
'quantity_per_account' => 'sometimes|integer|min:0',
152153
'auto_apply' => 'sometimes|boolean',
153154
], $discount_code_rules);
@@ -156,7 +157,7 @@ public static function buildForAdd(array $payload = []): array
156157
case DomainAuthorizedSummitRegistrationPromoCode::ClassName:
157158
{
158159
$specific_rules = [
159-
'allowed_email_domains' => 'sometimes|json',
160+
'allowed_email_domains' => ['sometimes', new AllowedEmailDomainsArray()],
160161
'quantity_per_account' => 'sometimes|integer|min:0',
161162
'auto_apply' => 'sometimes|boolean',
162163
];
@@ -285,7 +286,7 @@ public static function buildForUpdate(array $payload = []): array
285286
case DomainAuthorizedSummitRegistrationDiscountCode::ClassName:
286287
{
287288
$specific_rules = array_merge([
288-
'allowed_email_domains' => 'sometimes|json',
289+
'allowed_email_domains' => ['sometimes', new AllowedEmailDomainsArray()],
289290
'quantity_per_account' => 'sometimes|integer|min:0',
290291
'auto_apply' => 'sometimes|boolean',
291292
], $discount_code_rules);
@@ -294,7 +295,7 @@ public static function buildForUpdate(array $payload = []): array
294295
case DomainAuthorizedSummitRegistrationPromoCode::ClassName:
295296
{
296297
$specific_rules = [
297-
'allowed_email_domains' => 'sometimes|json',
298+
'allowed_email_domains' => ['sometimes', new AllowedEmailDomainsArray()],
298299
'quantity_per_account' => 'sometimes|integer|min:0',
299300
'auto_apply' => 'sometimes|boolean',
300301
];

‎app/ModelSerializers/Summit/Registration/PromoCodes/DomainAuthorizedSummitRegistrationDiscountCodeSerializer.php‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class DomainAuthorizedSummitRegistrationDiscountCodeSerializer
2222
extends SummitRegistrationDiscountCodeSerializer
2323
{
2424
protected static $array_mappings = [
25-
'AllowedEmailDomains' => 'allowed_email_domains:json_array',
25+
'AllowedEmailDomains' => 'allowed_email_domains:json_string_array',
2626
'QuantityPerAccount' => 'quantity_per_account:json_int',
2727
'AutoApply' => 'auto_apply:json_boolean',
2828
];
@@ -44,8 +44,11 @@ public function serialize($expand = null, array $fields = [], array $relations =
4444
if (!$code instanceof DomainAuthorizedSummitRegistrationDiscountCode) return [];
4545
$values = parent::serialize($expand, $fields, $relations, $params);
4646

47-
// RE-ADD allowed_ticket_types (parent discount serializer unsets it)
48-
if (in_array('allowed_ticket_types', $relations) && !isset($values['allowed_ticket_types'])) {
47+
// RE-ADD allowed_ticket_types (parent discount serializer unsets it).
48+
// Check both relations (default serialization) and expand (explicit ?expand= request).
49+
$needs_allowed_ticket_types = in_array('allowed_ticket_types', $relations)
50+
|| (!empty($expand) && str_contains($expand, 'allowed_ticket_types'));
51+
if ($needs_allowed_ticket_types && !isset($values['allowed_ticket_types'])) {
4952
$ticket_types = [];
5053
foreach ($code->getAllowedTicketTypes() as $ticket_type) {
5154
$ticket_types[] = $ticket_type->getId();

‎app/ModelSerializers/Summit/Registration/PromoCodes/DomainAuthorizedSummitRegistrationPromoCodeSerializer.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class DomainAuthorizedSummitRegistrationPromoCodeSerializer
2222
extends SummitRegistrationPromoCodeSerializer
2323
{
2424
protected static $array_mappings = [
25-
'AllowedEmailDomains' => 'allowed_email_domains:json_array',
25+
'AllowedEmailDomains' => 'allowed_email_domains:json_string_array',
2626
'QuantityPerAccount' => 'quantity_per_account:json_int',
2727
'AutoApply' => 'auto_apply:json_boolean',
2828
];

‎app/Models/Foundation/Summit/Factories/SummitPromoCodeFactory.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ public static function populate(SummitRegistrationPromoCode $promo_code, Summit
293293
break;
294294
case DomainAuthorizedSummitRegistrationDiscountCode::ClassName:{
295295
if(isset($data['allowed_email_domains']))
296-
$promo_code->setAllowedEmailDomains(json_decode($data['allowed_email_domains'], true));
296+
$promo_code->setAllowedEmailDomains($data['allowed_email_domains']);
297297
if(isset($data['quantity_per_account']))
298298
$promo_code->setQuantityPerAccount(intval($data['quantity_per_account']));
299299
if(isset($data['auto_apply']))
@@ -308,7 +308,7 @@ public static function populate(SummitRegistrationPromoCode $promo_code, Summit
308308
break;
309309
case DomainAuthorizedSummitRegistrationPromoCode::ClassName:{
310310
if(isset($data['allowed_email_domains']))
311-
$promo_code->setAllowedEmailDomains(json_decode($data['allowed_email_domains'], true));
311+
$promo_code->setAllowedEmailDomains($data['allowed_email_domains']);
312312
if(isset($data['quantity_per_account']))
313313
$promo_code->setQuantityPerAccount(intval($data['quantity_per_account']));
314314
if(isset($data['auto_apply']))

‎app/Rules/AllowedEmailDomainsArray.php‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
<?php namespace App\Rules;
2+
/*
3+
* Copyright 2026 OpenStack Foundation
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
**/
14+
15+
use Illuminate\Contracts\Validation\Rule;
16+
17+
/**
18+
* Class AllowedEmailDomainsArray
19+
* @package App\Rules
20+
*
21+
* Validates that the value is an array of allowed email domain patterns.
22+
* Each entry must match one of:
23+
* - @domain.com (exact domain match, starts with @)
24+
* - .tld (TLD/suffix match, starts with .)
25+
* - user@example.com (exact email match)
26+
*/
27+
final class AllowedEmailDomainsArray implements Rule
28+
{
29+
/**
30+
* @param string $attribute
31+
* @param mixed $value
32+
* @return bool
33+
*/
34+
public function passes($attribute, $value): bool
35+
{
36+
if (!is_array($value))
37+
return false;
38+
39+
foreach ($value as $element) {
40+
if (!is_string($element) || empty(trim($element)))
41+
return false;
42+
43+
$element = trim($element);
44+
45+
// @domain.com — must have at least one char after @
46+
if (str_starts_with($element, '@')) {
47+
if (!preg_match('/^@[\w][\w.-]+$/', $element))
48+
return false;
49+
}
50+
// .tld — must have at least one char after .
51+
elseif (str_starts_with($element, '.')) {
52+
if (!preg_match('/^\.\w+$/', $element))
53+
return false;
54+
}
55+
// user@example.com — standard email-like pattern
56+
elseif (str_contains($element, '@')) {
57+
if (!preg_match('/^[^@\s]+@[\w][\w.-]+$/', $element))
58+
return false;
59+
}
60+
else {
61+
return false;
62+
}
63+
}
64+
return true;
65+
}
66+
67+
/**
68+
* @return string
69+
*/
70+
public function message(): string
71+
{
72+
return trans('The :attribute must be an array of valid email domain patterns (@domain.com, .tld, or user@example.com).');
73+
}
74+
}

‎doc/promo-codes-for-early-registration-access.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,9 @@ Deviations from the SDS captured during implementation. Each entry is either **O
484484
- `php artisan clear-compiled`
485485

486486
**Review Follow-ups:**
487-
- None
487+
- [x] **`allowed_email_domains` validation is broken for natural API usage (MUST-FIX):** `getJsonPayload()` calls `Request::json()->all()` which returns an already-decoded PHP array. Laravel's `'sometimes|json'` rule requires `is_string($value)` — it returns false for a PHP array, so every real request sending `"allowed_email_domains": ["@acme.com"]` (the natural representation) is rejected with a 422. Additionally, `SummitPromoCodeFactory::populate()` calls `json_decode($data['allowed_email_domains'], true)` on what is already a PHP array — a TypeError in PHP 8 if ever reached. Fix: replace `'sometimes|json'` with a custom `AllowedEmailDomainsRule` that accepts a pre-decoded PHP array and validates each entry matches `@domain`, `.tld`, or `user@email` format (per D3 resolution plan). Also remove the `json_decode()` call from the factory — the value is already an array. Apply in both `buildForAdd` and `buildForUpdate`.
488+
- [x] **`expand=allowed_ticket_types` silently drops field on discount variant (SHOULD-FIX):** `AbstractSerializer::_expand()` sets `$values['allowed_ticket_types']` from the expand mapping, then `SummitRegistrationDiscountCodeSerializer::serialize()` unconditionally does `unset($values['allowed_ticket_types'])`, then the child re-add guard in `DomainAuthorizedSummitRegistrationDiscountCodeSerializer::serialize()` checks `in_array('allowed_ticket_types', $relations)` — which is false when the field was requested via `?expand=`. Field disappears from the response. Fix: extend the re-add condition to also check `!empty($expand) && str_contains($expand, 'allowed_ticket_types')`.
489+
- [x] **`json_array` is not a recognized serializer type (NIT):** Both new serializers declare `'AllowedEmailDomains' => 'allowed_email_domains:json_array'` but `AbstractSerializer` has no `case 'json_array'` in its formatter switch — the mapping is a silent NOP. Works in practice because `getAllowedEmailDomains()` returns a PHP array which the response encoder serializes correctly. Fix: rename to `json_string_array` for correctness.
488490

489491
---
490492

0 commit comments

Comments
 (0)