Feature request for the addition of module hooks for insert, update, and delete of card data to allow other modules to react to these operations.

CommentFileSizeAuthor
#3 commerce_cardonfile-add-crud-hooks-1808166-3.patch3.64 KBrecrit
#1 commerce_cardonfile-add-crud-hooks-1808166-1.patch3.64 KBrecrit

Comments

recrit’s picture

recrit commented
Status: Active » Needs review
StatusFileSize
new commerce_cardonfile-add-crud-hooks-1808166-1.patch3.64 KB

The attached patch ...

  • adds module hooks for insert, update, delete
  • implements hook_hook_info for the new hooks
  • adds commerce_cardonfile.api.php to describe the provided hooks

In testing the new hooks, I ran into #1539344: Error in commerce_cardonfile_data_delete so this patch includes a fix for the delete query.

jpstrikesback’s picture

jpstrikesback commented

This is pretty cool but I believe the delete invocations should be placed before the db_delete i.e. before the db is changed, so that other modules can act before it is gone (even tho you pass the original $card_data). This seems to be the pattern in core (see hook_node_delete)

recrit’s picture

recrit commented
StatusFileSize
new commerce_cardonfile-add-crud-hooks-1808166-3.patch3.64 KB

nice catch... re-rolled patch

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

This makes sense, although I'm wondering if we shouldn't just make it a real entity, then we'd get the standard entity CRUD hooks for free. There is an issue for that at #1777352: Expose Card Data as a Drupal Entities.

* Hook documentation from this issue would still be useful, maybe it could be merged into the other one.

recrit’s picture

recrit commented

I agree with the entity route, however, it's an architectural change and would cause issues with contrib modules using card on file. Entity based could be the 2.x branch.

dwkitchen’s picture

dwkitchen commented
Status: Needs review » Closed (won't fix)

This has been done in 2.x by moving to entities.