diff --git a/app/Http/Controllers/Admin/FlightController.php b/app/Http/Controllers/Admin/FlightController.php index 93623c1f..89411211 100644 --- a/app/Http/Controllers/Admin/FlightController.php +++ b/app/Http/Controllers/Admin/FlightController.php @@ -166,21 +166,9 @@ class FlightController extends Controller { $input = $request->all(); - # See if flight number exists with the route code/leg - $where = [ - 'flight_number' => $input['flight_number'], - ]; - - if (filled($input['route_code'])) { - $where['route_code'] = $input['route_code']; - } - - if (filled($input['route_leg'])) { - $where['route_leg'] = $input['route_leg']; - } - - $flights = $this->flightRepo->findWhere($where); - if ($flights->count() > 0) { + // Create a temporary flight so we can validate + $flight = new Flight($input); + if ($this->flightSvc->isFlightDuplicate($flight)) { Flash::error('Duplicate flight with same number/code/leg found, please change to proceed'); return redirect()->back()->withInput($request->all()); } @@ -270,22 +258,11 @@ class FlightController extends Controller $input = $request->all(); - # See if flight number exists with the route code/leg - $where = [ - ['id', '<>', $id], - 'flight_number' => $input['flight_number'], - ]; + # apply the updates here temporarily, don't save + # the repo->update() call will actually do it + $flight->fill($input); - if (filled($input['route_code'])) { - $where['route_code'] = $input['route_code']; - } - - if (filled($input['route_leg'])) { - $where['route_leg'] = $input['route_leg']; - } - - $flights = $this->flightRepo->findWhere($where); - if ($flights->count() > 0) { + if($this->flightSvc->isFlightDuplicate($flight)) { Flash::error('Duplicate flight with same number/code/leg found, please change to proceed'); return redirect()->back()->withInput($request->all()); } diff --git a/app/Interfaces/ImportExport.php b/app/Interfaces/ImportExport.php index e260b65f..80f2f33d 100644 --- a/app/Interfaces/ImportExport.php +++ b/app/Interfaces/ImportExport.php @@ -47,7 +47,7 @@ class ImportExport /** * Get the airline from the ICAO. Create it if it doesn't exist * @param $code - * @return \Illuminate\Database\Eloquent\Model + * @return Airline */ public function getAirline($code) { diff --git a/app/Models/Flight.php b/app/Models/Flight.php index c1836a2d..7f414c95 100644 --- a/app/Models/Flight.php +++ b/app/Models/Flight.php @@ -14,6 +14,7 @@ use PhpUnitsOfMeasure\Exception\NonStringUnitName; /** * @property string id * @property Airline airline + * @property integer airline_id * @property mixed flight_number * @property mixed route_code * @property mixed route_leg diff --git a/app/Services/FlightService.php b/app/Services/FlightService.php index af26a857..9dbcfbef 100644 --- a/app/Services/FlightService.php +++ b/app/Services/FlightService.php @@ -100,6 +100,43 @@ class FlightService extends Service return $flight; } + /** + * Check if this flight has a duplicate already + * @param Flight $flight + * @return bool + */ + public function isFlightDuplicate(Flight $flight) + { + $where = [ + ['id', '<>', $flight->id], + 'airline_id' => $flight->airline_id, + 'flight_number' => $flight->flight_number, + ]; + + $found_flights = $this->flightRepo->findWhere($where); + if($found_flights->count() === 0) { + return false; + } + + // Find within all the flights with the same flight number + // Return any flights that have the same route code and leg + // If this list is > 0, then this has a duplicate + $found_flights = $found_flights->filter(function($value, $key) use ($flight) { + if($flight->route_code === $value->route_code + && $flight->route_leg === $value->route_leg) { + return true; + } + + return false; + }); + + if($found_flights->count() === 0) { + return false; + } + + return true; + } + /** * Delete a flight, and all the user bids, etc associated with it * @param Flight $flight diff --git a/app/Services/ImportExport/FlightImporter.php b/app/Services/ImportExport/FlightImporter.php index d3f2e882..1567277d 100644 --- a/app/Services/ImportExport/FlightImporter.php +++ b/app/Services/ImportExport/FlightImporter.php @@ -75,6 +75,19 @@ class FlightImporter extends ImportExport // Get the airline ID from the ICAO code $airline = $this->getAirline($row['airline']); + // Check if the imported flight is a duplicate + $temp_flight = new Flight([ + 'airline_id' => $airline->id, + 'flight_number' => $row['flight_number'], + 'route_code' => $row['route_code'], + 'route_leg' => $row['route_leg'], + ]); + + if($this->flightSvc->isFlightDuplicate($temp_flight)) { + $this->errorLog('Error in row '.$index.': Duplicate flight number detected'); + return false; + } + // Try to find this flight $flight = Flight::firstOrNew([ 'airline_id' => $airline->id, diff --git a/resources/views/admin/flights/fields.blade.php b/resources/views/admin/flights/fields.blade.php index 26836103..4686aef4 100644 --- a/resources/views/admin/flights/fields.blade.php +++ b/resources/views/admin/flights/fields.blade.php @@ -223,9 +223,13 @@
- {{ Form::label('active', 'Active:') }} - {{ Form::hidden('active', 0, false) }} - {{ Form::checkbox('active', null, ['class' => 'form-control icheck']) }} +
+ +
diff --git a/tests/FlightTest.php b/tests/FlightTest.php index 0f449bbb..8dacd99a 100644 --- a/tests/FlightTest.php +++ b/tests/FlightTest.php @@ -35,6 +35,57 @@ class FlightTest extends TestCase return $flight; } + /** + * Test adding a flight and also if there are duplicates + */ + public function testDuplicateFlight() + { + $this->user = factory(App\Models\User::class)->create(); + $flight = $this->addFlight($this->user); + + // first flight shouldn't be a duplicate + $this->assertFalse($this->flightSvc->isFlightDuplicate($flight)); + + $flight_dupe = new Flight([ + 'airline_id' => $flight->airline_id, + 'flight_number' => $flight->flight_number, + 'route_code' => $flight->route_code, + 'route_leg' => $flight->route_leg, + ]); + + $this->assertTrue($this->flightSvc->isFlightDuplicate($flight_dupe)); + + # same flight but diff airline shouldn't be a dupe + $new_airline = factory(App\Models\Airline::class)->create(); + $flight_dupe = new Flight([ + 'airline_id' => $new_airline->airline_id, + 'flight_number' => $flight->flight_number, + 'route_code' => $flight->route_code, + 'route_leg' => $flight->route_leg, + ]); + + $this->assertFalse($this->flightSvc->isFlightDuplicate($flight_dupe)); + + # add another flight with a code + $flight_leg = factory(App\Models\Flight::class)->create([ + 'airline_id' => $flight->airline_id, + 'flight_number' => $flight->flight_number, + 'route_code' => 'A', + ]); + + $this->assertFalse($this->flightSvc->isFlightDuplicate($flight_leg)); + + // Add both a route and leg + $flight_leg = factory(App\Models\Flight::class)->create([ + 'airline_id' => $flight->airline_id, + 'flight_number' => $flight->flight_number, + 'route_code' => 'A', + 'route_leg' => 1, + ]); + + $this->assertFalse($this->flightSvc->isFlightDuplicate($flight_leg)); + } + public function testGetFlight() { $this->user = factory(App\Models\User::class)->create();