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

There were some weird constants I stripped out from the Asset model #15540

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

uberbrady
Copy link
Collaborator

@uberbrady uberbrady commented Sep 20, 2024

We have a chunk of weird constants and methods that we really didn't need to have in the Asset model. This PR pulls those out and just looks directly at assigned_type and compares to things like User::class et al.

I think this is easier to read and understand, and the additional abstractions were just confusing.

Tests pass, but that doesn't mean much. I'm happy to take a more deep pass through the code if we decide that this is even a direction we like.

Oh, and there were some code improvements I made to the assetLoc method that I think make it a bit cleaner to read.

Copy link

what-the-diff bot commented Sep 20, 2024

PR Summary

  • Optimization in Asset Handling: Code that was redundant or not in use, like the portion handling check if an asset is assigned to a user in AssetCheckinController.php, has been removed. This results in a neater code structure and less clutter.

  • Improved Clarity in AccessoryCheckout.php: Better comment notation has been added to improve overall code readability and maintenance for future developers.

  • Enhanced Recursion Control: A new constant, MAX_ASSET_ITERATIONS_FOR_LOCATION, has been introduced in Asset.php. This will be used to limit the depth of checking asset locations, adding an additional layer of protection against unwanted infinite loops.

  • Logic Refactoring: Decision-making processes in Asset.php have been refined and superfluous methods are removed, making our code leaner, easier to understand, and more efficient.

  • Improved Location Checking: The manner in which asset locations are checked has been refined in assetLoc method in Asset.php, resulting in more efficient and clearer code.

  • Updated Testing Procedures: Tests in StoreAssetTest.php have been altered such that they now check the assigned type directly instead of indirectly. This change paves the way for more straightforward and error-proof testing of code. The clearer test cases will facilitate pinpointing and rectifying problems faster.

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.

1 participant