Skip to content
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

Add `--from-post=<post_id>` flag to create duplicate posts #154

Merged
merged 9 commits into from May 8, 2018

Conversation

3 participants
@sagarnasit
Copy link
Contributor

sagarnasit commented Mar 14, 2018

I here modified the create command to generate a duplicate post.

@danielbachhuber
Copy link
Member

danielbachhuber left a comment

👍 Great start! Can you include functional tests too?

*
* [--from-post=<post_id>]
* : The post id of a post to be duplicated.
*

This comment has been minimized.

@danielbachhuber

danielbachhuber Mar 20, 2018

Member

Looks like this is spaces instead of tabs. Can you correct to tabs?

This comment has been minimized.

@sagarnasit

sagarnasit Mar 21, 2018

Author Contributor

yes, I will correct it.

* # Create a duplicate post with same post data.
* $ wp post create --from-post=123 --post_title='Different Title'
* Success: Created post 2350.
*/

This comment has been minimized.

@danielbachhuber

danielbachhuber Mar 20, 2018

Member

Ditto spaces -> tabs.

This comment has been minimized.

@sagarnasit

sagarnasit Mar 21, 2018

Author Contributor

My bad. I will change it too.

$post_id = $post_arr['ID'];
unset( $post_arr['post_date'] );
unset( $post_arr['post_date_gmt'] );
unset( $post_arr['guid'] );

This comment has been minimized.

@danielbachhuber

danielbachhuber Mar 20, 2018

Member

WHy did you decide to skip these values?

This comment has been minimized.

@sagarnasit

sagarnasit Mar 21, 2018

Author Contributor

This values will be duplicated too. I skip post_date and post_date_gmt so that values can be reset according to the current date at the time of duplicating. Reason to unset guid is that duplicated post guid will same as the original.

If you have any suggestions please let me know.

