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

[10.x] Make $path optional in Filesystem methods #44395

Merged
merged 4 commits into from
Nov 2, 2022
Merged

[10.x] Make $path optional in Filesystem methods #44395

merged 4 commits into from
Nov 2, 2022

Conversation

innocenzi
Copy link
Contributor

Follow-up of #44384, since this contains breaking changes to method signatures, this PR is targeted at master instead of 9.x.


This PR makes the $path parameter optional in the following methods:

  • FilesystemAdapter#putFile
  • FilesystemAdapter#putFileAs
  • UploadedFile#store
  • UploadedFile#storeAs
  • UploadedFile#storePublicly
  • UploadedFile#storePubliclyAs

Since #44105 (thanks Frank), I've been using the scoped driver in order to be able not to repeat paths throughout my codebases. It allows me to do the following:

// Before
Storage::disk('s3')->putFile('chat/attachments', $uploadedFile);
//                           ~~~~~~~~~~~~~~~~~~  This could be repeated in some places

// After
Storage::disk(Disk::ChatAttachments)->putFile('', $uploadedFile)

With this PR, it's possible to omit the first argument, so one could do this instead:

Storage::disk(Disk::ChatAttachments)->putFile($uploadedFile)

@innocenzi innocenzi changed the title Make $path optional in Filesystem methods [10.x] Make $path optional in Filesystem methods Sep 30, 2022
@ar2r
Copy link
Contributor

ar2r commented Oct 4, 2022

It will be more usefull like this, but i dont need that:

Storage::disk('s3')->putFiles('chat/attachments', [$uploadedFile1,$uploadedFile2]);

Your code allows you to upload files without controlling the directory and the level of access to this file. the path can be defined in a constant and you can reuse it every time, instead of a string.

@innocenzi
Copy link
Contributor Author

I'd rather the constant be the disk, that's why I do that. The path is defined in the disk definition, so, yes, I do control the directory.

@taylorotwell
Copy link
Member

Just curious - why would the put method not allow this?

@innocenzi
Copy link
Contributor Author

That's an omission. Added it 👍

Copy link
Contributor

@milwad-dev milwad-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good

@taylorotwell taylorotwell merged commit 8d47416 into laravel:master Nov 2, 2022
@innocenzi innocenzi deleted the feat/fs-optional-path branch November 9, 2022 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants