-
Notifications
You must be signed in to change notification settings - Fork 103
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
Create job advert #4
base: master
Are you sure you want to change the base?
Conversation
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.
Specific comments are left where needed. In general:
- Make sure the code format is PSR2 compliant. You can install this package in Sublime and build the files before committing.
- All functions should be properly documented, taking into account all input params and their type. You can use DocBlockr in Sublime.
- When adding other classes with "use", make sure they are needed in the current class.
$jobData = $request->all(); | ||
$jobData = $this->jobAdvertRepository->create($jobData); | ||
$request->session()->flash('success', trans('app.pim.job_advert.store_success')); | ||
return redirect()->route('recruitment.job_advert.edit', $jobData->id); } |
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.
Make sure the format of your code follows PSR2 standards. Check this package for Sublime that builds the code for you.
/** | ||
* Store a newly created resource in storage. | ||
* | ||
* @param \Illuminate\Http\Request $request |
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.
Make sure the functions are documented properly, using the actual type of the input parameters and the response. Check other controllers for reference.
namespace App\Modules\Recruitment\Http\Controllers; | ||
|
||
use Illuminate\Http\Request; | ||
use App\Http\Controllers\Controller; |
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.
After finishing with the rest of the functions in this controller, check what of these "use" statements is not needed and remove them.
namespace App\Modules\Recruitment\Http\Requests; | ||
|
||
use Illuminate\Foundation\Http\FormRequest; | ||
use Illuminate\Validation\Rule; |
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.
You don't need Rule and Route here. Make sure you only include the classes you use.
{ | ||
$rules = [ | ||
'title' => ['required'], | ||
'image' => ['required'] |
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.
The image is not required, you can remove it. Please add the description as well as required field.
|
||
use DB; | ||
use App\Repositories\EloquentRepository; | ||
use App\User; |
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.
Remove the unnecessary use statements.
@@ -0,0 +1,109 @@ | |||
@extends('layouts.main') | |||
@section('content') |
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.
In cases like this, when the file is copied from another module but not yet adapted, please remove irrelevant data and leave the basic structure only so it's not confusing.
@@ -25,6 +25,7 @@ public function register() | |||
Settings\Repositories\SalaryComponentsRepository::class => [Settings\Repositories\Interfaces\SalaryComponentsRepositoryInterface::class], | |||
Pim\Repositories\EmployeeRepository::class => [Pim\Repositories\Interfaces\EmployeeRepositoryInterface::class], | |||
Pim\Repositories\CandidateRepository::class => [Pim\Repositories\Interfaces\CandidateRepositoryInterface::class], | |||
Recruitment\Repositories\JobAdvertRepository::class => [Recruitment\Repositories\Interfaces\JobAdvertRepositoryInterface::class], |
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.
Please move this below together with the rest of the Recruitment bindings.
resources/lang/en/app.php
Outdated
'update_success' => 'The job advertisement details were successfully updated.', | ||
'delete_success' => 'The job advertisement was successfully removed.', | ||
'additional' => 'Additional details', | ||
'preferences' => [ |
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.
The preferences part is forgotten here, right? Please check.
routes/web.php
Outdated
@@ -166,6 +166,7 @@ | |||
'destroy' => 'candidates.destroy' | |||
]]); | |||
|
|||
|
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.
Please remove this extra new line.
Add BrowserStack logo Adjust BrowserStack URL - readme change BrowserStack logo format Update browserstack url Update Readme Add BrowserStack reference to readme Update Readme
…or gender and date clausole for birth date
Fix UTF encoding issue
…admin panel, employee login added
Replaced required rules on gender and birth date fields
…or gender and date clausole for birth date
…admin panel, employee login added
…employee_login
…admin panel, employee login added
…admin panel, employee login added
…employee_leave
Employee leave
Employee time log
Dashboard documents
Employee salary
Added new subsection for creating job advertisements inside the Recruitment section. New migration, new form with validation, Resource Controller, Model, Repository and Interface.