---
# System prepended metadata

title: 'Rough file by file analysis:'

---

# Rough file by file analysis:


## AuthController
- The code uses the Validator facade for validation, instead of using $request->validate. The facade creates unnecessary boilerplate
- Emails are hardcoded into controllers instead of using them through a service
- The application doesn't use a unified response structure
- Array indices used without proper validation, will throw a fatal error - 89
- DB facade used instead of model - 114

## CheckListItemController
- Functions created without access modifiers
- Models used directly inside controllers instead of going through services/repositories
- No response returned if lead is not found - 50
- Duplicate queries ran multiple times for no reason - 61
- request() method used instead of validating input and using $request->validated()
- use of property setters instead of using $model->update([]);
- Auth facade used in multiple places instead of using $request->user(), less boilerplate

## LeadController
- Direct use or role() function instead of using permission middleware
- $request->property_name will fail without validation if not present in the request body - 41, 44, 47, etc
- Poor code indentation
- Some of the properties used in the Store method are not present in the validation request. Will throw a fatal if not present in the body
- Eloquent::create used in loop instead of using batch insert, poor performance
- $request->agent_id used but not validated, will fail if not present in request body
- $status_callback and $agent_callback are used only once, no need to store in variable and waste memory
- Again, use of random response structure - 191
- Use of duplicate queries, poor performance
- Extremely bad code at line 206->221, can be improved using $request->validate
- code after line 224 will throw a fatal error if lead wasn't found
- $leadActivity[0] used without verifying that index exists, will fail
- False "successfully deleted" response even if lead wasn't found - 253

## Notes controller
- lead_id is not validated, this will fail with improper request body - 27
- in-memory filter is used instead of whereIn SQL clause, will perform worse
- lead_id is not validated, this will fail with improper request body - 49
- status is success while lead wasn't found, alse the response status is 200 instead of 404 - 53
- too much boilterplate 51, 53
- no validation at all for update method
- in-memory filter - 79
- same problems with the delete method

## User detail
- wrong response code - 58
- not all of the fields are validated, will throw a fatal error
- wrong naming convention for $UserDetail, use camelCase or snake_case
- another example of poor code at line 98->105

## UserObserver
- request() helper used in an observer! this oberver will also be used in non HTTP flows (queues), and will fail
- multiple points of roles assign, role_id in users table and model_has_roles table 


# Overall bad coding practices
- Controller methods are not using camelCase for function names
- Error responses are returned with 200 status code at multiple places
- All api requests are using the POST method, will cause problems when creating interceptors, should use GET, POST, PUT and DELETE
- roles are hardcoded into the code. We should instead hardcode permission middlewares and later assign permissions to arbitraray roles.
- Code is not indented almost anywhere and looks like spaghetti code
- multiple points of roles assign, role_id in users table and model_has_roles table 
- All the business logic is in the controllers instead of it being in services for code reusability and maintainance
- Most code is riddled with single-point-of-failure problems.

# Reviewer notes:
After giving a rough look to the codebase, I find that there are multiple developers working on this code. One of them is fairly experienced and writes good code, but very little of the code is theirs, mostly the initial setup. Rest of the code looks to be written by an entry level developer, probably under an year of experience or poor understanding of low-level concepts such as code secutiy and performance. 

Senior level coder: Haider Javed
Junior programmer with most of the code: Mohammad Awais Sarwar