From 6e6ba850800280efcd98b8eb5fd7023e7f72e16a Mon Sep 17 00:00:00 2001 From: Nabeel S Date: Thu, 6 May 2021 12:42:56 -0400 Subject: [PATCH] Fix for duplicated/wrong expenses being applied #915 (#1173) * Fix for duplicated/wrong expenses being applied #915 * Style fixes --- app/Models/Expense.php | 3 +- app/Services/Finance/PirepFinanceService.php | 39 ++++--- tests/FinanceTest.php | 104 ++++++++++++++++++- 3 files changed, 128 insertions(+), 18 deletions(-) diff --git a/app/Models/Expense.php b/app/Models/Expense.php index cf36f46f..23063bed 100644 --- a/app/Models/Expense.php +++ b/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', diff --git a/app/Services/Finance/PirepFinanceService.php b/app/Services/Finance/PirepFinanceService.php index 41ea80ed..d8c6a35d 100644 --- a/app/Services/Finance/PirepFinanceService.php +++ b/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 - && $expense->ref_model_id === $pirep->aircraft->subfleet->id - ) { - $memo = "Subfleet Expense: {$expense->name} ({$pirep->aircraft->subfleet->name})"; - $transaction_group = "Subfleet: {$expense->name} ({$pirep->aircraft->subfleet->name})"; - } elseif ($expense->ref_model === Aircraft::class - && $expense->ref_model_id === $pirep->aircraft->id - ) { - $memo = "Aircraft Expense: {$expense->name} ({$pirep->aircraft->name})"; - $transaction_group = "Aircraft: {$expense->name} " - ."({$pirep->aircraft->name}-{$pirep->aircraft->registration})"; + if ($expense->ref_model === Subfleet::class) { + if ((int) ($expense->ref_model_id) === $pirep->aircraft->subfleet->id) { + $memo = "Subfleet Expense: $expense->name ({$pirep->aircraft->subfleet->name}) dd"; + $transaction_group = "Subfleet: $expense->name ({$pirep->aircraft->subfleet->name})"; + } else { // Skip any subfleets that weren't used for this flight + return; + } + } elseif ($expense->ref_model === Aircraft::class) { + if ((int) ($expense->ref_model_id) === $pirep->aircraft->id) { + $memo = "Aircraft Expense: $expense->name ({$pirep->aircraft->name})"; + $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}"; - $transaction_group = "Expense: {$expense->name}"; + // Skip any expenses that aren't for the airline this flight was for + 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})"; - $transaction_group = "Airport: {$expense->ref_model_id}"; + $memo = "Airport Expense: $expense->name ($expense->ref_model_id)"; + $transaction_group = "Airport: $expense->ref_model_id"; $debit = Money::createFromAmount($expense->amount); diff --git a/tests/FinanceTest.php b/tests/FinanceTest.php index deb64574..466957d9 100644 --- a/tests/FinanceTest.php +++ b/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()); + } }