Improve Manage teacher #16
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the ProfessorController by adding validation request classes for store and update operations, and wraps the destroy method in a database transaction for data integrity.
Key Changes:
- Added
StoreProfessorRequestandUpdateProfessorRequestclasses for input validation - Wrapped the
destroymethod in a DB transaction to ensure atomic deletion of professor and user records
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| StoreProfessorRequest.php | New validation request class defining required fields for creating professors |
| UpdateProfessorRequest.php | New validation request class defining optional fields for updating professors |
| ProfessorController.php | Updated store/update methods to use request classes and added transaction handling to destroy method |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| return [ | ||
| 'name' => [ 'required', 'string', 'max:255', ], | ||
| 'email' => [ 'required', 'string','email', 'max:255', Rule::unique('users', 'email'), ], |
There was a problem hiding this comment.
Missing import for Rule class. Add use Illuminate\Validation\Rule; at the top of the file.
|
|
||
| return [ | ||
| 'name' => [ 'sometimes', 'string', 'max:255', ], | ||
| 'email' => [ 'sometimes', 'string','email', 'max:255', Rule::unique('users', 'email')->ignore($professorId), ], |
There was a problem hiding this comment.
Missing import for Rule class. Add use Illuminate\Validation\Rule; at the top of the file.
|
|
||
| DB::commit(); | ||
|
|
||
| retrun response()->json(null, 204); |
There was a problem hiding this comment.
Corrected spelling of 'retrun' to 'return'.
| retrun response()->json(null, 204); | |
| return response()->json(null, 204); |
| DB::commit(); | ||
|
|
||
| retrun response()->json(null, 204); | ||
| } catch (\Throwable e) { |
There was a problem hiding this comment.
Missing $ before variable name. Should be catch (\Throwable $e).
| } catch (\Throwable e) { | |
| } catch (\Throwable $e) { |
luigieli
force-pushed
the
manage-teacher
branch
from
cca0ef1 to
ec42509
Compare
1 participant
This PR intend to:
StoreandUpdaterequests toProfessorController.destroymethod onProfessorController.