if( isset( $assoc_args['from-post'] ) ) {
$post = $this->fetcher->get_check( $assoc_args['from-post'] );
$post_arr = get_object_vars( $post );
$post_id = $post_arr['ID'];

This comment has been minimized.

@danielbachhuber

danielbachhuber Mar 20, 2018

Member

$post_arr['ID'] should be unset, correct?

This comment has been minimized.

@sagarnasit

sagarnasit Mar 21, 2018

Author Contributor

yes, you are right. Actually, if we don't unset id it will still generate new id. But, for a safer side, I will make correct it.

* @param $post_id id of the post.
* @return array
*/
private function get_metadata( $post_id = null ) {

This comment has been minimized.

@danielbachhuber

danielbachhuber Mar 20, 2018

Member

Instead of making $post_id optional, we should make it required:

get_metadata( $post_id = null )

This comment has been minimized.

@sagarnasit

sagarnasit Mar 21, 2018

Author Contributor

sure.

if ( empty( $assoc_args['meta_input'] ) ) {
$assoc_args['meta_input'] = $this->get_metadata( $post_id );
}

This comment has been minimized.

@danielbachhuber

danielbachhuber Mar 20, 2018

Member

Do we need to persist taxonomy data too?

This comment has been minimized.

@sagarnasit

sagarnasit Mar 21, 2018

Author Contributor

This part is for overwriting existing metadata if a user wants. Categories and tags can be duplicated too. what are your thoughts about it?

This comment has been minimized.

@danielbachhuber

danielbachhuber Mar 21, 2018

Member

Yes, I think categories and tags should be duplicated too.

@sagarnasit

This comment has been minimized.

Copy link
Contributor Author

sagarnasit commented Mar 22, 2018

@danielbachhuber I made changes you asked and added taxonomy duplication as well. I don't have any idea about functional testing so I haven't done testing yet.

@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Mar 22, 2018

@sagarnasit I can help you go through setting up the testing.

You should first check whether you can even run the existing tests.

As the Functional Tests help states, you'll need to prepare your database server first:

Before running the functional tests, you’ll need a MySQL (or MariaDB) user called wp_cli_test with the password password1 that has full privileges on the MySQL database wp_cli_test. Running the following as root in MySQL should do the trick:

GRANT ALL PRIVILEGES ON wp_cli_test.* TO "wp_cli_test"@"localhost" IDENTIFIED BY "password1";

Then, to run the entire test suite:

./vendor/bin/behat --expand

Or to test a single feature:

./vendor/bin/behat features/core.feature

sagarnasit added some commits Apr 1, 2018

@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Apr 2, 2018

Looking into the Travis failure: #161

@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Apr 3, 2018

@sagarnasit I should have addressed the test failure on PHP 5.3 now. Can you merge latest master into your PR to make the tests pass again?

@sagarnasit

This comment has been minimized.

Copy link
Contributor Author

sagarnasit commented Apr 14, 2018

Thanks @schlessera . I also added functional testing as @danielbachhuber said.

sagar.nasit@rtcamp.com
"""
{TERM_ID}
"""
@require-wp-4.4

This comment has been minimized.

@schlessera

schlessera Apr 19, 2018

Member

Blank line needed between scenarios (above the @require-wp-4.4 tag).

@@ -171,7 +178,28 @@ public function create( $args, $assoc_args ) {
}
$array_arguments = array( 'meta_input' );
$assoc_args = \WP_CLI\Utils\parse_shell_arrays( $assoc_args, $array_arguments );
$assoc_args = \WP_CLI\Utils\parse_shell_arrays($assoc_args, $array_arguments);

This comment has been minimized.

@schlessera

schlessera Apr 19, 2018

Member

This entire block (lines 181-202) seems to have been inadvertently reformatted to PSR2 with the last commit.

Please adapt the formatting to adhere to WPCS instead.

*
* @return array
*/
private function get_tags( $post_id ) {

This comment has been minimized.

@schlessera

schlessera Apr 19, 2018

Member

The code path for tags is not covered by the tests.

@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Apr 19, 2018

@sagarnasit This looks good already. There's some whitespace that got messed up during the last commit that you need to redress, and I would like to also have a test that covers the "tags" code.

sagar.nasit@rtcamp.com added some commits Apr 21, 2018

sagar.nasit@rtcamp.com
@sagarnasit

This comment has been minimized.

Copy link
Contributor Author

sagarnasit commented Apr 21, 2018

@schlessera Added Test for Tags. Looks Good?

@schlessera
Copy link
Member

schlessera left a comment

Code looks good. Only some minor stuff as a final cleanup, and then we can merge.

/**
* Get Tags of a post.
*
* @param $post_id postid of the post.

This comment has been minimized.

@schlessera

schlessera Apr 23, 2018

Member

Should be:

@param $post_id ID of the post.
/**
* Get Categories of a post.
*
* @param $post_id postid of the post.

This comment has been minimized.

@schlessera

schlessera Apr 23, 2018

Member

Should be:

@param $post_id ID of the post.
/**
* Get post metadata.
*
* @param $post_id id of the post.

This comment has been minimized.

@schlessera

schlessera Apr 23, 2018

Member

Should be:

@param $post_id ID of the post.

When I run `wp post term add {POST_ID} post_tag {TAG_ID} --by=id`
Then STDOUT should contain:
"""*

This comment has been minimized.

@schlessera

schlessera Apr 23, 2018

Member

There's a random asterisk added here.

sagar.nasit@rtcamp.com
@sagarnasit

This comment has been minimized.

Copy link
Contributor Author

sagarnasit commented Apr 25, 2018

@schlessera Made changes you asked for.

@schlessera schlessera changed the title create subcommand extended to generate duplicate post Add `--from-post<post_id>` flag to create duplicate posts May 8, 2018

@schlessera schlessera changed the title Add `--from-post<post_id>` flag to create duplicate posts Add `--from-post=<post_id>` flag to create duplicate posts May 8, 2018

@schlessera schlessera merged commit cf1741d into wp-cli:master May 8, 2018

1 check passed

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

@schlessera schlessera added this to the 1.3.0 milestone May 8, 2018

@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented May 8, 2018

Thank you for the pull request, @sagarnasit !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.