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

Planning for Next Major Version #52

Closed
frasmage opened this issue Dec 30, 2016 · 22 comments
Closed

Planning for Next Major Version #52

frasmage opened this issue Dec 30, 2016 · 22 comments

Comments

@frasmage
Copy link
Collaborator

Hello to all watchers and contributors (paging @sebdesign),

With the new year approaching, I think it is time to start planning the next major version of this package. Most pressingly, with PHP 5.6 passing into security-only support tomorrow night, I think it is time to drop support for PHP 5.x branch and fully embrace the new features of PHP 7.x. I will be working on this over the next few weeks, as I have time.

A new major version presents an opportunity to add any number of other changes that we want to the codebase. So I would like to ask if there are any new features that should be incorporated into the package at the same time. Is there anything that you would like to see built into the package? Are there any methods that should really be moved or renamed? Noticed any code smells that I overlooked? Now is the time to ask.

Feel free to make any suggestions in this thread. Pull Requests also very much welcome!

@frasmage
Copy link
Collaborator Author

frasmage commented Dec 30, 2016

A few ideas that I had lingering:

  • convert docs from RST to markdown. ReadTheDocs supports both, but markdown is much easier to read and write.
  • fix the 2 tests that are inconsistent in travis
  • add a mass deletion method to the MediaCollection

@robbielove
Copy link

robbielove commented Jan 14, 2017

Could you please integrate pagination of Media::inDirectory() and $object->getMedia('resources') etc, results

@frasmage
Copy link
Collaborator Author

Hi @robbielove,

all of the Media and Mediable query scopes are simply applying conditions to the Laravel query builder, so the native paginate() functionality should work just fine.

Media::inDirectory('disk', 'dir')->paginate();

On the other hand, once you get to getMedia() all media attached to the model has already been loaded, so there is no efficiency benefit to pagination. If you simply want to split up the display of media, consider the chunk() method on the collection.

If you have a huge number of media attached to one model, you can use the paginate() method within a load() or with() statement to limit the results.

