Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Sorting Based On EOL Date #12392

Closed
wants to merge 4 commits into from
Closed

Conversation

corydlamb
Copy link
Contributor

@corydlamb corydlamb commented Jan 22, 2023

Allowing sorting via EOL.

Description

Saw a post ( #12372 ) requesting this, gave this change a test on our server and it seems to work fairly well. If an asset has a buy date and EOL on the model it can sort by the dates.

Fixes #12372

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Tested on my server, another really small change in the asset presenter file.

  • Test A
  • Test B

Test Configuration:

  • PHP version: 8.1.11
  • MySQL version 15.1
  • Webserver version Apache 2.4.38
  • OS version Turnkey Linux (updated to latest version of SNIPE (6.0.14)

Checklist:

@what-the-diff
Copy link

what-the-diff bot commented Jan 22, 2023

  • The field 'eol' is now sortable and visible.

@corydlamb corydlamb changed the base branch from master to develop January 22, 2023 06:34
@corydlamb corydlamb mentioned this pull request Jan 22, 2023
12 tasks
@corydlamb corydlamb changed the title Update AssetPresenter.php Allow Sorting Based On EOL Date Jan 22, 2023
@snipe
Copy link
Owner

snipe commented Jan 22, 2023

EOL on the asset isn't sortable because it's not a native database field, it's calculated.

'eol' => ($asset->purchase_date != '') ? Helper::getFormattedDateObject($asset->present()->eol_date(), 'date') : null,

public function eol_date()
{
if (($this->purchase_date) && ($this->model->model) && ($this->model->model->eol)) {
$date = date_create($this->purchase_date);
date_add($date, date_interval_create_from_date_string($this->model->model->eol.' months'));
return date_format($date, 'Y-m-d');
}
}

@corydlamb
Copy link
Contributor Author

corydlamb commented Jan 22, 2023

EOL on the asset isn't sortable because it's not a native database field, it's calculated.

'eol' => ($asset->purchase_date != '') ? Helper::getFormattedDateObject($asset->present()->eol_date(), 'date') : null,

public function eol_date()
{
if (($this->purchase_date) && ($this->model->model) && ($this->model->model->eol)) {
$date = date_create($this->purchase_date);
date_add($date, date_interval_create_from_date_string($this->model->model->eol.' months'));
return date_format($date, 'Y-m-d');
}
}

Oh I see, my apologies!

@snipe
Copy link
Owner

snipe commented Jan 22, 2023

I think we'd need to do something more like this to make it work: #12393

@snipe snipe closed this Jan 22, 2023
@snipe
Copy link
Owner

snipe commented Jan 22, 2023

I'd be happy to go into more detail on that PR if you have questions - it does get a bit in the weeds. Definitely appreciate you trying to solve it via this PR though. As you can see, it's a little trickier than it first seems offhand, as so many things are.

@corydlamb
Copy link
Contributor Author

I'd be happy to go into more detail on that PR if you have questions - it does get a bit in the weeds. Definitely appreciate you trying to solve it via this PR though. As you can see, it's a little trickier than it first seems offhand, as so many things are.

Thank you so much! I really appreciate how welcoming and freindly to novices like myself y'all are! I guess I'm just curious as to why calculated values shouldn't be sorted. There's a lot of reading up I should probably do on php tbh 😅

@snipe
Copy link
Owner

snipe commented Jan 22, 2023

It's honestly largely just the complexity of the query in the database (AND people have asked to be able to edit that date directly in the past, so we can kill two birds with one stone here.) Even though we technically only officially support MySQL and MariaDB, I do know that some folks are using other databases (pgSQL, SQLite, etc) so we generally try to keep those queries as simple as possible for maximum compatibility. That said, my PR could get rejected by my team come Monday, so... we'll see :D

So it's a bit more of a product decision than it is a technical one, and there's no way you could have known every little thing that folks have asked for here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants