search

DeuxHuitHuit / craft-cloudflare-stream

Upload your videos assets to Cloudflare Stream from Craft CMS assets interface
https://plugins.craftcms.com/cloudflare-stream
Other
1 stars 1 forks source link

(fix) Check for rootPath property #8

Closed curtishenson closed 1 month ago

curtishenson commented 9 months ago

Seems like not all file systems have the rootPath property.

We ran into an issue similar to #7 but with the servd assets plugin. Which is S3 under the hood.

Checking for rootPath and defaulting to an empty string seems to make everything work again.

I wasn't able to test a local filesystem to see if this caused any issues there.

nitriques commented 1 month ago

Hey @curtishenson

Sorry for the late response. Thanks for the fix! I do not use S3, but we do aim to be compatible with as many FS as possible.

I added 7fa52dc to your branch, it's purely cosmetic.

This will be release with craft 5 support. Let us know if you need that for craft 4, as it can be easily backported.

Thanks again !

curtishenson commented 3 weeks ago

Awesome!

pixelcab commented 1 week ago

Hey @curtishenson & @nitriques, I'm relatively new to CraftCMS, and I may simply be missing a configuration or env variables somewhere, but while using the Local fs and setting the craft-cloudflare-stream setting to use form input instead of public URL. The getAssetFolderPath method will only return null for the $path value, which would cause a failure in the uploadVideoByPath method.

After reverting the changes, this PR implemented (locally) the $path was able to be generated, minus the Volume subfolder (which I've removed for now), and the video was uploaded to CF Stream without issue.

This is a long-winded way of saying, either would you be able to point me in the direction of what config value I may need to include, or this change accidentally breaks Local FS.

Thanks in advance, and I appreciate your efforts here.

nitriques commented 1 week ago

Hey @pixelcab !

I'm relatively new to CraftCMS, and I may simply be missing a configuration or env variables somewhere,

Welcome! We DO want beginners to be able to use the extension, so this is great !

We do use the local FS all the time with this plugin and it works pretty well on our end.

The getAssetFolderPath() function does not even know about the setting and should work exactly the same way in either case.

Could I ask you to use dd() to get the values of your root path ? The only way I see the $path variable being null is if your rootPath is null. AFAICT, the rootPath of the local FS will never be null (this may point to a different problem).

Also, could you provide the complete stack trace of the error you are seeing with the current code ? Thanks !