spatie / laravel-medialibrary

Associate files with Eloquent models
https://spatie.be/docs/laravel-medialibrary
MIT License
5.78k stars 1.08k forks source link

New 'getDownloadFilename' method for extensibility #3504

Closed chrispage1 closed 10 months ago

chrispage1 commented 10 months ago

When downloading a file by utilising Responsable, the file naming at the point of download isn't very flexible.

I've added a method that allows extending the Media class, and in turn the download file name, rather than having to create my own download controller for a filament project I'm working on (for some reason in Filament the file names are by default obfuscated). I can then simply extend the functionality by overriding the method

    public function getDownloadFilename(): string
    {
        return $this->name . '.' . pathinfo($this->file_name, PATHINFO_EXTENSION);
    }

In addition I've utilised Symfony's HeaderUtils class (as Laravel does) to generate the disposition, ensuring it's sanitised and always correctly formatted.

Thanks!

freekmurze commented 10 months ago

Seems like the testsuite is failing because of these changes. Could you take a look at that?

chrispage1 commented 10 months ago

Seems like the testsuite is failing because of these changes. Could you take a look at that?

Sure, its changed the disposition structure slightly, I'll reconsider that element 👍

chrispage1 commented 10 months ago

@freekmurze I've changed the way its being sanitised as it turns out the HeaderUtils is a bit too strict, e.g. 'my file name.png' would be converted to 'my-file-name.png' but there's no reason why a file name couldn't include spaces when downloading.

So instead escaping quotes and using ascii formatting.

Thanks!

freekmurze commented 10 months ago

Thanks!