Checking out specific seats for licenses #5887
Conversation
| return redirect()->route('licenses.index')->with('error', 'There are no available seats for this license'); | ||
| } | ||
| } else { | ||
| $licenseSeat = LicenseSeat::where('id', '=', $seatId)->first(); |
There was a problem hiding this comment.
We should probably confirm that the license seat belongs to the license ID we think it does here, no? And fail with an error if the seat ID doesn't match the license ID?
| $licenseSeat = LicenseSeat::where('id', '=', $seatId)->first(); | ||
| if (!$licenseSeat) { | ||
| return redirect()->route('licenses.index')->with('error', 'License seat is not available for checkout'); | ||
| } |
There was a problem hiding this comment.
This looks great--I wonder if it's cleaner to do something like:
$licenseSeat = LicenseSeat::find($seatId); // This will return null if seatId is null.
if (is_null($licenseSeat) {
$licenseSeat = $license->freeSeat();
if (!$licenseSeat) { return "No Available seats" }
}
Would seem to keep the logic a little cleaner... I know this predates you, but part of the logic above can be consensed as LicenseSeat::where('id', '=', $licenseSeat->id), which just seems silly :)
There was a problem hiding this comment.
@Azerothian Are you interesting in the rework @dmeltzer suggested here? I don't want to merge if you're working on that.
There was a problem hiding this comment.
I reworked this method quite a bit in #5916. -- https://github.com/snipe/snipe-it/pull/5916/files#diff-150d5333297ece1729c0c3ef7c758630R56 Might be better to just merge here as-is and then I'll touch that when I rebase the monster.
When a user clicks checkout on the seat page in licences, it will specifically checkout that seat for the assigned user or asset.
relates to #4413