-
-
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
User department now visible in side pane of asset view page #13184
Conversation
PR Summary
|
Can you do me a solid and throw a screenshot in this PR summary? |
in the main post |
…used in a later PR
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 teensie change
@@ -45,4 +45,5 @@ | |||
'alert_details' => 'Please see below for details.', | |||
'custom_export' => 'Custom Export', | |||
'mfg_warranty_lookup' => ':manufacturer Warranty Status Lookup', | |||
'user_department' => 'User Department: ', |
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 actually try not to include the colon in the translations these days and instead put it in the blade IIRC
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.
fixed. saw a few others that had this formatting. I didn't change them, but will make sure NEW translations put the colon on the blade
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.
Yeah, there are a few places where it's inconsistent :(
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 more small change
@@ -919,6 +919,10 @@ | |||
</li> | |||
@endif | |||
|
|||
@if((isset($asset->assignedTo)) && ($asset->assignedTo->department!='')) |
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.
I'd probably change this to:
@if((isset($asset->assignedTo)) && ($asset->assignedTo->department))
so that we're making sure that the dept actually exists
Thank you for this line of code! Extremely helpful for our school to know which department our user (students in our case) belongs to when viewing assets. Would love to be able to also add Manager as well under the User Department. The Manager happens to be the "teacher's" name. @akemidx THANKS!! |
Department for a user is now visible in side pane when looking at an asset view.
Department is NOT visible on the index page as to reduce confusion, as we do not allow assets to be checked out to departments.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
tested locally
Checklist: