Improve License Editing to add or delete seats

[exula]

If the number of seats change for license this goes through and only deletes seats which are not assigned to a user.
If there are no available seats to delete it returns an error message (which looks like was already in the lang file)

Adding seats is easy and it just does it.

I've logged the deletion and addition of seats along the way.

** This is my first real pull request, I appreciate any help in making it better.

[exula]

This fixes issue #125

[snipe]

Nice work, @exula! Thanks! Moving forward, I'd like to move more of this logic into the model instead of the controller (fat models, skinny controllers), but your code looks good and it fixes a legitimate bug, so I'm going to merge this request and we can refactor later.

[exula]

Great, that makes total sense. Anything else in the future I hack at i'll keep that in mind.

[madd15]

When I changed a license to a smaller number in the history it shows i removed x number of seats but it only removes one from the license. eg 10 - 9 should be 1 but I got 9

[snipe]

Not sure I understand. If you changed it from 10 to 9, it would only take off one license seat.

[madd15]

i changed from 10 to 1 and was left with 9

[exula]

I'll take a look at this tomorrow when I get a chance. I probably totally screwed up my first commit.

[snipe]

@madd15 and to clarify, you had no users assigned to those 9, right? So they should be available for removal.

[madd15]

correct

[snipe]

Thanks - looking into it now. Apologies for the trouble.

[snipe]

I think I see what's happening. The number of seats in the licenses table is updating, but the number of records in the license seats isn't decreasing accordingly.

[snipe]

I think I got it with d1499de (and bef5170 because I'm a dumbass).

@exula - simple mistake to make. When you use ->first() after you pull the array of objects, it still treats that ->first() as the same record. So basically, your loop deleted the same record $x times, where $x is abs($difference). It took me a few to figure it out, too :)

So in this kind of circumstance, where you're not re-retrieving that array of objects within the loop itself and you just want to iterate through the loop, pop() would be a better choice.

So $seats->first()->delete(); becomes $seats->pop()->delete();

@madd15 - this issue should be resolved with the latest on develop (I'll be merging into master later tonight). Can you check and see if this is working for you now?

[exula]

@snipe That makes perfect sense. Bonehead mistake on my part.
Thanks for catching that, now I know the difference.

[snipe]

👍

[madd15]

Works like a charm

[snipe]

@madd15 - sweet, thanks for the quick reply!

[madd15]

Now when i set the seats to a higher number i get this error;

$logaction->checkedout_to = NULL; is the line that errors

ErrorException
Creating default object from empty value
open: C:\inetpub\wwwroot\snipe\app\controllers\admin\LicensesController.php
for ($i=1; $i <= $difference; $i++) {
//Create a seat for this license
$license_seat = new LicenseSeat();
$license_seat->license_id = $license->id;
$license_seat->user_id = Sentry::getId();
$license_seat->assigned_to = 0;
$license_seat->notes = NULL;
$logaction->checkedout_to = NULL;
$license_seat->save();
}

[snipe]

Balls. Sorry about that. I had to change that to NULL for mysql strict mode. It's not happening for me, but I'll check again.

[snipe]

Oh FFS. Bad copy+paste. I'm an idiot and should not be allowed to write code.

[snipe]

What line is that on, can you tell? I can't find that block of text in the Licenses controller.

[madd15]

Its on line 246

[snipe]

Hmm... https://github.com/snipe/snipe-it/blob/develop/app/controllers/admin/LicensesController.php - I don't see that there.

[madd15]

:/ ok the error has changed now

Illuminate \ Database \ QueryException
SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'checkedout_to' cannot be null (SQL: insert into asset_logs (asset_id, asset_type, user_id, note, checkedout_to, action_type) values (4, software, 1, 9 seats, , delete seats))

[snipe]

Did you run the DB migrations? php artisan migrate

[madd15]

i thought i had but i will do it again

[madd15]

Ok I'm a noob and its now working after migration

[snipe]

Haha - excellent. I'd rather have you be a noob than me be a crappy coder. :P

[madd15]

Haha :P

[exula]

Is it wrong that i'm glad the second half of problems here weren't my fault? ;)

[madd15]

Nope

[snipe]

LOL not at all, @exula