berlindb / core

All of the required core code
MIT License
252 stars 27 forks source link

Consider adding automatically prefixed pre and post action hooks on CRUD methods within query class #138

Open arraypress opened 2 years ago

arraypress commented 2 years ago

Here is an example of a function within my code base for a third-party plugin I am building for EDD 3.0:

function delete_automation( $automation_id = 0 ) {
    $automation_query = new Automation_Query();

    // Pre-increase order coupon rule use count
    do_action( 'eddot_pre_delete_automation', $automation_id );

    $retval = $automation_query->delete_item( $automation_id );

    // Pre-increase order coupon rule use count
    do_action( 'eddot_post_delete_automation', $retval, $automation_id );

    return $retval;
}

For almost all methods I had to add pre and post do_actions passing in the ID and return value, which bloats the code base a bit too much for my liking.

Much like the transition method hook, it would be useful to have these actions automatically generated for all the main CRUD methods. Ideally it should be possible to override that prefix too if you are building an add-on for another plugin (like EDD 3.0). Maybe this override could be defined in the query class schema or somewhere similar.

robincornett commented 1 year ago

This is something we've discussed wanting for EDD, specifically when we introduced hooks for adding, updating, and deleting order items (here's what we did for adding).

On the one hand, those are hooks that can get triggered frequently and often--technically every time an object is changed somehow. On the other hand, there are a lot of different ways, times, and places to interact with [order] data, and trying to make sure that one thing happens consistently (or potentially happens) with every change requires a hook at this level.

So we've done hooks like edd_order_item_added, which in Berlin would, I think, be something along these lines:

$this->apply_prefix( $this->item_name ) . '_added'

which would ideally be tucked right before the $item_id is returned.

arraypress commented 1 year ago

@robincornett,

I actually use BerlinDB in a lot of my own personal projects and built out an extension of the query class that adds CRUD actions/filters to almost all the methods. Here is what it looks like:

class Query_Base extends Query {

  public function add_item( $data = array() ) {

    // Pre add database item action
    do_action( $this->apply_prefix( "pre_add_{$this->item_name}" ), $data );

    $data = apply_filters( $this->apply_prefix( "add_{$this->item_name}_data" ), $data );

    $should_add_item = apply_filters( $this->apply_prefix( "should_add_{$this->item_name}" ), true, $data );

    if ( false === $should_add_item ) {
      return false;
    }

        // Do the query
    $result = parent::add_item( $data );

    // Post add database item action
    do_action( $this->apply_prefix( "post_add_{$this->item_name}" ), $result, $data );

        // Return result
    return $result;
  }

  public function update_item( $item_id = 0, $data = array() ) {

    do_action( $this->apply_prefix( "pre_update_{$this->item_name_plural}" ), $item_id, $data );

    $data = apply_filters( $this->apply_prefix( "update_{$this->item_name}_data" ), $data, $item_id );

        // Do the query
    $result = parent::update_item( $item_id, $data );

    do_action( $this->apply_prefix( "post_update_{$this->item_name_plural}" ), $result, $item_id, $data );

    // Return result
    return $result;
  }

  public function delete_item( $item_id = 0 ) {

    do_action( $this->apply_prefix( "pre_delete_{$this->item_name_plural}" ), $item_id );

        // Do the query
    $result = parent::delete_item( $item_id );

    do_action( $this->apply_prefix( "post_delete_{$this->item_name_plural}" ), $result, $item_id );

    // Return result
    return $result;
  }

}

Maybe EDD could do it a similar way? Keeps everything a lot more DRY imho.