Rewrite add/remove bids code w additional tests

app/Http/Controllers/Api/UserController.php

@@ -2,6 +2,7 @@
 
 namespace App\Http\Controllers\Api;
 
+use App\Exceptions\BidExists;
 use App\Http\Resources\Bid as BidResource;
 use App\Http\Resources\Pirep as PirepResource;
 use App\Http\Resources\Subfleet as SubfleetResource;
@@ -98,7 +99,6 @@ class UserController extends Controller
      * @return mixed
      * @throws \Illuminate\Database\Eloquent\ModelNotFoundException
      * @throws \App\Exceptions\BidExists
-     * @throws \App\Services\Exception
      */
     public function bids(Request $request)
     {

app/Interfaces/Controller.php

@@ -71,28 +71,6 @@ abstract class Controller extends \Illuminate\Routing\Controller
         return $fields;
     }
 
-    /**
-     * Run a validation
-     * @param $request
-     * @param $rules
-     * @return bool
-     * @throws \Symfony\Component\HttpKernel\Exception\BadRequestHttpException
-     */
-    /*public function validate($request, $rules)
-    {
-        if ($request instanceof Request) {
-            $validator = Validator::make($request->all(), $rules);
-        } else {
-            $validator = Validator::make($request, $rules);
-        }
-
-        if (!$validator->passes()) {
-            throw new BadRequestHttpException($validator->errors(), null, 400);
-        }
-
-        return true;
-    }*/
-
     /**
      * Simple normalized method for forming the JSON responses
      * @param $message

app/Models/Bid.php

@@ -16,6 +16,10 @@ class Bid extends Model
         'flight_id',
     ];
 
+    protected $casts = [
+        'user_id' => 'integer',
+    ];
+
     /**
      * Relationships
      */

app/Models/Flight.php

@@ -18,6 +18,7 @@ use PhpUnitsOfMeasure\Exception\NonStringUnitName;
  * @property mixed      flight_number
  * @property mixed      route_code
  * @property int        route_leg
+ * @property bool       has_bid
  * @property Collection field_values
  * @property Collection fares
  * @property Collection subfleets

app/Models/User.php

