Closed nickolasjadams closed 3 years ago
I'm not entirely sure if I got the entire thing down or not. I don't understand how to go from my current branch to yours to test your so I just removed my .git and the files and then went into your branch and brought it all down. I do see the 403 error and if I put in http://127.0.0.1:8000/[business_name_slug]
it does load just fine, but I don't get redirected to that, it still takes me to http://127.0.0.1:8000/dashboard
which serves me a 403 error.
Not sure if this is the code or maybe I didn't get everything down to my computer and am just missing a controller or something?
Once we figure this out I have brands controller, model, and routes created, if you would like you can take a look at that branch.
Couple thoughts, I was going through the files to see what changes you made and where my local files have the issues. I noticed your do your functions with an underscore instead of camel-case, is that intentional for this or? If there isn't specific reasoning I think we should do camel-case for consistency throughout the project.
I'm also wondering if maybe we should create a slug column for a user's business name rather than having to deconstruct the business name every time someone signs in and changes pages. The positive to creating a slug at the start is we could then allow a user to choose what that slug is. I can approve this pull request and we could address these things later if you'd like, but I think they're things we should probably address before going much further.
I see you made the business name unique in order to make the handle/slug unique, I think we really do need to make an additional column for slug and make that unique. I think if a Company puts in it's business name and gets "this business name is already taken" they're going to be confused and frustrated, but when they know their creating a handle for the address, it's easier to understand that someone else already owns that handle and they just need a unique one for their own store.
The getSlugFromId method can be refactored for direct indexing with a collection->pluck. That would result in a constant runtime. I can't figure out any way to achieve the same runtime for the getIdFromSlug method though, which is used in the middleware. So I do think that a slug column is a good idea.
I'm not sure we should allow users to change their slugs. I don't think it provides much value to the client, and it creates technical debt. For example. if we want to provide a feature in the future that allows us to give users DNS info programmatically, then if they are able to change their slug at any time, we would have to do file io on the vhost file each time as well. I think that feature provides much more value than the former, and it creates a complex additional step. Though this might be something we can't avoid if we allow for business name changes. This is probably a discussion to slap an icebox label on.
I've added the slug column to the users table, and refactored the middleware to use it.
I was doing snake casing because I'd noticed php functions follow that. But looking at class methods they are using camelCase. So I changed that as well.
After seeding the application
php artisan migrate:fresh --seed
Login to mysql and run
select id, business_name, email from users where id < 10;
Use one of these emails to login with the password "password" You will be redirected to
that corresponds the logged in user.
Try to visit another user's dashboard.
You'll be given a forbidden 403 message.
All other links in the navigation should work with way now too.