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

Plugins List Generate URL based on getCurrentPanel()->getId() can get wrong when using not ID as slug when tenancy enabled #21

Closed
eelco2k opened this issue Jul 24, 2024 · 6 comments
Labels
bug Something isn't working enhancement New feature or request

Comments

@eelco2k
Copy link

eelco2k commented Jul 24, 2024

in resources/views/pages/table.blade.php

the route: filament()->getCurrentPanel()->getId() can get wrong when you use another key as the routekey when tenancy is enabled. the extra parameter "tenant" should be added to the route() helper

original:

<x-filament::icon-button :tooltip="trans('filament-plugins::messages.plugins.actions.generate')" tag="a" href="{{route('filament.'.filament()->getCurrentPanel()->getId().'.resources.tables.index', ['module'=>$item->module_name])}}">

modified:

@if ((bool) filament()->hasTenancy())
    @php
       $urlParams = [
          'tenant' => filament()->getTenant(),
          'module' => $item->module_name,
      ];
   @endphp
@else
    @php
        $urlParams = [
            'module' => $item->module_name,
        ];
    @endphp
@endif
<x-filament::icon-button :tooltip="trans('filament-plugins::messages.plugins.actions.generate')" tag="a" href="{{ route('filament.' . filament()->getCurrentPanel()->getId() . '.resources.tables.index', $urlParams) }}">
@3x1io 3x1io added the bug Something isn't working label Jul 26, 2024
@3x1io
Copy link
Member

3x1io commented Jul 26, 2024

hi, @eelco2k thanks for your report, I will try to fix it ASAP, thanks for your support.

@eelco2k
Copy link
Author

eelco2k commented Jul 26, 2024

No problem! ✌🏽

Regarding the issue: I still find it strange that it does not fill the correct tenant param itself correctly via the route() helper function. It should, as I can remember i had this issue before where I needed to add the “tenant” param explicitly. But after configuring somewhere (have to check where that is) that the default routeParam for the tenant would be different. I could remove the “tenant” param for all the routes() as extra param. So it is weird that in your package it does not work.

Thank you for following up on this issue

@eelco2k
Copy link
Author

eelco2k commented Jul 26, 2024

I think it was related to Route Model Binding

but as Model/Table does not use this via getRouteKeyName() function it uses the default (which returns “id”) from the inherited Eloquent Model getRouteKeyName() function.

Perhaps you could move the logic to Table model into that getRouteKeyName() function where it checks if tenancy is enabled and. If so return

filament()->getTenant()->{filament()->getTenant()->getRouteKeyName()};

@eelco2k
Copy link
Author

eelco2k commented Jul 27, 2024

i've changed the code above a little bit, as the param 'tenant' also accepts filament->getTentant() (so it uses the Model associated with the Tenancy. and will use and also fallback to the correct routekeyname )

@php
$urlParams = ['module' => $item->module_name];
if ((bool) filament()->hasTenancy()) {
   $urlParams['tenant'] = filament()->getTenant();
}
@endphp
<x-filament::icon-button :tooltip="trans('filament-plugins::messages.plugins.actions.generate')" tag="a"
                    href="{{ route('filament.' . filament()->getCurrentPanel()->getId() . '.resources.tables.index', $urlParams) }}">

@3x1io
Copy link
Member

3x1io commented Jul 29, 2024

hi @eelco2k i think the last one maybe the best solution and I will try to apply it, thanks for your support.

@3x1io 3x1io added the enhancement New feature or request label Jul 29, 2024
@3x1io 3x1io changed the title Plugins List Generate URL based on getCurrentPanel()->getId() can get wrong when using not ID as slug when tenancy enabled Plugins List Generate URL based on getCurrentPanel()->getId() can get wrong when using not ID as slug when tenancy enabled Sep 8, 2024
@3x1io
Copy link
Member

3x1io commented Sep 16, 2024

fixed on the last version.

@3x1io 3x1io closed this as completed Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants