laravel-api-debugger icon indicating copy to clipboard operation
laravel-api-debugger copied to clipboard

Check response data type before appending

Open mycarrysun opened this issue 5 years ago • 7 comments

My implementation of responses sometimes has integers as the response content. This will of course produce an error trying to access an index of an integer like an array.

I have added a check to make sure the response data is an array before trying to attach the debug property to the response.

Also: I have added a small convenience for seeing the total time of all the queries in the query collection.

mycarrysun avatar Mar 22 '19 20:03 mycarrysun

@mlanin added :)

mycarrysun avatar May 20 '19 18:05 mycarrysun

Tests are failing now :(

mlanin avatar May 21 '19 06:05 mlanin

@mlanin looks like for some reason the $this->json() method is returning the standard Illuminate\Http\Response type instead of the TestResponse, which makes sense why it can't find the assertJsonMissingExact() method.

Did I do anything wrong in the return type?

/** @test */
public function is_does_not_add_debug_if_response_is_integer()
{
    $this->app['router']->get('foo', function () {
        lad('test if integer');

        return response()->json(1234);
    });

    $this->json('get', '/foo')
         ->assertStatus(200)
         ->assertJsonMissingExact([
            'debug' => [
                'dump' => [
                    'test if integer',
                ]
            ]
         ]);
}

mycarrysun avatar May 22 '19 19:05 mycarrysun

Tests are failing for php 5.6 version. The highest laravel version that supports this is 5.4.36. But this version doesn't have assertJsonMissingExact method :(

mlanin avatar May 24 '19 08:05 mlanin

Can we include this version requirement of laravel in composer.json then? The copy I have installed v5.7.28.

mycarrysun avatar May 25 '19 00:05 mycarrysun

But this package supports 5.4 version. Your PR is really good, please, adjust your test and I'll merge it asap :)

mlanin avatar Jun 17 '19 08:06 mlanin

But this package supports 5.4 version. Your PR is really good, please, adjust your test and I'll merge it asap :)

I've updated the composer.json to use version of packages that are compatible with your requirements so composer doesn't try to install a newer version. I also added requirements for sqlite since it was missing from my machine and the tests were failing (Ubuntu 19).

mycarrysun avatar Sep 25 '19 20:09 mycarrysun

Closing from lack of activity from the maintainer. Feel free to reopen if you want it merged.

mycarrysun avatar Nov 09 '22 16:11 mycarrysun