Added #6695: add API endpoint for license seats

[marcquark]

This PR implements a dedicated API endpoint for license seats. The endpoint can be used to get all seats of a particular license, get one specific seat aswell as update one specific seat's assignments. The new controller contains logic to determine whether an update operation is a checkin or checkout and adds logs accordingly.

• Implements API Endpoint for License seats #6695
• Implements missing checkout / check-in on licenses API #6875

It also deprecates the existing /licenses/{id}/seats route, which was only used in one instance anyway. It the output format remains the same, so should be backwards compatible.

There are also a bunch of tests. I haven't yet fully understood on what data the testing framework operates. But the seeded DB data only contains licenses that haven't been checked out to anyone. Hence i think a proper test for checkin cannot be implemented at the moment.

[marcquark]

Hmm, i don't really know how to simplify/reduce the complexity Codacy is complaining about.

Sure, could skip sanity checks and some other validation logic, but i'd rather have a slightly more complex function than an API that doesn't tell the user why the input is not plausible...

Similarly, i could of course make dedicated checkin/checkout functions but the point of this endpoint is to have an easy way of manipulating seats externally. This is especially useful if the user doesn't agree with the way snipe-it determines how and when these fields should be populated (e.g. the annoying bug that seats which are assigned to assets stick with the original user when an asset is reassigned).
But i'd also not like an API endpoint that doesn't log actions. That'll just lead to more confusion for users that only use the GUI.

[jkour]

@marcquark Codacy suggestions don't really make sense in this case I believe.

I wonder how would the functions measure if you created smaller functions for all those ifs...

Disclaim: I do not code in php so it is just a general observation

[marcquark]

That's what i thought too. For the sanity checks, it feels like there should be some elegant way to inject those into Laravel for specific routes or methods of the controller, but i couldn't find a decent example online. Having that could avoid having to copy+paste the same logic twice. The approach of moving it into a private function and calling that became complex quickly, so i bailed out.

As far as the other logic goes, i'm actually less happy with what i produced now vs the original PR. Got the codacy score slightly lower, but it's now harder to understand imho.

@snipe must there absolutely be a green tick in the codacy check for the PR to be accepted?

[jkour]

TBH I don't really follow the suggestion that "else" statements should be completely avoided

[jkour]

@marcquark Is it possible to get your fork and install it myself?

[snipe]

Just a heads up - once v5 is out, we're going to be completely reworking the license section to make it work a bit more like asset models, with seats being more like assets.

[jkour]

@snipe When do you reckon it will be out? Any plans in place?

[marcquark]

@jkour sure you can just pull the develop branch from my forked repo. though i don't know whether that is a wise thing to do. after all if major changes are coming with v5 it's very likely that the endpoint will be shuffled around. don't know how urgently you need the feature but it may not be worth adjusting all you tools and then redo everything in a few months or so

[jkour]

@marcquark Not in a rush really...Maybe I will wait for the new version

Thanks anyway

[marcquark]

Hi @snipe,

so it looks like v5 didn't include any major changes to the license model and API. Hence i'd like to bring back attention to this PR. Any chance to get it merged at some point? A lot of people have been waiting for this feature for quite some time now.

[welcome]

Congrats on merging your first pull request! 🎉🎉🎉