Lighthouse: @scout directive unnecessarily constrains supported Eloquent directives

Created on 12 May 2021  路  6Comments  路  Source: nuwave/lighthouse

Describe the bug

A change to Lighthouse in the past few months has introduced an unnecessary validation check on the @search directive use which throws an exception whenever one of several other denoted argument directives are included in a query. This is likely intended to order to perform a validation check to best support the Algolia search driver, which indeed provides a very narrow feature set on account of searches being processed by a remote server.

I see this is as an error in execution to presuppose the appropriate argument set. For example the laravel-tntsearch driver supports most of the Eloquent query methods, because it executes locally and can afford to filter down indexed search results with an SQL query when so desired.

The relevant validation check is here: https://github.com/nuwave/lighthouse/blob/d6944386e2020dec767f236025e0d80704638ef2/src/Scout/ScoutEnhancer.php#L70

I suggest that Lighthouse should not be applying this validation check as it breaks support for some Scout search drivers.

Expected behavior/Solution

Don't apply/presume this validation check; make the app developer responsible for this.

Steps to reproduce

  1. Set up Lighthouse with TNTSearch Scout driver
  2. Build a query which mixes the @search directive with other Eloquent-based directives such as query scopes
  3. Run query and observe error in response

Lighthouse Version

5.x - Using 5.3.0 at the moment myself.

bug

All 6 comments

Consider the difference between the ArgBuilderDirective and ScoutBuilderDirective interfaces. Using the TNTSearch driver, what would a call to Model::search() return?

Oops I totally derped with this, I just totally forgot how I solved this before. My bad. :( Thanks for your patience.

NOPE, I was wrong, this totally breaks TNTsearch.

So I was implementing this using a (crude) builder method like this:

  /**
   * Add a limit constrained upon the query.
   *
   * @param  \Illuminate\Database\Query\Builder|\Illuminate\Database\Eloquent\Builder  $builder
   * @param  mixed $value
   * @return \Illuminate\Database\Query\Builder|\Illuminate\Database\Eloquent\Builder
   */
  public function projectTeamMember($builder, array $value)
  {
    $user = $value['user'];
    $role = $value['role'] ?? null;

    if ($builder instanceof ScoutBuilder) {
      return $builder->constrain(Project::whereTeamWithUser($user, $role));
    } else {
      return $builder->whereTeamWithUser($user, $role);
    }
  }

This totally works with TNTsearch and will be fine except Lighthouse just blocks it because it decides scopes are not okay.

Edit: I should clarify that I am using the @builder directive here, this might be what is being blocked.

Nit: Your code is inconsistently typed, the condition $builder instanceof ScoutBuilder will never be true if the PHPDoc is correct. This is precisely the reason why I separated the directive interfaces.

I agree that your use case should indeed work. I guess you are using @scope? We might open up some directives that implement ArgBuilderDirective to also implement ScoutBuilderDirective.

image

Out of those, only @eq and @trashed also implement ScoutBuilderDirective. Given that they defer to user defined code, which may or may not work, we should probably open up @scope and @builder. Any other directive that would also work or we could make work?

Regarding the docblock, true, I basically copied and pasted the docblock from documentation and haven't fussed with that too much yet. I personally prefer to use typing in the method declaration whenever that is practical.

I'm not using @scope here because of the need to check for the ScoutBuilder and then make use of the constrain() method. This is using the @builder directive.

The query definition (here "teamMember" used together with "search" doesn't work)

extend type Query @guard {
  projects(
    teamMember: ProjectTeamMemberInput @builder(method: "App\\GraphQL\\Builders\\QueryScopes@projectTeamMember")
    search: String @trim @search
    name: String @trim @eq
    projectType: ID @eq(key: "project_type_id")
    projectStatus: ID @eq(key: "project_status_id")
    office: ID @eq(key:"office_id")
    orderBy: _ @orderBy(columns: ["id", "name", "created_at", "updated_at", "record_due_at"])
  ): [Project!]! @paginate @can(ability: "viewScoped", injectArgs: true)
Was this page helpful?
0 / 5 - 0 ratings

Related issues

a-ssassi-n picture a-ssassi-n  路  3Comments

alexwhb picture alexwhb  路  4Comments

basudebpalfreelancer picture basudebpalfreelancer  路  4Comments

nguyentrongbang picture nguyentrongbang  路  3Comments

eriktisme picture eriktisme  路  4Comments