Laravel JSON / Intermittent head scratch

[rexehuk]

Converting website over to Laravel, and have a weird error that is driving me mad.

Got a controller, firing out reviews (randomised to page). This should be producing JSON as standard in laravel which it is, however this javascript intermittently fails!

	<script type="text/javascript">
	jQuery.ajax({
    url: 'review/random/3',
    dataType: 'json',
	method: 'GET',
    success: function(data){reviews_data(data)}
});

function reviews_data(data)
{
    reviews = data;
	console.log (reviews);
	reviews.forEach (function(review) {
		document.getElementById("reviews").innerHTML+= '<div class="col-sm-4 portfolio-item" id="review">	<div class="review" data-stars="1,2,3,4,5">	<p class="reviewContent">' + review.reviewtext + '</p>	<p class="reviewName">' + review.name + '</p>	<div class="rating-score">	<div class="rating-5">' + review.reviewrating + '</div>	</div>	</div>	</div>';		
	});
};
</script>

The error is reviews.forEach is not a function

I've looked at the JSON and it's intermittently not providing indexes? Maybe 1/10 it'll produce a working JSON that won't crash the JS code.

Working JSON (from Chrome Console via a console.log)

Object {1: Object, 2: Object, 3: Object}
1: Object
created_at: "2015-12-24 15:59:42"
id: 2
name: "Review two"
published: 1
published_at: "0000-00-00 00:00:00"
reviewrating: 3
reviewtext: "This is some text for the review"
updated_at: "2015-12-24 15:59:42"
__proto__: Object
2: Object
created_at: "2015-12-24 15:59:42"
id: 4
name: "Review four"
published: 1
published_at: "0000-00-00 00:00:00"
reviewrating: 3
reviewtext: "This is some text for the review"
updated_at: "2015-12-24 15:59:42"
__proto__: Object
3: Object
created_at: "2015-12-24 15:59:42"
id: 5
name: "Review Five"
published: 1
published_at: "0000-00-00 00:00:00"
reviewrating: 5
reviewtext: "This is some text for the reviewThis is some text for the reviewThis is some text for the reviewThis is some text for the reviewThis is some text for the review"
updated_at: "2015-12-24 15:59:42"
__proto__: Object
__proto__: Object

Failing JSON feed

[Object, Object, Object]
0: Object
created_at: "2015-12-24 15:59:42"
id: 1
name: "Review one"
published: 1
published_at: "0000-00-00 00:00:00"
reviewrating: 5
reviewtext: "This is some text for the review"
updated_at: "2015-12-24 15:59:42"
__proto__: Object
1: Object
created_at: "2015-12-24 15:59:42"
id: 2
name: "Review two"
published: 1
published_at: "0000-00-00 00:00:00"
reviewrating: 3
reviewtext: "This is some text for the review"
updated_at: "2015-12-24 15:59:42"
__proto__: Object
2: Object
created_at: "2015-12-24 15:59:42"
id: 4
name: "Review four"
published: 1
published_at: "0000-00-00 00:00:00"
reviewrating: 3
reviewtext: "This is some text for the review"
updated_at: "2015-12-24 15:59:42"
__proto__: Object
length: 3
__proto__: Array[0]

Cookies and kudos if you can crack the puzzle!

 

[iKettles]

Rather than relying on the prototypical method forEach why don't you use an indexed for loop?

for(var i = 0; i < reviews.length; i ++) {
// do something
}

Also if you're already using jQuery you may as well use its helper methods, eg. $.each for looping through arrays and for adding markup use $.('#reviews').append(html)

 

[rexehuk]

Rather than relying on the prototypical method forEach why don't you use an indexed for loop?

for(var i = 0; i < reviews.length; i ++) {
// do something
}

Also if you're already using jQuery you may as well use its helper methods, eg. $.each for looping through arrays and for adding markup use $.('#reviews').append(html)

Mind walking me through a bit more?

I've looked at jQuery.each but no real information on how to achieve the same result... i.e. for each JSON object, output the key data > here <

 

[iKettles]

Mind walking me through a bit more?

I've looked at jQuery.each but no real information on how to achieve the same result... i.e. for each JSON object, output the key data > here <

For the regular indexed for loop, just use the current index in the loop to fetch that item from the array:

for(var i = 0; i < reviews.length; i ++) {
console.log(reviews[i].id]);
}

You could also use for...in if you need the key as well:

for (key in object) {
console.log(object[key]);
}

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...in

And for the jQuery helper:

$.each( reviews, function( key, value ) {
  console.log( key + " : " + value );
});

http://api.jquery.com/jquery.each/

And as always, as it's impossible to guarantee the state of the object you're given, check the object has the key first:

if(obj.hasOwnProperty(key)) {
// do something
}

