Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

AJAX request not working as it should #32995

Closed
billwagner3 opened this issue May 28, 2020 · 14 comments
Closed

AJAX request not working as it should #32995

billwagner3 opened this issue May 28, 2020 · 14 comments

Comments

@billwagner3
Copy link

billwagner3 commented May 28, 2020 •
edited

Laravel Version: 6.18.13
PHP Version: 7.3.6
Database Driver & Version:
MySQL: Amazon RDS version 5.7.22.
Driver: MySQL Connector/J 8:8.0.15[Latest]

Description: I have a form in a blade template that uses the jQuery AJAX method as well as .sortable, which is used to rearrange customers that were called from the DB. .sortable is working fine. When the "Save" button is pushed, it returns the "data" value as null.

Steps To Reproduce: In order to fully replicate this, one would need a MySQL database as it pulls customer data, which you will see from the code. Probably just using placeholders will be enough to repro.

I have attached a template (edit-route.blade.txt) that has all the JS in it (but there is one jQuery file that is NOT in the template, that only worked when being served up by webpack in the app.js file--it is this here:
datatables.js download here: https://datatables.net/download/

Template is here:
edit-route.blade.txt

Here is the Controller:
RoutesController.txt

Here's what my routes look like:

Route::get('/routes/edit-route/{id}', 'RoutesController@edit')->name('edit-route');
Route::post('/routes/edit-route/{id}', 'RoutesController@update')->name('edit-route');

This should be enough to repro the issue.
@driesvints
Copy link
Member

Please don't re-open an issue. Try a support channel like I asked in the previous one. Maybe open a discussion on this repo.

@billwagner3
Copy link
Author

billwagner3 commented May 28, 2020 via email

@DevJMD
Copy link

DevJMD commented May 29, 2020 •
edited

I don’t understand. This looks like a bug. I’ve posted on SO and gotten zero response because of this. How do I get it addressed? I have been through any possible online post to address this issue. How do you report bugs then?
…
On Thu, May 28, 2020 at 2:45 AM Dries Vints @.***> wrote: Please don't re-open an issue. Try a support channel like I asked in the previous one. Maybe open a discussion on this repo. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#32995 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADIM5JQUOKS7TXHWAUJJDLTRTYXD5ANCNFSM4NMVYWFA .

Are you sure you have your jQuery script and HTML setup correctly? It looks wrong to me. It doesn't look like a bug, you're just not implementing it correctly via the documentation.

See here, where you have both:

const url = "{{ url('routes/edit-route/{id}') }}"

and

<form name="order" class="form-group" method="POST" action="{{ url('routes/edit-route/{id}') }}">

Where exactly do you believe the URL helper is fetching that {id} from? If you look the HTML in the source code, what exactly is the <form> method showing? Is it showing an actual integer id or is it still showing {id}?

To me, it looks like you should be using:

{{ route('edit-route', $route->id) }}

The url() helper doesn't know how to interpret {id} - url() only accepts strings that build a complete URL string - it does not process parameters unless you specify an actual valid PHP variable, such as url('/my/url', $model->id) and not just using url('/my/url/{id}').

As above, you should be using the route() helper with named routes, not url() necessarily.

See the docs here how to use it properly: https://laravel.com/docs/7.x/urls#urls-for-named-routes

Edit: Look up Resource Routes, this will make your life easier using the correct methods for creating, updating and deleting records and will give you named routes for each resource. If you're unsure what your routes are named, run php artisan route:list and this will output all your routes, including resource routes.

https://laravel.com/docs/7.x/controllers#resource-controllers

Then simply use them where needed with route('resource.name', $model) - you pass the route name in the first parameter and then the model, model key (i.e., $model->id) or any parameters your ModelController method requires. In your instance, you need to pass $model->id as the second parameter.

You can, however, use url() but you need to set the parameter as a second argument. However, if you ever change the URL of your route, using url() would break so I'd highly consider using route()

For example, as above, you could change your implementation to this but strongly consider name routing.

{{ url('routes/edit-route', $route->id) }}

The reason you couldn't find anything on SO or frantically finding a blog post surrounding your problem is because this "bug" doesn't exist, it's just you were implementing routing wrong in your templates. 😉

The reason you were also getting Status 200 is because the third segment (which is {id} in your case) in your URL is technically classed as a parameter and the route you have setup recognises this.

And maybe name your existing routes to be a little more consistent:

Route::get('/routes/edit-route/{id}', 'RoutesController@edit')->name('route.edit');
Route::post('/routes/edit-route/{id}', 'RoutesController@update')->name('route.update');

Then you can take advantage of implicit route-model binding. Instead of using:

{{ Form::open() }}

Use:

{{ Form::model($model, ['route' => ['route.update', $model->id]) }}

And this will pre-populate any fields where the field name matches the model attribute.

For example, assume you have a database field called title:

{!! Form::text('title', null, ['class' => 'form-control mb-4', 'placeholder' => '...']) !!}

Will automatically fill the title in for you without explicitly defining the variable.

Happy coding 😎

@DevJMD DevJMD mentioned this issue May 29, 2020
Closed
@billwagner3
Copy link
Author

billwagner3 commented May 29, 2020 via email

@DevJMD
Copy link

DevJMD commented May 29, 2020 •
edited

Hi Bill,

From what I can see, your implementation would technically never work because Laravel's url() helper isn't being used correctly whatsoever. You have to specify the route parameter with a PHP variable because url() does not magically know what {id} is.

Also, I actually meant what does your form action output, not the method, sorry.

Are you seeing this in your browsers Dev Tools:

<form name="order" class="form-group" method="POST" action="routes/edit-route/{id}">

And

const url = "routes/edit-route/{id}"

Or this:

<form name="order" class="form-group" method="POST" action="routes/edit-route/1">

And

const url = "routes/edit-route/1"

Please read what url() is actually capable of: https://laravel.com/docs/7.x/helpers#method-url - the way you have it set in your code is 100% incorrect and would never work under any circumstance. I think you're getting muddled up with what url() is used for.

url() and route() are two completely different functions. You should only really using url() for static files on your server. route() is made perfectly for its intent - to link dynamically to a named route which you have declared you're using with:

Route::get('/routes/edit-route/{id}', 'RoutesController@edit')->name('edit-route');

What you're effectively doing is hardcoding URL's in to your system which is completely negates any reason to use named routes.

You need to dynamically link your URL using exactly what route() is meant for. Otherwise there is no reason for you to use named routes at all.

The only way you can split url() in to parameters mid-way is to do:

url(sprintf("foo/%d/bar/%d/show", 333, 444));

Would result in /foo/333/bar/444

Or as you would need if you're set on using url(), you would absolutely need to use:

url('routes/edit-route', $model->id)

Would result in routes/edit-route/1

There's no two way about it, that is exactly what the url() helper does. Please see this answer on Stack Overflow:

https://stackoverflow.com/questions/34636572/how-to-generate-a-url-with-laravels-url-generator-helper-with-parameters-that-a

Also note: you have two named URL's that are exactly the same. This is quite dangerous as it could conflict very easily. Try using dot-notated routes such as routes.edit and routes.update. This could be why you're having side-effects by seeing a status 200.

The following example works for me 100% and the following will highlight what you're doing wrong.

web.php

<?php

Route::get('/routes/edit-route/{id}', function() {
	// Return view...
})->name('edit-route');

Route::post('/routes/edit-route/{id}', function(Request $request, $id) {
	return response()->json([ 'order' => $request->all(), 'id' => $id ]);
})->name('edit-route');

// ...
// Using hardcoded URL
const url1 = "{{ url('routes/edit-route', 1) }}"
// Using dynamically linked Route URL
const url2 = "{{ route('edit-route', 1) }}"
// WRONG - this will NOT work (it's what you are doing in your code.)
const url3 = "{{ url('routes/edit-route/{id}') }}"

$(document).ready(function() {

	// Get CSRF token
	const token = '{{ csrf_token() }}'

	// Set CSRF in header
	$.ajaxSetup({
		headers: {
			'X-CSRF-TOKEN': token
		}
	});


	$.ajax({
		type: 'POST',
		data: { order: { foo: 'bar' } },
		cache: false,
		dataType: 'json',
		url: url1, // better to use `url2`
		statusCode: {
			200: function (data) {
				console.log('data', data);
			}
		}
	});
});

With the above, I am getting the correct data and returning it back to the client successfully which results in:

data > order: { foo: "bar" }, id: 1

You need to change your implementation of url() - it's not going to work with the way you have it set. $id will always be null in your response because that's what you are sending. You are not sending an integer, you're sending null so your query will always return null because it's looking for a parameter - of which you aren't sending with your existing implementation.

url('/my/route', $model->id)
         ^         ^
          Link      Parameter

// Will result in: "http://mysite.com/my/route/1"


url('/my/route/{id}')

// Will result in "http://mysite.com/my/route/{id}" <-- WRONG

The parameter is required here, not two ifs or buts about it. Hope it makes sense!

Kind Regards,
James

@billwagner3
Copy link
Author

billwagner3 commented May 29, 2020 via email

@devcircus
Copy link
Contributor

Push a stripped down sample repo to github when u get a chance. This is basic functionality and if it was broken, this repo would be full of issues. So the best bet is to make a repo that has the issue, Push it up and link to it here. Someone will figure it out.

@billwagner3
Copy link
Author

billwagner3 commented Jun 2, 2020 via email

@DevJMD
Copy link

DevJMD commented Jun 2, 2020 •
edited

Hi Clayton. Here's my repo with the issue. I've worked many hours trying to handle this, so if someone knows what to do, it would be greatly appreciated. https://github.com/billwagner3/ajax_issue/blob/master/readme.md Best Regards, Bill Wagner
…
On Fri, May 29, 2020 at 3:26 PM Clayton Stone @.***> wrote: Push a stripped down sample repo to github when u get a chance. This is basic functionality and if it was broken, this repo would be full of issues. So the best bet is to make a repo that has the issue, Push it up and link to it here. Someone will figure it out. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#32995 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADIM5JTNTWUNIS3BKL6ZT7DRUAZDHANCNFSM4NMVYWFA .

Hi Bill,

I've tested your code (and seen you have update it with the correct route params)

This is working for me absolutely fine, but I had to change your code a little bit. You are trying to submit a form as usual. You don't appear to be implementing your JS correctly (or you have done this by mistake). Where you have:

edit-route.blade.php
<button id="button" onclick="sendOrderToServer()"  type="submit" class="btn btn-primary  btn-lg">Save</button>

This will fire the function sendOrderToServer() but it's also firing the form as if you're normally submitting it without AJAX.

I had to do this:

HTML

<button id="button" type="submit" class="btn btn-primary btn-lg js-submit-form" >Save</button>

Javascript

$('.js-submit-form').on('click', sendOrderToServer.bind(this));

function sendOrderToServer(event) {

        // Prevent form from submitting normally.
        event.preventDefault();
        
        // ... rest of script
}

Then, in RoutesController.php

<?php

namespace App\Http\Controllers;

use Illuminate\Http\Request;
use Illuminate\Support\Facades\DB;

class RoutesController extends Controller
{
	public function update(Request $request) {

        // Update data here...

        // Then return response...
		return response()->json(array(
            'success' => true,
            'order' => $request->post('order')),
            200,
            array( 'allow origin' => 'Access-Control-Allow-Origin')
        );
	}
}

This returned the both the data in the right order and success => true. You would have got null on submission because your <form> doesn't contain an input with the name order.

There is no bugs with Laravel here regarding AJAX so I guess it's down to a bit of practice and learning. :)

Edit: I've just noticed you're using the name attribute on a form element. This really serves no purpose and does not translate to POST parameters within Laravel, hence why you're seeing null. name on a form is a legacy attribute which was replaced with using id. name does not represent data on or within forms, it merely acts as an identifier.

Again, aside the obvious, I think it's worth brushing up on JavaScript and working with forms. There's tons of articles on the web that will guide you forward. :) There is little else I can do at this point as it's down to a question of personal implementation rather than any form of bug.

There's 100% no bug with PHP/Laravel here - just so you know.

Cheers,
James

@devcircus
Copy link
Contributor

Noticed the same thing. The above by @DevJMD definitely works.

@billwagner3
Copy link
Author

billwagner3 commented Jun 3, 2020 via email

@billwagner3
Copy link
Author

billwagner3 commented Jun 3, 2020 via email

@billwagner3
Copy link
Author

billwagner3 commented Jun 3, 2020 via email

@digitcreators
Copy link

digitcreators commented Jul 21, 2022 •
edited

var html = '<a href="'+('{{route('your.route.name', [':id', 'source' => $yourQueryPrams])}}').replace(':id', yourID)+'">';

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@driesvints @DevJMD @billwagner3 @devcircus @digitcreators