Added EOL as de-normed and sortable on assets

[snipe]

Addresses #12392 and fixes #2372. This solves the issue of sorting by EOL date by de-norming that field in the database versus calculating it used off of the model EOL.

This will definitely need some thorough testing before we can to take it into master.

[what-the-diff]

• Added a new field to the assets table called asset_eol_date
• Updated all of the views and controllers that use this data, including adding it as an option in search filters
• Migrated existing EOL dates from models into assets (if they exist) so we don't lose any information when upgrading Snipe-IT

[uberbrady]

I think this generally makes a lot of sense, and could easily solve quite a few birds with very few stones. But I do have some concerns. One of them is inline, and specific, referring to the API changes.

But the next is a little more general. What happens if someone put the wrong model in when they initially input the asset, and then 'fixed it' later when they realize their mistake (honestly, not very likely, but certainly possible)? Would that update the de-normed field? And the same for purchase date (which is a lot more likely).

If the objective were just sorting, I can confirm that the following SQL - although quite vile - does work:

select
    assets.id,
    asset_tag,
    models.name,
    models.eol,
    date_add(purchase_date, INTERVAL models.eol month)
from
    assets
    INNER JOIN models ON (model_id = models.id)
ORDER BY
    date_add(purchase_date, INTERVAL models.eol month);

Though I can't confirm exactly how standards-compliant that is (or how well it might work under other database engines). Nor do I know how well that would perform (I would assume the answer would be "very badly"). And I have no idea how to represent that in Eloquent.

I think my thinking might be to add just a little bit more to handle all of the use-cases in the best way we can.

First off we would have a denormed-only field that always has the effective actual EOL date. This is never directly updated by the user, period. It's only ever calculated. Then we have another field which is the 'overriden_eol_date' - which is directly editable by the user, and nullable, and default-null.

Then, we do something (callback on pre-save?) that follows an algorithm like this:

if($this->overriden_eol_date) { 
   $this->denormed_eol_date = $this->overriden_eol_date; 
} else {
   $this->denormed_eol_date = $this->purchase_date + months($this->model->eol);
}

(Pardon my pseudocode).

This has the advantage of making the user's intent clearer. The moment they override the EOL date, that will always take precedence. They can delete that override if they want, then it will get calculated off the purchase date and EOL-per-model setting. Additionally, we can write an Artisan command that will recalculate, for every single asset, the value of the denormed field again, and no data (nor any 'intent') will ever be lost.

My general policy on denorming is that if you're using it for performance or functionality purposes, then it should never be editable. This way you don't lose data if you end up having to recalculate it.

Other possible ways of handling the issue are certainly through UI and clever bits of Javascript - changing the purchase date or model giving you a suggested 'recalculate' button which will let you recalculate based on any new settings (if a difference is detected).

All of that whining aside, I do like this PR, and I love its simplicity. I know it'd perform well and I know it would solve a lot of problems for a lot of users. Please do let me know if I can explain any of my thoughts on this further, and thanks again for your hard work on this!

[uberbrady]

I really do prefer this naming, but for backwards-compatibility reasons I think it might be better to go another way. My logic is this: users using the API are going to expect eol to be the actual eol date (and not a "date interval").

Since the date interval can be different than the defined eol date now (because it's been denormed), it definitely does make sense to return the 'default' eol duration, so that people can take action (if needed) on assets whose model-defined EOL no longer matches up with the asset-denormed-eol interval.

So I'd like to propse that we have eol return the denormed field and maybe something like default_eol to be the newly-defined value that describes the 'default' EOL for assets of this particular model.

[snipe]

Backwards compatibility will also make this super confusing though, since eol exists on the models field, and we'll likely end up with some ambiguous clauses. I'm honestly torn.