Feat:user controller created and route name added for profile by alamriku · Pull Request #42 · alamriku/LMS

alamriku · 2020-11-23T18:05:12Z

No description provided.

me-shaon · 2020-11-24T04:45:28Z

+        $this->middleware('auth');
+    }
+
+    public function profile($id){


PSR coding style violation.
{ should be on the next line.

Fix it in other places too.

me-shaon · 2020-11-24T04:46:39Z

+    }
+
+    public function profile($id){
+        $user = User::where('id',$id)->first();


PSR coding style violation.
where('id',$id), there should be a space after comma, like this: where('id', $id)

Fix it in other places too.

me-shaon · 2020-11-24T04:48:14Z

+        $this->middleware('auth');
+    }
+
+    public function profile($id){


Rather than passing an id and querying to find the user with that id, just bind Route model here.

me-shaon · 2020-11-24T04:48:50Z

+    }
+
+    public function profileUpdate(Request $request){
+            $validator = Validator::make($request->all(),[


Don't write validation logic here. Create separate Request file with validation logic.

me-shaon · 2020-11-24T04:49:22Z

+            $profile->name=$request->name;
+            $profile->email=$request->email;
+            if($request->filled('password')){
+                dd('test');


non-functioning, broken code.

me-shaon · 2020-11-24T04:49:38Z

+                return redirect()->back()->with('success','nothing updated');
+            }
+
+


Remove redundant empty lines.

me-shaon · 2020-11-24T04:50:03Z

+                $profile->password=$request->password;
+            }
+           $profile->update();
+            if($profile->wasChanged()){


No need to have this check.

me-shaon · 2020-11-24T04:51:10Z

    <div class="text-center navbar-brand-wrapper d-flex align-items-center justify-content-center">
-        <a class="navbar-brand brand-logo mr-5" href="index.html"><img src="images/logo.svg" class="mr-2" alt="logo"/></a>
-        <a class="navbar-brand brand-logo-mini" href="index.html"><img src="images/logo-mini.svg" alt="logo"/></a>
+        <a class="navbar-brand brand-logo mr-5" href="index.html"><img src="{{asset('asset')}}/images/logo.svg" class="mr-2" alt="logo"/></a>


The whole image path should be inside asset method.

me-shaon · 2020-11-24T04:53:53Z

 })->middleware(['auth'])->name('dashboard');

+Route::get('profile/{id}',[UserController::class,'profile'])->name('profile');
+Route::put('profile-update',[UserController::class,'profileUpdate'])->name('profile-update');


The route should be profile/update.

me-shaon · 2020-11-24T04:54:26Z

    return view('admin.dashboard');
 })->middleware(['auth'])->name('dashboard');

+Route::get('profile/{id}',[UserController::class,'profile'])->name('profile');


As the profile should be only for the 'logged in' user, no need to pass the user id here.

me-shaon · 2020-11-28T16:52:23Z

+    {
+        $users = User::paginate(5);
+        return view('user.list', compact('users'));
+    }


Add an empty line after the method.

me-shaon · 2020-11-28T16:52:37Z

+
+    public function profileUpdate(ProfileRequest $request)
+    {
+            $request->validated();


The indentation is wrong.

me-shaon · 2020-11-28T16:53:35Z

+    }
+    public function index()
+    {
+        $users = User::paginate(5);


Don't hardcode the paginate value here. Set it as a constant in a global 'Constant' file or in the Model.

me-shaon · 2020-11-28T17:06:11Z

    use HasFactory, Notifiable,SoftDeletes;
    const User ="User";
-    const Role="Role";
+    const Author="Author";


The constant should be ROLE_AUTHOR.

me-shaon · 2020-11-28T17:07:49Z

 })->middleware(['auth'])->name('dashboard');

+Route::get('profile/{user}',[UserController::class,'profile'])->name('profile');
+Route::get('user/list',[UserController::class,'index'])->name('user-list');


The route should be users.

me-shaon · 2020-11-28T17:08:33Z


+Route::get('profile/{user}',[UserController::class,'profile'])->name('profile');
+Route::get('user/list',[UserController::class,'index'])->name('user-list');
+Route::delete('user/delete/{user}',[UserController::class,'destroy'])->name('user-delete');


