Fix Breaking Change in Form/Uploader

[onairmarc]

Removed && config('livewire.temporary_file_upload.directory') !== null from Form/Uploader on Line 103 due to a breaking change.

Livewire's config file defaults to null for livewire.temporary_file_upload.directory. If this config key has a value of null, Livewire internally sets this to livewire-tmp. PR #451 introduces a breaking change for people who have not customized livewire.temporary_file_upload.directory.

The temporary fix is to manually set a config value for livewire.temporary_file_upload.directory. The permanent fix is the changes made in this PR.

[awcodes]

There's got to be a better way to handle this. I think your Pr is relevant but I feel that chaining if conditionals is a bad code smell. Could definitely use some help here because cloud storage basically negates the point of the plugin and it's integrate with glide.

[onairmarc]

There probably is a better way to go about doing this. Perhaps we could extract these checks into their own private or protected methods to improve readability/maintainability. If you like, I could work on that. From an isolation perspective, I think that should be a separate PR, but I can do that here if you'd like.

I'm not sure how cloud storage negates the point of this package; from my perspective, it only adds value to the package. If Curator didn't have cloud storage, it wouldn't have been a contender when I was going through my selection process.

[awcodes]

The problem with cloud storage is the plugin defaults to serving media from the host server. Which is the intended usage. Hence the glide implementation. When using a cloud provider that implementation actually becomes a performance detriment unless the model is overridden at the app level.

[awcodes]

It is a different way of thinking about media in a way that we havn't been programmed to think of media. If you prefer a more traditional approach I would recommend the media library plugin from Ralph.

[onairmarc]

I still think that cloud storage will be crucial for this package. There's some media that, yes, is served directly from the cloud storage provider, but other media is secured behind the application for which this package is perfect. Ralph's package was intriguing, but this part of my application (Basic CMS for Churches) might be open-sourced, so paid packages weren't an option, at least in my case.

Yes, technically, there is a performance hit on the first request for an image, but the same thing can be said for many things in application development. A proper caching strategy will aid in the recovery from the performance hit.

I think there might also be configuration options that would allow for the desired Glide functionality to pair nicely with the need for cloud storage. One of those configuration options/changes could be moving the default configuration to the database. If cloud_storage is true, serve media from cloud storage directly, if false, serve through the package.

My current implementation actually bypasses the package entirely when serving through cloud storage due to certain media being set to public in the cloud storage drive, so even right now, this package doesn't introduce a performance hit in that regard.

These are just some ideas that I hope come across as trying to be helpful. Interested to hear your thoughts.

[awcodes]

I do agree that cloud support can be vital just always have trouble with it. What fixes one person's app breaks another. It's crazy.

What are your thoughts on a service class / "provider" instead of configs for how to serve the images?

I'm thinking there could be an interface too for things like thumbnail url, etc.

But that route would most likely be a breaking change and have to wait for v4 of the plugin.

[onairmarc]

Like a class per cloud provider? I think that could be very promising? There could be a BaseStorageProvider and some basic default storages providers like S3 and DigitalOcean both of which extend BaseStorageProvider. This could enable people to more easily implement custom storage providers in the future.

When you say interface for thumbnails, are you talking about language interfaces or url interfaces? Couldn't these just be traits that utilize the access methods within the class?

[awcodes]

Yea. I'm thinking something along the lines of the font providers and avatar providers in filament.

The benefit of the interface is that that the provider can be swapped in and out but the provider has to have methods for thumbnail etc so the plugin can call them off of the provider instead of the model so that the table, files in the panel etc all depend on that implementation.

[awcodes]

Could eliminate the need for overriding the model.

[onairmarc]

I think that could be interesting and probably the right move! Are you planning on removing the ability to override the model or just making it where it would no longer be necessary but still could if you wanted to?

[awcodes]

I would still support overriding it because it would still need to be should you have to setup inverse relationships for some reason or you are customizing the db table.

[onairmarc]

Wonderful! I am happy to help however I can. Just let me know what you need!

[awcodes]

I do think this approach would have to be implemented in v4 though as, at least in my head at the moment, it would be a breaking change.

[onairmarc]

100% agree that it should be v4