Bug fix/Extend Laravel 5.3 "custom" project

For a background on this project, please read here: https://laracasts.com/discuss/channels/code-review/problematic-non-conventional-project

Greetings!

I'm quite new to Laravel but have been reading/watching courses non-stop for the last 2-3 weeks. That said, I'm in current need of help on how to get started fixing/improving a Laravel 5.3 "Client Portal" (running on an Apache server w/ PHP 5.6).

Like I said on the aforementioned topic, my predecessor built it, but from what I can tell they customised/improvised a lot of the document/folders naming/placing, so I'm feeling quite dumb trying to find my way around it.

Basically, it's a "Client Portal" where employees log in, register interpreters, clients, there's a calendar for bookings, and other related data.

Here's a list I got from my employer, with some other bugs I found that need fixing, as well as some features to be added:

- Mixed languages (ie: The system understands that Brazilian Portuguese and Portugal Portuguese are the same language): 
I'm guessing it's a matter of simply creating a different record for each on the database, with different IDs. 

- Forgot password doesn't work: 
The email gets sent to the user, the link in it works, you get to type in a new password twice, but then it just doesn't change. This is the one fix I'm working on right now, and none of the files match up with documentation instructions/guides/tutorials. 

Seems to me like, instead of doing "php artisan make:auth", they just chose a directory and created whatever they felt like.

- Date formatting in American: 
Just a matter of finding which file is handling this. 

- Interpreter Availability missing: 
Table showing interpreters has no "availability" column.

- Can't Add comments to each interpreter:
Will just add the field.

- Can't prevent duplicates: 
If two employees add the same interpreter, the system allows it. 

- Can't delete bookings (administrator function):
Need to setup different privileges to employees (users), as to allow some of them to remove calendar bookings.

- Can't delete interpreters/employees:
Need to setup a button and functionality to delete users/interpreters.

- Can't filter by hour appointment:
Need to add further filtering options to bookings.

- Set default to current date:
Calendar default view isn't current date.

- Can't see that inactive interpreter has inactive status when creating a booking:
Also, inactive interpreters show up on their list. My plan is to make a secondary list for inactive interpreters (like an archive) and move them from the main list.

- Active/inactive filter not working:
The above solution should indirectly solve this.

- Second languages not searching:
Interpreters, more often than not, have multiple languages but there's no way to filter those, only the "primary" language.

- Add priority to interpreters closest to the booking's location: 
Will probably have to integrate Google Maps API functionality to calculate location distances.

What I hope to find here in Laracasts/Discuss is obviously not someone to do the work for me, but someone to suggest where I should begin, taking in consideration the "custom" nature of this project.

Thanks for reading this far, I really do appreciate any and all input!

D9705996

7 years ago

Level 51

Do you have an existing test suite to help you ensure that when you refactor you dont break anything. If not you need to identify how the existing application should work and write tests for all of the key behaviour in the system. You dont necessarily need 100% code coverage but you need to cover how your customer uses the application now so it still does this after making changes.

Once you have covered the existing behaviour start writing tests for the things in the list that should work e.g. an interpreter should have comments. Your test will fail as it doesn't work, you then make that test pass and assuming your existing tests dont fail you can cross that item off the list.

Just make sure you use git or some other version control system so you have a history of changes and can revert if anything goes sideways. Also dont work on the production code. Setup a separate test environment so you can work in isolation.

You might want read through this article as it will help you figure out the existing codebase as although your project is bespoke the needs to be some basic starting points e.g. routes, middleware, controllers and probably just haven't followed laravel convention.

https://laravel-news.com/navigating-a-new-laravel-codebase

Overthane

7 years ago

Level 1

@D9705996 - Thanks for all your advice! I did follow it and decided to brush up on my Git knowledge, I've now setup a repo for the project.

Unfortunately my employer, upon knowing that I was performing tests careful and properly, ordered me to skip testing suites and attempt to apply the fixes directly to the code.

I'm using Valet to view the project on my localhost, using a copy of the Database being currently used by the live project.

If anyone's interested, here's the repo:

https://github.com/Overthane/wpportal

I've managed to check off two (minor) items from the list, on the "Debug" branch.

Thanks again!

D9705996

7 years ago

Level 51

@OVERTHANE

Unfortunately my employer, upon knowing that I was performing tests careful and properly, ordered me to skip testing suites and attempt to apply the fixes directly to the code.

WOW. I know from previous experience the world of pain you will end up in without a test suite. A small seemingly innocuous change breaks core functionality, you have a fear of change as you dont have confidence to make changes so everything takes many times more, debugging problems is a nightmare as the breaking change might have been months ago.

For your own sanity if nothing else I would continue testing as much as you can without compromising your employers delivery dates (I would be pushing back that tests are vital if it were my employer)

It's easier to ask for forgiveness than permission!!!

If you want a good reference book I highly recommend TDD by example by Kent beck and refactoring by Martin Fowler. I have 1st edition which is java orientated bug still relevant but link is for the new one which is javascript

D9705996

7 years ago

Level 51

@OVERTHANE - I had a look at your github repo and I have the following starters advice

For each of your issue create a test to prove that you can reproduce the issue. Then fix it and close the issue I know you have been asked not to but you dont want to accidentally break it again later you can then be sure forever that this bit of your codd work
Havd a look at https://guides.github.com/activities/hello-world/ about creating branches/ merge requests. Really useful for tracking the changes you make for each issue in isolation and let's you review in the future, it's also how anyone else can contribute to your project (e.g. if I wanted to fix an issue for you I could create a copy of your repo, make the changes and then create a merge request that you could approve, reject, discuss)
Your routes dont follow restful convention e.g. using post routes to delete things. Not an issue as such but sticking to a convention helps readability https://laravel.com/docs/5.7/controllers#resource-controllers
Your models are using a weird hybrid of raw SQL and query builder for no obvious reason. A lot of the code could be simplified with eloquent - https://laravel.com/docs/5.7/eloquent
Your controllers have validation logic all over the place and is pretty bespoke. Look at form requests as they can do z lot of the heavy lifting for you in a centralised place. https://laravel.com/docs/5.7/validation#form-request-validation
Authentication is being performed in your controller routes with repeated code. You can use the auth middleware to remove this and use policies for your authorization. https://laravel.com/docs/5.7/authorization
Ids are being passed as request parameters. Look at route model binding to automatically get model by its id - https://laravel.com/docs/5.7/routing#route-model-binding

I know you aren't really in a position to refactorwrite tests so you might have to keep going with what you've got but I would try chipping away at some of the code if you can as it will get easier and the above are the quickest wins IMO but I feel for you as you have inherited an absolute beast

Good luck

Please or to participate in this conversation.