OrchardCMS / OrchardCore

Orchard Core is an open-source modular and multi-tenant application framework built with ASP.NET Core, and a content management system (CMS) built on top of that framework.
https://orchardcore.net
BSD 3-Clause "New" or "Revised" License
7.42k stars 2.39k forks source link

How Attached MediaField keeps the original file name? #4563

Closed chinasqzl closed 2 years ago

chinasqzl commented 5 years ago

Attached MediaField uploaded as a GUID + real file name, saved and changed to MD5.xxx, is there such a necessary, or has other options to retain the original file name?

matiasmolleja commented 5 years ago

No other options as it is now. When we designed how it should work we tried to preserve disk space as much as possible. We use the hash to check if a newly added file is already there. If it is there we don't upload the new file.

We do that here: https://github.com/OrchardCMS/OrchardCore/blob/f21b24cf4f2a7ab441bae03eba09f9322ab235f8/src/OrchardCore.Modules/OrchardCore.Media/Services/AttachedMediaFieldFileService.cs#L105

Skrypt commented 5 years ago

Hash is usefull for avoiding the issue of people naming their files with similar names. You don't want to override files when uploading new ones and you can't really remember all the file names you used before. I think here, what @chinasqzl wants is to add a name portion after the hash so that the file names themselves could be indexed by a search engine for SEO or could be also for adding to a Lucene index. For that matter, I prefer that we make use of a Title field for images. Most of the time people don't look at their image names and they just upload it even if they have strange chars and else. Ex: ( "my-file_06-copy2.jpg")

deanmarcussen commented 5 years ago

Yes I like the idea of appending the source file as an extension to the hash. We have had a requirement to know the original file name with this field, but had to capture it manually in a text field. I can imagine it’s not totally uncommon to need to maintain the original file name

sebastienros commented 5 years ago

That could be part of the data stored on the field instead.

deanmarcussen commented 5 years ago

What about a dictionary on the field for metadata such as this, and perhaps an alt tag value from an alt-editor @sebastienros ?

sebastienros commented 5 years ago

We have talked about tags already for standard media fields. I would be ok with that only as another property on the field. But that means changing the ui significantly. But no dictionary, it's not open ended. Well-known properties only.

chinasqzl commented 5 years ago

File name is especially important when the user uploads an attachment, not a picture

DrewBrasher commented 3 years ago

I'm using an Attached Media Field to attach pdf files to content items and I also need it to display the original name both in the admin view and the default view. I would be happy to help with a solution to this if given some direction on how this should be implemented.

deanmarcussen commented 3 years ago

That could be part of the data stored on the field instead.

Here's an example of how we store extra data on the media field.

https://github.com/OrchardCMS/OrchardCore/blob/78d6a459b1e63cd1d53e13699cf059f5c8372986/src/OrchardCore.Modules/OrchardCore.Media/Drivers/MediaFieldDriver.cs#L149

A similar solution for this with the extension methods etc would be great.

DrewBrasher commented 3 years ago

Here's an example of how we store extra data on the media field.

https://github.com/OrchardCMS/OrchardCore/blob/78d6a459b1e63cd1d53e13699cf059f5c8372986/src/OrchardCore.Modules/OrchardCore.Media/Drivers/MediaFieldDriver.cs#L149

A similar solution for this with the extension methods etc would be great.

Should it be a selectable option like Anchors?

DrewBrasher commented 3 years ago

@deanmarcussen before I get to far, does this look like I'm on the right track? https://github.com/DrewBrasher/OrchardCore/commit/6348d92d019671f09040ee10bdb9f79e288e1ed5

deanmarcussen commented 3 years ago

Yes, looks good thanks

I don't think it'll be a selectable option like anchors, that was because we required some ui around it (or not if the user didn't want it.)

Might call it AttachedFileNames because it will only apply to the attached editor I think?

DrewBrasher commented 3 years ago

Created pull request https://github.com/OrchardCMS/OrchardCore/pull/10570 for this.

DrewBrasher commented 2 years ago

@deanmarcussen I've had to go ahead and start using that pull request in a production site. Is this still something the Orchard team is interested in supporting or should I work on a way to achieve this through a separate module?

Skrypt commented 2 years ago

We need to review back the PR. It is summer time, people are busy, sorry for that.

DrewBrasher commented 2 years ago

I understand

hishamco commented 2 years ago

May be I come too late but

is there such a necessary, or has other options to retain the original file name?

I don't think it's a good idea to keep the original name, assume I upload a client picture "Jon.png", after awhile I create a blog that I need to upload a picture of the blogger. So, if Jon is there, either we need to notifiy the user that the image already there or overwrite it (which is a problematic).

Generating a GUID or random name avoid such naming issues

DrewBrasher commented 2 years ago

@hishamco My changes do not change the way filenames are generated or stored. It only adds a property to the field that contains the original file name so someone can use it if they want similar to mediaText or anchor. The reason I need this is because my admin users need to attach non image files like pdfs and audio files and since those do not have a thumbnail image like a picture file, once a file is attached they cannot tell what files have already been attached. Here is a screenshot of what it looks like for reference:

image

With the changes I made, I can now override the admin display in my own module so that it displays the original file name instead of a string of characters and no thumbnail.

hishamco commented 2 years ago

@hishamco My changes do not change the way filenames are generated or stored. It only adds a property to the field that contains the original file name so someone can use it if they want similar to mediaText or anchor.

No problem. BTW I didn't look into your PR but I comment based on the issue I saw ;)

PiemP commented 2 years ago

Sorry guys but, for my opinion, save file with an hash name is something that reduce the site accessibility. This feature is more important than what is seems. For a blind user is not beautiful to download a file that have a strange name. And the MediaField, in the Attached mode, is the only field that allow to upload file in a position that is easy manageable by a security role's: reducing risks of wrong deletion.

Add another fields in the MediaField to save the original file name could cause a lot of misunderstanding when update from one version of OrchardCore to another one. If would proceed in this way it could be necessary to document this changes well. To keep this modify more "transparent", for the old OrchardCore version, could be useful to edit MediaField templates to have the possibility to download the file with the original name (and not the hash one) without doing something strange in the code (the HTML5 download attribute is enough for this?).

Hash is usefull for avoiding the issue of people naming their files with similar names

OK, but is simple to return an exception, add an error to the model and say to user about the necessity to change the file name. If Orchard change the file name, it do an abuse, if the user have named a file in a logical way. Probably is not the focus of the MediaField to be used with pdf or other documents...but for me, at this time, could be impossible to move my uploaded documents to another specific field for this purpose.

Another question: what happen if I delete a content item that have some files related and uploaded with a MediaField in Attached mode? If we reuse a file with the same hash in another content item ... what happen if I delete a content item that is related to this file?

Sorry guys, hope to be not inappropriate. I hope to been useful in some way and not polemic. Is not my intent to be polemic and sorry for my bad written english.