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

Use native disk methods instead of the pseudo scoped disks #844

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

LukeTowers
Copy link
Member

@LukeTowers LukeTowers commented Feb 21, 2023

WIP, requires wintercms/storm#141

@LukeTowers LukeTowers added the enhancement PRs that implement a new feature or substantial change label Feb 21, 2023
@what-the-diff
Copy link

what-the-diff bot commented Feb 21, 2023

  • Removed the cms.storage config and moved it to filesystems.php
  • Added a new disk for each of uploads, media and resized files (scoped disks)
  • Updated ImageResizer::getAvailableSources() to use scoped disks instead of cms storage config values
  • Updated MediaLibrary::getStorageDisk() to use DISK constant instead of cms storage config value
    5-6: Moved getPublicPath(), getStorageDirectory(), isPublic() methods from File model into base class as they are used by other models that extend this one too (e9d8f7b1c). Also updated them so they work with scoped disks now rather than using the old CMS Storage configuration options which have been removed in favour or Laravel's Filesystem Adapter system now being used everywhere throughout Winter Storm codebase where possible/appropriate - see https://github.com/octobercms/winterstorm#filesystem-adapter for more information on how these changes affect you if at all! :)

@LukeTowers
Copy link
Member Author

@jaxwilko this was the WIP PR for breaking out those disks, I don't think it would be too difficult to finish. Perhaps we should also use properties instead of class constants for storing the disks so that we can more easily switch away from the singleton pattern later for the medialibrary etc.

Do you want to take a look and see what's left to push it across the finish line?

@jaxwilko
Copy link
Member

@LukeTowers Happy to do so but it's going to be middle of next week before i can start :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement PRs that implement a new feature or substantial change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants