[5.3] Add Files folder to Media component and to "FileSystem local" adapter

[Fedik]

Pull Request for Issue # .

Summary of Changes

Trying to add /files folder, addittionaly to existing /images folder. It is not very logical to have PDF (and other) documents under /images.

The changes affects new installations. Existing installations should continue to work as usual.

Testing Instructions

Check that Media manager works. Upload images/files in Media manager view, upload images using Media field in Article editing, etc.

Actual result BEFORE applying this Pull Request

Works

Expected result AFTER applying this Pull Request

Works

Link to documentations

Please select:

• [ ] Documentation link for docs.joomla.org:
• [ ] No documentation changes for docs.joomla.org needed
• [ ] Pull Request link for manual.joomla.org: TBD
• [ ] No documentation changes for manual.joomla.org needed

[dgrammatiko]

@Fedik please add the files folder also here: https://github.com/joomla/joomla-cms/blob/1548e3085f94ac3a6cb45a5d4c6a09749524c5bb/libraries/src/Helper/PublicFolderGeneratorHelper.php#L169

[dgrammatiko]

FWIW I would prefer the folder to be named user_uploads than files

[Fedik]

Added.

I think files is a good name, it also will produce a good links. Let's say Company Site uploads a document, then link will be kind of example.com/files/foo-bar.pdf. example.com/user_uploads/foo-bar.pdf will not looks "professional" :wink:

[Fedik]

Currently there still a limitation (pretty huge), the custom field for Media does not allow to select anything else than folders under /images. It is hardcoded https://github.com/joomla/joomla-cms/blob/5e3d8489c3f5bea3074c6005b9be42b2d9927d04/plugins/fields/media/params/media.xml#L6-L9

I think it will need another field to allow to pick folders from Media manager adapters, but that something for another PR.

[dgrammatiko]

Currently there still a limitation (pretty huge), the custom field for Media does not allow to select anything else than folders under /images

You're right, there's a @TODO for this: https://github.com/joomla/joomla-cms/blob/1548e3085f94ac3a6cb45a5d4c6a09749524c5bb/layouts/joomla/form/field/media.php#L158

[dgrammatiko]

One more thing, could you add an .htaccess file inside the files folder with content:

<Files *.php>
  deny from all
</Files>

Since this is a new folder we can restrict it to ONLY static files. (the same should be applied to images and media folders but that's irrelevant to this PR)

[Fedik]

there's a TODO for this

Actualy, that "todo" can be already trashed, since we have type="images,documents", attribute. What i meant, is media field configuration for Custom fields.

One more thing, could you add an .htaccess file inside the files folder

Hmhm, I can, but this can affect people who already have /files folder, and use it on its own purpuse. Not sure here.

[richard67]

It seems the system tests need to be adapted, too. Currently they fail at file api/com_media/Files.cy.js:

Test that media files API endpoint
    1) can deliver a list of files
    2) can deliver a list of files in a subfolder
    ✓ can deliver a list of files with an adapter (346ms)
    3) can search in filenames
    4) can deliver a single file
    5) can deliver a single file with the url
    6) can deliver a single folder
    7) can create a file without adapter
    8) can create a folder without adapter
    9) can create a file with adapter
    10) can create a folder with adapter
    11) can update a file without adapter
    12) can update a folder without adapter
    13) can update a file with adapter
    14) can update a folder with adapter
    15) can delete a file without adapter
    16) can delete a folder without adapter
    ✓ can delete a file with adapter (325ms)
    ✓ can delete a folder with adapter (315ms)

  3 passing (38s)
  16 failing

[Fedik]

It seems the system tests need to be adapted, too

Yeap, I will look on it later

[brianteeman]

[Fedik]

@brianteeman ?

[laoneo]

I really like it, just the name is for me too generic. I would rather go with documents, if you name them files, then should the images also be moved there. Or you can go with /files/images and /files/documents. But having a generic files in root which directly contains everything except images is for me not logical.

[Fedik]

I will keep it generic. We definitely can do /files/images, sometime in future. 1 step at time :)

[Fedik]

I found a bug in media manager API, it always return path without adapter https://github.com/joomla/joomla-cms/blob/dfb572de83d16a25e7eee673b3875e18750e688b/api/components/com_media/src/Model/MediumModel.php#L235

And when you doing API call with non default adapter, like rename folder local-potato:/folder to local-potato:/folder-new it produces 404 error. Because it looking in default adapter local-files:/folder-new instead of requested local-potato:/folder-new.

[brianteeman]

I am a bit confused about the need for this PR.

If I create a folder myself in the root of the web space then I can already achieve everything that this PR does - what am i missing?

[Fedik]

what am i missing?

You need to create the folder by yourself. That what you missing :wink: I offer to have it in the core by default, so you do not have to create it by yourself every time.

[brianteeman]

what am i missing?

You need to create the folder by yourself. That what you missing 😉 I offer to have it in the core by default, so you do not have to create it by yourself every time.

Then I dont see the point in adding this

[Fedik]

I do not know, man. I do not know what to say. It is absolutely basic thing that should be done a decades ago.

Look, we have allowed uploads bmp,gif,jpg,jpeg,png,webp,avif,ico, mp3,m4a,mp4a,ogg, mp4,mp4v,mpeg,mov, odg,odp,ods,odt,pdf, png,ppt,txt,xcf,xls,csv And by default all goes in to /images folder, it confuses newcomer very much.

There still limitation that need to address, like hardcoded /images path in many places, but that for the future.

[brianteeman]

It is absolutely basic thing that should be done a decades ago.

It was and we removed it

[sousa9g]

From a Joomla extension developer view, this change is much needed. It is helpful not only for the core com_media. Thank you for adding it.

[MacJoom]

I have tested this item :white_check_mark: successfully on 2c7e0af42053da361863186128664322e7a2a0e8

Tested on 5.2 Alpha 2

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43532.}

[KingLouis1]

I have tested this item :white_check_mark: successfully on 2c7e0af42053da361863186128664322e7a2a0e8

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43532.}

[richard67]

@MacJoom This pull request has a conflicting file, so I can't set it to RTC.

[MacJoom]

@MacJoom This pull request has a conflicting file, so I can't set it to RTC.

Yeah just noticed, set back to untested

[peterpeter]

I have tested this item :white_check_mark: successfully on 2c7e0af42053da361863186128664322e7a2a0e8

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43532.}

[richard67]

@Fedik Could you fix the conflict in the integration test file? When that is done this PR can be set to RTC. Human test are valid as the conflict is only in that file which does not have effect on the tested functionality.

[crimle]

I have tested this item :white_check_mark: successfully on 2c7e0af42053da361863186128664322e7a2a0e8

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43532.}

[Fedik]

synced

[richard67]

RTC

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43532.}

[laoneo]

Please do not merge till there is an agreement in production about our future folder strategy.