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

Standardize Asset EOL Date #13585

Merged
merged 53 commits into from
Oct 2, 2023
Merged

Standardize Asset EOL Date #13585

merged 53 commits into from
Oct 2, 2023

Conversation

spencerrlongg
Copy link
Collaborator

@spencerrlongg spencerrlongg commented Sep 11, 2023

Description

Second attempt at this PR after blowing up the first attempt with a rebase.

This should help us get to more of a denormalized Asset EOL Date, and hopefully remove the need for any on-the-fly calculation of that date. The migration calculates and updates all fields where possible, and the observer and asset model controller should always make sure that the value is calculated when it can be. Furthermore, this adds an eol_explicit column to Assets to indicate when asset_eol_date does not match what the calculated date would be.

On the frontend the only change of note is adding an indicator that the asset_eol_date is explicitly set (or differing from what would be calculated)

image

Type of change

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

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • API
  • GUI
  • Importer

Test Configuration:

  • PHP version: 8.1
  • MySQL version 8.1

Checklist:

@what-the-diff
Copy link

what-the-diff bot commented Sep 11, 2023

PR Summary

  • Enhancement of Asset Lifecycle Management
    In AssetModelsController.php and AssetsController.php, the logic has been updated to better handle the lifecycle data of the assets (asset_eol_date and eol fields). This includes an improved method to store and update lifecycle information. This will enable more accurate and reliable lifecycle management for the assets.

  • Update on Data Importing Features
    In the Importer.php, and AssetImporter.php, the ability to import asset_eol_date, an important lifecycle date for assets, has been added which will make data imports more detailed and useful.

  • Commented-out Logic in ItemImporter.php
    The logic related to asset_eol_date in ItemImporter.php has been commented out as it is currently not needed. This means asset_eol_date won't be processed when importing items data.

  • Improvement on Asset's Data Structure
    In Asset.php, the data structure for assets has been improved - new rules have been introduced, and certain data types have been explicitly defined. This will ensure a more uniform data structure for assets, and prevent potential inconsistencies.

  • Enhancements in Asset Presentation
    In AssetPresenter.php, the method of computing the eol_date (end of life date) for assets has been refined. This now offers a more accurate representation of asset lifecycle data.

  • Update on Test Data Generation
    In AssetFactory.php, the logic to generate test data has been updated to include asset_eol_date, thereby providing a more comprehensive dataset for testing.

  • Database Update with Migration
    A new migration file has been added which introduces a new field (eol_explicit) and modifies an existing field (asset_eol_date). This will better accommodate the updated asset management functionality while ensuring the integrity of existing data.

@spencerrlongg spencerrlongg marked this pull request as ready for review September 14, 2023 21:21
@marcusmoore
Copy link
Collaborator

FYI: I started reviewing this and will wrap up tomorrow.

Copy link
Collaborator

@marcusmoore marcusmoore 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 all of the effort put into this. I know it was a heavy lift.

I ran into some exceptions while testing it out that we gotta address before merging 😕

app/Models/Asset.php Outdated Show resolved Hide resolved
app/Models/Asset.php Outdated Show resolved Hide resolved
app/Models/Asset.php Outdated Show resolved Hide resolved
database/factories/AssetFactory.php Show resolved Hide resolved
app/Http/Controllers/AssetModelsController.php Outdated Show resolved Hide resolved
app/Http/Controllers/Assets/AssetsController.php Outdated Show resolved Hide resolved
app/Http/Controllers/Assets/AssetsController.php Outdated Show resolved Hide resolved
app/Http/Controllers/AssetModelsController.php Outdated Show resolved Hide resolved
resources/views/partials/forms/edit/eol_date.blade.php Outdated Show resolved Hide resolved
Copy link
Collaborator

@marcusmoore marcusmoore left a comment

Choose a reason for hiding this comment

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

Looks good! 😄

@snipe snipe merged commit 129e9b9 into snipe:develop Oct 2, 2023
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.

4 participants