-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Added #14426: Makes all Manufacturer links dynamic, not just warranty lookup #14530
Conversation
PR Summary
|
…() method in hardware/view.blade.php
Please let me know if there's a better way to do this or if there's anything else I can do to get this merged. @snipe Thanks. |
@@ -281,21 +281,21 @@ | |||
<li> {{ $asset->model->manufacturer->name }}</li> | |||
@endcan | |||
|
|||
@if (($asset->model) && ($asset->model->manufacturer->url)) | |||
@if (($asset->model->manufacturer) && ($asset->model->manufacturer->url!='')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do actually have to keep the model check here, in case someone deleted a model directly from the database. In that case, ($asset->model->manufacturer)
will 500, since we're looking for a property on a relationship that doesn't exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small change, but otherwise this looks good!
Never heard back from you. I'm going to accept this PR and make that change myself. Thanks! |
Congrats on merging your first pull request! 🎉🎉🎉 |
Description
As requested in #14470, I'm retargeting for develop branch. I couldn't find an easy way to do this so I just started over. Deleted, reforked, made changes against the develop branch and retested on testing environment with debugging enabled.
This makes all manufacturer links dynamic just like warranty lookup.
Added #14426
Type of change
How Has This Been Tested?
I've tested working on latest from my test environment.
Test Configuration:
Checklist: