From 586769fbf0277e647be2c97cb5f2e97968651744 Mon Sep 17 00:00:00 2001 From: Nabeel Shahzad Date: Fri, 2 Mar 2018 17:29:11 -0600 Subject: [PATCH] Fix tests and docs/better checks #130 --- ...1807_create_journal_transactions_table.php | 2 +- app/Listeners/FinanceEvents.php | 24 +++++++++++++- app/Models/Aircraft.php | 11 +++++++ app/Models/Airline.php | 1 + app/Models/Airport.php | 1 + app/Models/Expense.php | 2 ++ app/Models/Fare.php | 4 +++ app/Models/Pirep.php | 17 ++++++---- app/Models/Rank.php | 3 ++ app/Models/User.php | 4 ++- app/Repositories/BaseRepository.php | 14 ++++++-- app/Repositories/JournalRepository.php | 30 +++++++++++++++-- app/Services/FinanceService.php | 33 ++++++++++++++++--- tests/FinanceTest.php | 6 ++-- 14 files changed, 132 insertions(+), 20 deletions(-) diff --git a/app/Database/migrations/2018_02_28_231807_create_journal_transactions_table.php b/app/Database/migrations/2018_02_28_231807_create_journal_transactions_table.php index ab4d10fb..bb296282 100644 --- a/app/Database/migrations/2018_02_28_231807_create_journal_transactions_table.php +++ b/app/Database/migrations/2018_02_28_231807_create_journal_transactions_table.php @@ -22,7 +22,7 @@ class CreateJournalTransactionsTable extends Migration $table->char('currency', 5); $table->text('memo')->nullable(); $table->char('ref_class', 32)->nullable(); - $table->text('ref_class_id')->nullable(); + $table->string('ref_class_id', 36)->nullable(); $table->timestamps(); $table->dateTime('post_date'); $table->softDeletes(); diff --git a/app/Listeners/FinanceEvents.php b/app/Listeners/FinanceEvents.php index d86bb106..e322ee6e 100644 --- a/app/Listeners/FinanceEvents.php +++ b/app/Listeners/FinanceEvents.php @@ -2,7 +2,8 @@ namespace App\Listeners; -use \App\Events\PirepAccepted; +use App\Events\PirepAccepted; +use App\Events\PirepRejected; use App\Services\FinanceService; /** @@ -26,14 +27,35 @@ class FinanceEvents PirepAccepted::class, 'App\Listeners\FinanceEvents@onPirepAccept' ); + + $events->listen( + PirepRejected::class, + 'App\Listeners\FinanceEvents@onPirepReject' + ); } /** * Kick off the finance events when a PIREP is accepted * @param PirepAccepted $event + * @throws \UnexpectedValueException + * @throws \InvalidArgumentException + * @throws \Exception + * @throws \Prettus\Validator\Exceptions\ValidatorException */ public function onPirepAccept(PirepAccepted $event) { $this->financeSvc->processFinancesForPirep($event->pirep); } + + /** + * Delete all finances in the journal for a given PIREP + * @param PirepRejected $event + * @throws \UnexpectedValueException + * @throws \InvalidArgumentException + * @throws \Exception + */ + public function onPirepReject(PirepRejected $event) + { + $this->financeSvc->deleteFinancesForPirep($event->pirep); + } } diff --git a/app/Models/Aircraft.php b/app/Models/Aircraft.php index 290a8773..d7a4d545 100644 --- a/app/Models/Aircraft.php +++ b/app/Models/Aircraft.php @@ -5,6 +5,17 @@ namespace App\Models; use App\Models\Enums\AircraftStatus; use App\Support\ICAO; +/** + * @property mixed subfleet_id + * @property string name + * @property string icao + * @property string registration + * @property string hex_code + * @property Airport airport + * @property Subfleet subfleet + * @property int status + * @property int state + */ class Aircraft extends BaseModel { public $table = 'aircraft'; diff --git a/app/Models/Airline.php b/app/Models/Airline.php index 982f4840..f7f47a7e 100644 --- a/app/Models/Airline.php +++ b/app/Models/Airline.php @@ -6,6 +6,7 @@ use App\Models\Traits\JournalTrait; /** * Class Airline + * @property Journal journal * @package App\Models */ class Airline extends BaseModel diff --git a/app/Models/Airport.php b/app/Models/Airport.php index 0d81f776..c306e155 100644 --- a/app/Models/Airport.php +++ b/app/Models/Airport.php @@ -6,6 +6,7 @@ use Illuminate\Notifications\Notifiable; /** * Class Airport + * @property float ground_handling_cost * @package App\Models */ class Airport extends BaseModel diff --git a/app/Models/Expense.php b/app/Models/Expense.php index 8480cb3e..c015efe7 100644 --- a/app/Models/Expense.php +++ b/app/Models/Expense.php @@ -4,6 +4,8 @@ namespace App\Models; /** * Class Expense + * @property float amount + * @property string name * @package App\Models */ class Expense extends BaseModel diff --git a/app/Models/Fare.php b/app/Models/Fare.php index 18e7d5d2..5400fed5 100644 --- a/app/Models/Fare.php +++ b/app/Models/Fare.php @@ -4,6 +4,10 @@ namespace App\Models; /** * Class Fare + * @property integer capacity + * @property float cost + * @property float price + * @property mixed code * @package App\Models */ class Fare extends BaseModel diff --git a/app/Models/Pirep.php b/app/Models/Pirep.php index 916b9d2d..bc46d187 100644 --- a/app/Models/Pirep.php +++ b/app/Models/Pirep.php @@ -13,13 +13,16 @@ use PhpUnitsOfMeasure\Exception\NonStringUnitName; /** * Class Pirep * + * @property string flight_number + * @property string route_code + * @property string route_leg * @property integer airline_id + * @property Aircraft aircraft * @property Airline airline * @property Airport arr_airport * @property Airport dep_airport - * @property mixed flight_number - * @property mixed route_code - * @property mixed route_leg + * @property integer flight_time In minutes + * @property User user * @package App\Models */ class Pirep extends BaseModel @@ -221,7 +224,7 @@ class Pirep extends BaseModel * Do some cleanup on the route * @param $route */ - public function setRouteAttribute($route) + public function setRouteAttribute($route): void { $route = strtoupper(trim($route)); $this->attributes['route'] = $route; @@ -231,7 +234,7 @@ class Pirep extends BaseModel * Check if this PIREP is allowed to be updated * @return bool */ - public function allowedUpdates() + public function allowedUpdates(): bool { if($this->state === PirepState::CANCELLED) { return false; @@ -321,7 +324,9 @@ class Pirep extends BaseModel public function transactions() { return $this->hasMany(JournalTransaction::class, 'ref_class_id') - ->where('ref_class', __CLASS__); + ->where('ref_class', __CLASS__) + ->orderBy('credit', 'desc') + ->orderBy('debit', 'desc'); } public function user() diff --git a/app/Models/Rank.php b/app/Models/Rank.php index 67ab1f8e..2185437f 100644 --- a/app/Models/Rank.php +++ b/app/Models/Rank.php @@ -4,6 +4,9 @@ namespace App\Models; /** * Class Rank + * @property int hours + * @property float manual_base_pay_rate + * @property float acars_base_pay_rate * @package App\Models */ class Rank extends BaseModel diff --git a/app/Models/User.php b/app/Models/User.php index a755a632..a98dbb2a 100755 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -14,11 +14,13 @@ use Laratrust\Traits\LaratrustUserTrait; * @property string $email * @property string $password * @property string $api_key - * @property string $flights + * @property Flight[] $flights * @property string $flight_time * @property string $remember_token * @property \Carbon\Carbon $created_at * @property \Carbon\Carbon $updated_at + * @property Rank rank + * @property Journal journal * @mixin \Illuminate\Notifications\Notifiable * @mixin \Laratrust\Traits\LaratrustUserTrait */ diff --git a/app/Repositories/BaseRepository.php b/app/Repositories/BaseRepository.php index 4f2a5935..f321a1c6 100644 --- a/app/Repositories/BaseRepository.php +++ b/app/Repositories/BaseRepository.php @@ -56,10 +56,20 @@ abstract class BaseRepository extends \Prettus\Repository\Eloquent\BaseRepositor * @param $order_by * @return $this */ - public function whereOrder($where, $sort_by, $order_by) + public function whereOrder($where, $sort_by, $order_by='asc') { return $this->scopeQuery(function($query) use ($where, $sort_by, $order_by) { - return $query->where($where)->orderBy($sort_by, $order_by); + $q = $query->where($where); + # See if there are multi-column sorts + if(\is_array($sort_by)) { + foreach($sort_by as $key => $sort) { + $q = $q->orderBy($key, $sort); + } + } else { + $q = $q->orderBy($sort_by, $order_by); + } + + return $q; }); } } diff --git a/app/Repositories/JournalRepository.php b/app/Repositories/JournalRepository.php index d8aa45ef..2f3f69fd 100644 --- a/app/Repositories/JournalRepository.php +++ b/app/Repositories/JournalRepository.php @@ -137,8 +137,9 @@ class JournalRepository extends BaseRepository implements CacheableInterface } /** - * @param Journal $journal * @param Carbon $date + * @param Journal $journal + * @param Carbon|null $start_date * @return Money * @throws \UnexpectedValueException * @throws \InvalidArgumentException @@ -173,6 +174,8 @@ class JournalRepository extends BaseRepository implements CacheableInterface * @param $object * @param null $journal * @return array + * @throws \UnexpectedValueException + * @throws \InvalidArgumentException */ public function getAllForObject($object, $journal=null) { @@ -185,7 +188,10 @@ class JournalRepository extends BaseRepository implements CacheableInterface $where['journal_id'] = $journal->id; } - $transactions = $this->findWhere($where); + $transactions = $this->whereOrder($where, [ + 'credit' => 'desc', + 'debit' => 'desc' + ])->get(); return [ 'credits' => new Money($transactions->sum('credit')), @@ -193,4 +199,24 @@ class JournalRepository extends BaseRepository implements CacheableInterface 'transactions' => $transactions, ]; } + + /** + * Delete all transactions for a given object + * @param $object + * @param null $journal + * @return void + */ + public function deleteAllForObject($object, $journal = null) + { + $where = [ + 'ref_class' => \get_class($object), + 'ref_class_id' => $object->id, + ]; + + if ($journal) { + $where['journal_id'] = $journal->id; + } + + $this->deleteWhere($where); + } } diff --git a/app/Services/FinanceService.php b/app/Services/FinanceService.php index ca9f58ac..fa3f9eba 100644 --- a/app/Services/FinanceService.php +++ b/app/Services/FinanceService.php @@ -96,15 +96,26 @@ class FinanceService extends BaseService return $pirep; } + /** + * @param Pirep $pirep + */ + public function deleteFinancesForPirep(Pirep $pirep) + { + $this->journalRepo->deleteAllForObject($pirep); + } + /** * Collect all of the fares and then post each fare class's profit and * the costs for each seat and post it to the journal * @param $pirep + * @throws \UnexpectedValueException + * @throws \InvalidArgumentException * @throws \Prettus\Validator\Exceptions\ValidatorException */ public function payFaresForPirep($pirep): void { $fares = $this->getReconciledFaresForPirep($pirep); + /** @var \App\Models\Fare $fare */ foreach ($fares as $fare) { Log::info('Finance: PIREP: ' . $pirep->id . ', fare:', $fare->toArray()); @@ -128,12 +139,14 @@ class FinanceService extends BaseService /** * Collect all of the expenses and apply those to the journal * @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) { Log::info('Finance: PIREP: ' . $pirep->id . ', expense:', $expense->toArray()); @@ -154,6 +167,8 @@ class FinanceService extends BaseService /** * Collect and apply the ground handling cost * @param Pirep $pirep + * @throws \UnexpectedValueException + * @throws \InvalidArgumentException * @throws \Prettus\Validator\Exceptions\ValidatorException */ public function payGroundHandlingForPirep(Pirep $pirep) @@ -337,17 +352,27 @@ class FinanceService extends BaseService # find the right subfleet $override_rate = $rank->subfleets() ->where('subfleet_id', $subfleet_id) - ->first() - ->pivot; + ->first(); + + if($override_rate) { + $override_rate = $override_rate->pivot; + } if($pirep->source === PirepSource::ACARS) { Log::debug('Source is ACARS'); $base_rate = $rank->acars_base_pay_rate; - $override_rate = $override_rate->acars_pay; + + if($override_rate) { + $override_rate = $override_rate->acars_pay; + } + } else { Log::debug('Source is Manual'); $base_rate = $rank->manual_base_pay_rate; - $override_rate = $override_rate->manual_pay; + + if($override_rate) { + $override_rate = $override_rate->manual_pay; + } } Log::debug('pilot pay: base rate=' . $base_rate . ', override=' . $override_rate); diff --git a/tests/FinanceTest.php b/tests/FinanceTest.php index 326b7b20..e48770fc 100644 --- a/tests/FinanceTest.php +++ b/tests/FinanceTest.php @@ -575,8 +575,8 @@ class FinanceTest extends TestCase $transactions = $journalRepo->getAllForObject($pirep); - $this->assertCount(4, $transactions['transactions']); - $this->assertEquals(3000, $transactions['credits']->getValue()); - $this->assertEquals(1520, $transactions['debits']->getValue()); + $this->assertCount(6, $transactions['transactions']); + $this->assertEquals(3020, $transactions['credits']->getValue()); + $this->assertEquals(1540, $transactions['debits']->getValue()); } }