-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Added #17687: Add User Responsible for a maintenances. #17907
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
base: develop
Are you sure you want to change the base?
Conversation
44a66f5
to
443e030
Compare
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.
This looks pretty good, thanks! I have a few requests here, but also I think I'd probably want to reverse the logic in the name, Responsible User
versus User Responsible
.
It could also get a little confusing, since this "responsible" could be interpreted as the person responsible for causing a maintenance event, versus the person responsible for ensuring the maintenance is handled.
I was actually already working on this issue (I have a branch that wasn't pushed up yet.) It's often better to ask first, just so we're not duplicating efforts :)
'completion_date' => 'date_format:Y-m-d|nullable|after_or_equal:start_date', | ||
'notes' => 'string|nullable', | ||
'cost' => 'numeric|nullable|gte:0|max:99999999999999999.99', | ||
'user_responsible_id' => 'nullable|integer' |
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 probably want a check here to make sure the user's ID exists in the user table.
'asset.supplier' => ['name'], | ||
'asset.assetstatus' => ['name'], | ||
'supplier' => ['name'], | ||
'user_responsible' => ['first_name', 'last_name'], |
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 think you want to refer to the relationship here, no? userResponsible
down below.
* | ||
* @return \Illuminate\Database\Eloquent\Relations\Relation | ||
*/ | ||
public function maintenanceCausedBy() |
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.
Can we rename this to maintenanceCausedByUser
or causedByUser
for clarity?
Add #17687.
This adds a User responsible field to maintenances. That way, we can track who cost a lot to a company.
The information is available both in the Users & the maintenance page.