Skip to content
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

Filterable fields don't resolve correctly when both Index and General variants exist #6423

Closed
tylernathanreed opened this issue Jun 12, 2024 · 1 comment
Labels
bug Verified bug by the Nova team fix incoming A fix is in review
Milestone

Comments

@tylernathanreed
Copy link

tylernathanreed commented Jun 12, 2024

  • Laravel Version: 10.48.12
  • Nova Version: 4.34.2
  • PHP Version: 8.2.13

Description:

When a resource has a fieldsForIndex method, and defines a field using the same attribute as another field in the fields method, where the field defined in fieldsForIndex is marked as filterable, and the other field using the same attribute isn't, Nova is unable to find the filterable field.

This happens because in ResolvesFields@filterableFields, Nova is grabbing all fields in the fields, fieldsForIndex, and fieldsForDetail method, and then applying a unique collection filter on the fields. If there's two fields with the same attribute, the unique filter arbitrary picks the first one, rather than the one that is marked as filterable.

It's worth noting that the ResolvesFields@filterableFields method does call the withOnlyFilterableFields filter, but this filter reduces the collection to fields that have the potential to be filterable, not fields that are actually marked as filterable. Once things bubble up into ResolvesFilters@resolveFiltersFromFields, and $field->resolveFilter($request) is called, which is where fields that truly are marked as filterable get filtered.

Suggestion Solution:

In ResolvesFields@filterableFields, before the unique filter, you could first sort the fields by whether or not they are marked as filterable. This would guarantee that if there are duplicate fields (by attribute), the field that's marked as filterable will get picked first.

Alternatively, in ResolvesFields@buildAvailableFields, you could grab from the fields defined by the $methods parameter first, and push in the fields defined in the fields method last.

Detailed steps to reproduce the issue on a fresh Nova installation:

Here's an example:

public function fieldsForIndex(Request $request)
{
    return [
        Fields\Text::make('Title')
            ->sortable()
            ->filterable()
    ];
}

public function fields(Request $request)
{
    return [
        Fields\Text::make('Title')
            ->rules(['required', 'string', 'max:255'])
    ];
}

In short, this results in "no filterable fields", despite the resource reading as, "the title field is filterable".

To break this down, ResolvesFields@availableFieldsOnIndexOrDetail pulls in fields first, then fieldsForIndex. This means that the non-filterable field comes first in the collection. Then in ResolvesFields@filterableFields, both fields will pass the withOnlyFilterableFields collection filter, has both fields have the potential to be filterable, even though only one of them actually has a filter callback. The unique collection filter right after that then drops the second field (which came from fieldsForIndex, and was actually marked as filterable). Once this bubbles up to ResolvesFilters@resolveFiltersFromFields, the method will return an empty collection.

Other Arrangements

It's worth noting that if the same field is defined in fieldsForIndex and fieldsForDetail, this issue doesn't happen. However, in a lot of cases, I define my index fields separately from my general fields, as they tend to be different enough (or ordered differently) that it's easier to just define it in a separate method.

@tylernathanreed tylernathanreed changed the title Filterable fields don't resolve correctly when both Index and Detail variants exist Filterable fields don't resolve correctly when both Index and General variants exist Jun 12, 2024
@crynobone crynobone added the bug Verified bug by the Nova team label Jun 14, 2024
@crynobone crynobone added the fix incoming A fix is in review label Jul 9, 2024
@crynobone crynobone added this to the 4.x milestone Jul 23, 2024
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Verified bug by the Nova team fix incoming A fix is in review
Projects
None yet
Development

No branches or pull requests

2 participants