From 6b5cf382245c6697577390746c8491f0c706773b Mon Sep 17 00:00:00 2001 From: Nabeel S Date: Thu, 26 Nov 2020 16:44:57 -0500 Subject: [PATCH] Calculate percentage properly instead of adding to the base value #925 (#942) * Calculate percentage properly instead of adding to the base value #925 --- app/Services/FareService.php | 6 ++-- app/Support/Math.php | 11 ++++--- tests/FinanceTest.php | 57 ++++++++---------------------------- tests/MathTest.php | 20 ++++++------- 4 files changed, 33 insertions(+), 61 deletions(-) diff --git a/app/Services/FareService.php b/app/Services/FareService.php index 8647c5c5..2c06d3ee 100644 --- a/app/Services/FareService.php +++ b/app/Services/FareService.php @@ -147,7 +147,7 @@ class FareService extends Service { if (filled($pivot->price)) { if (strpos($pivot->price, '%', -1) !== false) { - $fare->price = Math::addPercent($fare->price, $pivot->price); + $fare->price = Math::getPercent($fare->price, $pivot->price); } else { $fare->price = $pivot->price; } @@ -155,7 +155,7 @@ class FareService extends Service if (filled($pivot->cost)) { if (strpos($pivot->cost, '%', -1) !== false) { - $fare->cost = Math::addPercent($fare->cost, $pivot->cost); + $fare->cost = Math::getPercent($fare->cost, $pivot->cost); } else { $fare->cost = $pivot->cost; } @@ -163,7 +163,7 @@ class FareService extends Service if (filled($pivot->capacity)) { if (strpos($pivot->capacity, '%', -1) !== false) { - $fare->capacity = floor(Math::addPercent($fare->capacity, $pivot->capacity)); + $fare->capacity = floor(Math::getPercent($fare->capacity, $pivot->capacity)); } else { $fare->capacity = floor($pivot->capacity); } diff --git a/app/Support/Math.php b/app/Support/Math.php index bf48fa73..948ba533 100644 --- a/app/Support/Math.php +++ b/app/Support/Math.php @@ -27,18 +27,19 @@ class Math return $override_rate; } - return static::addPercent($base_rate, $override_rate); + // It is a percent, so apply it + return static::getPercent($base_rate, $override_rate); } /** - * Add/subtract a percentage to a number + * Apply a percentage to a number * * @param $number * @param $percent * * @return float */ - public static function addPercent($number, $percent): float + public static function getPercent($number, $percent): float { if (!is_numeric($number)) { $number = (float) $number; @@ -48,6 +49,8 @@ class Math $percent = (float) $percent; } - return $number + ($number * ($percent / 100)); + $val = $number * ($percent / 100); + + return $val; } } diff --git a/tests/FinanceTest.php b/tests/FinanceTest.php index 65c278fd..a7f5098a 100644 --- a/tests/FinanceTest.php +++ b/tests/FinanceTest.php @@ -165,37 +165,6 @@ class FinanceTest extends TestCase return [$user, $pirep, $fares]; } - /*public function testFlightFaresNoOverride() - { - $flight = factory(Flight::class)->create(); - $fare = factory(Fare::class)->create(); - - $this->fareSvc->setForFlight($flight, $fare); - $subfleet_fares = $this->fareSvc->get($flight); - - $this->assertCount(1, $subfleet_fares); - $this->assertEquals($fare->price, $subfleet_fares->get(0)->price); - $this->assertEquals($fare->capacity, $subfleet_fares->get(0)->capacity); - - // - // set an override now - // - $this->fareSvc->setForFlight($flight, $fare, [ - 'price' => 50, 'capacity' => 400, - ]); - - // look for them again - $subfleet_fares = $this->fareSvc->getForFlight($flight); - - $this->assertCount(1, $subfleet_fares); - $this->assertEquals(50, $subfleet_fares[0]->price); - $this->assertEquals(400, $subfleet_fares[0]->capacity); - - // delete - $this->fareSvc->delFareFromFlight($flight, $fare); - $this->assertCount(0, $this->fareSvc->getForFlight($flight)); - }*/ - /** * Make sure that the API is returning the fares properly for a subfleet on a flight * https://github.com/nabeelio/phpvms/issues/899 @@ -355,13 +324,13 @@ class FinanceTest extends TestCase $subfleet = factory(Subfleet::class)->create(); $this->fleetSvc->addSubfleetToFlight($subfleet, $flight); - $percent_incr = '20%'; - $percent_decr = '-20%'; + $percent_incr = '120%'; + $percent_decr = '80%'; $percent_200 = '200%'; - $new_price = Math::addPercent($fare->price, $percent_incr); - $new_cost = Math::addPercent($fare->cost, $percent_decr); - $new_capacity = Math::addPercent($fare->capacity, $percent_200); + $new_price = Math::getPercent($fare->price, $percent_incr); + $new_cost = Math::getPercent($fare->cost, $percent_decr); + $new_capacity = Math::getPercent($fare->capacity, $percent_200); $this->fareSvc->setForFlight($flight, $fare, [ 'price' => $percent_incr, @@ -455,9 +424,9 @@ class FinanceTest extends TestCase $percent_decr = '-20%'; $percent_200 = '200%'; - $new_price = Math::addPercent($fare->price, $percent_incr); - $new_cost = Math::addPercent($fare->cost, $percent_decr); - $new_capacity = Math::addPercent($fare->capacity, $percent_200); + $new_price = Math::getPercent($fare->price, $percent_incr); + $new_cost = Math::getPercent($fare->cost, $percent_decr); + $new_capacity = Math::getPercent($fare->capacity, $percent_200); $this->fareSvc->setForSubfleet($subfleet, $fare, [ 'price' => $percent_incr, @@ -499,7 +468,7 @@ class FinanceTest extends TestCase 'cost' => 250, ]); - $fare3_price = Math::addPercent($fare3->price, 300); + $fare3_price = Math::getPercent($fare3->price, 300); // Assign another one to the flight, that's not on the subfleet // This one should NOT be returned in the list of fares @@ -623,7 +592,7 @@ class FinanceTest extends TestCase // Change to a percentage $manual_pay_rate = '50%'; - $manual_pay_adjusted = Math::addPercent( + $manual_pay_adjusted = Math::getPercent( $rank->manual_base_pay_rate, $manual_pay_rate ); @@ -931,7 +900,7 @@ class FinanceTest extends TestCase // $this->assertCount(9, $transactions['transactions']); $this->assertEquals(3020, $transactions['credits']->getValue()); - $this->assertEquals(2060, $transactions['debits']->getValue()); + $this->assertEquals(2050, $transactions['debits']->getValue()); // Check that all the different transaction types are there // test by the different groups that exist @@ -987,7 +956,7 @@ class FinanceTest extends TestCase // $this->assertCount(9, $transactions['transactions']); $this->assertEquals(3020, $transactions['credits']->getValue()); - $this->assertEquals(2060, $transactions['debits']->getValue()); + $this->assertEquals(2050, $transactions['debits']->getValue()); // Check that all the different transaction types are there // test by the different groups that exist @@ -1026,7 +995,7 @@ class FinanceTest extends TestCase $transactions = $journalRepo->getAllForObject($pirep2); $this->assertEquals(3020, $transactions['credits']->getValue()); - $this->assertEquals(2160, $transactions['debits']->getValue()); + $this->assertEquals(2150, $transactions['debits']->getValue()); // Check that all the different transaction types are there // test by the different groups that exist diff --git a/tests/MathTest.php b/tests/MathTest.php index 033306ad..1cef0586 100644 --- a/tests/MathTest.php +++ b/tests/MathTest.php @@ -13,16 +13,16 @@ class MathTest extends TestCase public function testAddPercent() { $tests = [ - ['expected' => 112, 'fn' => Math::addPercent(100, 12)], - ['expected' => 112, 'fn' => Math::addPercent(100, '12')], - ['expected' => 112, 'fn' => Math::addPercent(100, '12%')], - ['expected' => 112, 'fn' => Math::addPercent(100, '12 %')], - ['expected' => 112, 'fn' => Math::addPercent('100 ', '12')], - ['expected' => 112.5, 'fn' => Math::addPercent('100', '12.5')], - ['expected' => 88, 'fn' => Math::addPercent('100', -12)], - ['expected' => 88, 'fn' => Math::addPercent('100', '-12')], - ['expected' => 88, 'fn' => Math::addPercent('100', '-12 %')], - ['expected' => 88, 'fn' => Math::addPercent('100', '-12%')], + ['expected' => 112, 'fn' => Math::getPercent(100, 112)], + ['expected' => 112, 'fn' => Math::getPercent(100, '112')], + ['expected' => 112, 'fn' => Math::getPercent(100, '112%')], + ['expected' => 112, 'fn' => Math::getPercent(100, '112%')], + ['expected' => 112, 'fn' => Math::getPercent('100 ', '112')], + ['expected' => 112.5, 'fn' => Math::getPercent('100', '112.5')], + ['expected' => 88, 'fn' => Math::getPercent('100', 88)], + ['expected' => 88, 'fn' => Math::getPercent('100', '88')], + ['expected' => 88, 'fn' => Math::getPercent('100', '88 %')], + ['expected' => 88, 'fn' => Math::getPercent('100', '88%')], ]; foreach ($tests as $test) {