sudar / bulk-delete

Bulk Delete is a WordPress Plugin that allows you to delete posts, pages and users in bulk based on different conditions and filters.
https://bulkwp.com
GNU General Public License v2.0
33 stars 8 forks source link

Delete Posts by Post status - Custom Post status - Add tests #189

Closed mariadanieldeepak closed 6 years ago

mariadanieldeepak commented 6 years ago

This should be done after #217

Once this task is done, the helper functions should be moved to the tests library repo.

While sending this task for code review create a new issue for moving the helper functions to the the test library repo

mariadanieldeepak commented 6 years ago

@sudar, @rajanit2000,

I see few things to be corrected.

https://github.com/sudar/bulk-delete/blob/dev/6.0.0/tests/wp-unit/include/Core/Posts/Modules/DeletePostsByStatusModuleTest.php#L215

force_delete should be true since we're deleting permanently and not trashing it.

Similar changes to be made in the following lines too.

The following function name should indicate if the function deletes or trashes. Also, fix the force_delete flag accordingly.

The following functions are missing their counterparts (i.e. Either Trashed TC is written but not deleted the vice versa)

@rajanit2000,

Glad that you were able to write test cases. Totally appreciate it. Also, do note that the Test cases are like frontline soldiers that should take the first hit when a release is planned. So keeping them solid saves us from pain.

Also, how about naming the post statuses similar to the process in baking a Pizza?

https://github.com/sudar/bulk-delete/blob/dev/6.0.0/tests/wp-unit/include/Core/Posts/Modules/DeletePostsByStatusModuleTest.php#L462

Ex. Preparing Dough, Baking, Topping. ;) Leaving some Bread crumbs in the code makes it even more delicious. What do you think?

sudar commented 6 years ago

@mariadanieldeepak

Thanks for reviewing the tests and for coming up with the list of changes.

Let's create a new issue to track them instead of reopening the old one. That way we won't have any issues while naming the branch.

mariadanieldeepak commented 6 years ago

@sudar,

Let's create a new issue to track them instead of reopening the old one. That way we won't have any issues while naming the branch.

A separate issue has been created and you can find it here

And hence closing this issue.