@@ -10,18 +10,19 @@ use Illuminate\Notifications\Notifiable;
 use Laratrust\Traits\LaratrustUserTrait;
 
 /**
- * @property integer        $id
+ * @property integer        id
- * @property string         $name
+ * @property string         name
- * @property string         $email
+ * @property string         email
- * @property string         $password
+ * @property string         password
- * @property string         $api_key
+ * @property string         api_key
+ * @property mixed          ident
  * @property string         curr_airport_id
  * @property string         home_airport_id
- * @property Flight[]       $flights
+ * @property Flight[]       flights
- * @property string         $flight_time
+ * @property string         flight_time
- * @property string         $remember_token
+ * @property string         remember_token
- * @property \Carbon\Carbon $created_at
+ * @property \Carbon\Carbon created_at
- * @property \Carbon\Carbon $updated_at
+ * @property \Carbon\Carbon updated_at
  * @property Rank           rank
  * @property Journal        journal
  * @property string         pilot_id

app/Services/FlightService.php

@@ -35,7 +35,8 @@ class FlightService extends Service
         FlightRepository $flightRepo,
         NavdataRepository $navdataRepo,
         UserService $userSvc
-    ) {
+    )
+    {
         $this->fareSvc = $fareSvc;
         $this->flightRepo = $flightRepo;
         $this->navDataRepo = $navdataRepo;
@@ -114,15 +115,15 @@ class FlightService extends Service
         ];
 
         $found_flights = $this->flightRepo->findWhere($where);
-        if($found_flights->count() === 0) {
+        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) {
+        $found_flights = $found_flights->filter(function ($value, $key) use ($flight) {
-            if($flight->route_code === $value->route_code
+            if ($flight->route_code === $value->route_code
                 && $flight->route_leg === $value->route_leg) {
                 return true;
             }
@@ -130,7 +131,7 @@ class FlightService extends Service
             return false;
         });
 
-        if($found_flights->count() === 0) {
+        if ($found_flights->count() === 0) {
             return false;
         }
 
@@ -199,43 +200,53 @@ class FlightService extends Service
      * Allow a user to bid on a flight. Check settings and all that good stuff
      * @param Flight $flight
      * @param User   $user
-     * @return Bid|null
+     * @return mixed
      * @throws \App\Exceptions\BidExists
      */
     public function addBid(Flight $flight, User $user)
     {
-        # If it's already been bid on, then it can't be bid on again
+        # Get all of the bids for this user. See if they're allowed to have multiple
-        if ($flight->has_bid && setting('bids.disable_flight_on_bid')) {
+        # bids
-            Log::info($flight->id.' already has a bid, skipping');
+        $bids = Bid::where('user_id', $user->id)->get();
-            throw new BidExists();
+        if ($bids->count() > 0 && setting('bids.allow_multiple_bids') === false) {
+            throw new BidExists('User "'.$user->id.'" already has bids, skipping');
         }
 
-        # See if we're allowed to have multiple bids or not
+        # Get all of the bids for this flight
-        if (!setting('bids.allow_multiple_bids')) {
+        $bids = Bid::where('flight_id', $flight->id)->get();
-            $user_bids = Bid::where(['user_id' => $user->id])->first();
+        if ($bids->count() > 0) {
-            if ($user_bids) {
+            # Does the flight have a bid set?
-                Log::info('User "'.$user->id.'" already has bids, skipping');
+            if ($flight->has_bid === false) {
-                throw new BidExists();
+                $flight->has_bid = true;
+                $flight->save();
+            }
+
+            # Check all the bids for one of this user
+            foreach ($bids as $bid) {
+                if ($bid->user_id === $user->id) {
+                    Log::info('Bid exists, user='.$user->ident.', flight='.$flight->id);
+                    return $bid;
                 }
             }
 
-        # See if this user has this flight bid on already
+            if (setting('bids.allow_multiple_bids') === false) {
-        $bid_data = [
+                throw new BidExists('A bid already exists for this flight');
+            }
+        } else {
+            if ($flight->has_bid === true) {
+                Log::info('Bid exists, flight='.$flight->id.'; no entry in bids table, cleaning up');
+            }
+        }
+
+        $bid = Bid::firstOrCreate([
             'user_id'   => $user->id,
-            'flight_id' => $flight->id
+            'flight_id' => $flight->id,
-        ];
+        ]);
-
-        $user_bid = Bid::where($bid_data)->first();
-        if ($user_bid) {
-            return $user_bid;
-        }
-
-        $user_bid = Bid::create($bid_data);
 
         $flight->has_bid = true;
         $flight->save();
 
-        return $user_bid;
+        return $bid;
     }
 
     /**
@@ -245,16 +256,18 @@ class FlightService extends Service
      */
     public function removeBid(Flight $flight, User $user)
     {
-        $user_bid = Bid::where([
+        $bids = Bid::where([
-            'flight_id' => $flight->id, 'user_id' => $user->id
+            'flight_id' => $flight->id,
-        ])->first();
+            'user_id'   => $user->id
+        ])->get();
 
-        if ($user_bid) {
+        foreach ($bids as $bid) {
-            $user_bid->forceDelete();
+            $bid->forceDelete();
         }
 
         # Only flip the flag if there are no bids left for this flight
-        if (!Bid::where('flight_id', $flight->id)->exists()) {
+        $bids = Bid::where('flight_id', $flight->id)->get();
+        if ($bids->count() === 0) {
             $flight->has_bid = false;
             $flight->save();
         }

tests/FlightTest.php

@@ -373,7 +373,11 @@ class FlightTest extends TestCase
      */
     public function testBids()
     {
+        $this->settingsRepo->store('bids.allow_multiple_bids', true);
+        $this->settingsRepo->store('bids.disable_flight_on_bid', false);
+
         $user = factory(User::class)->create();
+        $user2 = factory(User::class)->create();
         $headers = $this->headers($user);
 
         $flight = $this->addFlight($user);
@@ -387,12 +391,13 @@ class FlightTest extends TestCase
         $flight = Flight::find($flight->id);
         $this->assertTrue($flight->has_bid);
 
-        # Check the table and make sure thee entry is there
+        # Check the table and make sure the entry is there
-        $this->expectException(\App\Exceptions\BidExists::class);
+        $bid_retrieved = $this->flightSvc->addBid($flight, $user);
-        $this->flightSvc->addBid($flight, $user);
+        $this->assertEquals($bid->id, $bid_retrieved->id);
 
         $user->refresh();
-        $this->assertEquals(1, $user->bids->count());
+        $bids = $user->bids;
+        $this->assertEquals(1, $bids->count());
 
         # Query the API and see that the user has the bids
         # And pull the flight details for the user/bids
@@ -407,13 +412,25 @@ class FlightTest extends TestCase
 
         $body = $req->json()['data'];
         $req->assertStatus(200);
-        $this->assertEquals($flight->id, $body[0]['id']);
+        $this->assertEquals($flight->id, $body[0]['flight_id']);
+
+        # have a second user bid on it
+        $bid_user2 = $this->flightSvc->addBid($flight, $user2);
+        $this->assertNotNull($bid_user2);
+        $this->assertNotEquals($bid_retrieved->id, $bid_user2->id);
 
         # Now remove the flight and check API
 
         $this->flightSvc->removeBid($flight, $user);
 
         $flight = Flight::find($flight->id);
+
+        # user2 still has a bid on it
+        $this->assertTrue($flight->has_bid);
+
+        # Remove it from 2nd user
+        $this->flightSvc->removeBid($flight, $user2);
+        $flight->refresh();
         $this->assertFalse($flight->has_bid);
 
         $user->refresh();