Skip to content

E
El-Captain Sports Management
Commit c1c1e9bc authored by Mahmoud Aglan's avatar Mahmoud Aglan
Browse files
Options

feat(pricing): let a sibling discount apply to one membership tier only 

A compound charges a resident 2,200 and a non-resident 2,800, and the second
child of a non-resident 2,400 — while a resident's second child stays at 2,200,
because the resident price is already the discounted one. A sibling_order rule
could say "second child" but not "and only for non-residents", and the
arithmetic has no shortcut: a flat 400 off takes the resident to 1,800, and a
percentage landing on 2,400 from 2,800 lands on 1,886 from 2,200.

The config-only alternative was a second stackable rule holding
fixed_price 220000 to push residents back up. That works arithmetically and is
the wrong answer: it stores the tier's base price in a second table. The day
fees rise and only base_prices is edited — the obvious place — that rule
silently forces every resident back to the old number, with no error and no
missing-price failure to notice. It also prints a +200 EGP "discount" line on a
resident sibling's invoice, because appliedRules is stored verbatim on it.

So sibling_order and family_size gain an optional membership_type list in their
conditions, evaluated as an AND alongside the existing range.

conditions is jsonb with no CHECK constraint, so no migration is needed. The key
is absent from every rule authored before now, isset() is false, and evaluation
is bit-identical for every existing rule on every tenant — additive in the same
sense a nullable column is. It is accepted only where the schema declares it;
normalize() strips it from an age or loyalty rule rather than storing a
condition the engine will never consult. describe() appends the tier to the
existing clause so the picker's rejection reason stays a sentence.

