Employee leave #14
Employee leave #14
Conversation
vtozija
commented
Jul 14, 2017
vtozija
commented
- Employees asking for leave added in employee panel
- Admins apporving employee's leave added in admin panel
…d in header_employee
…employee_leave
| */ | ||
| public function getDatatable(EmployeeRepository $employeeRepository) | ||
| { | ||
| $id = $employeeRepository->findBy('email', Auth::user()->email)->first()->id; |
There was a problem hiding this comment.
You can get the id the same way as you get the email
Auth::user()->id
| public function getDatatable(EmployeeRepository $employeeRepository) | ||
| { | ||
| $id = $employeeRepository->findBy('email', Auth::user()->email)->first()->id; | ||
| return Datatables::of($this->employeeLeaveRepository->findBy('user_id', $id, ['id', 'user_id', 'leave_type_id', 'start_date', 'end_date', 'approved'])) |
There was a problem hiding this comment.
We don't need the user column in the table, nor the form for adding new leaves. This is for the logged user only and that info is redundant.
| */ | ||
| public function store(EmployeeLeaveRequest $request) | ||
| { | ||
| $employeeLeaveData = $request->all(); |
There was a problem hiding this comment.
It's very insecure to get the user id from the input request. Even though I only have the option to choose my user from the dropdown, I could inspect the element and change it's value before submitting the form. If I know the id of some other employee, I could easily request a leave in their name.
| $path = $request->attachment->store('uploads/leaves'); | ||
| $employeeLeaveData['attachment'] = $path; | ||
| } | ||
| $employeeLeaveData['approved'] = "pending"; |
There was a problem hiding this comment.
When using status columns, always use boolean instead of a string. We have only two values available and keeping strings instead wastes unnecessary resources.
| * @param \App\Modules\Pim\Repositories\EmployeeRepository $employeeRepository | ||
| * @return \Illuminate\Http\Response | ||
| */ | ||
| public function edit($id, LeaveTypeRepository $leaveTypeRepository, EmployeeRepository $employeeRepository) |
There was a problem hiding this comment.
This needs to have additional check in case the user opens other person's leave request. The way it is now, if I have the request id I could open the edit page and change the details.
| * @param \App\Modules\Leave\Http\Requests\EmployeeLeaveRequest $request | ||
| * @return \Illuminate\Http\Response | ||
| */ | ||
| public function store(EmployeeLeaveRequest $request) |
There was a problem hiding this comment.
We need separate request for this because the user id is picked up from session, not input.
| @@ -987,4 +987,34 @@ | |||
| Breadcrumbs::register('employee.home', function($breadcrumbs) | |||
| { | |||
| $breadcrumbs->push(trans('app.home'), route('employee.home')); | |||
| }); | |||
|
|
|||
There was a problem hiding this comment.
Breadcrumbs should go to leaves, not back to home page directly.
routes/breadcrumbs.php
Outdated
| $breadcrumbs->push($breadcrumb['title'], route('employee.leaves.update', $breadcrumb['id'])); | ||
| }); | ||
|
|
||
| Breadcrumbs::register('employee.leaves.destroy', function($breadcrumbs) |
There was a problem hiding this comment.
Only add breadcrumbs for existing pages, destroy doesn't need a breadcrumb.
