Woocommerce send unique pdf

Question

I have developed a code to send tickets (PDF files) of a specific product in the order confirmation. I place the PDFs in a folder (i.e. Tickets) on the server after the purchase is completed. The code retrieves the corresponding number of PDFs based on the quantity of the item, sends them in the confirmation email, and then moves them to another folder (i.e. UsedTickets).

Is the code well-written? Are there any issues if there are multiple purchases happening simultaneously?

add_filter( 'woocommerce_email_attachments', 'add_pdf_attachment_to_wc_email', 10, 3 );
function add_pdf_attachment_to_wc_email( $attachments, $email_id, $order ) {
 // Check if the email is 'customer_completed_order' and if the product in the cart is 8059.
 if ( $email_id === 'customer_completed_order' && $order && $order->get_item_count() ) {
 $pdf_files = glob( get_template_directory() . '/Tickets/*.pdf' );
 $pdf_count = 0;
 $pdf_attachments = array(); // Creating an empty list for the names of the PDF files that will be attached.
 foreach ( $order->get_items() as $item ) {
 if ( $item->get_product_id() === 8059 ) {
 $pdf_count += $item->get_quantity();
 for ( $i = 0; $i < $item->get_quantity(); $i++ ) {
 $pdf_path = $pdf_files[ $pdf_count - $item->get_quantity() + $i % count( $pdf_files ) ];
 $pdf_attachments[] = $pdf_path; // Adding the PDF file to the list of attachments.
 }
 }
 }
 // Moving the PDF files to the 'UsedTickets' folder.
 foreach ( $pdf_attachments as $pdf_path ) {
 $pdf_filename = basename( $pdf_path );
 $new_path = get_template_directory() . '/UsedTickets/' . $pdf_filename;
 if ( rename( $pdf_path, $new_path ) ) {
 error_log( 'PDF successfully moved to UsedTickets folder.: ' . $pdf_filename );
 } else {
 error_log( 'An error occurred while moving the PDF to the UsedTickets folder: ' . $pdf_filename );
 }
 $attachments[] = $new_path; // Adding the PDF file to the email attachments.
 }
 }
 return $attachments; // Returning the updated list of attachments.
}

Answer 1

function add_pdf_attachment_to_wc_email( $attachments, $email_id, $order ) {

This function could have a better name. It does not exactly operate on a woocommerce email, and there's a singular / plural distinction I'm getting hung up on. Consider spelling it add_pdfs_to_email_attachments to clarify that the 1st argument shall be side effected.

---

 if ( $email_id === 'customer_completed_order' && $order && $order->get_item_count() ) {

Surely that third guard clause is superfluous, no? Imagine the count is zero. Iterating through zero items will be harmless.

If we do not get past this if, say because of ID mismatch, should we maybe throw or log an error to notify caller that he called us in vain? Or perhaps the function documentation should clarify that adding zero or more PDFs is fine.

---

 if ( $item->get_product_id() === 8059 ) {

Please give a symbolic name to this magic number.

---

$pdf_count - $item->get_quantity() + $i % count( $pdf_files )

That's a complex expression. It deserves a unit test, perhaps a helper method, or minimally a // comment.

The invariants maintained by your several state variables are not self-documented by just the identifier names.

Breaking out a function get_pdf_attachments ... helper would be useful.

Since you didn't pass in GLOB_NOSORT, glob() promises that "the pathnames are sorted alphabetically". This expression appears to rely on that.

any issues if there are multiple purchases happening simultaneously?

I am worried that glob looping over multiple '/Tickets/*.pdf' files can interact badly with the subsequent foreach & for loops when multiple customers submit orders at about the same time. Consider adding automated stress tests to your test suite which examine this race. To make the race easier to see, during testing insert a sleep() of a few seconds right after the glob, giving 2nd customer a better chance to click submit and overlap with 1st customer processing.

Presumably the *.pdf filenames include a GUID or other uniquifying element, so they can't collide. Document the expected format of filenames in the code.

It's unclear why the rename() would ever fail, but I'm sure you had a reason for handling that case.

---

This code appears to achieve its design goals.

With improved documentation and testing I would be willing to delegate or accept maintenance tasks on it.