CharrafiMed / global-search-modal

Enhances filamentphp's global search by transforming it into a modal for a better user experience
https://filamentphp.com/plugins/charrafimed-global-search-modal
72 stars 16 forks source link

Bug: [Missing parameter: tenant] on renderHook when not authenticated #57

Closed OccTherapist closed 1 month ago

OccTherapist commented 1 month ago

When i am not authenticated I get this exception

Missing required parameter for [Route: filament.admin.networks.resources.tasks.edit] [URI: admin/{tenant}/networks/tasks/{record}/edit] [Missing parameter: tenant]. 

When I disable your plugin it works fine and i can authenticate. Then I can enable the plugin again and it works.

CharrafiMed commented 1 month ago

hey @OccTherapist

I don't believe this is a bug but rather an issue with how the plugin is being used. If you check the Filament discussions, you'll find this type of error comes up frequently.

Could you provide more details on how the plugin is configured in your project, especially regarding tenant management and authentication?

OccTherapist commented 1 month ago

Thank you for your fast response!

I configured the plugin in the AdminPanelProvider by just adding \CharrafiMed\GlobalSearchModal\GlobalSearchModalPlugin::make(), to the plugins array.

In fact I use a custom tenant name.

Here are my User Model relationships:


public function facilities(): \Illuminate\Database\Eloquent\Relations\BelongsToMany

    {
        return $this->belongsToMany(Facility::class, 'facility_user', 'user_id', 'facility_id');
    }

    public function teams(): \Illuminate\Database\Eloquent\Relations\BelongsToMany
    {
        return $this->facilities();
    }

And here is my Tenant:


public function users(): \Illuminate\Database\Eloquent\Relations\BelongsToMany
    {
        return $this->belongsToMany(User::class);
    }

    public function members(): \Illuminate\Database\Eloquent\Relations\BelongsToMany
    {
        return $this->users();
    }
CharrafiMed commented 1 month ago

oki! did you extend Pivot in the Facility model or Illuminate\Database\Eloquent\Model??

CharrafiMed commented 1 month ago

It would be really helpful if you could create a minimal reproduction repository that demonstrates this weird behavior when not authenticated.

OccTherapist commented 1 month ago

No, it's an basic Eloquent Model. But I added a custom RouteKey:

public function getRouteKeyName()
    {
        return 'slug';
    }
OccTherapist commented 1 month ago

I will try to setup a minimal repository today.

For now maybe this stackTrace helps...

log.log

CharrafiMed commented 1 month ago

emmmm... Could you try checking what Filament::getTenant() returns ? You can do a quick dd(Filament::getTenant()) to see .

OccTherapist commented 1 month ago

It would return null, when I'm not authenticated. Wouldn't it?

CharrafiMed commented 1 month ago

Yeah, that’s expected.

I see someone solve a similar issue here

OccTherapist commented 1 month ago

I think I narrowed the bug a little bit.

The authentication works and I get redirected correctly when I remove all the $recordTitleAttribute from my resources. Thats obvious, because there wont be any page search feature then.

It also works when i remove laravel breezy 2FA plugin with Force 2FA enabled.

It seems that the tenant is not set yet, when the 2FA is not completed, but the search already loads and therefore wants the urls to have a tenant.

Then what I don't understand yet is, that the urls already have my tenants slug when Breezys 2FA page kicks in.

OccTherapist commented 1 month ago

Maybe it would solve this issue when the global search modal only is rendered by the renderhook when the authentication and 2FA are completed and the user actually is redirected to the panel.

CharrafiMed commented 1 month ago

Maybe it would solve this issue when the global search modal only is rendered by the renderhook when the authentication and 2FA are completed and the user actually is redirected to the panel.

I understand your idea about rendering the global search modal only after authentication and 2FA completion. However, how do we handle the panels that don't have an authentication system and are designed to be publicly accessible?

CharrafiMed commented 1 month ago

I think I need to play around with the boot method in the plugin to make things configurable.

please in your free time could you try to reproduce the issue by creating a minimal repository, It would help me see how I can fix this issue in scenarios like yours.

OccTherapist commented 1 month ago

I created a reproduction repository: https://github.com/OccTherapist/minimal-bug-reproduction

There are a few useless files, since I used the filament starter package. Hope you can ignore them.

Installation:

Username: admin Password: admin

OccTherapist commented 1 month ago

This issue is also happening on the famous filament-tour plugin by @JibayMcs

Here are the issues

You can see it in my pull request https://github.com/JibayMcs/filament-tour/pull/24

CharrafiMed commented 1 month ago

In the past 15 minutes, I cloned your repo and started thinking about the solution. I saw your PR, but I believe the issue we're facing is quite different. this is the entry file of the plugin