Pinned by 13 tests, the first of which is the one that matters to the other
tenants: absent key, unchanged behaviour.
Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
parent 5c5d1330
Hide whitespace changes
Inline Side-by-side
Showing with 244 additions and 7 deletions
app/Domain/Pricing/Services/PricingService.php
View file @ c1c1e9bc
......@@ -429,8 +429,10 @@ private function evaluateConditions(PricingRule $rule, array $context): bool
return match ($rule->rule_type) {
\App\Domain\Pricing\Enums\PricingRuleType::Age => $this->evaluateRange($context['age'], $conditions),
\App\Domain\Pricing\Enums\PricingRuleType::MembershipDuration => $this->evaluateRange($context['membership_duration_months'], $conditions),
\App\Domain\Pricing\Enums\PricingRuleType::FamilySize => $this->evaluateRange($context['family_size'], $conditions),
\App\Domain\Pricing\Enums\PricingRuleType::SiblingOrder => $this->evaluateRange($context['sibling_order'], $conditions),
\App\Domain\Pricing\Enums\PricingRuleType::FamilySize => $this->evaluateRange($context['family_size'], $conditions)
&& $this->evaluateOptionalTier($context, $conditions),
\App\Domain\Pricing\Enums\PricingRuleType::SiblingOrder => $this->evaluateRange($context['sibling_order'], $conditions)
&& $this->evaluateOptionalTier($context, $conditions),
\App\Domain\Pricing\Enums\PricingRuleType::MembershipType => $this->evaluateInList($context['membership_type'] ?? null, $conditions),
\App\Domain\Pricing\Enums\PricingRuleType::Classification => $this->evaluateInList($context['classification'], $conditions),
\App\Domain\Pricing\Enums\PricingRuleType::EnrollmentVolume => $this->evaluateRange($context['enrollment_count'], $conditions),
......@@ -465,6 +467,31 @@ private function evaluateRange(?int $value, array $conditions): bool
return true;
}
/**
* The optional membership-tier filter carried by a sibling or family-size rule.
*
* A club whose sibling price differs for only one tier — the second child of a
* non-resident pays less, a resident's price is already the discounted one —
* has no way to say that with a range alone. Expressing it as a second stacked
* rule would store that tier's base price in a second place, and the day fees
* change and only `base_prices` is updated, the rule silently forces the old
* number back. So the filter lives on the rule that owns the condition.
*
* An absent key means both tiers, which is what every rule authored before this
* existed means — so behaviour is unchanged wherever it was never set.
*/
private function evaluateOptionalTier(array $context, array $conditions): bool
{
if (empty($conditions['membership_type'])) {
return true;
}
return $this->evaluateInList(
$context['membership_type'] ?? null,
['values' => $conditions['membership_type']],
);
}
private function evaluateInList(mixed $value, array $conditions): bool
{
$allowed = $conditions['values'] ?? [];
......
app/Domain/Pricing/Support/ConditionSchema.php
View file @ c1c1e9bc
......@@ -27,12 +27,20 @@
*
* keys — exactly what may appear in the conditions JSONB
* unit — Arabic unit shown beside range inputs
*
* `sibling_order` and `family_size` additionally accept an optional
* `membership_type` list. A club whose sibling price differs for only one
* tier — the second child of a non-resident pays less, a resident's price is
* already the discounted one — cannot say that with a range alone, and
* saying it with a second stacked rule would put that tier's base price in a
* second place, to drift the next time fees change. The key is optional and
* absent everywhere it was never authored, so an existing rule is unaffected.
*/
private const SCHEMA = [
'age' => ['kind' => self::KIND_RANGE, 'keys' => ['min', 'max'], 'unit' => 'سنة'],
'membership_duration' => ['kind' => self::KIND_RANGE, 'keys' => ['min', 'max'], 'unit' => 'شهر'],
'family_size' => ['kind' => self::KIND_RANGE, 'keys' => ['min', 'max'], 'unit' => 'أبناء'],
'sibling_order' => ['kind' => self::KIND_RANGE, 'keys' => ['min', 'max'], 'unit' => 'الترتيب'],
'family_size' => ['kind' => self::KIND_RANGE, 'keys' => ['min', 'max', 'membership_type'], 'unit' => 'أبناء'],
'sibling_order' => ['kind' => self::KIND_RANGE, 'keys' => ['min', 'max', 'membership_type'], 'unit' => 'الترتيب'],
'enrollment_volume' => ['kind' => self::KIND_RANGE, 'keys' => ['min', 'max'], 'unit' => 'برنامج'],
'loyalty' => ['kind' => self::KIND_RANGE, 'keys' => ['min', 'max'], 'unit' => 'شهر'],
'membership_type' => ['kind' => self::KIND_LIST, 'keys' => ['values'], 'unit' => null],
......@@ -88,6 +96,17 @@ public static function normalize(PricingRuleType|string $type, array $input): ar
$out[$k] = (int) $input[$k];
}
}
// Only where the schema declares it — a tier filter on an age or
// loyalty rule would be silently ignored by the engine, so it is
// stripped here rather than stored as a lie.
if (in_array('membership_type', self::keys($type), true)) {
$tiers = $input['membership_type'] ?? [];
$tiers = is_array($tiers) ? $tiers : [$tiers];
$tiers = array_values(array_intersect($tiers, self::MEMBERSHIP_TYPES));
if ($tiers) {
$out['membership_type'] = $tiers;
}
}
break;
case self::KIND_LIST:
......@@ -147,10 +166,13 @@ public static function normalize(PricingRuleType|string $type, array $input): ar
public static function validationRules(PricingRuleType|string $type): array
{
return match (self::kind($type)) {
self::KIND_RANGE => [
self::KIND_RANGE => array_merge([
'conditions.min' => 'nullable|integer|min:0|max:200',
'conditions.max' => 'nullable|integer|min:0|max:200|gte:conditions.min',
],
], in_array('membership_type', self::keys($type), true) ? [
'conditions.membership_type' => 'nullable|array',
'conditions.membership_type.*' => 'string|in:' . implode(',', self::MEMBERSHIP_TYPES),
] : []),
self::KIND_LIST => [
'conditions.values' => 'required|array|min:1',
'conditions.values.*' => 'required|string|max:50',
......@@ -210,7 +232,7 @@ public static function describe(PricingRuleType|string $type, array $c): string
return $noun;
};
return match ($key) {
$sentence = match ($key) {
'age' => $range('للأعمار'),
'membership_duration' => $range('لمدة ' . term('membership_indefinite')),
'family_size' => $range('لأسرة عدد أبنائها'),
......@@ -241,6 +263,17 @@ public static function describe(PricingRuleType|string $type, array $c): string
'custom' => 'يدوي — يُطبق بالاختيار فقط',
default => '',
};
// The optional tier filter, appended rather than folded into the noun so
// the sentence still reads as one clause: "للابن رقم ٢ فأكثر لغير المقيم".
if (! empty($c['membership_type']) && in_array('membership_type', self::keys($key), true)) {
$sentence .= ' ل' . implode('، ', array_map(
fn ($v) => self::membershipTypeLabel($v),
$c['membership_type']
));
}
return $sentence;
}
private static function describeSchedule(array $c): string
......
tests/Unit/SiblingDiscountByMembershipTierTest.php 0 → 100644
View file @ c1c1e9bc
<?php
namespace Tests\Unit;
use App\Domain\Pricing\Models\PricingRule;
use App\Domain\Pricing\Services\PricingService;
use App\Domain\Pricing\Support\ConditionSchema;
use ReflectionMethod;
use Tests\TestCase;
/**
* A sibling discount that applies to only one membership tier.
*
* OC-Sport's compounds charge a resident 2,200 and a non-resident 2,800, and the
* second child of a non-resident 2,400 — while a resident's second child stays at
* 2,200, because the resident price is already the discounted one. A
* `sibling_order` rule could express the "second child" half but not the "and only
* for non-residents" half, and the arithmetic has no shortcut: a flat 400 off
* takes a resident to 1,800, and a percentage that lands on 2,400 from 2,800
* lands on 1,886 from 2,200.
*
* The alternative was a second stacked rule holding `fixed_price 220000` to push
* residents back up — which puts the resident base price in two places. The day
* fees rise and only `base_prices` is edited, that rule silently forces every
* resident back to the old number, with no error and no missing-price failure to
* notice. So the tier filter lives on the rule that owns the condition.
*
* The key is optional. Every rule authored before it existed has no
* `membership_type` in its conditions, so the first test here is the one that
* matters to the other tenants: absent key, unchanged behaviour.
*/
class SiblingDiscountByMembershipTierTest extends TestCase
{
// ---- the engine -------------------------------------------------------
public function test_a_sibling_rule_without_the_tier_key_still_matches_both_tiers(): void
{
$rule = $this->rule('sibling_order', ['min' => 2]);
$this->assertTrue($this->ruleMatches($rule, ['sibling_order' => 2, 'membership_type' => 'member']));
$this->assertTrue($this->ruleMatches($rule, ['sibling_order' => 2, 'membership_type' => 'non_member']));
}
public function test_a_sibling_rule_scoped_to_non_members_skips_a_member(): void
{
$rule = $this->rule('sibling_order', ['min' => 2, 'membership_type' => ['non_member']]);
$this->assertFalse($this->ruleMatches($rule, ['sibling_order' => 2, 'membership_type' => 'member']));
}
public function test_a_sibling_rule_scoped_to_non_members_matches_a_non_member(): void
{
$rule = $this->rule('sibling_order', ['min' => 2, 'membership_type' => ['non_member']]);
$this->assertTrue($this->ruleMatches($rule, ['sibling_order' => 2, 'membership_type' => 'non_member']));
}
public function test_the_tier_filter_does_not_widen_the_range(): void
{
// The right tier is still not enough — a first child is not a sibling.
$rule = $this->rule('sibling_order', ['min' => 2, 'membership_type' => ['non_member']]);
$this->assertFalse($this->ruleMatches($rule, ['sibling_order' => 1, 'membership_type' => 'non_member']));
}
public function test_an_unknown_tier_in_the_context_fails_closed(): void
{
// evaluateInList fails closed, so a rule scoped to a tier never becomes an
// academy-wide discount because the buyer's tier could not be resolved.
$rule = $this->rule('sibling_order', ['min' => 2, 'membership_type' => ['non_member']]);
$this->assertFalse($this->ruleMatches($rule, ['sibling_order' => 2, 'membership_type' => null]));
}
public function test_family_size_carries_the_same_filter(): void
{
$rule = $this->rule('family_size', ['min' => 2, 'membership_type' => ['member']]);
$this->assertTrue($this->ruleMatches($rule, ['family_size' => 3, 'membership_type' => 'member']));
$this->assertFalse($this->ruleMatches($rule, ['family_size' => 3, 'membership_type' => 'non_member']));
}
public function test_both_tiers_listed_is_the_same_as_no_filter(): void
{
$rule = $this->rule('sibling_order', ['min' => 2, 'membership_type' => ['member', 'non_member']]);
$this->assertTrue($this->ruleMatches($rule, ['sibling_order' => 2, 'membership_type' => 'member']));
$this->assertTrue($this->ruleMatches($rule, ['sibling_order' => 2, 'membership_type' => 'non_member']));
}
// ---- the schema -------------------------------------------------------
public function test_normalize_keeps_the_tier_on_a_sibling_rule(): void
{
$out = ConditionSchema::normalize('sibling_order', [
'min' => '2',
'membership_type' => ['non_member'],
]);
$this->assertSame(['min' => 2, 'membership_type' => ['non_member']], $out);
}
public function test_normalize_strips_the_tier_from_a_rule_type_that_cannot_use_it(): void
{
// The engine only consults the filter for sibling_order and family_size.
// Storing it on an age rule would be a condition that reads as a promise
// and is never evaluated.
$out = ConditionSchema::normalize('age', [
'min' => 6,
'membership_type' => ['non_member'],
]);
$this->assertSame(['min' => 6], $out);
}
public function test_normalize_drops_a_tier_value_that_is_not_a_tier(): void
{
$out = ConditionSchema::normalize('sibling_order', [
'min' => 2,
'membership_type' => ['non_member', 'platinum'],
]);
$this->assertSame(['non_member'], $out['membership_type']);
}
public function test_normalize_accepts_a_bare_string_tier(): void
{
$out = ConditionSchema::normalize('sibling_order', [
'min' => 2,
'membership_type' => 'member',
]);
$this->assertSame(['member'], $out['membership_type']);
}
public function test_the_builder_validates_the_tier_only_where_it_is_allowed(): void
{
$this->assertArrayHasKey('conditions.membership_type', ConditionSchema::validationRules('sibling_order'));
$this->assertArrayHasKey('conditions.membership_type', ConditionSchema::validationRules('family_size'));
$this->assertArrayNotHasKey('conditions.membership_type', ConditionSchema::validationRules('age'));
}
public function test_the_sentence_names_the_tier(): void
{
$with = ConditionSchema::describe('sibling_order', ['min' => 2, 'membership_type' => ['non_member']]);
$without = ConditionSchema::describe('sibling_order', ['min' => 2]);
// The rejection reason in the discount picker has to stay a sentence, so
// the tier is appended to the existing clause rather than replacing it.
$this->assertStringContainsString('للابن رقم 2', $with);
$this->assertStringContainsString(ConditionSchema::membershipTypeLabel('non_member'), $with);
$this->assertStringNotContainsString(ConditionSchema::membershipTypeLabel('non_member'), $without);
}
// ---- helpers ----------------------------------------------------------
private function rule(string $type, array $conditions): PricingRule
{
return new PricingRule([
'rule_type' => $type,
'conditions' => $conditions,
]);
}
/**
* evaluateConditions() is private and stays that way — it is engine internals,
* not an API. Reaching it directly keeps these tests free of a database, which
* matters because none of the pricing migrations run on sqlite.
*/
private function ruleMatches(PricingRule $rule, array $context): bool
{
$method = new ReflectionMethod(PricingService::class, 'evaluateConditions');
$method->setAccessible(true);
return $method->invoke(new PricingService(), $rule, $context);
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment