Keep file name of attached media files #10570

DrewBrasher · 2021-10-27T18:42:08Z

No description provided.

dnfadmin · 2021-10-27T18:42:21Z

All CLA requirements met.

Skrypt · 2021-10-27T20:33:02Z

Awesome we have a new contributor 🐰

sebastienros · 2021-10-28T17:33:44Z

It's making us wonder if we want to display something that actually doesn't exist. Also if it would make sense to keep the original filename in the new file computed one.

deanmarcussen · 2021-10-28T17:35:59Z

For reference when we discuss this further. That's what a file name looks like
https://localhost:44300/blog1/media/mediafields/Article/41vqf90jr6ey03m859ts1tb759/0deb0e222f66fc43f580f76f8b51d522.jpg

/mediafields/.../GUID/MD5Hash.jpg

DrewBrasher · 2021-10-28T17:55:07Z

What if instead of prepending a guid to the file name and then hashing that for the final file name it used that guid for another sub folder to keep from overwriting files with the same name and then just keep the original name?

So then the path would look like this: /mediafields/.../ContainerItemGuid/FileGuid/OriginalFileName.jpg

DrewBrasher · 2021-11-05T11:35:39Z

It's making us wonder if we want to display something that actually doesn't exist. Also if it would make sense to keep the original filename in the new file computed one.

I would prefer the original name be kept if it can be. That way if a user downloads the file it will have the original name. What do you think of the suggestion in my previous comment?

deanmarcussen

Reviewing the issue, I agree with @matiasmolleja when this was originally designed, the hash was an important part of the design for the feature.

The suggestion was to provide a less well known field, which could be used when displaying the media item (in the front end, not the admin) to provide some context to the user as to the file they might be downloading.

I'm open to being convinced we could show this information in the admin as well, but it would have to be an addition the the actual file name created from the hash.

I'm open to seeing how we could maintain the hash and the original file name (without hacks), but I don't see a particular way forward there.

deanmarcussen · 2021-11-06T10:27:02Z

src/OrchardCore.Modules/OrchardCore.Media/Drivers/MediaFieldDriver.cs