<?php

namespace CharrafiMed\GlobalSearchModal;

use Filament\Panel;
use Filament\Contracts\Plugin;
use Filament\View\PanelsRenderHook;
use Illuminate\Support\Facades\Blade;
use CharrafiMed\GlobalSearchModal\Concerns\HasMaxWidth;
use CharrafiMed\GlobalSearchModal\Concerns\CanHandleRTL;
use CharrafiMed\GlobalSearchModal\Concerns\HasPlaceHolder;
use CharrafiMed\GlobalSearchModal\Concerns\CanUseCustomViews;
use CharrafiMed\GlobalSearchModal\Concerns\HasSearchItemTree;
use CharrafiMed\GlobalSearchModal\Concerns\CanExpandUrlTarget;
use CharrafiMed\GlobalSearchModal\Concerns\CanBeSwappableOnMobile;
use CharrafiMed\GlobalSearchModal\Concerns\CanHighlightQueryMatches;
use CharrafiMed\GlobalSearchModal\Concerns\HasAccessibilityElements;
use CharrafiMed\GlobalSearchModal\Concerns\CanCustomizeModalBehaviors;
use CharrafiMed\GlobalSearchModal\Concerns\CanInteractWithLocalStorage;

class GlobalSearchModalPlugin implements Plugin
{
    use CanCustomizeModalBehaviors;
    use CanInteractWithLocalStorage;
    use CanBeSwappableOnMobile;
    use HasMaxWidth;
    use CanExpandUrlTarget;
    use CanUseCustomViews;
    use HasSearchItemTree;
    use CanHighlightQueryMatches;
    use HasAccessibilityElements;
    use HasPlaceHolder;

    public static function make()
    {
        return app(static::class);
    }

    public function getId(): string
    {
        return 'global-search-modal';
    }

    public function register(Panel $panel): void
    {
        $panel->renderHook(
            PanelsRenderHook::BODY_START,
            fn (): string => Blade::render('@livewire("global-search-modal" )'),
        );
    }

    public function boot(Panel $panel): void
    {
        //
    }
}

The only solution I'm considering at the moment is setting a condition for rendering the plugin. I think this could resolve issues like yours. It could be something like this:


    public function register(Panel $panel): void
    {if($conditon){
        $panel->renderHook(
            PanelsRenderHook::BODY_START,
            fn (): string => Blade::render('@livewire("global-search-modal" )'),
        );
}
    }

And the condition could check whether a user is authenticated or use another closure to detect specific conditions."

CharrafiMed commented 1 month ago

My initial idea was incorrect because the register method is executed only once. After logging into the panel, the plugin is still not rendered. I believe this issue requires core support, as we need to modify something in Filament itself to account for tenant parameters in URLs when not authenticated.

OccTherapist commented 1 month ago

Why it is only executed once? You mean once per request? Or are you referring to SPA Mode?

I had the same idea as you. What else do you think of render hooks with scopes? see Filament documentation

Maybe there is a way to build an exclusion scope like all but Auth Pages or so...

Or even one that checks if the user is authenticated.

edeoliv commented 1 month ago

I get the same error if a user is not logged in yet or if he/she logout.

imagen

CharrafiMed commented 1 month ago

hey @OccTherapist ! Recently, while using your minimal repo to troubleshoot the issue, I encountered this error:

image I was able to solve it by adding this method to the App\Models\Facility

 public function facility()
    {
        return $this->members()->where('team_id', Filament::getTenant()->id);
    }

and it seems also the case for the activity log model image

However, regarding the solution to the issue, Filament currently doesn't support exception scopes. Even if we consider submitting a PR to Filament v3, it likely won't be accepted, as they only accept new features for v4. So, we’ll have to wait until v4 is released.

One thing I will add to the plugin now is the ability to add scopes, which seems to resolve the issue. However, we all agree this could become cumbersome for projects with multiple resources and pages. For now, though, this is the only workaround we've discovered.

CharrafiMed commented 1 month ago

hey @edeoliv! Are your project scenarios like @OccTherapist ??

edeoliv commented 1 month ago

hey @edeoliv! Are your project scenarios like @OccTherapist ??

My project uses a multi-tenancy architecture, but in my case, I’m facing an issue where I can't access the admin panel directly. Instead, I have to log in through the public interface first, and only then can I access the admin panel. If no user is logged in, I encounter an error when trying to access the admin panel directly.

CharrafiMed commented 1 month ago

Yeah, that's the same problem as @OccTherapist. For now, the only hack that @OccTherapist found is to prevent access to pages outside authentication by using scopes that exclude those pages see

CharrafiMed commented 2 days ago

hey @OccTherapist! i think this problem has been fixed due last release.