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

Feature: Allow customization of URL generation (alternative) #214

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

niladam
Copy link

@niladam niladam commented Jan 17, 2025

Hello @pxlrbt

After the discussion in #212 i went ahead and came up with an alternative solution.

Now you can configure the URL generation like this in a boot() method in a ServiceProvider.

use pxlrbt\FilamentExcel\FilamentExport;

FilamentExport::createExportUrlUsing(function ($export) {
    $fileInfo = pathinfo($export['filename']);
    $filenameWithoutExtension = $fileInfo['filename'];
    $extension = $fileInfo['extension'];

    return URL::temporarySignedRoute(
        'your-custom-route',
        now()->addHours(2),
        ['path' => $filenameWithoutExtension, 'extension' => $extension]
    );
});

I had to relocate the handling of the notifications to the new class FiamentExport and while there i split the handling of the notification methods for easier maintenance going forward.

Let me know your thoughts about this.

P.S: This also updates the readme :)

Thank you!:)

@niladam
Copy link
Author

niladam commented Jan 17, 2025

I guess merging this should close #212

@niladam
Copy link
Author

niladam commented Jan 22, 2025

hello @pxlrbt -- i'm sorry for pushing this (i know it's usually frowned upon) -- but i was wondering if you had a chance to look at this and maybe have any other suggestions or think this is good enough :)

Thank you!:)

@pxlrbt
Copy link
Owner

pxlrbt commented Jan 23, 2025

Hey,
thanks for taking the time to draft another version. I had a look at it and just wanted to have a second look at the refactoring in my code editor, but I'm currently travelling. I will merge this the next days.

@niladam
Copy link
Author

niladam commented Jan 23, 2025

Hey,

thanks for taking the time to draft another version. I had a look at it and just wanted to have a second look at the refactoring in my code editor, but I'm currently travelling. I will merge this the next days.

Safe travels! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants