Refactor CRUD::delete method to support advanced DELETE queries

[shvlv]

The current implementation relies on wpdb::delete that supports the simple where x = y conditions only. It's not only incomplete but also dangerous because Where::$comparisonOperator and Where::$logicalOperator are just ignored, so the result is unpredictable. The following change provides a way to use existing QueryBuilder functionality to generate DELETE SQL. The documentation describes some restrictions.

Additionally, I updated actions/cache to v4 to make CI tests run.

[defunctl]

This is great, thanks a lot @shvlv !

I stopped commenting, but I'd like to see the following:

1. Asserting the posts you created exist before they are deleted.
2. Rather than assertNotNull, assert the type or an actual value.

[shvlv]

@defunctl I've updated the test approach.
But regarding the release, could we not make it as 1.2 so we can start to use it in the short term? I think there are no breaking changes, I didn't update any of the existing tests.

[defunctl]

@defunctl I've updated the test approach. But regarding the release, could we not make it as 1.2 so we can start to use it in the short term? I think there are no breaking changes, I didn't update any of the existing tests.

@shvlv I was suggesting 2.0.0 because this changes the underlying functionality of a used method, but is also a bug fix.

Are there no scenarios you can think of where someone is unintentionally relying on existing functionality and this update could break things for them, even though we're correcting the logic?

[shvlv]

Are there no scenarios you can think of where someone is unintentionally relying on existing functionality and this update could break things for them, even though we're correcting the logic?

For the current functionality, the only way it works as expected is to use only a simple where query with = comparison:

DB::table('posts')
			->where('post_type', 'delete_like_test')
			->delete();

If someone used something different, like whereLike or whereIn, they should get completely wrong results. Is it possible that someone relies on those buggy results? 🤔

[defunctl]

Are there no scenarios you can think of where someone is unintentionally relying on existing functionality and this update could break things for them, even though we're correcting the logic?

For the current functionality, the only way it works as expected is to use only a simple where query with = comparison:

DB::table('posts')
			->where('post_type', 'delete_like_test')
			->delete();

If someone used something different, like whereLike or whereIn, they should get completely wrong results. Is it possible that someone relies on those buggy results? 🤔

That is my exact worry. That they even wrote tests for those buggy results, so it's really just a question of do we want to potentially release that type of issue right now, or when they move to 2.0.0 which is more expected with a major release that things could "break"

[defunctl]

Looking good, your call on the release version, I personally think 2.0.0 so there aren't as many surprises, but I'll leave that up to you.

@borkweb may still want to have a look though :)

[borkweb]

I love this change and the code looks great, @shvlv. The likelihood that this will cause problems with current uses out in the wild is incredibly low due to the nature of how limited the delete() call was.

I'm ok with 1.2 and accept that there may be annoying edge case bugs that'll creep up due to bad implementations elsewhere.

[bordoni]

@dpanta94 has put some good work on this should be aware of the changes.