@@ -123,6 +145,8 @@ public override async Task<IDisplayResult> UpdateAsync(MediaField field, IUpdate

                field.Paths = items.Where(p => !p.IsRemoved).Select(p => p.Path).ToArray() ?? new string[] { };

+                field.SetAttachedFileNames(items.Where(i => !i.IsRemoved).Select(i => i.AttachedFileName).ToArray());


I think this should only happen when the editor is attached. Around line 115

I moved that line up in 5280f52

deanmarcussen · 2021-11-06T10:33:49Z

src/OrchardCore.Modules/OrchardCore.Media/Drivers/MediaFieldDriver.cs

+                        {
+                            var lastSlash = items[i].Path.LastIndexOf('/');
+                            var nameOnly = items[i].Path[(lastSlash + 1)..];
+                            items[i].AttachedFileName = nameOnly[36..]; // remove unique id from name


It would be better if the javascript app includes the AttachedFileName on it's post, than having to do this .

I removed this code in 99ece16 but I'm still trying to figure out where to include it in the JavaScript app.

@deanmarcussen, If I understand how this works correctly, when you select or drop in a file it immediately posts the file and saves it to a temporary location and then moves it once you save the content type the file is attached to. Is that correct and if so, at which step should the AttachedFileName be set?

I think I figured it out but I'm not sure I did it the best way. Stepping through the JavaScript I did not see another way but I may have missed something. It's in this commit: 481d4f3

deanmarcussen · 2021-11-06T10:35:10Z

src/OrchardCore.Modules/OrchardCore.Media/wwwroot/Scripts/media.js

@@ -3351,7 +3356,7 @@ Vue.component('mediaFieldThumbsContainer', {
                        <div class="media-container-main-item-title card-body">\
                            <a href="javascript:;" class="btn btn-light btn-sm float-right inline-media-button delete-button"\
                                v-on:click.stop="selectAndDeleteMedia(media)"><i class="fa fa-trash" aria-hidden="true"></i></a>\
-                            <span class="media-filename card-text small text-danger" :title="media.name">{{ media.name }}</span>\
+                            <span class="media-filename card-text small text-danger" :title="media.name">{{ media.attachedFileName == null ? media.name : media.attachedFileName }}</span>\


I agree with @sebastienros here, we shouldn't be showing anything different in the edit UI here.

I reverted my changes to the admin display in 5cd1a81

DrewBrasher · 2021-11-08T16:24:02Z

@deanmarcussen I will update my changes to only store the file name, not display it and make the other changes you suggested here.

I do think something needs to be done to the admin display for files that don't have a thumbnail image like pdfs because right now it just shows a blank thumbnail and a hashed name so admin users have no indicator of what files they have already attached. But maybe that should be a separate issue or discussion.

deanmarcussen · 2021-12-13T10:21:52Z

merge conflicts to fix

# Conflicts: # src/OrchardCore.Modules/OrchardCore.Media/wwwroot/Scripts/media.js # src/OrchardCore.Modules/OrchardCore.Media/wwwroot/Scripts/media.min.js

…e name not the original file name

…the Attached editor

DrewBrasher · 2021-12-14T23:57:53Z

merge conflicts to fix

The conflicts were with the wwwroot\Scripts\media.js and media.min.js files. I resolved the conflict by overwriting mine with the OrchardCMS main. These files were updated by gulp. Should they be excluded from git? If not, I will add my generated files back to this once it is working.

…e name not the original file name.

deanmarcussen · 2021-12-15T07:49:28Z

These files were updated by gulp. Should they be excluded from git? If not, I will add my generated files back to this once it is working.

You'll need to regenerate with gulp, and commit to source control (we have to keep the generated files in source control because not all users, use the NuGet releases, some use the repository directly)

I recomend using gulp watch and just changing the media.js file so it rebuilds, rather than trying to rebuild the entire pipeline

DrewBrasher · 2022-02-16T14:00:15Z

Since this pull request has code that was changed then reverted, if it would make for a cleaner pull request, I can cancel this and make my changes on a fresh fork and do a new pull request if you want. Just let me know.

# Conflicts: # src/OrchardCore.Modules/OrchardCore.Media/wwwroot/Scripts/media.js # src/OrchardCore.Modules/OrchardCore.Media/wwwroot/Scripts/media.min.js

src/OrchardCore.Modules/OrchardCore.Media/Drivers/MediaFieldDriver.cs

sebastienros · 2022-08-18T17:22:54Z

Is it breaking anything in its current state when migrating from a previous version? What happens if the metadata is not preset?

DrewBrasher · 2022-08-19T12:39:46Z

Is it breaking anything in its current state when migrating from a previous version? What happens if the metadata is not preset?

@sebastienros It does not seem to be breaking anything when migrating from a previous version. Files that were attached before the migration will have the metadata added with a null value.

Here is what I did to test it:

Checked out the release/1.4 branch and ran the cms.web project.
Created a site with the Agency Theme and added a content type with a media field set to use the attached editor
Created a new item, attached a document, and published.
Checked out my branch and ran the cms.web project.
Viewed the content item and inspected the file object using the Vue explorer in Chrome.
Attached another file to the same content item as before and published.
Viewed the content item and inspected the file objects using the Vue explorer in Chrome.

PiemP · 2022-09-22T14:57:25Z

Related to #4563

DrewBrasher and others added 3 commits October 25, 2021 11:18

WIP: Adding filename to media field data

6348d92

Merge branch 'OrchardCMS:main' into main

2336532

Added AttachedFileName to the data stored on the MediaField.

f76ce85

DrewBrasher mentioned this pull request Oct 27, 2021

How Attached MediaField keeps the original file name？ #4563

Closed

hishamco requested a review from deanmarcussen October 27, 2021 19:22

deanmarcussen reviewed Nov 6, 2021

View reviewed changes

Skrypt mentioned this pull request Dec 9, 2021

Media added via code deleted from ContentItem if updated manually #10843

Closed

Merge remote-tracking branch 'upstream/main' into main

22461d5

# Conflicts: # src/OrchardCore.Modules/OrchardCore.Media/wwwroot/Scripts/media.js # src/OrchardCore.Modules/OrchardCore.Media/wwwroot/Scripts/media.min.js

DrewBrasher marked this pull request as draft December 14, 2021 13:44

DrewBrasher added 3 commits December 14, 2021 16:24

Reverted changes to the admin display so that it shows the actual fil…

5cd1a81

…e name not the original file name

Moved the call to SetAttachedFileName so that it is only called when …

5280f52

…the Attached editor

Removed code I added to parse name out of the path.

99ece16

DrewBrasher added 2 commits December 14, 2021 20:30

Merge branch 'OrchardCMS:main' into main

420900e

Reverted changes to the admin display so that it shows the actual fil…

59c144b

…e name not the original file name.

DrewBrasher added 3 commits February 7, 2022 12:10

Merge branch 'OrchardCMS:main' into main

6ac37dd

Merge branch 'OrchardCMS:main' into main

d595a10

Include AttachedFileName when uploading new files.

481d4f3

DrewBrasher marked this pull request as ready for review February 10, 2022 20:23

DrewBrasher requested a review from deanmarcussen February 16, 2022 14:00

DrewBrasher added 5 commits March 1, 2022 06:12

Merge branch 'OrchardCMS:main' into main

1b0965c

Merge branch 'OrchardCMS:main' into main

6a8c555

Merge remote-tracking branch 'upstream/release/1.4' into main

6226ad3

# Conflicts: # src/OrchardCore.Modules/OrchardCore.Media/wwwroot/Scripts/media.js # src/OrchardCore.Modules/OrchardCore.Media/wwwroot/Scripts/media.min.js

Merge remote-tracking branch 'upstream/main' into main

19b07a6

# Conflicts: # src/OrchardCore.Modules/OrchardCore.Media/wwwroot/Scripts/media.js # src/OrchardCore.Modules/OrchardCore.Media/wwwroot/Scripts/media.min.js

Rebuilt media js files

dfc1bda

Skrypt added the needs triage label Aug 5, 2022

ns8482e requested changes Aug 7, 2022

View reviewed changes

src/OrchardCore.Modules/OrchardCore.Media/Drivers/MediaFieldDriver.cs Show resolved Hide resolved

src/OrchardCore.Modules/OrchardCore.Media/Drivers/MediaFieldDriver.cs Show resolved Hide resolved

DrewBrasher requested a review from ns8482e August 9, 2022 13:49

sebastienros merged commit e2d8c29 into OrchardCMS:main Sep 22, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Keep file name of attached media files #10570

Keep file name of attached media files #10570

DrewBrasher commented Oct 27, 2021

dnfadmin commented Oct 27, 2021 •

edited

Loading

Skrypt commented Oct 27, 2021

sebastienros commented Oct 28, 2021

deanmarcussen commented Oct 28, 2021

DrewBrasher commented Oct 28, 2021

DrewBrasher commented Nov 5, 2021

deanmarcussen left a comment

deanmarcussen Nov 6, 2021

DrewBrasher Dec 14, 2021

deanmarcussen Nov 6, 2021

DrewBrasher Dec 14, 2021

DrewBrasher Feb 7, 2022

DrewBrasher Feb 10, 2022

deanmarcussen Nov 6, 2021

DrewBrasher Dec 14, 2021

DrewBrasher commented Nov 8, 2021

deanmarcussen commented Dec 13, 2021

DrewBrasher commented Dec 14, 2021

deanmarcussen commented Dec 15, 2021

DrewBrasher commented Feb 16, 2022

sebastienros commented Aug 18, 2022

DrewBrasher commented Aug 19, 2022

PiemP commented Sep 22, 2022 •

edited

Loading

		@@ -123,6 +145,8 @@ public override async Task<IDisplayResult> UpdateAsync(MediaField field, IUpdate

		field.Paths = items.Where(p => !p.IsRemoved).Select(p => p.Path).ToArray() ?? new string[] { };

		field.SetAttachedFileNames(items.Where(i => !i.IsRemoved).Select(i => i.AttachedFileName).ToArray());

Keep file name of attached media files #10570

Keep file name of attached media files #10570

Conversation

DrewBrasher commented Oct 27, 2021

dnfadmin commented Oct 27, 2021 • edited Loading

Skrypt commented Oct 27, 2021

sebastienros commented Oct 28, 2021

deanmarcussen commented Oct 28, 2021

DrewBrasher commented Oct 28, 2021

DrewBrasher commented Nov 5, 2021

deanmarcussen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DrewBrasher commented Nov 8, 2021

deanmarcussen commented Dec 13, 2021

DrewBrasher commented Dec 14, 2021

deanmarcussen commented Dec 15, 2021

DrewBrasher commented Feb 16, 2022

sebastienros commented Aug 18, 2022

DrewBrasher commented Aug 19, 2022

PiemP commented Sep 22, 2022 • edited Loading

dnfadmin commented Oct 27, 2021 •

edited

Loading

PiemP commented Sep 22, 2022 •

edited

Loading