Support gift cards in admin order refunds template

[xristos3490]

Closes #31413

Specification

Please check the issue description for additional details.

Changelog

Dev - Added woocommerce_admin_order_should_render_refunds hook to allow control over the refunds UI within the order editor.

[manospsyx]

@woocommerce/proton see pdqkbG-6O-p2#comment-173

[barryhughes]

Where possible we should avoid placing filter hooks inside templates and we should also document any new hooks we add.

I realize the duplication of the logic predates your change, and you were just wrapping it in a filter, but this is actually a pretty nice opportunity to clean it up. We could for example place the hook inside WC_Meta_Box_Order_Items::output() (adding a docblock to explain what it does) and then pass the result in here as a variable.

[xristos3490]

Thanks for the review, @barryhughes! Sounds reasonable!

[barryhughes]

Thanks for submitting this: seems reasonable but a few small changes are needed.

[xristos3490]

Tests are failing.

We could for example place the hook inside WC_Meta_Box_Order_Items::output() (adding a docblock to explain what it does) and then pass the result in here as a variable.

Adding a filter into the WC_Meta_Box_Order_Items::output() function creates the need to include the same filter into multiple WC_AJAX endpoints due to how each AJAX admin handler renders the order items table. e.g., WC_AJAX::maybe_add_order_item() Line 973 and others.

Instead of writing the same filter in multiple places, we could:

• Refactor the way we render the html-order-items.php in all accuracies by using a global render function, for example, in the WC_Meta_Box_Order_Items class, which will not rely on global variables or;
• Leave the filter inline inside the template as is.

Moreover, I think introducing a new variable inside the html-order-items.php, or a dedicated render method for that matter, could potentially break 3rd party extensions using this template to include additional functionality in the order items table.

Another way could be to introduce a global helper function, e.g., woocommerce_admin_should_render_refunds( $order ) that will have this filter in its return statement -- this should play well along with 3rd party code.

In my opinion, and because the template in question is widely used, I would opt for leaving the filter inline. Although, I understand your point that this filter would be used twice in the same template, which is not ideal.

Thoughts?

[barryhughes]

You're right, it doesn't seem viable in this case and there isn't a particularly clean way to tidy this up without a larger body of work.

Moreover, I think introducing a new variable inside the html-order-items.php, or a dedicated render method for that matter, could potentially break 3rd party extensions using this template to include additional functionality in the order items table.
...
In my opinion, and because the template in question is widely used, I would opt for leaving the filter inline.

It still seems a shame to repeat it unnecessarily by inlining it. Given that all of the other data available to the template remains the same, do you see difficulties if we do something along these lines (ie, place the filter once at the top of the template):

<?php
/**
 * Order items HTML for meta box.
 *
 * @package WooCommerce\Admin
 */

/**
 * Docblock for the new filter.
 */
$render_refunds = (bool) apply_filters( 'woocommerce_admin_order_should_render_refunds', 0 < $order->get_total() - $order->get_total_refunded() || 0 < absint( $order->get_item_count() - $order->get_item_count_refunded() ), $order->get_id(), $order );
?>

<!-- Other template code -->

<?php if ( $render_refunds ) : ?>
    <!-- Refund HTML -->
<?php endif; ?>

<!-- Other template code -->

[xristos3490]

It still seems a shame to repeat it unnecessarily by inlining it.

I feel the same way!

do you see difficulties if we do something along these lines

No. Placing the filter once at the top of the template seems like a good idea at this point!

[barryhughes]

Excellent, thanks. I tweaked the docblock a little (I believe this will actually ship in 6.4) and once tests pass I'll go ahead and merge. Thanks for the contribution :-)

[github-actions]

Hi @barryhughes, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:

• Add the status: needs changelog label
• Add the status: needs testing instructions label

[xristos3490]

Thanks, @barryhughes! 🙇
I'll keep an eye on this to pass this filter/fix into the extension!