Add `--skip-copy` flag to allow import of media from local filesystem without moving on disk by phlbnks · Pull Request #21 · wp-cli/media-command

phlbnks · 2017-06-21T12:28:02Z

Allow import of media from local filesystem without moving on disk

Fixes #24

danielbachhuber · 2017-06-21T16:20:04Z

@emirpprime Each pull request should only contain one conceptual change. Can you make sure this pull request only contains the specific change you're submitting?

This reverts commit 3f27d46.

phlbnks · 2017-06-21T17:24:21Z

@danielbachhuber Done, I think - I could only see that there was a WPCS commit in there accidentally.

danielbachhuber

Couple of changes to start with. Still thinking this through generally.

danielbachhuber · 2017-06-21T22:46:32Z

 	 * [--desc=<description>]
 	 * : "Description" field (post content) of attachment post.
 	 *
+	 * [--import_only]


What do you think about calling this --skip-copy or similar, to better communicate that we're skipping existing behavior?

Yes that makes sense to me too.

danielbachhuber · 2017-06-21T22:50:05Z

+      """
+      Success: Imported 1 of 1 items.
+      """
+    And the {CACHE_DIR}/large-image.jpg file should exist


We should also assert that there's no file in the new destination

Will do. I should get this updated later today.

@danielbachhuber I'm struggling to work out how to write this.
Can you point me ot any docs or examples that would help?

I guess I need to learn what Behat actually does with the image if it imports it? Does it move to {CACHE_DIR}/wp-content/uploads/year/month?
Then I think I'll need to create a new variable in FeatureContext.php for {UPLOADS_DIR} to use in the test. Is that about right?

Thanks!

You should just need And the wp-content/uploads/large-image.jpg file should not exist - see media-regenerate.feature for examples.

Edit - Should add that Daniel is offline till Wednesday slack#cli.

Thanks @gitlost, can't believe I didn't think to check in there :/

schlessera · 2017-06-25T09:18:05Z

