From 9d3953f3acb70b441f86374f138d4a6bf707f10f Mon Sep 17 00:00:00 2001 From: Nabeel Shahzad Date: Mon, 5 Mar 2018 22:49:42 -0600 Subject: [PATCH] Refactor expenses; move finance service classes; add daily/monthly skeletons #130 #136 --- app/Database/factories/ExpenseFactory.php | 4 +- app/Database/factories/NavdataFactory.php | 2 +- app/Database/factories/UserFactory.php | 2 +- ...018_02_26_185121_create_expenses_table.php | 4 +- app/Database/seeds/sample.yml | 9 - .../Controllers/Admin/FinanceController.php | 6 +- app/Http/Controllers/Api/PirepController.php | 6 +- app/Listeners/FinanceEvents.php | 4 +- app/Repositories/ExpenseRepository.php | 5 - app/Services/Finance/DailyFinanceService.php | 20 ++ .../Finance/MonthlyFinanceService.php | 20 ++ .../PirepFinanceService.php} | 183 +++++++----------- app/Support/Math.php | 22 ++- tests/FinanceTest.php | 32 ++- 14 files changed, 164 insertions(+), 155 deletions(-) create mode 100644 app/Services/Finance/DailyFinanceService.php create mode 100644 app/Services/Finance/MonthlyFinanceService.php rename app/Services/{FinanceService.php => Finance/PirepFinanceService.php} (79%) diff --git a/app/Database/factories/ExpenseFactory.php b/app/Database/factories/ExpenseFactory.php index e074e3a9..15ae4fe9 100644 --- a/app/Database/factories/ExpenseFactory.php +++ b/app/Database/factories/ExpenseFactory.php @@ -6,9 +6,7 @@ use Faker\Generator as Faker; $factory->define(App\Models\Expense::class, function (Faker $faker) { return [ 'id' => null, - 'airline_id' => function () { - return factory(App\Models\Airline::class)->create()->id; - }, + 'airline_id' => null, 'name' => $faker->text(20), 'amount' => $faker->randomFloat(2, 100, 1000), 'type' => ExpenseType::FLIGHT, diff --git a/app/Database/factories/NavdataFactory.php b/app/Database/factories/NavdataFactory.php index 929f2189..dedebbda 100644 --- a/app/Database/factories/NavdataFactory.php +++ b/app/Database/factories/NavdataFactory.php @@ -1,7 +1,7 @@ define(App\Models\Navdata::class, function (Faker $faker) { return [ diff --git a/app/Database/factories/UserFactory.php b/app/Database/factories/UserFactory.php index 690aa9c1..2ac82906 100644 --- a/app/Database/factories/UserFactory.php +++ b/app/Database/factories/UserFactory.php @@ -1,7 +1,7 @@ define(App\Models\User::class, function (Faker $faker) { diff --git a/app/Database/migrations/2018_02_26_185121_create_expenses_table.php b/app/Database/migrations/2018_02_26_185121_create_expenses_table.php index 5098782a..1a6b58c2 100644 --- a/app/Database/migrations/2018_02_26_185121_create_expenses_table.php +++ b/app/Database/migrations/2018_02_26_185121_create_expenses_table.php @@ -1,8 +1,8 @@ airlineRepo = $airlineRepo; diff --git a/app/Http/Controllers/Api/PirepController.php b/app/Http/Controllers/Api/PirepController.php index a282a33f..43c108ca 100644 --- a/app/Http/Controllers/Api/PirepController.php +++ b/app/Http/Controllers/Api/PirepController.php @@ -26,7 +26,7 @@ use App\Models\PirepComment; use App\Repositories\AcarsRepository; use App\Repositories\JournalRepository; use App\Repositories\PirepRepository; -use App\Services\FinanceService; +use App\Services\Finance\PirepFinanceService; use App\Services\GeoService; use App\Services\PIREPService; use App\Services\UserService; @@ -47,7 +47,7 @@ class PirepController extends RestController /** * PirepController constructor. * @param AcarsRepository $acarsRepo - * @param FinanceService $financeSvc + * @param PirepFinanceService $financeSvc * @param GeoService $geoSvc * @param JournalRepository $journalRepo * @param PirepRepository $pirepRepo @@ -56,7 +56,7 @@ class PirepController extends RestController */ public function __construct( AcarsRepository $acarsRepo, - FinanceService $financeSvc, + PirepFinanceService $financeSvc, GeoService $geoSvc, JournalRepository $journalRepo, PirepRepository $pirepRepo, diff --git a/app/Listeners/FinanceEvents.php b/app/Listeners/FinanceEvents.php index e322ee6e..2fb3e1dc 100644 --- a/app/Listeners/FinanceEvents.php +++ b/app/Listeners/FinanceEvents.php @@ -4,7 +4,7 @@ namespace App\Listeners; use App\Events\PirepAccepted; use App\Events\PirepRejected; -use App\Services\FinanceService; +use App\Services\Finance\PirepFinanceService; /** * Subscribe for events that we do some financial processing for @@ -16,7 +16,7 @@ class FinanceEvents private $financeSvc; public function __construct( - FinanceService $financeSvc + PirepFinanceService $financeSvc ) { $this->financeSvc = $financeSvc; } diff --git a/app/Repositories/ExpenseRepository.php b/app/Repositories/ExpenseRepository.php index ad0b39da..b215899c 100644 --- a/app/Repositories/ExpenseRepository.php +++ b/app/Repositories/ExpenseRepository.php @@ -37,8 +37,6 @@ class ExpenseRepository extends BaseRepository implements CacheableInterface if($ref_class) { $where['ref_class'] = $ref_class; - } else { - $where[] = ['ref_class', '=', null]; } $expenses = $this->findWhere($where); @@ -52,12 +50,9 @@ class ExpenseRepository extends BaseRepository implements CacheableInterface if ($ref_class) { $where['ref_class'] = $ref_class; - } else { - $where[] = ['ref_class', '=', null]; } $airline_expenses = $this->findWhere($where); - $expenses = $expenses->concat($airline_expenses); } diff --git a/app/Services/Finance/DailyFinanceService.php b/app/Services/Finance/DailyFinanceService.php new file mode 100644 index 00000000..46efa754 --- /dev/null +++ b/app/Services/Finance/DailyFinanceService.php @@ -0,0 +1,20 @@ +pirepSvc = $pirepSvc; } - /** - * Determine from the base rate, if we want to return the overridden rate - * or if the overridden rate is a percentage, then return that amount - * @param $base_rate - * @param $override_rate - * @return float|null - */ - public function applyAmountOrPercent($base_rate, $override_rate=null): ?float - { - if (!$override_rate) { - return $base_rate; - } - - # Not a percentage override - if (substr_count($override_rate, '%') === 0) { - return $override_rate; - } - - return Math::addPercent($base_rate, $override_rate); - } - /** * Process all of the finances for a pilot report. This is called * from a listener (FinanceEvents) @@ -92,7 +73,7 @@ class FinanceService extends BaseService # Now start and pay from scratch $this->payFaresForPirep($pirep); $this->payExpensesForPirep($pirep); - $this->paySubfleetExpenses($pirep); + $this->payExpensesEventsForPirep($pirep); $this->payGroundHandlingForPirep($pirep); $this->payPilotForPirep($pirep); @@ -147,16 +128,34 @@ class FinanceService extends BaseService * @param Pirep $pirep * @throws \UnexpectedValueException * @throws \InvalidArgumentException - * @throws \Prettus\Validator\Exceptions\ValidatorException */ public function payExpensesForPirep(Pirep $pirep): void { - $expenses = $this->getExpenses($pirep); - /** @var \App\Models\Expense $expense */ - foreach ($expenses as $expense) { + $expenses = $this->expenseRepo->getAllForType( + ExpenseType::FLIGHT, + $pirep->airline_id + ); + + /** + * Go through the expenses and apply a mulitplier if present + */ + $expenses->map(function ($expense, $i) use ($pirep) + { + if ($expense->multiplier) { + # TODO: Modify the amount + } Log::info('Finance: PIREP: ' . $pirep->id . ', expense:', $expense->toArray()); + # Get the transaction group name from the ref_class name + # This way it can be more dynamic and don't have to add special + # tables or specific expense calls to accomodate all of these + $transaction_group = 'Expense'; + if($expense->ref_class) { + $ref = explode('\\', $expense->ref_class); + $transaction_group = end($ref); + } + $debit = Money::createFromAmount($expense->amount); $this->journalRepo->post( $pirep->airline->journal, @@ -165,42 +164,56 @@ class FinanceService extends BaseService $pirep, 'Expense: ' . $expense->name, null, - 'Expenses' + $transaction_group ); - } + }); } /** - * Pay out the expenses for the subfleet + * Collect all of the expenses from the listeners and apply those to the journal * @param Pirep $pirep + * @throws \UnexpectedValueException + * @throws \InvalidArgumentException * @throws \Prettus\Validator\Exceptions\ValidatorException */ - public function paySubfleetExpenses(Pirep $pirep) + public function payExpensesEventsForPirep(Pirep $pirep): void { - $subfleet = $pirep->aircraft->subfleet; - $subfleet_expenses = Expense::where([ - 'ref_class' => Subfleet::class, - 'ref_class_id' => $subfleet->id, - ])->get(); - - if(!$subfleet_expenses) { + /** + * Throw an event and collect any expenses returned from it + */ + $gathered_expenses = event(new ExpensesEvent($pirep)); + if (!\is_array($gathered_expenses)) { return; } - foreach ($subfleet_expenses as $expense) { + foreach ($gathered_expenses as $event_expense) { + if (!\is_array($event_expense)) { + continue; + } - Log::info('Finance: PIREP: '.$pirep->id - .'; subfleet expense: "'.$expense->name.'", cost: "'.$expense->amount); + foreach ($event_expense as $expense) { + # Make sure it's of type expense Model + if (!($expense instanceof Expense)) { + continue; + } - $this->journalRepo->post( - $pirep->airline->journal, - null, - Money::createFromAmount($expense->amount), - $pirep, - 'Subfleet ('.$subfleet->type.'): '.$expense->name, - null, - 'Subfleet Expense' - ); + # If an airline_id is filled, then see if it matches + if ($expense->airline_id !== $pirep->airline_id) { + continue; + } + + $debit = Money::createFromAmount($expense->amount); + + $this->journalRepo->post( + $pirep->airline->journal, + null, + $debit, + $pirep, + 'Expense: ' . $expense->name, + null, + $expense->transaction_group ?? 'Expenses' + ); + } } } @@ -325,7 +338,7 @@ class FinanceService extends BaseService if(filled($pirep->aircraft->subfleet->ground_handling_multiplier)) { // force into percent mode $multiplier = $pirep->aircraft->subfleet->ground_handling_multiplier.'%'; - return $this->applyAmountOrPercent( + return Math::applyAmountOrPercent( $pirep->arr_airport->ground_handling_cost, $multiplier ); @@ -334,68 +347,6 @@ class FinanceService extends BaseService return $pirep->arr_airport->ground_handling_cost; } - /** - * Send out an event called ExpensesEvent, which picks up any - * event listeners and check if they return a list of additional - * Expense model objects. - * @param Pirep $pirep - * @return mixed - */ - public function getExpenses(Pirep $pirep) - { - $event_expenses = []; - - $expenses = $this->expenseRepo->getAllForType( - ExpenseType::FLIGHT, - $pirep->airline_id, - Expense::class - ); - - /** - * Go through the expenses and apply a mulitplier if present - */ - $expenses = $expenses->map(function($expense, $i) use ($pirep) { - if(!$expense->multiplier) { - return $expense; - } - - // TODO Apply the multiplier from the subfleet - return $expense; - }); - - /** - * Throw an event and collect any expenses returned from it - */ - $gathered_expenses = event(new ExpensesEvent($pirep)); - if (!\is_array($gathered_expenses)) { - return $expenses; - } - - foreach ($gathered_expenses as $event_expense) { - if (!\is_array($event_expense)) { - continue; - } - - foreach($event_expense as $expense) { - # Make sure it's of type expense Model - if(!($expense instanceof Expense)) { - continue; - } - - # If an airline_id is filled, then see if it matches - if($expense->airline_id !== $pirep->airline_id) { - continue; - } - - $event_expenses[] = $expense; - } - } - - $expenses = $expenses->concat($event_expenses); - - return $expenses; - } - /** * Return the pilot's hourly pay for the given PIREP * @param Pirep $pirep @@ -435,7 +386,7 @@ class FinanceService extends BaseService } Log::debug('pilot pay: base rate=' . $base_rate . ', override=' . $override_rate); - return $this->applyAmountOrPercent( + return Math::applyAmountOrPercent( $base_rate, $override_rate ); diff --git a/app/Support/Math.php b/app/Support/Math.php index c1c0829c..badda574 100644 --- a/app/Support/Math.php +++ b/app/Support/Math.php @@ -8,6 +8,26 @@ namespace App\Support; */ class Math { + /** + * Determine from the base rate, if we want to return the overridden rate + * or if the overridden rate is a percentage, then return that amount + * @param $base_rate + * @param $override_rate + * @return float|null + */ + public static function applyAmountOrPercent($base_rate, $override_rate = null): ?float + { + if (!$override_rate) { + return $base_rate; + } + + # Not a percentage override + if (substr_count($override_rate, '%') === 0) { + return $override_rate; + } + + return static::addPercent($base_rate, $override_rate); + } /** * Add/subtract a percentage to a number @@ -25,8 +45,6 @@ class Math $percent = (float) $percent; } - return $number + ($number * ($percent/100)); } - } diff --git a/tests/FinanceTest.php b/tests/FinanceTest.php index cd803c9d..ccba5b2a 100644 --- a/tests/FinanceTest.php +++ b/tests/FinanceTest.php @@ -5,7 +5,7 @@ use App\Repositories\ExpenseRepository; use App\Services\PIREPService; use App\Repositories\JournalRepository; use App\Services\FareService; -use App\Services\FinanceService; +use App\Services\Finance\PirepFinanceService; use App\Services\FleetService; use App\Support\Math; use App\Support\Money; @@ -28,7 +28,7 @@ class FinanceTest extends TestCase $this->expenseRepo = app(ExpenseRepository::class); $this->fareSvc = app(FareService::class); - $this->financeSvc = app(FinanceService::class); + $this->financeSvc = app(PirepFinanceService::class); $this->fleetSvc = app(FleetService::class); $this->pirepSvc = app(PIREPService::class); } @@ -80,7 +80,6 @@ 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 */ - $fare_counts = []; $fares = factory(App\Models\Fare::class, 3)->create([ 'price' => 100, 'cost' => 50, @@ -571,7 +570,7 @@ class FinanceTest extends TestCase $airline = factory(App\Models\Airline::class)->create(); $airline2 = factory(App\Models\Airline::class)->create(); - $expense = factory(App\Models\Expense::class)->create([ + factory(App\Models\Expense::class)->create([ 'airline_id' => $airline->id ]); @@ -599,6 +598,23 @@ class FinanceTest extends TestCase $found = $expenses->where('airline_id', $airline2->id); $this->assertCount(0, $found); + + /* + * Test the subfleet class + */ + + factory(App\Models\Expense::class)->create([ + 'airline_id' => null, + 'ref_class' => \App\Models\Subfleet::class, + ]); + + $expenses = $this->expenseRepo->getAllForType( + ExpenseType::FLIGHT, + $airline->id, + \App\Models\Subfleet::class + ); + + $this->assertCount(1, $expenses); } /** @@ -610,8 +626,7 @@ class FinanceTest extends TestCase $journalRepo = app(JournalRepository::class); [$user, $pirep, $fares] = $this->createFullPirep(); - - $journal = $user->airline->initJournal(config('phpvms.currency')); + $user->airline->initJournal(config('phpvms.currency')); # Override the fares $fare_counts = []; @@ -635,12 +650,13 @@ class FinanceTest extends TestCase $this->assertEquals(1840, $transactions['debits']->getValue()); # Check that all the different transaction types are there + # test by the different groups that exist $transaction_types = [ - 'Expenses' => 1, + 'Expense' => 1, + 'Subfleet' => 1, 'Fares' => 3, 'Ground Handling' => 1, 'Pilot Pay' => 2, # debit on the airline, credit to the pilot - 'Subfleet Expense' => 1, ]; foreach($transaction_types as $type => $count) {