Commit ae261a7
fix(promo-codes): address CodeRabbit findings — CSV domain import and migration rollback
CodeRabbit flagged 6 issues on PR #525. After independent validation (Codex),
2 were confirmed as real bugs, 2 were false positives, and 2 were
informational/misframed.
Fixed (validated as real):
- **CSV import TypeError:** `allowed_email_domains` was not exploded from its
pipe-delimited CSV string before reaching `setAllowedEmailDomains(array)`,
causing a TypeError on domain-authorized code import. Added the same
`explode('|', ...)` normalization used by all other CSV list fields in both
the add and update import paths.
- **Migration down() failure:** Dropping the joined domain-authorized tables
did not remove orphaned base-table rows, so narrowing the ClassName ENUM
would fail if any domain-authorized promo codes existed. Added a DELETE
statement before the ALTER TABLE.
Dismissed (validated as false positives):
- `remaining_quantity_per_account = null` in MemberDiscountCode serializer is
correct — Member types do not have per-account quantity.
- Discover route already has OAuth2 auth via the `api` middleware group and
an explicit controller-level null-member guard. Adding `auth.user` would
break it (requires authz_groups, intentionally removed in 138c1f8).
Deferred:
- `boolval("false")` pattern is pre-existing across the factory (not
introduced by this PR); warrants a separate cleanup.
- Multi-level TLD validation regex (`.co.uk`) is an enhancement, not a bug
in the current domain-matching logic.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>1 parent 19e5f53 commit ae261a7
2 files changed
Lines changed: 14 additions & 0 deletions
File tree
- app/Services/Model/Imp
- database/migrations/model
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
642 | 642 | | |
643 | 643 | | |
644 | 644 | | |
| 645 | + | |
| 646 | + | |
| 647 | + | |
| 648 | + | |
645 | 649 | | |
646 | 650 | | |
647 | 651 | | |
| |||
745 | 749 | | |
746 | 750 | | |
747 | 751 | | |
| 752 | + | |
| 753 | + | |
| 754 | + | |
| 755 | + | |
748 | 756 | | |
749 | 757 | | |
750 | 758 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
100 | 100 | | |
101 | 101 | | |
102 | 102 | | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
103 | 109 | | |
104 | 110 | | |
105 | 111 | | |
| |||
0 commit comments