+				$post_array['post_status'] = 'inherit';
+
+				$success = wp_insert_attachment( $post_array, $file, $assoc_args['post_id'] );
+				if ( ! is_wp_error( $success ) ) {


You should switch the conditions and bail early on error.

So, instead of this:

if ( ! is_wp_error( $success ) ) { // Handle success. } if ( is_wp_error( $success ) ) { // Handle failure. continue; }

you should prefer something like this:

if ( is_wp_error( $success ) ) { // Handle failure. continue; } // Handle success.

@schlessera I've updated the error handling part.

Since $success is used in areas these changes don't touch, shall I submit that as a seperate PR once this is resolved? Or is it ok to handle it in here?

schlessera · 2017-06-25T09:20:06Z

+				$post_array['post_mime_type'] = $wp_filetype['type'];
+				$post_array['post_status'] = 'inherit';
+
+				$success = wp_insert_attachment( $post_array, $file, $assoc_args['post_id'] );


As the variable $success is being used several times here, and mostly refers to a "non-success", I would prefer the name to be changed to something like $result instead. Naming it $success seems misleading.

(Yes, I know that name was already in the code... 😄 )

We can handle this in a subsequent PR.

phlbnks · 2017-06-27T14:11:10Z

I don't think that check fail was due to my code...

gitlost · 2017-06-27T14:20:33Z

@emirpprime no it's ImageMagick doing its thing (randomly seg faulting) - trying to make it go away in #26

danielbachhuber · 2017-06-28T14:08:55Z

      """

+  Scenario: Import a file as attachment from a local image and leave it in it's current location
+    Given download:


After Given download, you'll also need to run:

And I run `wp option update uploads_use_yearmonth_folders 0`

Otherwise, your file path assertion below will always be invalid, because WordPress will always used date-based folders.

danielbachhuber · 2017-06-28T14:10:15Z

@emirpprime Can you merge master to adopt the testing fix?

danielbachhuber · 2017-07-03T12:54:45Z

Can you merge master to adopt the testing fix?

I did this:

$ git remote add emirpprime https://github.com/emirpprime/media-command.git
$ git fetch emirpprime
remote: Counting objects: 108, done.
remote: Total 108 (delta 64), reused 64 (delta 64), pack-reused 44
Receiving objects: 100% (108/108), 14.30 KiB | 0 bytes/s, done.
Resolving deltas: 100% (66/66), completed with 7 local objects.
From https://github.com/emirpprime/media-command
 * [new branch]        allow_unsafe -> emirpprime/allow_unsafe
 * [new branch]        import_only  -> emirpprime/import_only
 * [new branch]        master       -> emirpprime/master
 * [new branch]        wpcs         -> emirpprime/wpcs
$ gco import_only
Branch import_only set up to track remote branch import_only from emirpprime.
Switched to a new branch 'import_only'
$ gm master
Auto-merging src/Media_Command.php
Auto-merging features/media-import.feature
Merge made by the 'recursive' strategy.
 bin/install-imagick.sh        |  6 +++---
 features/media-import.feature | 16 ++++++++--------
 src/Media_Command.php         | 14 ++++++++++++--
 3 files changed, 23 insertions(+), 13 deletions(-)
$ gp emirpprime
Counting objects: 22, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (19/19), done.
Writing objects: 100% (22/22), 2.72 KiB | 0 bytes/s, done.
Total 22 (delta 13), reused 0 (delta 0)
remote: Resolving deltas: 100% (13/13), completed with 7 local objects.
To https://github.com/emirpprime/media-command.git
   6c763ecf..054115d1  import_only -> import_only

Can handle the issue in a subsequent PR

danielbachhuber · 2017-07-03T14:05:42Z

Thanks for your work on this, @emirpprime !

Add `--skip-copy` flag to allow import of media from local filesystem without moving on disk

Phil Banks added 4 commits June 21, 2017 11:59

functional test for --import_only

af02f1c

add --import_only

995dad7

fix broken remote file import

a94eaba

update --import_only test

7201e90

phlbnks mentioned this pull request Jun 21, 2017

Allow import without moving files on disk #24

Closed

Phil Banks added 2 commits June 21, 2017 18:17

Revert "WPCS fixes"

eeb9836

This reverts commit 3f27d46.

updated noun in test

422a202

danielbachhuber requested changes Jun 21, 2017

View reviewed changes

rename to --skip-copy and improve test

e5ea370

schlessera previously requested changes Jun 25, 2017

View reviewed changes

bail early on error

6c763ec

This was referenced Jun 27, 2017

Find a way to install PHP imagick which doesn't seg fault in PHP 7.0 and 7.1. #7

Closed

Try ImageMagick 7.0.3-10 for PHP 7.1 and 6.9.6-8 for the rest. #26

Merged

danielbachhuber requested changes Jun 28, 2017

View reviewed changes

danielbachhuber changed the title ~~--import_only~~ Add --skip-copy flag to allow import of media from local filesystem without moving on disk Jun 28, 2017

Merge branch 'master' into import_only

054115d

danielbachhuber added this to the 1.0.4 milestone Jul 3, 2017

danielbachhuber added command:media Related to 'media' command command:media-import Related to 'media import' command labels Jul 3, 2017

Make sure the file isn't copied over

38b6176

danielbachhuber approved these changes Jul 3, 2017

View reviewed changes

danielbachhuber merged commit a43c457 into wp-cli:master Jul 3, 2017

phlbnks deleted the import_only branch July 10, 2017 08:33

danielbachhuber added a commit that referenced this pull request Nov 18, 2022

Merge pull request #21 from emirpprime/import_only

3bcc48f

Add `--skip-copy` flag to allow import of media from local filesystem without moving on disk

Conversation

phlbnks commented Jun 21, 2017 • edited by danielbachhuber Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danielbachhuber commented Jun 21, 2017

Uh oh!

phlbnks commented Jun 21, 2017

Uh oh!

danielbachhuber left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gitlost Jun 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phlbnks commented Jun 27, 2017

Uh oh!

gitlost commented Jun 27, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielbachhuber commented Jun 28, 2017

Uh oh!

danielbachhuber commented Jul 3, 2017

Uh oh!

danielbachhuber commented Jul 3, 2017

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

phlbnks commented Jun 21, 2017 •

edited by danielbachhuber

Loading

gitlost Jun 22, 2017 •

edited

Loading