Conversation
| $department_code = $faculty->department_code; | ||
|
|
||
| if ($faculty && $faculty->isHOD()) { | ||
| $courses = Course::with(['Classroom','Subject','Faculty'])->get(); |
There was a problem hiding this comment.
Classroom, Subject okke lower case alle vende
| $classrooms = Classroom::all(); | ||
| $faculties = Faculty::where('department_code',$department_code)->get(); | ||
| $subjects = Subject::where('department_code',$department_code)->get(); | ||
| $types = CourseTypes::getKeys(); |
There was a problem hiding this comment.
You can use this enum directly in views, no need to pass from controller
| $course->subject_code = $data["subject"]; | ||
| // Here is room for improvement instead of again querying in database table,we can infer it from above | ||
| // message/ do we need this in the first place <<Semester>> | ||
| $course->semester = Classroom::where('id',$data["classroom"])->get('semester')->first()->semester; |
There was a problem hiding this comment.
use find to get by id. Classroom::find($data["classroom"])->semester
There was a problem hiding this comment.
do course need a semester as it is always linked to classroom?
There was a problem hiding this comment.
Good question, classroom was added after semester, so we do need to to think about it.
But an obvious reason for semester is Minor courses. They don't have a particular classroom.
Maybe we could make semester null for other courses, and add a getter getSemester() to Course model, which will either get from classroom or from the semester field.
| $editCourse->faculty_id=$data["faculty"]; | ||
| $editCourse->classroom_id = $data["classroom"]; | ||
| $editCourse->subject_code = $data["subject"]; | ||
| $editCourse->type = CourseTypes::getValue($data["type"]); |
There was a problem hiding this comment.
Should all fields be updatable?
Faculty is ok, because they can be changed in between.
But subject, classroom and course_type?
| * Move Course creation to model | ||
| * Javascipt function to check subject code is unique or not in client side | ||
| * Optimise Course creation | ||
| * Update method - PUT/PATCH ? |
| */ | ||
| $auth_user = Auth::user(); | ||
| $faculty = $auth_user->faculty; | ||
| $department_code = $faculty->department_code; |
There was a problem hiding this comment.
Admin can create subjects. They may no be a faculty.
Checkout faculty.store, where I encountered a similar situation
There was a problem hiding this comment.
Isn't it better that HOD's do subject creation , since subject belongs to a department
There was a problem hiding this comment.
Admin can literally do anything, admin is GOD, so don't restrict admin.
|
@rkjcb camelCase is the convention, for almost all variable names, checkout that best practices repo. |
rkjcb
changed the title
2 participants
No description provided.