This checks the object itself has the property and it's not inherited from its prototype.

 

[rexehuk]

Cheers for that, really helped. Still some cleanup to do but got it working now with >1 objects, just need to put some handling for 0/1 but won't be an issue in production as there are plenty in the db. Also I'll skip assigning variables and just output them direct into the document.

function reviews_data(data)
{
    reviews = data;
    var reviewCount = 1;
    for(var i = 0; i < reviews.length; i ++) {

        console.log(reviews[i].id);
        console.log(reviews[i].name);
        var reviewID = reviews[i].id;
        var reviewName = reviews[i].name;
        var reviewText = reviews[i].reviewtext;
        var reviewRating = reviews[i].reviewrating;

        document.getElementById("reviews").innerHTML+= '<div class="col-sm-4 portfolio-item" id="review_' + reviewCount +'"> <div class="review" data-stars="1,2,3,4,5"> <p class="reviewContent">' + reviewText + '</p>	<p class="reviewName">' + reviewName + '</p> <input id="review-rating' + reviewCount +'" class="rating" data-stars="' + reviewRating + '" data-step="1" value="5" disabled="true" data-size="xs" /></div></div></div>';

        $('#review-rating' + reviewCount).rating('refresh', {
            showClear:true,
            disabled:true,
            showCaption:false,
            showClear:false
        });
        
        reviewCount++;
    }

Hopefully isn't too terrible!

 

[iKettles]

Cheers for that, really helped. Still some cleanup to do but got it working now with >1 objects, just need to put some handling for 0/1 but won't be an issue in production as there are plenty in the db. Also I'll skip assigning variables and just output them direct into the document.

function reviews_data(data)
{
    reviews = data;
    var reviewCount = 1;
    for(var i = 0; i < reviews.length; i ++) {

        console.log(reviews[i].id);
        console.log(reviews[i].name);
        var reviewID = reviews[i].id;
        var reviewName = reviews[i].name;
        var reviewText = reviews[i].reviewtext;
        var reviewRating = reviews[i].reviewrating;

        document.getElementById("reviews").innerHTML+= '<div class="col-sm-4 portfolio-item" id="review_' + reviewCount +'"> <div class="review" data-stars="1,2,3,4,5"> <p class="reviewContent">' + reviewText + '</p>	<p class="reviewName">' + reviewName + '</p> <input id="review-rating' + reviewCount +'" class="rating" data-stars="' + reviewRating + '" data-step="1" value="5" disabled="true" data-size="xs" /></div></div></div>';

        $('#review-rating' + reviewCount).rating('refresh', {
            showClear:true,
            disabled:true,
            showCaption:false,
            showClear:false
        });
        
        reviewCount++;
    }

Hopefully isn't too terrible!

My pleasure

It's best practice to keep markup out of your JS as it's a pain to maintain and not that readable. Instead you could include a blank hidden version of your markup in the template and then use $(selector).clone(). You may as well use jQuery's append function to add the markup to its parent. I would do it like this:

var review = $('#blankReview').clone().prop('id', 'review-rating'+(i+1));
review.find('.reviewName').text(reviewName);
// the rest of your variables
$('#reviews').append(review);

This makes your code much more readable and easier to maintain. You can also use the current index in the loop instead of incrementing your own reviewCount.

What's the issue you're having with just a single object/an empty array? The loop won't run if the reviews array is empty and will only run once if there is a single item.

 

[kaku]

You are treating the symptom, not solving the actual problem.

You can see in the "failing" example on the first line: [Object, Object, Object] (array of objects)
And on the "working" example: Object {1: Object, 2: Object, 3: Object} (object with multiple child objects)

You need to decide how your response JSON should be formatted (array of "reviews", or an object with several "review" properties) and make sure your server consistently provides that response - my opinion is that it makes logical sense to have an array of 1-or-more objects since each "review" is an individual entity - there is no reason for them to be grouped under a single parent.

My first port of call would be to check the code in your Laravel controller - this needs to provide a consistently-formatted response every time.

 

[rexehuk]

My first port of call would be to check the code in your Laravel controller - this needs to provide a consistently-formatted response every time.

Yeah shortly after posting above, I did more digging and come to the conclusion that laravel 5.0 has a bug. Talked in their slack and no one can solve it! Gave them all the code, and all I'm doing is using proprietary laravel functions/methods to do this.

When using random row retrieval, it sends back named / unnamed objects in what seems like a pluck out of a hat.

I can bypass it by doing something like getting the 3 latest reviews, but it would have been nice to randomly show some reviews each time someone visits.

I've left it alone for now whilst I get the rest of the site done, really liking laravel overall though! First time getting deep into an MVC framework myself hands on. Nevertheless, the javascript you provided is a bit cleaner, so not all lost .