ResourceDetailController & ResourceUpdateController should use Resource::detailQuery when checking existence

[dmason30]

• Nova Version: 4.18.0, 4.19.0

Description

I have had resources since Nova 3.x work that use a non primary key as the $request->resourceId by using a custom indexQuery and detailQuery on the resource class.

Recent changes toResourceDetailController, ResourceReplicateController and ResourceUpdateController are now throwing a 404 when trying to access them since 4.18.0 because it is querying the resource expecting $request->resourceId to contain the primary key.

I think all queries by id (e.g. on detail) should be using the Resource::detailQuery to check existence so that any customisations to the query are applied.

[4.18.0] Nova now throws a 404 when attempting to replicate a resource that doesn't exist

This added the following to ResourceDetailController, ResourceReplicateController and ResourceUpdateController in their respective __invoke methods...

abort_unless($request->findModelQuery()->exists(), 404);

This should be changed to use the Resource::detailQuery like so

abort_unless(tap($request->findModelQuery(), function ($query) use ($request) {
    $resource = $request->resource();
    $resource::detailQuery($request, $query);
})->exists(), 404);

[4.19.0] Added new breadcrumbs menu via the Nova::withBreadcrumbs method.

This added another query in the new breadcrumbs method in ResourceDetailController and ResourceUpdateController:

$resource = $request->findResourceOrFail();

This should also be changed to use the Resource::detailQuery:

$resource = $request->newResourceWith(tap($request->findModelQuery(), function ($query) use ($request) {
    $resource = $request->resource();
    $resource::detailQuery($request, $query);
})->firstOrFail());

Example where this is now throwing 404 - fixed with the above changes

I have an indexQuery using unions to show a fixed list of setting types that are editable:

The underlying model has many rows per setting type, when you view or edit setting it will use the type column as the $request->resourceId on the detail and update routes.

In the screenshot above, the fields Endpoint, API Key etc are actually a separate rows in the table, but as you can see the detail URL contains the setting type and the Resource::detailQuery handles resolving the $request->resourceId..

[crynobone]

We probably need to add a code example on https://github.com/laravel/nova-dusk-suite so we can avoid any regression related to this.

Can you please either add a Resource (with Model, migration, factory) example here or PR the example to https://github.com/laravel/nova-dusk-suite

[crynobone]

In the screenshot above, the fields Endpoint, API Key etc are actually a separate rows in the table, but as you can see the detail URL contains the setting type and the Resource::detailQuery handles resolving the $request->resourceId..

But why wouldn't you just set protected $primaryKey = 'type'; in the Eloquent model for such example?

[dmason30]

In the screenshot above, the fields Endpoint, API Key etc are actually a separate rows in the table, but as you can see the detail URL contains the setting type and the Resource::detailQuery handles resolving the $request->resourceId..

But why wouldn't you just set protected $primaryKey = 'type'; in the Eloquent model for such example?

I just experimented with this and unfortunately I need the primaryKey to still be id for certain Eloquent functions to work as expected e.g. $model->touch().

Simplified Replication Example

My example above is a very specific use case and I am struggling to find a simple use case that would make sense in the real world. However, this arbitrary example below will have worked in 4.17.2 but would have been broken since 4.18.0:

Model

- id (primary key)
- uuid
- ...

Resource

class ExampleResource extends Resource {
   public function fields(NovaRequest $request) {
     return [
       ID::make('UUID', 'uuid'),
       // ...
     ]
   }
   
   public function detailQuery(NovaRequest $request, $query) {
     $query->orWhere('uuid', $request->resourceId);
   }
   
   public static function redirectAfterCreate(NovaRequest $request, $resource)
   {
      return '/resources/'.static::uriKey().'/'.$resource->uuid;
   }

   public static function redirectAfterUpdate(NovaRequest $request, $resource)
   {
     return '/resources/'.static::uriKey().'/'.$resource->uuid;
   }
}

Workaround

I have managed a workaround for this issue by overriding findModelQuery via a custom request class and binded it to force it to use detailQuery if the Resource implements a specific interface..

public function findModelQuery($resourceId = null)
{
    $resource = $this->newResource();

    if ($resource instanceof PrefersDetailQuery) {
        return $resource->detailQuery($this, $this->newQueryWithoutScopes());
    }

    return parent::findModelQuery($resourceId);
}

$this->app->bind(ResourceCreateOrAttachRequest::class, CustomResourceCreateOrAttachRequest::class);
$this->app->bind(ResourceDetailRequest::class, CustomResourceDetailRequest::class);
$this->app->bind(UpdateResourceRequest::class, CustomUpdateResourceRequest::class);
$this->app->bind(
  ResourceUpdateOrUpdateAttachedRequest::class,
  CustomResourceUpdateOrUpdateAttachedRequest::class,
);

class ApplicationSettingResource extends Resource implements PrefersDetailQuery

[crynobone]

I feel like we can opt to rely on Eloquent getRouteKey() and getRouteKeyName() when developer want to use slug/uuid based route key insteads of default $primaryKey. To do this we need to modify:

• ID to expose $routeKey
• Resource redirectAfterCreate() and redirectAfterUpdate() need to be updated.
• All pages controllers (Inertia) should query based on routeKeyName() as "key" and return primaryKey value to response as "props".

At the moment we 100% rely on getKey() but Laravel\Nova\Resource does uses Illuminate\Http\Resources\DelegatesToResource