Simplify toResponse method on ViewModel

[SamuelNitsche]

Hey, I am not quite sure but I think the toResponse method on ViewModel.php can be simplified. The original method looks like this:

public function toResponse($request): Response
{
    if ($request->wantsJson()) {
        return new JsonResponse($this->items());
    }

    if ($this->view) {
        return response()->view($this->view, $this);
    }

    return new JsonResponse($this->items());
}

I think you can remove the first if statement so the method looks like this:

public function toResponse($request): Response
{
    if ($this->view) {
        return response()->view($this->view, $this);
    }

    return new JsonResponse($this->items());
}

[coderatio]

I think it's cleaner too.

[freekmurze]

Feel free to send a PR, all current tests should pass.

[sebastiandedeyne]

I don't think this is correct. Right now you can specify a view for plain requests and fall back to JSON automatically in AJAX requests. The simplified version doesn't handle that case.

If the tests still pass after the suggested change, I think we need another test case (unless I'm overlooking something)

[SamuelNitsche]

With the new version all currently available tests pass. I don’t think that this breaks something since the if statement des the same as the Json fallback.

Edit: If there is no view the json response is returned. Therefore the first if statement is not necessary.

[freekmurze]

But if there is a view present in the view model and the request wants json a response we still need to return the json representation. @sebastiandedeyne is right in saying that we need an extra test to cover this scenario.

[SamuelNitsche]

Got it. Then the current implementation is perfectly fine. As you said another test would be good.

[freekmurze]

I've correct the tests. When removing

if ($request->wantsJson()) {
    return new JsonResponse($this->items());
}

the tests will now fail. Thanks for bringing this to our attention.

[SamuelNitsche]

Yeah, makes totally sense. Thanks for clarifying.