Fix for duplicated/wrong expenses being applied #915 (#1173)

* Fix for duplicated/wrong expenses being applied #915

* Style fixes

app/Models/Expense.php

@@ -13,6 +13,7 @@ use App\Models\Traits\ReferenceTrait;
  * @property string flight_type
  * @property string ref_model
  * @property string ref_model_id
+ * @property bool   charge_to_user
  *
  * @mixin \Illuminate\Database\Eloquent\Builder
  */
@@ -36,7 +37,7 @@ class Expense extends Model
     ];
 
     public static $rules = [
-        'active'         => 'boolean',
+        'active'         => 'bool',
         'airline_id'     => 'integer',
         'amount'         => 'float',
         'multiplier'     => 'bool',

app/Services/Finance/PirepFinanceService.php

@@ -304,20 +304,29 @@ class PirepFinanceService extends Service
             }
 
             // Form the memo, with some specific ones depending on the group
-            if ($expense->ref_model === Subfleet::class
+            if ($expense->ref_model === Subfleet::class) {
-                && $expense->ref_model_id === $pirep->aircraft->subfleet->id
+                if ((int) ($expense->ref_model_id) === $pirep->aircraft->subfleet->id) {
-            ) {
+                    $memo = "Subfleet Expense: $expense->name ({$pirep->aircraft->subfleet->name}) dd";
-                $memo = "Subfleet Expense: {$expense->name} ({$pirep->aircraft->subfleet->name})";
+                    $transaction_group = "Subfleet: $expense->name ({$pirep->aircraft->subfleet->name})";
-                $transaction_group = "Subfleet: {$expense->name} ({$pirep->aircraft->subfleet->name})";
+                } else { // Skip any subfleets that weren't used for this flight
-            } elseif ($expense->ref_model === Aircraft::class
+                    return;
-                      && $expense->ref_model_id === $pirep->aircraft->id
+                }
-            ) {
+            } elseif ($expense->ref_model === Aircraft::class) {
-                $memo = "Aircraft Expense: {$expense->name} ({$pirep->aircraft->name})";
+                if ((int) ($expense->ref_model_id) === $pirep->aircraft->id) {
-                $transaction_group = "Aircraft: {$expense->name} "
+                    $memo = "Aircraft Expense: $expense->name ({$pirep->aircraft->name})";
-                    ."({$pirep->aircraft->name}-{$pirep->aircraft->registration})";
+                    $transaction_group = "Aircraft: $expense->name "
+                        ."({$pirep->aircraft->name}-{$pirep->aircraft->registration})";
+                } else { // Skip any aircraft expenses that weren't used for this flight
+                    return;
+                }
             } else {
-                $memo = "Expense: {$expense->name}";
+                // Skip any expenses that aren't for the airline this flight was for
-                $transaction_group = "Expense: {$expense->name}";
+                if ($expense->airline_id && $expense->airline_id !== $pirep->airline_id) {
+                    return;
+                }
+
+                $memo = "Expense: $expense->name";
+                $transaction_group = "Expense: $expense->name";
             }
 
             $debit = Money::createFromAmount($expense->amount);
@@ -360,8 +369,8 @@ class PirepFinanceService extends Service
         $expenses->map(function (Expense $expense, $i) use ($pirep) {
             Log::info('Finance: PIREP: '.$pirep->id.', airport expense:', $expense->toArray());
 
-            $memo = "Airport Expense: {$expense->name} ({$expense->ref_model_id})";
+            $memo = "Airport Expense: $expense->name ($expense->ref_model_id)";
-            $transaction_group = "Airport: {$expense->ref_model_id}";
+            $transaction_group = "Airport: $expense->ref_model_id";
 
             $debit = Money::createFromAmount($expense->amount);
 

tests/FinanceTest.php

@@ -11,6 +11,7 @@ use App\Models\Expense;
 use App\Models\Fare;
 use App\Models\Flight;
 use App\Models\Journal;
+use App\Models\JournalTransaction;
 use App\Models\Pirep;
 use App\Models\Subfleet;
 use App\Models\User;
@@ -81,28 +82,31 @@ class FinanceTest extends TestCase
 
         $this->fleetSvc->addSubfleetToRank($subfleet['subfleet'], $rank);
 
+        /** @var Airport $dpt_apt */
         $dpt_apt = factory(Airport::class)->create([
             'ground_handling_cost' => 10,
             'fuel_jeta_cost'       => 10,
         ]);
 
+        /** @var Airport $arr_apt */
         $arr_apt = factory(Airport::class)->create([
             'ground_handling_cost' => 10,
             'fuel_jeta_cost'       => 10,
         ]);
 
-        /** @var \App\Models\User $user */
+        /** @var User $user */
         $user = factory(User::class)->create([
             'rank_id' => $rank->id,
         ]);
 
-        /** @var \App\Models\Flight $flight */
+        /** @var Flight $flight */
         $flight = factory(Flight::class)->create([
             'airline_id'     => $user->airline_id,
             'dpt_airport_id' => $dpt_apt->icao,
             'arr_airport_id' => $arr_apt->icao,
         ]);
 
+        /** @var Pirep $pirep */
         $pirep = factory(Pirep::class)->create([
             'flight_number'  => $flight->flight_number,
             'flight_type'    => FlightType::SCHED_PAX,
@@ -124,6 +128,7 @@ class FinanceTest extends TestCase
          * Add fares to the subfleet, and then add the fares
          * to the PIREP when it's saved, and set the capacity
          */
+        /** @var Fare $fares */
         $fares = factory(Fare::class, 3)->create([
             'price'    => 100,
             'cost'     => 50,
@@ -781,7 +786,10 @@ class FinanceTest extends TestCase
      */
     public function testPirepExpenses()
     {
+        /** @var Airline $airline */
         $airline = factory(Airline::class)->create();
+
+        /** @var Airline $airline2 */
         $airline2 = factory(Airline::class)->create();
 
         factory(Expense::class)->create([
@@ -1014,4 +1022,96 @@ class FinanceTest extends TestCase
             $this->assertEquals($count, $find->count());
         }
     }
+
+    /**
+     * @throws Exception
+     */
+    public function testPirepFinancesExpensesMultiAirline()
+    {
+        /** @var Airline $airline */
+        $airline = factory(Airline::class)->create();
+
+        $journalRepo = app(JournalRepository::class);
+
+        // Add an expense that's only for a cargo flight
+        factory(Expense::class)->create(
+            [
+                'airline_id'  => null,
+                'amount'      => 100,
+                'flight_type' => FlightType::SCHED_CARGO,
+            ]
+        );
+
+        [$user, $pirep, $fares] = $this->createFullPirep();
+        $user->airline->initJournal(setting('units.currency', 'USD'));
+
+        factory(Expense::class)->create(
+            [
+                'airline_id'  => $user->airline->id,
+                'amount'      => 100,
+                'flight_type' => FlightType::SCHED_CARGO,
+            ]
+        );
+
+        factory(Expense::class)->create(
+            [
+                'airline_id'  => $airline->id,
+                'amount'      => 100,
+                'flight_type' => FlightType::SCHED_CARGO,
+            ]
+        );
+
+        // There shouldn't be an expense from this subfleet
+        /** @var Subfleet $subfleet */
+        $subfleet = factory(Subfleet::class)->create();
+        factory(Expense::class)->create([
+            'airline_id'   => null,
+            'amount'       => 100,
+            'ref_model'    => Subfleet::class,
+            'ref_model_id' => $subfleet->id,
+        ]);
+
+        // Override the fares
+        $fare_counts = [];
+        foreach ($fares as $fare) {
+            $fare_counts[] = [
+                'fare_id' => $fare->id,
+                'price'   => $fare->price,
+                'count'   => 100,
+            ];
+        }
+
+        $this->fareSvc->saveForPirep($pirep, $fare_counts);
+
+        // This should process all of the
+        $pirep = $this->pirepSvc->accept($pirep);
+
+        $transactions = $journalRepo->getAllForObject($pirep);
+
+        /** @var JournalTransaction $transaction */
+        /*foreach ($transactions['transactions'] as $transaction) {
+            echo $transaction->memo."-"."\n";
+        }*/
+
+        // Check that all the different transaction types are there
+        // test by the different groups that exist
+        $transaction_tags = [
+            'fuel'            => 1,
+            'airport'         => 1,
+            'expense'         => 1,
+            'subfleet'        => 2,
+            'fare'            => 3,
+            'ground_handling' => 2,
+            'pilot_pay'       => 2, // debit on the airline, credit to the pilot
+        ];
+
+        foreach ($transaction_tags as $type => $count) {
+            $find = $transactions['transactions']->where('tags', $type);
+            $this->assertEquals($count, $find->count(), $type);
+        }
+
+        //        $this->assertCount(9, $transactions['transactions']);
+        $this->assertEquals(3020, $transactions['credits']->getValue());
+        $this->assertEquals(2050.0, $transactions['debits']->getValue());
+    }
 }