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

No collection fallback when using getTemporaryUrl #1898

Closed
nessor opened this issue May 24, 2020 · 6 comments
Closed

No collection fallback when using getTemporaryUrl #1898

nessor opened this issue May 24, 2020 · 6 comments

Comments

@nessor
Copy link

nessor commented May 24, 2020

PHP: v7.4.5
Laravel: v7.9.2
Media Library: 8.2.7

Hey guys,

I have noticed that apparently when using the function getTemporaryUrl the fallback url from the collection is not used.

The collection:

public function registerMediaCollections(): void
    {
        $this
            ->addMediaCollection('avatars')
            ->useFallbackUrl('/images/backend/no-avatar.png')
            ->useFallbackPath(public_path('/images/backend/no-avatar.png'));
    }

The call:

<img src="{{ Auth::user()->getFirstMedia('avatars')->getTemporaryUrl(\Carbon\Carbon::now()->addMinutes(10)) }}" alt="avatar">

The error:

Call to a member function getTemporaryUrl() on null

But when I try to call the getFirstMediaUrl function everything is working fine

<img src="{{Auth::user()->getFirstMediaUrl('avatars')}}" alt="avatar">

Am I doing something wrong or can someone validate the problem?

@freekmurze
Copy link
Member

I accept a PR that improves this behaviour.

@nessor
Copy link
Author

nessor commented May 27, 2020

@freekmurze
Little question on the side.
I'm in the middle of writing tests. But since I'm developing on Windows, it's totally annoying with the paths.

I get the following faild test:

4) Spatie\MediaLibrary\Tests\Feature\Media\GetPathTest::it_can_get_a_path_of_a_derived_image
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'C:\GitLab\laravel-medialibrary\tests/TestSupport/temp/media/1/conversions/test-thumb.jpg'
+'C:\GitLab\laravel-medialibrary\tests/TestSupport/temp/media\1/conversions/test-thumb.jpg'

C:\GitLab\laravel-medialibrary\tests\Feature\Media\GetPathTest.php:27

Is it perhaps useful to replace all "\" with "/" in the tests? Or could any other problems arise then?

@freekmurze
Copy link
Member

Yeah, than it won't work on Unix based systems. Maybe you can solve this by using DIRECTORY_SEPARATOR somewhere.

@DenMette
Copy link
Contributor

Hi @freekmurze,

After some digging into the codebase, I think it will break more stuff than necessary to make it work. When I send back a new Media object, other test are failing hard. Maybe an optional could be a solution on the long run.

@nessor can't you use:

// Your code
Auth::user()->getFirstMedia('avatars')->getTemporaryUrl(\Carbon\Carbon::now()->addMinutes(10))

// What I suppose it needs to be
// filename: InteractsWithMedia.php for more information
Auth::user()->getFirstTemporaryUrl(\Carbon\Carbon::now()->addMinutes(10), 'avatars')

@nessor
Copy link
Author

nessor commented Jun 22, 2020

@DenMette Idk. Unfortunately, I have so less time at the moment, that I could not continue to work on it :(

@freekmurze
Copy link
Member

Closing this due to inactivity.

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

No branches or pull requests

3 participants