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

Feature: Add archive extraction functionality to filetree contextmenu #12775

Merged
merged 5 commits into from Dec 9, 2015

Conversation

Projects
None yet
3 participants
@exside
Contributor

exside commented Nov 21, 2015

What does it do ?

Based on the forum discussion here: http://forums.modx.com/thread/84971/unzip-function-for-file-manager#dis-post-497458

This adds a filetree context menu item "Extract file" for .zip files (zip only for now), the PR includes an additional permission (file_unpack) and corresponding lexicon entry plus all necessary modifications in the core to make it work. That includes an additional method unpack() in the modfilehandler class where this feature could be used in another context (sure, that functionality could be used before this PR via the xPDO Archive class when loaded properly). It also adds the unrelated method move() as an alias of the rename() method to the filehandler, that's just something I find useful to write meaningful code when using the filehandler (depending on what the intended action is you can use either method to make the code speak for himself).

Looks like that:
bildschirmfoto 2014-05-09 um 19 35 51

The file/folder structure inside the extracted archive will be preserved and the content of the archive will be extracted to the same folder as the archive lives in.

Why is it needed ?

Because extracting archives from the manager is awesome =)

Related issue(s)/PR(s)

This is the refactoring of the following PR (which was closed because I wanted to change too many things...again =D): #11352

Credits also go to Susan Ottwell for providing the right idea and approach to get this running!

Things to be aware

a) This will probably not work when you pull this into an existing installation of MODX as there is a new permission (file_unpack) that is "installed" when you install MODX, so basically to fully test it a new build of MODX has to be created and installed (or the permission has to be added manually)
b) As far as I'm aware the PHP ZipArchive Class (which is used by XPDOZip, which itself is used by this PR) overwrites existing files with files that have the same name inside the archive, I think it will not touch other files, but I wasn't very eager to tryout what happens if I for example extract an archive with a folder "assets" inside into the MODX_BASE_PATH...that example probably also shows the potential danger of that feature (if used unwisely)...what do you guys think, should we take care of that or should we "delegate" that responsibility to the user with the permission to unpack files?

@sottwell

This comment has been minimized.

Contributor

sottwell commented Nov 22, 2015

Oh, yes please! Someone had a patch for adding this some time ago, but it was a bit uncertain in that it did patch the core and didn't work well as newer versions of MODX were a bit different in the core. This is the one feature that I need to be able to get rid of FTP/SFTP altogether and work entirely in the MODX Manager!

@exside

This comment has been minimized.

Contributor

exside commented Nov 22, 2015

That patch was from me and was posted in #11352 =), this is basically a "simplyfied" (no modifications in XPDO repo, only zip archives) version of that PR!

@Mark-H Mark-H added this to the v2.5.0-pl milestone Nov 22, 2015

@sottwell

This comment has been minimized.

Contributor

sottwell commented Nov 23, 2015

Well, I patched the relevant files and it works great. So here's looking forward to seeing it merged and in the next release!

@exside

This comment has been minimized.

Contributor

exside commented Nov 26, 2015

If you have an installation that's suitable for such test (backup the original assets folder =D), could you test what happens if you pack a folder called "assets" with a few files in it (some with the same name, but no content for example as in the existing assets folder) and you unpack it inside the MODX_BASE_PATH...the question is, will that "kill"/replace the assets folder completely (which would be bad) or just replace existing files with the same name and add the new ones, e.g. "merge" the original and the packed one (that's better)?

@sottwell

This comment has been minimized.

Contributor

sottwell commented Nov 27, 2015

It merges, at least on my MAMP-based localhost. Not sure if different server configurations would have any effect.

@exside

This comment has been minimized.

Contributor

exside commented Nov 28, 2015

I believe not, as far as I read in the php-docs at http://php.net/manual/en/ziparchive.extractto.php this is the default behavior of the extractTo() method!

@Mark-H Mark-H merged commit d4d98a5 into modxcms:2.x Dec 9, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Mark-H

This comment has been minimized.

Collaborator

Mark-H commented Dec 9, 2015

Thanks @exside - works nicely!

@Mark-H Mark-H self-assigned this Dec 9, 2015

@exside

This comment has been minimized.

Contributor

exside commented Dec 9, 2015

Thanks for merging! @sottwell will love that =)

@sottwell

This comment has been minimized.

Contributor

sottwell commented Dec 9, 2015

Yes! Built and installed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment