abdullahshublaq / films Goto Github PK

I think $request->has('avater') is better

Change

<a href="index-2.html"> ....

to

<a href="/"> ....

• Installation:

When installing the app I faced some problem and found some missing steps.

-Composer update will install/update all dependency.

• Missing : copy .env.example .env

• Double install of laravel/passport. It's already inside your composer JSON. Don't need separate installation.

• php artisan migrate does not have --seed included

-After seed super admin is not created in the database.

• UI

• Use movie slug to show movies, now showing numbers

• Pretty url for seo > Actors URL can be improved

• Why contact us on every page? Create separate contact us page

• Change login Form Fields FIRST_NAME?

• Route
  -Why Duplicate
  Route::get('/user/reviews', 'ClientController@reviews');

• Controller
  -Refactor code Dashboard\mostly to all validate

  $attributes = $request->validate([
  'name' => 'required|string|max:30|min:3|unique:actors',
  'dob' => 'required|date',
  'overview' => 'required|string',
  'biography' => 'required|string',
  'avatar' => 'required|image',
  'background_cover' => 'required|image',
  ]);

convert to request class/service
or create a function like

private function validate()
{
    return request()->validate([
        'name' => 'required|string|max:30|min:3|unique:actors',
        'dob' => 'required|date',
        'overview' => 'required|string',
        'biography' => 'required|string',
        'avatar' => 'required|image',
        'background_cover' => 'required|image',
    ]);
}

and use, this will return an array which is validated. All extra request field will be exempt. ex: $attribute don't have $request->password values. you have to add it later.
$attributes = $this->validate();

• .idea folder must be included to .gitignore file. Why? It's phpstorm folder, unnecessary
• https://github.com/AbdullahShublaq/Films/blob/master/routes/api/api-v1.php#L26 - IMO anonymous functions should not be done in routes files
• https://github.com/AbdullahShublaq/Films/blob/master/routes/web.php#L25 - also personal opinion, but I like when all routes has it's name. Easier to do Post, because notation stays always the same: route('route_name')
• https://github.com/AbdullahShublaq/Films/blob/master/routes/web.php#L35 and https://github.com/AbdullahShublaq/Films/blob/master/routes/web.php#L42 - one is snake case, other - camel case. Choose one. Be consistent.
• https://github.com/AbdullahShublaq/Films/blob/master/app/Http/Controllers/Api/AuthController.php#L12 Form validation requests should be used there. Entire validation logic should be moved there
• Nice approach here: https://github.com/AbdullahShublaq/Films/blob/master/app/Http/Controllers/Dashboard/ClientController.php#L28 - Reviews doesn't have to be always negative 😊
• https://github.com/AbdullahShublaq/Films/blob/master/app/Http/Controllers/Dashboard/ClientController.php#L60 same story as before - please consider using form validation requests.
• https://github.com/AbdullahShublaq/Films/blob/master/app/Http/Controllers/Dashboard/ClientController.php#L85 I think this can be replaced with redirect()->route('dashboard.clients.index')->with('success', 'Client Added Successfully');
• https://github.com/AbdullahShublaq/Films/blob/master/app/Http/Controllers/Dashboard/ClientController.php#L95 Empty method. I know about ,, I will remove it later" - nothing is more permanent than word ,,temporary" :)
• Traits should have their own namespace https://github.com/AbdullahShublaq/Films/blob/master/app/favoritable.php 9 ( IMO )
• https://github.com/AbdullahShublaq/Films/blob/master/app/favoritable.php#L39 - return false; return true; TRUE and FALSE should be in lowercase. No one likes being screamed at. Not even functions.
• https://github.com/AbdullahShublaq/Films/blob/master/app/favoritable.php#L39 can be done in one line - think how :)
• Also also - Traits filename should start in uppercase.

Sorry for nitpicking in some places. In my opinion code should always stay consistent, use PSR standards.
Please consider phpCS ( code sniffer ) and phpMD ( mess detector ). These 2 tools will improve your code quality and cleanliness in general.

Good luck,

Karolis from OneSoft

Add this mimes:jpeg,png,jpg

I suppose u need to use deleted instead of deleting .
the reason is you cant be sure the Actor model will be deleted, maybe some kind of an error would stop Actor be deleted .

I feel you are duplicating the code using an additional method. I have written an article to reduce that duplication and make the code more maintainable. Read.

https://dev.to/bawa_geek/laravel-api-the-best-way-to-return-the-uniform-response-for-all-type-of-requests-2ao1

I feel you can do better in login method:

1- It will be better to move the validation to request form, and be more specific in your rules as follow:

$validation = $request->validate([
    'email' => 'required|string|email',
    'password' => 'required|string|min:8|max:20' // feel free to change the numbers ..
]);

2- It will more readable to change this part:

$login = [
    'email' => $request->email,
    'password' => $request->password
];

To:

$credentials = $request->only(['email','password']);

3- Last thing, I saw your API files, you duplicated the return response a lot, and that would be problem when in the future you decide to change the form of that response and add another things to it, would be nice to put it in one place:

• Go to Controller.php (the core of all controllers) and create new protected method like this:

protected function responseJson($status, $message, $token = null)
{
    return response([
        'status' => $status,
        'message' => $message,
        'token' => $token
    ]);
}

Then In any controller just call the responseJson one as follow:

return $this->responseJson('success', 'auth success',$token);

You have to clean up your api routes methods, you are just implementing one method (show or only) your API Controllers, so it is better to limit your request to that method, otherwise it may generate unexpected errors.

https://laravel.com/docs/7.x/controllers#restful-partial-resource-routes