MyModel::where(...)->with(['media' => function($q) {
$q->where('tag', 'resources')->paginate();
})->first();

Hope this helps.

@sebdesign
Copy link
Contributor

Hi @frasmage,

it's good to see this package going forward! While I don't have any specific idea in mind, I'd love to contribute in the code or the tests when the time comes.

I'll try to throw some ideas if something comes to mind!

@frasmage
Copy link
Collaborator Author

Thanks @sebdesign, I will let you know. I will admit that I got a bit distracted finishing up my other package for release, I should get started on this in the next week or two.

@kaju74
Copy link

kaju74 commented Feb 8, 2017

It would be cool to be able to add custom properties to media(s), like rendering properties or if an image is protected from unathorized users,....

@frasmage
Copy link
Collaborator Author

frasmage commented Feb 8, 2017

@kaju74 Ha! It just so happens that I have another package for exactly that type of thing plank/laravel-metable. Not something that I would integrate into the core, but trivially easy enough for a user to marry the two packages.

Create a subclass of Media which uses the Metable trait, set that subclass as your 'model' in config/media.php and you are good to go. You can now attach anything you please to you media record.

Cheers!

@sebdesign
Copy link
Contributor

Hi @frasmage ,

I have an idea for the next version: setting the file visibility per media. This could be set in the MediaUploader, and could be changed later through the Media model. Laravel's storage already allows that: https://laravel.com/docs/5.4/filesystem#file-visibility.

Cheers!

@frasmage
Copy link
Collaborator Author

Hey @sebdesign,

Neat! I think that could easily be implemented, with three parts:

  • MediaUploader modifer, e.g. makePrivate() and makePublic(),
  • Media method to check its file, e.g. $media->isPublic() and/or $media->isPrivate()
  • UrlGenerator adjustment to throw an exception if private files are accessed directly.

Is it worthwhile to record this value in the database? That would allow verifying the visibility with a query scope. However it records the information in two places, so they could fall out sync (granted that already happens if the file on disk is modified in any other way).

In other news, I am leaning back in the direction of making the SourceAdapters return either stream resources or psr7 Streams across the board for 3.0. Ran into some memory limit issues with loading larger files into memory, which I imagine wouldn't be an issue for streams.

PS: I know there hasn't been much movement on this from me in the past few months. This is due to some exciting yet time consuming developments in my personal life. Everything should be settled by early April. Hoping to get back to this project as soon as I can. Feel free to work on some PRs in the mean time!

@klaravel
Copy link

klaravel commented Apr 8, 2017

Hey @frasmage,

First of all this is awesome package.

Only one thing if available it would great.

  • Can we pass some array of Ids and it will set order for that model's tag media with that orders?

Thanks for making great package.

@frasmage
Copy link
Collaborator Author

frasmage commented Apr 8, 2017

Hi @klaravel,

syncMedia() should already achieve that. e.g.

$media = $post->getMedia('gallery');
$post->syncMedia($media->reverse()->modelKeys());

(you shouldn't need to unpack the collection, just giving an example of passing an array of ids)

@klaravel
Copy link

klaravel commented Apr 8, 2017

@frasmage,

Great. I will try this one.
Also Is there anyway can I manipulate image before upload?

Example: I would like to use some API or other code to optimize that image same time upload image.

I know that I can do it after upload image with it's location.

@frasmage
Copy link
Collaborator Author

frasmage commented Apr 9, 2017

@klaravel,

This question has been asked a couple of times:

I am hoping to add built in support for imagine/intervention image at some point in the future to simplify things.

If you have any other general questions, would you mind creating a separate thread so we don't clutter the next version discussion?

@frasmage
Copy link
Collaborator Author

Slowly making progress on this. Additional features that I am currently working on:

  • Intervention image integration
  • better error handling of uploadedFile in error state
  • cleaning up unit tests

I'll push my 3.0 branch once I am satisfied with these features.

@liran-co
Copy link

@liran-co
Copy link

Scratch that, saw your earlier comment regarding the same thing - would be cool to have it integrated :)

@frasmage
Copy link
Collaborator Author

Hi @liran-co,

Indeed. Given that both of these are intended to be small modular components, I'm not keen on forcing the user to download both, when not everyone necessarily needs both.

What might be a good solution is to add a section on how to integrate them yourself to the documentation.

@liran-co
Copy link

That would certainly work!

Anyway to return the last ordered item? ->first() returns first, what about last()?

@liran-co
Copy link

I'm with @sebdesign on the public / private feature, it would be super helpful. I'm trying to upload some items as public and others as private.

@pet1330
Copy link
Contributor

pet1330 commented Aug 27, 2017

It would be nice if there was an easy way to create temporary urls. I know this feature is already available for some cloud services like S3, but it would be nice if we could just do something like:

// expires after 20 minutes
$duration_in_seconds = 20*60;

$post->firstMedia('thumbnail')->getUrl($duration_in_seconds);

or even something like

$post->firstMedia('thumbnail')->expires($duration_in_seconds)->getUrl();

and have it propagate down to which ever disk type is being used.

EDIT: useful link

@frasmage
Copy link
Collaborator Author

Hi @pet1330,

Thanks for the suggestion. I didn't even know that functionality existed! I will see about adding some handling for it.

@chadidi
Copy link

chadidi commented Nov 24, 2017

@frasmage can you add connection method to the package
example:

database1: media, mediable.
database2: users, posts.

$media = MediaUploader::fromSource($request->file('thumbnail'))
                            ->useHashForFilename()
                            ->upload();
$post->attachMedia($media, 'thumbnail'); // i wanna attach the media on the first database
$post->thumbnail = $media->getUrl();

@frasmage frasmage closed this as completed Dec 5, 2019
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

8 participants