Co-Authors-Plus
Co-Authors-Plus copied to clipboard
Automattic
Add wp-cli command to delete guest authors, with option to reassign their posts.
Currently, there is no way to delete guest authors from the command line. This command fills that need.
It is activated using delete-guest-authors, and passing in the path to a CSV file containing a list of two columns: the ID to delete, and the login to assign the posts to (or "false" to not reassign).
The command runs dry by default, showing the end user the guest authors it would have attempted to delete, and the user login it would have attempted to transfer the posts to.
Passing --no-dry-run, and confirming that you really do want to delete guest authors, actively modifies the database.
I'm not sure why the Travis build fails, it passes in my fork:
https://github.com/smistephen/Co-Authors-Plus/runs/12256989
This looks great! Could we allow deletion of all guest authors that have linked accounts? See #550. Would be good for testing, because allows to basically revert the install process. Maybe the csv thing could become an option (not compulsory) and in some way incorporate this bulk automatic deletion. Only include guest authors with linked accounts as we wouldn't want to delete the others, as there would be no fallback in post assignment.
Totally :+1:
Could you open a separate issue for those improvements? This PR's already pretty big, haha.
Looking at #550, it seems like it could be its own independent PR. Am I reading it right?
Nod, it might be useful for the comment to explain the intent behind this since I wouldn't say it's a common pattern :)
On 14 August 2018 at 18:40, Stephen Smith [email protected] wrote:
@smistephen commented on this pull request.
In php/class-wp-cli.php https://github.com/Automattic/Co-Authors-Plus/pull/588#discussion_r210126512 :
);
$assoc_args = wp_parse_args( $assoc_args, $defaults );$dry_run = $assoc_args['dry-run'];// Safety first.$filename_status = validate_file( $args[0] );if ( 0 !== $filename_status ) {WP_CLI::error( __( 'The provided file cannot be accessed. Please use either relative or absolute pathnames, and avoid using directory traversal such as ../ and other methods.', 'co-authors-plus' ) );}$filename = $args[0];// Add custom error handling on fopen error.Ah. The custom error handling is there to avoid an ugly PHP warning and stack trace from being thrown to the console if the CSV file can't be opened, doesn't exist, or some other error.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Automattic/Co-Authors-Plus/pull/588#discussion_r210126512, or mute the thread https://github.com/notifications/unsubscribe-auth/AARpqYPBy7Pw6eRbB4T6uFU6dxWvrJgzks5uQ1HagaJpZM4V9Lsf .
One overall question - how can we check this code is going to work as expected in production?
Checks I've performed so far:
- run phpunit (passes)
- run phpcs (passes)
- look for performance problems and security vulnerabilities (nothing obvious)
- test function return values and variable existence (added appropriate error handling code)
- test on a sample WordPress instance (seems to work as promised)
- try to break it with faulty data (seems to fail gracefully as predicted)
And I'm always open to more opportunities :+1:
Hooray for @philipjohn pulling the gremlin out of Travis, builds pass now, as they should :+1:
run phpunit (passes)
Which unit tests did you look at when trying to assess whether this code works as intended?
try to break it with faulty data (seems to fail gracefully as predicted)
That's a good idea! What kind of faulty data did you use here?
Which unit tests did you look at when trying to assess whether this code works as intended?
I examined the test_delete_* tests (https://github.com/Automattic/Co-Authors-Plus/blob/master/tests/test-coauthors-guest-authors.php#L722), as well as running the entire unit test suite locally before creating the PR.
That's a good idea! What kind of faulty data did you use here?
I used:
- non-existent guest authors
- malformed guest author IDs (various strings containing characters and punctuation)
- non-existent assignees
- malformed assignees (same as the IDs)
As well as:
- filepaths that should work (
testFile.csv,~/testFile.csv,/home/steve/testFile.csv) - filepaths that shouldn't work (
../../../../mwahahahaha.csv) work - files that don't exist
- files that do exist but can't be read (I set the file's permissions to
111to test this).
@rebeccahum I appreciate this is an old PR but is there a reason this PR never got merged? Happy to help get this over the line if possible.
@chrissy-dev I think this just slipped the radar. First step would be to resolve any merge conflicts and test if the current PR still works please!
