Have just implemented slugs via the Laracasts videos and they work nicely—except for existing logic that uses {id}.
One example is the payment gateway:
http://bidbird.test/jobs//bidreserve
As you can see above the id is missing for the job.
//Job.php
/**
* Get the route key for the model.
*
* @return string
*/
public function getRouteKeyName()
{
return 'slug';
}
//BidReservesController.php
/**
* Store a newly created resource in storage.
*
* @param $jobId
* @return Response
* @throws ValidationException
*/
public function store($jobId)
{
$job = Job::Incomplete()->findOrFail($jobId);
...
//other logic
Can you use {id} for some logic and {slug} for others? Otherwise, it seems a tremendous amount of work needs to be done to fix this.
Upcoming in Laravel 7 you will be able to specify how implicit Route Model binding is done, but not right now.
https://laravel-news.com/implicit-route-model-binding
Right now you are not using Route Model binding in the BidReserveController, so can handle either id and slug columns to find the record depending on how you implement. This is really your only viable option for now.
You can create model called SlugJob or something like that and have it extend Job model. Overwrite $table property and set it to (I guess) protected $table = 'jobs';. After that overwrite on SlugJob:
public function getRouteKeyName(): string
{
return 'slug';
}
Then for routes where you want to use slug type hint SlugJob and where you want to use id type hint Job.
That should do it until L7 is out.
P.S. Remove getRouteKeyName method from Job model.
Thank you, @tykus . I missed that article in the newsletter.
It's my first site, so unfortunately have a lot of balls down the field only to have someone (slugs) intercept!!!
Hi thank you @bugsysha ,
Have implemented this in the job.php
/**
* Get a string path for the job
*
* @return string
*/
public function path()
{
return "/jobs/{$this->id}";
// return "/jobs/{$this->slug}";
}
Does the below conform to your approach?
<?php
namespace App;
use App\Job;
class SlugJob extends Job
{
protected $table = 'jobs';
/**
* Get a string path for the job
*
* @return string
*/
public function path()
{
// return "/jobs/{$this->id}";
return "/jobs/{$this->slug}";
}
/**
* Get the route key for the model.
*
* @return string
*/
public function getRouteKeyName(): string
{
return 'slug';
}
}
The issue facing now are these variables in the JobsController.php
public function show(SlugJob $job)
{
$downloads = $job->getMedia('document');
$hasPayedBidReserve =
auth()->user()
->bidReserves()
->where('job_id', $job->id)
->exists();
$hasPlacedBid =
auth()->user()
->bids()
->where('job_id', $job->id)
->exists();
return view('jobs.show', compact('job', 'downloads', 'hasPayedBidReserve', 'hasPlacedBid'));
}
'downloads', 'hasPayedBidReserve', 'hasPlacedBid'
are dependent on the $jobId whereas job can use the slug. This is something i've wrestled with—if you want several items to show on the page it seems like several controllers can make that happen.
In this case, if I move 'downloads', 'hasPayedBidReserve', 'hasPlacedBid' logic to the BidReservesController@show they don't render. I'm guessing this is because the JobsController is the workhorse for showing the page?
You can not call two controller methods in one request. If you need that additional data then it has to be returned by one controller. If that is something that is shown on multiple pages than you can use https://laravel.com/docs/master/views#sharing-data-with-all-views
Hi @bugsysha ,
I'm trying to implement this and I believe the {id} is being found, as the button I've got
<div class="jobsbutton">
<a href="/jobs/{{ $job->id }}">
<button type="button" class="btn btn-job">Details</button>
</a>
</div>
is finding the right number (in addition to using slug). However, no data is pulled from the database. So, basically it shows the right id but cannot locate the data.
However, the routes seem to be all jacked up.
class AppServiceProvider extends ServiceProvider
{
/**
* Bootstrap any application services.
*
* @return void
*/
public function boot()
{
View::share('job_id', '$job');
}
in the routes.web file:
// Jobs
Route::resource('/jobs', 'JobsController', ['except' => ['show']]);
Route::get('/jobs/{slug}', 'JobsController@show');
Route::get('/jobs/{job}', 'JobsController@show');
How do you make both {slug} and {job} work in the routes file? Thank you ~
@trevorpan you do not change the value in the route
public function show(SlugJob $job)
{}
Route::get('/jobs/{job}', 'JobsController@show');
// variable $job must match {job} in route
So you need only one route.
If you want to have same route to work with id and slug then your controller method must be something like
public function show($job)
{
$job = Job::where('id', $job)
->orWhere('slug', $job)
->firstOrFail();
}
Also then there is no need to have SlugJob. That approach is good only for two routes that use different route key names.
Slug, slug, slug has slugged me.
I re-watched the below video, and have re-implemented this in the AppServiceProvider: https://laracasts.com/series/lets-build-a-forum-with-laravel/episodes/13
/**
* Bootstrap any application services.
*
* @return void
*/
public function boot()
{
View::share('job', Job::all());
}
It's giving an error here when doing a payment post, e.g.
select * from `jobs` where `slug` = '22' limit 1
or when switching:
select * from `jobs` where `bidded` = 0 and `jobs`.`id` = 'algolia-test' limit 1
I tried changing the calls from $job->id to $job->slug however it still gives errors such as:
Method Illuminate\Database\Eloquent\Collection::bidReserves does not exist.
//Job.php model
/**
* Get a string path for the job
*
* @return string
*/
public function path()
{
// return "/jobs/{$this->id}";
return "/jobs/{$this->slug}";
}
/**
* Get the route key for the model.
*
* @return string
*/
public function getRouteKeyName(): string
{
return 'slug';
}
If getRoutKeyName() is uncommented then the slug doesn't work.
I also added, as you mentioned, this to the JobsController
$job = Job::where('id', $job->id)
->orWhere('slug', $job->slug)
->firstOrFail();
Below is a payment store function which attaches an order to the sale. One thing about slugs is that I didn't know to start using them immediately. A ton of logic is based on the $job->id.
Adding more difficulty, this app uses the Spatie Media-Library which bases the routing of media objects to the id. However, if in the browser a slug is present the media shows. So, I'm not sure if this is based on the View::share(). However, if you do a browser request such as bidbird.test/jobs/3 a 404 is returned. so I question if View::share() is working properly.
// BidReservesController.php
/**
* Store a newly created resource in storage.
*
* @param $job
* @return Response
* @throws ValidationException
*/
public function store(Job $job)
{
$job = Job::Incomplete()
// ->where('id', $job->id) // added line - may be removed
// ->orWhere('slug', $job->slug) // added line - may be removed
->findOrFail($job);
It seems mostly to be a path() issue. I had even tried:
//Job.php model
/**
* Get a string path for the job
*
* @return string
*/
public function path()
{
if ($this->id == 'job_id') {
return "/jobs/{$this->id}";
}
elseif ($this->slug == 'job_slug'){
return "/jobs/{$this->slug}";
}
//if ('job_id' == $this->id) {
// return "/jobs/{$this->id}";
//}
//elseif ('job_slug' == $this->slug){
// return "/jobs/{$this->slug}";
//}
}
but it was causing errors.
Do you have any more ideas? @bugsysha
You haven't shown here your full code. Reason you have Method Illuminate\Database\Eloquent\Collection::bidReserves does not exist is cause you are calling bidReserves method on a collection and not on a model instance.
Thank you for hanging in on this, @bugsysha . I've found quite a few posts on the topic. Most notably this one: https://github.com/laravel/framework/pull/30471
That's a ways out though.
If I'm not mistaken the slug routes are needed to display the pages in this app, and the id's are needed to process charges and retrieve media. Not saying this is ideal but it's where the app's at at the moment.
Not sure why that would be a collection instance. Before the slug issue, I had quite a number of tests passing on this, where it was creating a model instance of BidReserve, Order, etc..storing the records.
// this action was changed from $job->id to:
<form id="payment-form" action="/jobs/{{ $job->slug }}/bidreserve" method="post">
@csrf
...
<button class="stripeButton">Bid Reserve</button>
</form>
Here's what I have in the BidReservesController.php
/**
* Store a newly created resource in storage.
*
* @param $job
* @return Response
* @throws ValidationException
*/
public function store(Job $job)
{
$job = Job::Incomplete()
// ->where('id', $job->id) // BUGSYSHA's added line
// ->orWhere('slug', $job->slug) // BUGSYSHA's added line
->findOrFail($job);
request()->validate([
'amount' => 'required|numeric|min:1',
'stripeToken' => 'required'
]);
// Charging the bidder
try {
$bidReserve = $job->bidReserves()->create([
'amount' => 500,
'job_id' => $job->id,
]);
$order = $bidReserve->complete($this->paymentGateway, request('stripeToken'), auth()->user()->id, $job, $bidReserve);
return Route::redirect('jobs.show/{job->slug}')
->setStatusCode(201);
} catch (PaymentFailedException $e) {
return back()
->setStatusCode(422);
}
I hope this helps out. If there's another snippet you need please let me know. I looked into the api, path()seems to be reserved for that exact function. Had played around with a slugPath() but it did not have any effect, and I was having a hard time figuring out where to call that - which is probably due to path() being reserved and I presume global...
Not sure why that would be a collection instance.
Cause you are passing Job instance which you get by type hinting in your store method to findOrFail method on Model/Builder.
public function store(Job $job) // <-- this here
{
$job = Job::Incomplete()
// ->where('id', $job->id) // BUGSYSHA's added line
// ->orWhere('slug', $job->slug) // BUGSYSHA's added line
->findOrFail($job); // <-- this here is the same object that was retrieved from the database
You should not refetch that model from the database since it is already resolved when you add Job before $job in store method.
If I'm not mistaken the slug routes are needed to display the pages in this app, and the id's are needed to process charges and retrieve media. Not saying this is ideal but it's where the app's at at the moment.
So you do have two set of routes for that job model and one uses id and the other uses slug. Since this is the case then you can use that approach which I've wrote about.
Where you are using id:
// in route file
Route::get('/jobs/{job}', 'JobsController@show');
// in controller
public function store(Job $job)
And where you use slug:
// in route file
Route::get('/jobs/{job}', 'JobsController@show');
// in controller
public function store(SlugJob $job)
That is all you have to do in order to make it work. Then you can get different results from path method.
Have been messing around with this for a bit. You are a life - saver. Thank you ~ @bugsysha
Not sure how this was getting tripped up; it was crashing the page saying [document] was not a property on this collection.
@if(isset($job) && $job->document)
var files =
{!! json_encode($job->document) !!}
for (var i in files) {
var file = files[i]
this.options.addedfile.call(this, file)
file.previewElement.classList.add('dz-complete')
$('form').append('<input type="hidden" name="document[]" value="' + file.file_name + '">')
}
@endif
This is a section for a dropzone file uploader. In the create page all I had was this:
//get
/**
* Show the step 1 Form for creating a new product.
*
* @return \Illuminate\Http\Response
*/
public function create(Request $request)
{
return view('jobs.create');
}
// modified to
/**
* Show the step 1 Form for creating a new product.
*
* @return \Illuminate\Http\Response
*/
public function create(Request $request, Job $job)
{
return view('jobs.create', compact('job', 'request'));
}
After implementing {slug} and {job} it was not working with just Request $request. I had to add Job $job to the create method and pass it to the view.
Before the SlugJob that was not the case. It's strange to me, as the view doesn't really have a job, this is the page to create one..?
Not sure I understand the problem.
On the jobs.create page there's a form with a file dropzone for uploading media - pdf, jpg, etc.. For some reason the not yet created $job->document was crashing the page giving this error: [document] was not a property on this collection.
Prior to implementing your solution with SlugJob the jobs.create page rendered just fine. After implementing - the page does not render, unless Job $job is type hinted in the controller.
It was strange to me, because the page is crashing before the form is submitted (when the jobs.create page is fetched) — but by putting an empty variable on the controller `public function create(Request $request, Job $job) it seems to prevent the crash.
Have you had experience where a not yet created variable causes a page to crash? I know this is related to our original post, as that's all I've been working on since it occurred. (I think future people may benefit by this gotcha being addressed - if possible)
Thank you again ~ @bugsysha
@trevorpan can you paste here routes?
// Jobs route
Route::resource('/jobs', 'JobsController');
// Jobmedia route
Route::post('/jobs/media', 'JobsController@storeMedia')
->name('jobs.storeMedia');
public function storeMedia(Request $request, Job $job)
{
$path = storage_path('medialibrary/temp/');
if (!file_exists($path)) {
mkdir($path, 0777, true);
}
$file = $request->file('file');
$name = uniqid() . '_' . trim($file->getClientOriginalName());
$file->move($path, $name);
return response()->json([
'name' => $name,
'original_name' => $file->getClientOriginalName(),
]);
}
You're right again!
I can see this now, the addition of Job $job in this method was not required as it is governed by the public function store(Request $request, SlugJob $job). Is this an accurate assessment?
Have removed the Job $job from storeMedia to storeMedia(Request $request) as it originally was setup prior to implementing SlugJob. Also removed it from create().
Thank you for your expertise — Happy New Year, @bugsysha
Route::resource('/jobs', 'JobsController');
Resource route above will create routes with names like jobs.index, jobs.create, jobs.edit and so on.
Route::post('/jobs/media', 'JobsController@storeMedia')
->name('jobs.storeMedia');
Now since you've named your Route::post('/jobs/media', 'JobsController@storeMedia') with jobs.storeMedia it will generate URL's like jobs/{job}/media and that is why Job or JobSlug type hint was required. If you named it jobs-storeMedia it would not be nested and would not require Job or JobSlug type hint in the controllers method. But that would then cause other issues since it would think when you visit jobs/media that the media is the slug for Job.
Not sure if my English is good enough and that I've managed to explain this but feel free to ask and I'll try to find some other way to explain.
Thanks man. Happy holidays to you and just keep on learning.
Hi @bugsysha ,
Came across one little gotcha, that I finally got working.
/**
* Store a newly created resource in storage.
*
* @param $job
* @return Response
*/
public function store(SlugJob $job)
When you dd($job) it gives an instance of App\SlugJob (the extended eloquent model), so when you get to the next logic in the store method it tries to insert slug_job_id in addition to job_id. This was messing with the orders and charges and all sorts of things.
The trick? So simple.
// Job.php model
public function bidReserves()
{
return $this->hasMany(BidReserve::class, 'job_id');
}
Just specify the foreign key - it throws out slug_job_id on the App\SlugJob instance and proceeds as it originally did.
Thank you again!
Or you can on SlugJob add following:
public function getForeignKey(): string
{
return 'job_id';
}
Please or to participate in this conversation.