The route should be users/{user} with the DELETE method.

me-shaon · 2020-11-28T17:09:06Z

+Route::get('user/list',[UserController::class,'index'])->name('user-list');
+Route::delete('user/delete/{user}',[UserController::class,'destroy'])->name('user-delete');
+Route::put('profile/update',[UserController::class,'profileUpdate'])->name('profile-update');
+Route::put('user/ban/{user}',[UserController::class,'Ban'])->name('ban-user');


The route should be users/{user}/ban.

me-shaon · 2020-11-28T17:09:42Z

+           $profile->update();
+           return redirect()->back()->with('success','Profile updated');
+    }
+    public function Ban(User $user)


The method name shouldn't have Uppercase Letter at the beginning.

me-shaon · 2020-12-04T10:50:46Z

+    use Notifiable;
+    use SoftDeletes;
+
+    public const USER = "User";


Rename this to ROLE_USER.

me-shaon · 2020-12-04T10:51:03Z

+
+    public const USER = "User";
+    public const ROLE_AUTHOR = "Author";
+    public const BANNED = 1;


Rename this to STATUS_BANNED.

me-shaon · 2020-12-04T10:51:14Z

+            }
+           $profile->update();
+           return redirect()->back()->with('success','Profile updated');
+    }


You've missed this.

me-shaon · 2020-12-04T10:51:23Z

+    public function profile(User $user)
+    {
+        return view('profile', compact('user'));
+    }


Add an empty line after the method.

me-shaon · 2020-12-04T10:51:51Z

+    }
+    public function profileUpdate(ProfileRequest $request)
+    {
+        $request->validated();


This method call is redundant.

me-shaon · 2020-12-04T10:52:16Z

@@ -72,4 +73,9 @@ public function changedReturnStatus()
    {
        return $this->hasMany(ReturnRequest::class,'status_changed_by');
    }


You've missed this.

me-shaon · 2020-12-04T10:53:04Z

+    use SoftDeletes;
+
+    public const USER = "User";
+    public const ROLE_AUTHOR = "Author";


Shouldn't it be 'Librarian'? We don't have a 'Author' type user.

me-shaon · 2020-12-04T10:53:32Z

+            'name' => $this->faker->name,
+            'email' => $this->faker->unique()->safeEmail,
+            'role'=>User::User,
+            'phone'=>$this->faker->phoneNumber,


You've missed this.

me-shaon · 2020-12-04T10:54:15Z

+                            @php($i=1)
+                            @foreach($users as $user)
+                            <tr>
+                                <th>{{$i++}}</th>


Use $loop->index instead of managing your own loop variable.

me-shaon

Code Styling issues are not fixed yet.

alamriku · 2020-12-08T15:05:34Z

add an empty line after the method or the unused method are not removed?

me-shaon · 2020-12-08T16:19:47Z

add an empty line after the method or the unused method are not removed?

Both.

Feat:user controller created and route name added for profile

fa98a44

alamriku requested a review from me-shaon November 23, 2020 18:05

Feat:user profile update done

a68746e

me-shaon requested changes Nov 24, 2020

View reviewed changes

alamriku added 5 commits November 25, 2020 21:02

fix:review fixed

06ad27e

User List and user delete done

1165824

only users will be deleted

a85b9be

feat:user ban un-ban done

c801c9d

ban check on login

40cf8c9

me-shaon requested changes Nov 28, 2020

View reviewed changes

alamriku added 6 commits December 4, 2020 12:38

user login checking

fad8963

user dashboard blade file created

99e5df2

gate facade is used to authorize user list to only author

af183bc

refactor

08dab68

constant refactor

c4418d3

refactor

c50a242

me-shaon requested changes Dec 4, 2020

View reviewed changes

alamriku added 6 commits December 4, 2020 18:16

refactor

c577312

service class created for user

462846d

librarian form add

5ba5d27

librarier add by librarian done

5a2f3d9

author adding done

e78e8b5

File Service and author service added

0178b15

me-shaon requested changes Dec 8, 2020

View reviewed changes

		return redirect()->back()->with('success','nothing updated');
		}

Conversation

alamriku commented Nov 23, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment