Skip to content

Added missing relation as argument for hook methods #229

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.

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

Closed
wants to merge 1 commit into from

Conversation

aimeos
Copy link

@aimeos aimeos commented Mar 12, 2023

For both hooks, readRelated{name} and readingRelated{name}, it seems to be impossible to create an appropriate response at the moment because both hooks are called without $relation which is required for constructing the response:

$response = $this->{$hook}($model, $data, $request);
}
return $response ?: RelatedResponse::make(
$model,
$relation->name(),
$data,
)->withQueryParameters($request);

@lindyhopchris
Copy link
Contributor

@aimeos what is it you need from the relation object? I.e. what methods on that object are you looking to call within your hook?

@aimeos
Copy link
Author

aimeos commented Mar 13, 2023

Obviously, $relation->name() to build the RelatedResponse:

@lindyhopchris
Copy link
Contributor

Ok, but you don't need the name from the relation, because it's already in the method name that's being called.

I.e. if the JSON:API field is comments, then the method on the controller being called is readingRelatedComments. So you already know the field name is comments.

So not sure why you'd need to call $relation->name()? Or am I missing something?

@aimeos
Copy link
Author

aimeos commented Mar 13, 2023

OK, didn't knew that it can be static in that case. It would be great to have example code in the docs what you can do in the hooks, e.g. here and following sections: https://laraveljsonapi.io/docs/3.0/routing/controllers.html#fetch-many-aka-index

public function read( ?Post $post, PageQuery $query ) : DataResponse
{
    return \LaravelJsonApi\Core\Responses\DataResponse::make( $post )
      ->withMeta( ['key' => 'value'] )
      ->withQueryParameters( $query );
}

Or here: https://laraveljsonapi.io/docs/3.0/routing/controllers.html#fetch-related-aka-show-related

public function readingRelatedAuthor(Post $post, UserQuery $query): RelatedResponse
  return \LaravelJsonApi\Core\Responses\RelatedResponse::make( $post, 'author', $data )
      ->withMeta( ['key' => 'value'] )
      ->withQueryParameters( $request );
}

BTW: The docs contains an error because the hook functions must not return void.

@lindyhopchris
Copy link
Contributor

Closing this as I'm not planning on making any changes.

FYI the docs show a return type of void, because you don't have to return anything from a hook method. You could just implement a side-effect, e.g. dispatching a job to the Laravel queue. Obviously if you do return anything from a hook (e.g. a response is a good example), you can type-hint your hook to return that value. (Hooks are not defined on an interface anywhere, so the return type can be anything you want, including void.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants