feat(pricing): scope a discount to members or to non-members, and let a...

feat(pricing): scope a discount to members or to non-members, and let a branch-targeted one save at all

Membership tier decided only which base price was read; it was invisible to
every rule. A club wanting '10% off, members only' had no way to say it.
The nearest rule type, membership_duration, is tenure in months, which is a
different question and happily matches a non-member who has been around a
while. New membership_type rule type, with the CHECK constraint widened to
admit it — no existing row changes value, and the new one is unreachable
until a rule is authored with it.

The tier the rule matches is the same one that chose the base price, so a
discount and the price it discounts cannot disagree about who this is. An
unset tier reads as non_member, matching step 1, so a 'members only' rule
cannot quietly reach someone nobody ever classified.

And the bug the test for it found: NO branch-targeted discount could be
saved. pricing_rule_branches.academy_id is NOT NULL and the pivot has no
model, so no BelongsToAcademy hook filled it and a bare sync() died on a
not-null violation — every rule authored in the wizard with a branch
ticked. Stamped by hand, the way ProgramForm already does for
program_products.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

app/Domain/Pricing/Enums/PricingRuleType.php

@@ -6,6 +6,14 @@
 {
     case Age = 'age';
     case MembershipDuration = 'membership_duration';
+
+    /**
+     * Member vs non-member. Distinct from MembershipDuration, which is tenure
+     * in months and says nothing about whether the participant is a member at
+     * all.
+     */
+    case MembershipType = 'membership_type';
+
     case FamilySize = 'family_size';
     case SiblingOrder = 'sibling_order';
     case Classification = 'classification';
@@ -23,6 +31,9 @@ public function label(): string
         return match ($this) {
             self::Age => 'العمر',
             self::MembershipDuration => 'مدة ' . term('membership'),
+            // "عضو / غير عضو" in a club, "مقيم / غير مقيم" in a compound — the
+            // branch names its own people, so the label is composed, not typed.
+            self::MembershipType => term('member') . ' / ' . term('non_member'),
             self::FamilySize => 'حجم الأسرة',
             self::SiblingOrder => 'ترتيب الأخوة',
             self::Classification => 'التصنيف',

app/Domain/Pricing/Services/PricingService.php

@@ -75,9 +75,14 @@ public function calculate(
         // participant row exists — otherwise the busiest flow cannot use the engine.
         if ($contextOverride !== null) {
             $context = array_merge($contextOverride, $extraContext);
+            // The tier was already resolved above for the base price. A
+            // membership_type rule must read the same answer, or the discount
+            // and the price it discounts would disagree about who this is.
+            $context['membership_type'] ??= $membershipType;
             $rules = $rules->filter(fn (PricingRule $rule) => $this->evaluateConditions($rule, $context));
         } elseif ($participant) {
             $context = array_merge($this->buildParticipantContext($participant), $extraContext);
+            $context['membership_type'] ??= $membershipType;
             $rules = $rules->filter(fn (PricingRule $rule) => $this->evaluateConditions($rule, $context));
         } else {
             // No participant and no context: only unconditional rules apply
@@ -364,6 +369,10 @@ private function buildParticipantContext(Participant $participant): array
             'age' => $age,
             'gender' => $person?->gender,
             'classification' => $participant->classification ?? 'regular',
+            // Same default the base-price lookup uses at step 1: an unset tier
+            // is a non-member, so a "members only" rule cannot quietly reach
+            // someone nobody ever classified.
+            'membership_type' => $participant->membership_type?->value ?? 'non_member',
             'membership_duration_months' => $membershipMonths,
             'family_size' => $familySize,
             'sibling_order' => $siblingOrder,
@@ -422,6 +431,7 @@ private function evaluateConditions(PricingRule $rule, array $context): bool
             \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::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),
             \App\Domain\Pricing\Enums\PricingRuleType::Gender => $this->evaluateInList($context['gender'], $conditions),

app/Domain/Pricing/Support/ConditionSchema.php

@@ -35,6 +35,7 @@
         'sibling_order'       => ['kind' => self::KIND_RANGE,  'keys' => ['min', 'max'], '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],
         'classification'      => ['kind' => self::KIND_LIST,   'keys' => ['values'],     'unit' => null],
         'gender'              => ['kind' => self::KIND_LIST,   'keys' => ['values'],     'unit' => null],
         'branch'              => ['kind' => self::KIND_LIST,   'keys' => ['values'],     'unit' => null],
@@ -46,6 +47,7 @@
 
     /** Allowed values for list-kind rule types. */
     public const CLASSIFICATIONS = ['regular', 'vip', 'scholarship', 'staff_child', 'trial'];
+    public const MEMBERSHIP_TYPES = ['member', 'non_member'];
     public const GENDERS = ['male', 'female'];
     public const WEEKDAYS = ['saturday', 'sunday', 'monday', 'tuesday', 'wednesday', 'thursday', 'friday'];
 
@@ -215,6 +217,10 @@ public static function describe(PricingRuleType|string $type, array $c): string
             'sibling_order'       => $range('للابن رقم'),
             'enrollment_volume'   => $range('لعدد برامج'),
             'loyalty'             => $range('ل' . term('membership_indefinite')),
+            'membership_type'     => 'لـ' . implode('، ', array_map(
+                fn ($v) => self::membershipTypeLabel($v),
+                $c['values'] ?? []
+            )),
             'classification'      => 'لتصنيف: ' . implode('، ', array_map(
                 fn ($v) => self::classificationLabel($v),
                 $c['values'] ?? []
@@ -254,6 +260,21 @@ private static function describeSchedule(array $c): string
         return $parts ? implode(' ', $parts) : 'في مواعيد محددة';
     }
 
+    /**
+     * The branch's own word for the tier — عضو in a club, مقيم in a compound,
+     * مشترك on a beach. Read through term() rather than hardcoded, so a rule
+     * authored in one branch still reads correctly in the vocabulary of the
+     * branch looking at it.
+     */
+    public static function membershipTypeLabel(string $v): string
+    {
+        return match ($v) {
+            'member'     => term('member'),
+            'non_member' => term('non_member'),
+            default      => $v,
+        };
+    }
+
     public static function classificationLabel(string $v): string
     {
         return match ($v) {

app/Livewire/Pricing/CreatePricingRuleWizard.php

@@ -321,6 +321,13 @@ public function rules(): array
             ];
         }
 
+        // Same reasoning as the branch clause: conditions is a plain public
+        // array. A tier the engine has never heard of would simply match nobody
+        // — a discount that silently does nothing is worse than a rejected one.
+        if ($this->ruleType === PricingRuleType::MembershipType->value) {
+            $rules['conditions.values.*'] = ['required', Rule::in(ConditionSchema::MEMBERSHIP_TYPES)];
+        }
+
         return $rules;
     }
 
@@ -402,7 +409,15 @@ public function save(): void
                 }
 
                 if ($this->branchIds) {
-                    $rule->branches()->sync($this->branchIds);
+                    // academy_id is stamped by hand: pricing_rule_branches has
+                    // no model, so no BelongsToAcademy hook fills it, and the
+                    // column is NOT NULL. A bare sync() therefore failed with a
+                    // not-null violation — every branch-targeted discount
+                    // authored in this wizard died on save.
+                    $rule->branches()->sync(array_fill_keys(
+                        $this->branchIds,
+                        ['academy_id' => $rule->academy_id],
+                    ));
                 }
 
                 return $rule;

database/migrations/2026_09_14_000001_allow_membership_type_pricing_rules.php

+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Schema;
+
+/**
+ * Let a discount be scoped to members or to non-members.
+ *
+ * Until now the only thing the tier decided was which base price was read.
+ * A club that wanted "10% off, members only" had no way to say it: the closest
+ * rule type, membership_duration, is tenure in months, which is a different
+ * question and matches non-members who have simply been around a while.
+ *
+ * This only widens a CHECK constraint. No existing row changes value, and the
+ * new value is unreachable until a rule is authored with it.
+ */
+return new class extends Migration
+{
+    private const CONSTRAINT = 'pricing_rules_rule_type_check';
+
+    private const TYPES = [
+        'age', 'membership_duration', 'membership_type', 'family_size',
+        'sibling_order', 'classification', 'enrollment_timing',
+        'enrollment_volume', 'seasonal', 'gender', 'branch', 'day_time',
+        'loyalty', 'custom',
+    ];
+
+    public function up(): void
+    {
+        if (! Schema::hasTable('pricing_rules')) {
+            return;
+        }
+
+        $values = "'" . implode("', '", self::TYPES) . "'";
+
+        // Dropped by name and rebuilt rather than altered — Postgres has no
+        // ALTER CONSTRAINT for a CHECK. IF EXISTS so this is safe on a database
+        // that somehow never got the original.
+        DB::statement('ALTER TABLE pricing_rules DROP CONSTRAINT IF EXISTS ' . self::CONSTRAINT);
+        DB::statement(
+            'ALTER TABLE pricing_rules ADD CONSTRAINT ' . self::CONSTRAINT
+            . " CHECK (rule_type IN ({$values}))"
+        );
+    }
+
+    public function down(): void
+    {
+        if (! Schema::hasTable('pricing_rules')) {
+            return;
+        }
+
+        // Rules authored against the new value would violate the narrower
+        // constraint, so they are retired first. Deactivating rather than
+        // deleting: a discount someone applied is part of an invoice's history.
+        DB::table('pricing_rules')
+            ->where('rule_type', 'membership_type')
+            ->update(['is_active' => false, 'rule_type' => 'custom']);
+
+        $values = "'" . implode("', '", array_diff(self::TYPES, ['membership_type'])) . "'";
+
+        DB::statement('ALTER TABLE pricing_rules DROP CONSTRAINT IF EXISTS ' . self::CONSTRAINT);
+        DB::statement(
+            'ALTER TABLE pricing_rules ADD CONSTRAINT ' . self::CONSTRAINT
+            . " CHECK (rule_type IN ({$values}))"
+        );
+    }
+};

resources/views/livewire/pricing/partials/condition-fields.blade.php

@@ -28,6 +28,7 @@ class="w-full px-3 py-2 border border-gray-300 rounded-lg focus:ring-2 focus:rin
         $options = match($ruleType) {
             'gender' => collect(ConditionSchema::GENDERS)->mapWithKeys(fn($g) => [$g => $g === 'male' ? __('ذكور') : __('إناث')]),
             'classification' => collect(ConditionSchema::CLASSIFICATIONS)->mapWithKeys(fn($c) => [$c => ConditionSchema::classificationLabel($c)]),
+            'membership_type' => collect(ConditionSchema::MEMBERSHIP_TYPES)->mapWithKeys(fn($t) => [$t => ConditionSchema::membershipTypeLabel($t)]),
             default => $branches->mapWithKeys(fn($b) => [(string) $b->id => $b->name_ar]),
         };
     @endphp

resources/views/livewire/pricing/pricing-rule-form.blade.php

@@ -208,6 +208,25 @@ class="w-full max-w-xs px-4 py-2.5 text-sm border border-gray-300 rounded-lg foc
                             </div>
                             @break
 
+                        @case('membership_type')
+                            <div>
+                                <label class="block text-sm font-medium text-gray-700 mb-1">{{ __('يُطبق على') }}</label>
+                                {{-- conditions.values, not a bespoke key: ConditionSchema::normalize()
+                                     strips anything else, and a stripped condition means a discount
+                                     that applies to everyone. --}}
+                                <div class="flex flex-wrap gap-3 mt-2">
+                                    @foreach(\App\Domain\Pricing\Support\ConditionSchema::MEMBERSHIP_TYPES as $val)
+                                    <label class="min-h-[44px] inline-flex items-center gap-2">
+                                        <input type="checkbox" value="{{ $val }}"
+                                               x-model="conditions.values"
+                                               class="w-4 h-4 rounded border-gray-300 text-blue-600 focus:ring-blue-500">
+                                        <span class="text-sm text-gray-700">{{ \App\Domain\Pricing\Support\ConditionSchema::membershipTypeLabel($val) }}</span>
+                                    </label>
+                                    @endforeach
+                                </div>
+                            </div>
+                            @break
+
                         @case('classification')
                             <div>
                                 <label class="block text-sm font-medium text-gray-700 mb-1">{{ __('التصنيفات المؤهلة') }}</label>

tests/Feature/DiscountAndBranchScreensRenderTest.php

+<?php
+
+namespace Tests\Feature;
+
+use App\Domain\Identity\Models\Branch;
+use App\Domain\Identity\Services\BranchSettingsService;
+use App\Domain\Pricing\Enums\PricingRuleType;
+use App\Domain\Pricing\Support\ConditionSchema;
+use App\Models\User;
+use Livewire\Livewire;
+use Tests\TestCase;
+
+/**
+ * The screens where a discount is authored and a branch is configured.
+ *
+ * Both grew a new control that nothing else renders: the member/non-member
+ * condition on the discount builder, and the "this branch bills nothing"
+ * toggle on the branch form. A rule type whose condition inputs no view knows
+ * how to draw is a blank step the desk cannot get past, and neither the wizard
+ * nor the branch form was covered by any render test.
+ *
+ * Runs against a restored Postgres tenant; skips on the default SQLite suite:
+ *
+ *   DB_CONNECTION=pgsql DB_DATABASE=oc_sport_test ./vendor/bin/phpunit --filter DiscountAndBranchScreensRenderTest
+ */
+class DiscountAndBranchScreensRenderTest extends TestCase
+{
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        if (config('database.default') !== 'pgsql') {
+            $this->markTestSkipped('Needs a restored Postgres tenant; see the class comment.');
+        }
+
+        $academy = \App\Domain\Shared\Models\Academy::query()->first();
+
+        if (! $academy) {
+            $this->markTestSkipped('No academy in the restored tenant.');
+        }
+
+        app()->instance('current_academy', $academy);
+    }
+
+    private function anOwner(): User
+    {
+        $user = User::withoutGlobalScopes()
+            ->whereHas('primaryRole', fn ($q) => $q->where('slug', 'academy_owner'))
+            ->first();
+
+        if (! $user) {
+            $this->markTestSkipped('No academy_owner in the restored tenant.');
+        }
+
+        return $user;
+    }
+
+    public function test_the_discount_wizard_renders(): void
+    {
+        $this->actingAs($this->anOwner())
+            ->get(route('pricing.rules.wizard'))
+            ->assertOk();
+    }
+
+    public function test_the_membership_tier_condition_draws_its_own_choices(): void
+    {
+        // The list-kind partial falls through to a branch picker for any rule
+        // type it does not recognise, so an unrecognised one renders a step
+        // offering branches where it should offer members.
+        Livewire::actingAs($this->anOwner())
+            ->test(\App\Livewire\Pricing\CreatePricingRuleWizard::class)
+            ->call('startBlank')
+            ->set('ruleType', PricingRuleType::MembershipType->value)
+            ->assertOk()
+            ->assertSee(ConditionSchema::membershipTypeLabel('member'), escape: false)
+            ->assertSee(ConditionSchema::membershipTypeLabel('non_member'), escape: false);
+    }
+
+    public function test_a_members_only_discount_saves_with_its_condition_intact(): void
+    {
+        $branchId = Branch::query()->value('id');
+
+        $component = Livewire::actingAs($this->anOwner())
+            ->test(\App\Livewire\Pricing\CreatePricingRuleWizard::class)
+            ->call('startBlank')
+            ->set('ruleType', PricingRuleType::MembershipType->value)
+            ->set('nameAr', 'خصم اختبار للأعضاء')
+            ->set('adjustmentType', 'percentage_discount')
+            ->set('amount', '10')
+            ->set('conditions', ['values' => ['member']])
+            ->set('branchIds', $branchId ? [$branchId] : [])
+            ->set('effectiveFrom', now()->toDateString())
+            ->call('save')
+            ->assertHasNoErrors();
+
+        $rule = \App\Domain\Pricing\Models\PricingRule::withoutGlobalScopes()
+            ->where('name_ar', 'خصم اختبار للأعضاء')
+            ->latest('id')
+            ->first();
+
+        $this->assertNotNull($rule, 'The wizard reported success but wrote nothing.');
+        $this->assertSame(['values' => ['member']], $rule->conditions, 'ConditionSchema::normalize() dropped the condition.');
+
+        if ($branchId) {
+            // pricing_rule_branches.academy_id is NOT NULL and the pivot has no
+            // model to stamp it, so a bare sync() threw and no branch-targeted
+            // discount could be saved at all.
+            $this->assertSame(
+                [$branchId],
+                $rule->branches()->pluck('branches.id')->all(),
+                'The rule saved without the branch it was targeted at.'
+            );
+            $this->assertNotNull(
+                \Illuminate\Support\Facades\DB::table('pricing_rule_branches')
+                    ->where('pricing_rule_id', $rule->id)
+                    ->value('academy_id'),
+                'The pivot row carries no academy.'
+            );
+        }
+
+        $rule->branches()->detach();
+        $rule->forceDelete();
+
+        unset($component);
+    }
+
+    public function test_a_tier_the_engine_never_heard_of_is_refused(): void
+    {
+        Livewire::actingAs($this->anOwner())
+            ->test(\App\Livewire\Pricing\CreatePricingRuleWizard::class)
+            ->call('startBlank')
+            ->set('ruleType', PricingRuleType::MembershipType->value)
+            ->set('nameAr', 'خصم اختبار مرفوض')
+            ->set('amount', '10')
+            ->set('conditions', ['values' => ['vip']])
+            ->set('effectiveFrom', now()->toDateString())
+            ->call('save')
+            ->assertHasErrors('conditions.values.0');
+    }
+
+    public function test_the_branch_form_renders_and_offers_the_billing_toggle(): void
+    {
+        $this->actingAs($this->anOwner())
+            ->get(route('branches.create'))
+            ->assertOk()
+            ->assertSee('الإيرادات بتتحصل خارج النظام', escape: false);
+    }
+
+    public function test_the_billing_question_can_be_asked_from_the_console(): void
+    {
+        $branch = Branch::query()->first();
+
+        $this->assertNotNull($branch, 'The restored tenant has no branches.');
+
+        // GenerateRenewalInvoices asks this per enrolment, from a scheduled
+        // command that binds no academy. Reading it through the academy
+        // fallback threw "Target class [current_academy] does not exist" and
+        // would have taken the nightly billing run down with it.
+        app()->forgetInstance('current_academy');
+
+        $this->assertIsBool(app(BranchSettingsService::class)->billingHandledExternally($branch->id));
+    }
+
+    public function test_the_billing_toggle_round_trips_through_branch_settings(): void
+    {
+        $branch = Branch::query()->first();
+
+        $this->assertNotNull($branch, 'The restored tenant has no branches.');
+
+        $settings = app(BranchSettingsService::class);
+        $before = $settings->billingHandledExternally($branch->id);
+
+        try {
+            $settings->set($branch->id, BranchSettingsService::KEY_BILLING_EXTERNAL, '1', 'financial');
+
+            // A fresh instance, because the service memoises per request.
+            $this->assertTrue(app(BranchSettingsService::class)->billingHandledExternally($branch->id));
+
+            $settings->set($branch->id, BranchSettingsService::KEY_BILLING_EXTERNAL, '0', 'financial');
+            $this->assertFalse(app(BranchSettingsService::class)->billingHandledExternally($branch->id));
+        } finally {
+            $settings->set(
+                $branch->id,
+                BranchSettingsService::KEY_BILLING_EXTERNAL,
+                $before ? '1' : '0',
+                'financial'
+            );
+        }
+    }
+}