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

[Improvement] Manage EOL date by setup eol_explicit status #13806

Conversation

Robert-Azelis
Copy link
Contributor

@Robert-Azelis Robert-Azelis commented Oct 27, 2023

Description

This improvement allow setup EOL date manually by user on damand by set eol_explicit checkbox.

BEFORE:
image

AFTER:
If checkbox is not checked then EOL date is hiden and calculation of date is on the base Model EOL rate
image

If checkbox is checked then EOL date is showed and user can input EOL Date manually.
Status of checkbox is saved as eol_explicit value
image

In this way is much easier to manage EOL date and explicit marker status instead of build complex rules :)

Type of change

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

How Has This Been Tested?

Test Configuration:

  • PHP version: 8.1
  • MySQL version 10.6.5-MariaDB
  • Webserver version IIS
  • Web browser: MS Edge, Chrome

Checklist:

snipe and others added 30 commits May 10, 2023 10:01
Signed-off-by: snipe <snipe@snipe.net>
Signed-off-by: snipe <snipe@snipe.net>

# Conflicts:
#	public/css/dist/skins/skin-red-dark.css
#	public/css/dist/skins/skin-red-dark.min.css
#	public/mix-manifest.json
Signed-off-by: snipe <snipe@snipe.net>

# Conflicts:
#	public/css/build/app.css
#	public/css/build/overrides.css
#	public/css/dist/all.css
#	public/mix-manifest.json
Signed-off-by: snipe <snipe@snipe.net>

# Conflicts:
#	public/css/build/app.css
#	public/css/build/overrides.css
#	public/css/dist/all.css
#	public/mix-manifest.json
Signed-off-by: snipe <snipe@snipe.net>

# Conflicts:
#	config/version.php
#	public/css/dist/all.css
#	public/js/build/app.js
#	public/js/build/vendor.js
#	public/js/dist/all.js
#	public/mix-manifest.json
This should provide LDAPS support out of the box, and fix snipe#13129
Fixed snipe#13129: Add missing LDAP lib required for LDAPS support
Signed-off-by: snipe <snipe@snipe.net>

# Conflicts:
#	config/version.php
Signed-off-by: snipe <snipe@snipe.net>

# Conflicts:
#	public/css/dist/skins/skin-yellow-dark.css
#	public/css/dist/skins/skin-yellow-dark.min.css
#	public/mix-manifest.json
Signed-off-by: snipe <snipe@snipe.net>
Signed-off-by: snipe <snipe@snipe.net>

# Conflicts:
#	config/version.php
Signed-off-by: snipe <snipe@snipe.net>

# Conflicts:
#	public/css/dist/all.css
#	public/css/dist/bootstrap-table.css
#	public/js/dist/bootstrap-table.js
#	public/mix-manifest.json
Copy link
Collaborator

@spencerrlongg spencerrlongg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I think I see the advantage to this now.

Could we add some text to the checkbox explaining what it is? I think it'd be a little confusing as is. It could just say "Explicit" or something.

The one concern I have is if you check the box on an existing asset that has EOL months set on the model, the date box defaults to the calculated date (which makes sense) and then you can save an asset with eol_explicit set to true, even though it's the same as the calculated date. This could get a little confusing and defeat some of the purpose of the feature.

Could we do a diffInMonths() check to make sure that it's actually different before saving?

Thanks again!

@Robert-Azelis
Copy link
Contributor Author

Robert-Azelis commented Oct 31, 2023

Hi, do you mean about something like this?
image

image

Could we do a diffInMonths() check to make sure that it's actually different before saving?
It is not good idea, in some cases purchase date could be not set, so cannot use diffInMonths() function.
Beside, if user decide to set explicit marker manually by enable checkbox and set EOL date (even if leave this same as it was calulated), then shuld be aware that EOL date is resistance to any furhter changes of asset model EOL rate and asset purchase date.
I think additionl check by diffInMonths() function is not neccesary.

Any changes are not yet done, let's make sure that we agree ;)

@Robert-Azelis Robert-Azelis reopened this Oct 31, 2023
@spencerrlongg
Copy link
Collaborator

Yeah, I think that's more or less what I mean for the text.

We could still check if purchase_date is set and only do diffInMonths() if it has been, this way it's a little more obvious to the user when they select the option that the date is possibly still the same as the calculated date.

Not a hill I'm ready to die on though - I do see your point about being able to set it explicit to not change the eol_date so that you can change the model eol without updating specific assets.

@Robert-Azelis
Copy link
Contributor Author

I'm sorry, by mistake updated my branch by master, not sure how to revert changes, so re-created this PR under new #13846

@Robert-Azelis Robert-Azelis deleted the Robert-Azelis-eol_explicit_improvement branch November 7, 2023 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants