Label print button - #60
Conversation
📝 WalkthroughWalkthroughThe plugin adds AJAX shipment creation and status handling, shared order-list label printing, processing-method-specific behavior, debug asset versioning, updated translations, and expanded automated test coverage. ChangesShipment and label workflow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant OrderList
participant PrintFlow
participant CreateShipments
participant SendyAPI
OrderList->>PrintFlow: Start label-print flow
PrintFlow->>CreateShipments: Request shipment statuses
CreateShipments->>SendyAPI: Create or retrieve shipment
SendyAPI-->>CreateShipments: Return shipment status and packages
CreateShipments-->>PrintFlow: Return statuses
PrintFlow->>OrderList: Print labels or reload
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (9)
tests/ShipmentsStatusTest.php (1)
198-210: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a test for the unauthenticated case.
fake_transport()always writes the three token options, so every test authenticates. The endpoint has no coverage for the state in whichsendy_access_tokenis empty. In that stateApiClientFactory::buildConnectionUsingTokens()throws\RuntimeException, whichshipment_status()does not catch. A test that deletes the token options and asserts a JSON response would lock the fix for the issue raised inlib/Modules/Orders/CreateShipments.php.Do you want me to write that test?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/ShipmentsStatusTest.php` around lines 198 - 210, Add an unauthenticated-case test alongside test_an_api_error_is_reported_as_failed_with_a_flash_notice by deleting the token options without calling fake_transport(), dispatching the shipment-status request, and asserting it returns the expected JSON response rather than propagating the RuntimeException from ApiClientFactory::buildConnectionUsingTokens().tests/doubles.php (2)
28-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGuard the
WC_Orderstub withclass_exists().The stub is declared unconditionally. If any future bootstrap loads WooCommerce, PHP fatals with a duplicate class declaration, and the failure points at this file rather than at the new test.
♻️ Proposed fix
-class WC_Order -{ -} +if (! class_exists('WC_Order')) { + class WC_Order + { + } +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/doubles.php` around lines 28 - 30, Guard the WC_Order stub declaration with class_exists('WC_Order') so it is only defined when WooCommerce has not already loaded the class. Preserve the existing empty stub behavior when the class is unavailable.
202-217: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
send()fails with an undefined offset when the queue is empty.
fake_transport()accepts zero responses.ShipmentsStatusTest::test_orders_without_a_shipment_or_that_do_not_resolve_are_reported_as_none()does not callfake_transport()at all, so today this path is not reached. If a later test constructs the transport without responses and the code under test makes one call,$this->responses[0]emits an undefined-offset warning and returnsnull, which violates theResponsereturn type and throws aTypeError.The condition also makes the last response repeat for every further call. Make both behaviours explicit.
💚 Proposed fix
public function send(Request $request): Response { $this->lastRequest = $request; $this->requests[] = $request; - if (count($this->responses) > 1) { - return array_shift($this->responses); - } - - return $this->responses[0]; + if ($this->responses === []) { + throw new LogicException('Sendy_Fake_Transport received an unexpected request; no response was queued.'); + } + + // The last queued response is reused, so polling loops do not need one + // response per call. + return count($this->responses) > 1 ? array_shift($this->responses) : $this->responses[0]; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/doubles.php` around lines 202 - 217, Update the response handling in send() so an empty responses queue is handled explicitly without accessing index 0 or violating the Response return type, and ensure the final configured response is not implicitly reused on subsequent calls. Preserve the existing request tracking while defining the intended behavior for both empty and exhausted response queues.lib/Modules/Orders/CreateShipments.php (1)
46-70: 🩺 Stability & Availability | 🔵 TrivialThe loop makes one synchronous API call per order with no bound.
Each iteration calls the Sendy API in the request thread. The orders list allows a large selection, so a single request can exceed
max_execution_timeor the gateway timeout. The client then sees a failed request although some shipments already exist, and a retry re-enters the loop for the remaining orders only because of the_sendy_shipment_idguard.Consider capping the number of orders per request and letting the frontend flow send batches. That also gives the user progress feedback.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/Modules/Orders/CreateShipments.php` around lines 46 - 70, Limit the order-processing loop in the shipment creation flow to a defined maximum number of orders per request, using the existing _sendy_shipment_id guard for already-created shipments. Update the frontend submission flow to send the selected orders in batches and report progress between requests, while preserving the existing error handling and created-order tracking in create_shipment processing.lib/Modules/Orders/OrdersModule.php (1)
63-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFully qualify the
@throwstype.The file does not import
GuzzleException, so both@throws GuzzleExceptiontags resolve toSendy\WooCommerce\Modules\Orders\GuzzleException, which does not exist. This is the cause of the two PHPStanthrows.notThrowableerrors at Line 65 and Line 121. Use the fully qualified name, asSingle::handle_create_shipment_from_form()already does.♻️ Proposed fix
- * `@throws` GuzzleException + * `@throws` \GuzzleHttp\Exception\GuzzleException */ protected function create_shipment(\WC_Order $order, string $shopId, string $preferenceId, int $amount): voidApply the same change to the tag above
create_shipment_from_order()at Line 119.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/Modules/Orders/OrdersModule.php` around lines 63 - 70, Update the `@throws` annotations above create_shipment() and create_shipment_from_order() to use the fully qualified Guzzle exception class name, matching Single::handle_create_shipment_from_form(), so PHPStan resolves both tags to a throwable type.Source: Linters/SAST tools
lib/Modules/Orders/BulkActions.php (1)
59-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign the
sendy_amountfallback with the AJAX endpoint.
lib/Modules/Orders/CreateShipments.phpdefaultsamountto'1'. Here the fallback is'', which casts to0. For the Sendy processing method the amount field is not rendered, so the value is missing and the cast produces0. The smart-rules path ignores the amount today, so no user-visible defect exists. Use'1'to keep both entry points consistent.♻️ Proposed change
- $amount = sanitize_key($_REQUEST['sendy_amount'] ?? ''); + $amount = sanitize_key($_REQUEST['sendy_amount'] ?? '1');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/Modules/Orders/BulkActions.php` at line 59, Update the sendy_amount fallback in the bulk-actions request handling to use '1' instead of an empty string, matching the amount default used by CreateShipments.php and preserving the expected value when the field is omitted.resources/js/admin-order-bulk.js (1)
86-112: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueAdd a rejection handler to the modal promise chain.
window.sendyOpenCreateShipmentsModal()builds its promise aroundtb_show(). Iftb_show()throws, the promise rejects and the failure surfaces only as an unhandled rejection. Add acatchso the failure is logged, which matches the logging thatresources/js/orders-list-print-flow.jsperforms inwindow.sendyOrdersListPrintFlow.♻️ Proposed change
bulkActionsForm.submit(); - } ); + } ) + .catch( ( error ) => + console.error( + 'Sendy: opening the create shipments modal failed', + error + ) + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/admin-order-bulk.js` around lines 86 - 112, Update the promise chain from window.sendyOpenCreateShipmentsModal in the bulk shipment flow to add a rejection handler after the existing then callback. Log the caught error using the same established logging pattern as window.sendyOrdersListPrintFlow, while preserving the current field handling and form submission behavior.resources/js/orders-list-print-flow.js (1)
55-65: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueGuard the nonce element lookup.
document.getElementById( 'sendy-print-labels-nonce' )returnsnullwhenprint_labels_nonce_field()did not render, for example when a third-party plugin alters the orders screen id. The property read then raises aTypeErrorthat only reaches the console. Read the value defensively and fail with a clear message.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/orders-list-print-flow.js` around lines 55 - 65, Update fetchStatuses to guard the sendy-print-labels-nonce element lookup before reading its value; when the element is absent, fail with a clear error message instead of dereferencing null, while preserving the existing AJAX behavior when the nonce is available.tests/SingleCreateShipmentTest.php (1)
80-114: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep
wp_send_json_success()outside the recoverytryblock.
WPAjaxDieContinueExceptionextendsException, so the current catch invokeswp_send_json_error()after the success response in AJAX tests. Limit thetryblock to validation and shipment work, then callwp_send_json_success()after the catch. The test helper can then usedispatch_ajax()without repairing output buffers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/SingleCreateShipmentTest.php` around lines 80 - 114, Update the AJAX handler invoked by dispatch so its try block only covers validation and shipment work; keep the catch handling failures, then move wp_send_json_success() after the catch and outside recovery. Preserve the successful response payload, allowing dispatch to use dispatch_ajax() without output-buffer repair.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/Modules/Orders/BulkActions.php`:
- Around line 61-63: Update the bulk loop around create_shipment() to resolve
each order first, skip the iteration when wc_get_order($id) returns false, and
catch SendyException per order so failures do not abort processing of subsequent
orders. Match the unresolved-order and per-order error handling used by
CreateShipments while preserving the existing shipment arguments and amount
conversion.
In `@lib/Modules/Orders/CreateShipments.php`:
- Around line 39-41: Clamp the requested shipment amount to at least one package
in both entry points: update lib/Modules/Orders/CreateShipments.php lines 39-41
to derive the amount with absint and max(1, ...), then pass that clamped value
to create_shipment() and remember_previously_used(); apply the same max(1,
absint(...)) handling in lib/Modules/Orders/Single.php lines 128-133, replacing
the current casted sanitize_key value.
- Around line 190-201: Update both shipment handlers, handle_shipments_status()
and handle_create_shipments(), to handle RuntimeException from
ApiClientFactory::buildConnectionUsingTokens() in addition to SendyException.
Catch \Exception (or validate the connection before proceeding) and return the
handlers’ expected JSON error response instructing the user to reconnect,
instead of allowing an HTML 500 response.
- Around line 205-210: Update the generated-status branch in CreateShipments so
it returns ready only when shipment['packages'] exists and is non-empty;
otherwise preserve the appropriate non-ready outcome and avoid persisting an
empty _sendy_packages value. Keep the existing metadata save and ready flow
unchanged for valid package data.
In `@resources/js/orders-list-print-flow.js`:
- Around line 32-44: Update the ajaxPost helper to parse non-2xx JSON responses,
extract and propagate the server-provided message when available, and retain a
sensible fallback for malformed responses. In window.sendyOrdersListPrintFlow,
surface the propagated error message to the user before the existing console
reporting, or reload through the existing notice flow so failures such as
expired nonces are explained.
In `@tests/AssetVersionTest.php`:
- Around line 23-27: Update the test method containing $expected and
Plugin::asset_version($path) to exercise debug and non-debug configurations
independently rather than deriving the expected value from the same condition.
Explicitly assert the file modification timestamp when SCRIPT_DEBUG is enabled
and Plugin::VERSION when it is disabled, restoring the original configuration
between cases.
---
Nitpick comments:
In `@lib/Modules/Orders/BulkActions.php`:
- Line 59: Update the sendy_amount fallback in the bulk-actions request handling
to use '1' instead of an empty string, matching the amount default used by
CreateShipments.php and preserving the expected value when the field is omitted.
In `@lib/Modules/Orders/CreateShipments.php`:
- Around line 46-70: Limit the order-processing loop in the shipment creation
flow to a defined maximum number of orders per request, using the existing
_sendy_shipment_id guard for already-created shipments. Update the frontend
submission flow to send the selected orders in batches and report progress
between requests, while preserving the existing error handling and created-order
tracking in create_shipment processing.
In `@lib/Modules/Orders/OrdersModule.php`:
- Around line 63-70: Update the `@throws` annotations above create_shipment() and
create_shipment_from_order() to use the fully qualified Guzzle exception class
name, matching Single::handle_create_shipment_from_form(), so PHPStan resolves
both tags to a throwable type.
In `@resources/js/admin-order-bulk.js`:
- Around line 86-112: Update the promise chain from
window.sendyOpenCreateShipmentsModal in the bulk shipment flow to add a
rejection handler after the existing then callback. Log the caught error using
the same established logging pattern as window.sendyOrdersListPrintFlow, while
preserving the current field handling and form submission behavior.
In `@resources/js/orders-list-print-flow.js`:
- Around line 55-65: Update fetchStatuses to guard the sendy-print-labels-nonce
element lookup before reading its value; when the element is absent, fail with a
clear error message instead of dereferencing null, while preserving the existing
AJAX behavior when the nonce is available.
In `@tests/doubles.php`:
- Around line 28-30: Guard the WC_Order stub declaration with
class_exists('WC_Order') so it is only defined when WooCommerce has not already
loaded the class. Preserve the existing empty stub behavior when the class is
unavailable.
- Around line 202-217: Update the response handling in send() so an empty
responses queue is handled explicitly without accessing index 0 or violating the
Response return type, and ensure the final configured response is not implicitly
reused on subsequent calls. Preserve the existing request tracking while
defining the intended behavior for both empty and exhausted response queues.
In `@tests/ShipmentsStatusTest.php`:
- Around line 198-210: Add an unauthenticated-case test alongside
test_an_api_error_is_reported_as_failed_with_a_flash_notice by deleting the
token options without calling fake_transport(), dispatching the shipment-status
request, and asserting it returns the expected JSON response rather than
propagating the RuntimeException from
ApiClientFactory::buildConnectionUsingTokens().
In `@tests/SingleCreateShipmentTest.php`:
- Around line 80-114: Update the AJAX handler invoked by dispatch so its try
block only covers validation and shipment work; keep the catch handling
failures, then move wp_send_json_success() after the catch and outside recovery.
Preserve the successful response payload, allowing dispatch to use
dispatch_ajax() without output-buffer repair.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2aa284e9-ae6c-4813-8708-2ec2f082928e
📒 Files selected for processing (28)
docker/provision.shlanguages/sendy-nl_NL.molanguages/sendy-nl_NL.polanguages/sendy.potlib/Modules/Admin/Settings.phplib/Modules/Checkout.phplib/Modules/Orders/BulkActions.phplib/Modules/Orders/CreateShipments.phplib/Modules/Orders/OrdersModule.phplib/Modules/Orders/PrintLabels.phplib/Modules/Orders/RowActions.phplib/Modules/Orders/Single.phplib/Plugin.phplib/Utils/BlocksIntegration.phpresources/css/order-actions.cssresources/js/admin-order-bulk.jsresources/js/orders-list-print-flow.jssendy.phptests/AssetVersionTest.phptests/CreateShipmentsTest.phptests/PrintFlowAssetsTest.phptests/PrintLabelsTest.phptests/RowActionsTest.phptests/Sendy_Ajax_TestCase.phptests/ShipmentsStatusTest.phptests/SingleCreateShipmentTest.phptests/bootstrap.phptests/doubles.php
| foreach ($objectIds as $id) { | ||
| if (get_option('sendy_processing_method') === ProcessingMethod::WooCommerce) { | ||
| $this->create_shipment_from_order( | ||
| wc_get_order($id), | ||
| sanitize_key($_REQUEST['sendy_preference_id'] ?? ''), | ||
| sanitize_key($_REQUEST['sendy_shop_id'] ?? ''), | ||
| sanitize_key($_REQUEST['sendy_amount'] ?? ''), | ||
| ); | ||
|
|
||
| update_option('sendy_previously_used_preference_id', sanitize_key($_REQUEST['sendy_preference_id'] ?? '')); | ||
| update_option('sendy_previously_used_amount', sanitize_key($_REQUEST['sendy_amount'] ?? '')); | ||
| } else { | ||
| $this->create_shipment_with_smart_rules( | ||
| wc_get_order($id), | ||
| false, | ||
| sanitize_key($_REQUEST['sendy_shop_id'] ?? ''), | ||
| ); | ||
| } | ||
| $this->create_shipment(wc_get_order($id), $shopId, $preferenceId, (int) $amount); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Guard the resolved order and contain per-order failures in the bulk loop.
wc_get_order($id) returns false when an id does not resolve. create_shipment() declares \WC_Order $order (see lib/Modules/Orders/OrdersModule.php:65), so a false value raises a TypeError and the whole bulk request fails. The AJAX sibling handles this: lib/Modules/Orders/CreateShipments.php skips unresolved orders and catches SendyException per order. Apply the same handling here, otherwise one bad id or one API error aborts the remaining orders without a notice.
🛡️ Proposed fix
foreach ($objectIds as $id) {
- $this->create_shipment(wc_get_order($id), $shopId, $preferenceId, (int) $amount);
+ $order = wc_get_order($id);
+
+ if (! $order) {
+ continue;
+ }
+
+ try {
+ $this->create_shipment($order, $shopId, $preferenceId, (int) $amount);
+ } catch (SendyException $exception) {
+ // translators: %1$s contains the ID of the order, %2$s the error message
+ sendy_flash_admin_notice('error', sprintf(
+ __('Error while creating shipment for order #%1$s: %2$s', 'sendy'),
+ $order->get_id(),
+ $exception->getMessage(),
+ ));
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| foreach ($objectIds as $id) { | |
| if (get_option('sendy_processing_method') === ProcessingMethod::WooCommerce) { | |
| $this->create_shipment_from_order( | |
| wc_get_order($id), | |
| sanitize_key($_REQUEST['sendy_preference_id'] ?? ''), | |
| sanitize_key($_REQUEST['sendy_shop_id'] ?? ''), | |
| sanitize_key($_REQUEST['sendy_amount'] ?? ''), | |
| ); | |
| update_option('sendy_previously_used_preference_id', sanitize_key($_REQUEST['sendy_preference_id'] ?? '')); | |
| update_option('sendy_previously_used_amount', sanitize_key($_REQUEST['sendy_amount'] ?? '')); | |
| } else { | |
| $this->create_shipment_with_smart_rules( | |
| wc_get_order($id), | |
| false, | |
| sanitize_key($_REQUEST['sendy_shop_id'] ?? ''), | |
| ); | |
| } | |
| $this->create_shipment(wc_get_order($id), $shopId, $preferenceId, (int) $amount); | |
| } | |
| foreach ($objectIds as $id) { | |
| $order = wc_get_order($id); | |
| if (! $order) { | |
| continue; | |
| } | |
| try { | |
| $this->create_shipment($order, $shopId, $preferenceId, (int) $amount); | |
| } catch (SendyException $exception) { | |
| // translators: %1$s contains the ID of the order, %2$s the error message | |
| sendy_flash_admin_notice('error', sprintf( | |
| __('Error while creating shipment for order #%1$s: %2$s', 'sendy'), | |
| $order->get_id(), | |
| $exception->getMessage(), | |
| )); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/Modules/Orders/BulkActions.php` around lines 61 - 63, Update the bulk
loop around create_shipment() to resolve each order first, skip the iteration
when wc_get_order($id) returns false, and catch SendyException per order so
failures do not abort processing of subsequent orders. Match the
unresolved-order and per-order error handling used by CreateShipments while
preserving the existing shipment arguments and amount conversion.
| $shopId = sanitize_key($_POST['shop_id'] ?? ''); | ||
| $preferenceId = sanitize_key($_POST['preference_id'] ?? ''); | ||
| $amount = sanitize_key($_POST['amount'] ?? '1'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
amount reaches the API unclamped in both entry points. Both call sites cast a sanitize_key() result to int without checking the range, so '', '0' and 'abc' all produce a request for zero packages.
lib/Modules/Orders/CreateShipments.php#L39-L41: replacesanitize_key($_POST['amount'] ?? '1')withmax(1, absint($_POST['amount'] ?? 1))and pass the clamped value tocreate_shipment()andremember_previously_used().lib/Modules/Orders/Single.php#L128-L133: replace(int) sanitize_key($_REQUEST['amount'] ?? '1')withmax(1, absint($_REQUEST['amount'] ?? 1)).
📍 Affects 2 files
lib/Modules/Orders/CreateShipments.php#L39-L41(this comment)lib/Modules/Orders/Single.php#L128-L133
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/Modules/Orders/CreateShipments.php` around lines 39 - 41, Clamp the
requested shipment amount to at least one package in both entry points: update
lib/Modules/Orders/CreateShipments.php lines 39-41 to derive the amount with
absint and max(1, ...), then pass that clamped value to create_shipment() and
remember_previously_used(); apply the same max(1, absint(...)) handling in
lib/Modules/Orders/Single.php lines 128-133, replacing the current casted
sanitize_key value.
| if ($status === 'generated') { | ||
| $order->update_meta_data('_sendy_packages', $shipment['packages'] ?? []); | ||
| $order->save(); | ||
|
|
||
| return 'ready'; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
generated without packages reports ready but stores nothing printable.
If the API reports the status as generated and omits packages, this branch persists an empty array and returns ready. That contradicts the ready definition at Line 186, which requires a non-empty packages array. The flow then tries to print an order for which no package data exists.
🐛 Proposed fix
- if ($status === 'generated') {
- $order->update_meta_data('_sendy_packages', $shipment['packages'] ?? []);
- $order->save();
-
- return 'ready';
- }
+ if ($status === 'generated' && ! empty($shipment['packages'])) {
+ $order->update_meta_data('_sendy_packages', $shipment['packages']);
+ $order->save();
+
+ return 'ready';
+ }
+
+ if ($status === 'generated') {
+ return 'pending';
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ($status === 'generated') { | |
| $order->update_meta_data('_sendy_packages', $shipment['packages'] ?? []); | |
| $order->save(); | |
| return 'ready'; | |
| } | |
| if ($status === 'generated' && ! empty($shipment['packages'])) { | |
| $order->update_meta_data('_sendy_packages', $shipment['packages']); | |
| $order->save(); | |
| return 'ready'; | |
| } | |
| if ($status === 'generated') { | |
| return 'pending'; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/Modules/Orders/CreateShipments.php` around lines 205 - 210, Update the
generated-status branch in CreateShipments so it returns ready only when
shipment['packages'] exists and is non-empty; otherwise preserve the appropriate
non-ready outcome and avoid persisting an empty _sendy_packages value. Keep the
existing metadata save and ready flow unchanged for valid package data.
Matches the minimum supported WordPress version of WooCommerce 8.2.
When creating a shipping label for a single order, the label in the modal now reads singular.
bc37e2e to
f2a11f8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@resources/js/orders-list-print-flow.js`:
- Line 38: Update the request flow around ajaxPost to create an AbortController,
pass its signal to fetch, and abort the request after a bounded timeout. Clear
the timeout in a finally block so both successful and failed requests release
the timer and the print button cannot remain disabled indefinitely.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2be89a4c-9af3-4a5f-97e6-28006cdcaa96
📒 Files selected for processing (17)
languages/sendy-nl_NL.molanguages/sendy-nl_NL.polanguages/sendy.potlib/Modules/Orders/BulkActions.phplib/Modules/Orders/CreateShipments.phplib/Modules/Orders/OrdersModule.phplib/Modules/Orders/RowActions.phplib/Modules/Orders/Single.phplib/Plugin.phpreadme.txtresources/js/orders-list-print-flow.jssendy.phptests/BulkActionsTest.phptests/CreateShipmentsTest.phptests/ShipmentsStatusTest.phptests/SingleCreateShipmentTest.phptests/doubles.php
🚧 Files skipped from review as they are similar to previous changes (12)
- sendy.php
- languages/sendy-nl_NL.po
- lib/Plugin.php
- languages/sendy.pot
- lib/Modules/Orders/BulkActions.php
- lib/Modules/Orders/Single.php
- lib/Modules/Orders/RowActions.php
- lib/Modules/Orders/CreateShipments.php
- tests/ShipmentsStatusTest.php
- lib/Modules/Orders/OrdersModule.php
- tests/doubles.php
- tests/CreateShipmentsTest.php
| body.append( 'order_ids[]', orderId ); | ||
| } | ||
|
|
||
| const response = await fetch( ajaxurl, { method: 'POST', body } ); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the polling path and locate existing print-flow test coverage.
rg -n -C 5 'POLL_TIMEOUT|fetch\(|ajaxPost|fetchStatuses|waitUntilReady' \
resources/js/orders-list-print-flow.js
fd -t f . | rg 'PrintFlow|orders-list-print-flow|jest|vitest'Repository: sendynl/woocommerce-plugin
Length of output: 4237
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- orders-list-print-flow.js ---'
cat -n resources/js/orders-list-print-flow.js | sed -n '1,225p'
printf '%s\n' '--- browser targets and script loading ---'
rg -n -i -C 3 'browserslist|browser.?support|polyfill|orders-list-print-flow|wp_enqueue_script|script_loader_tag' \
package.json composer.json webpack* vite* resources includes src tests 2>/dev/null || true
printf '%s\n' '--- AJAX error and button cleanup call sites ---'
rg -n -C 5 'class AjaxError|AjaxError|runPrintFlow|finally|disabled' resources/js testsRepository: sendynl/woocommerce-plugin
Length of output: 23057
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- existing timeout pattern ---'
cat -n resources/js/print-labels.js | sed -n '1,45p'
printf '%s\n' '--- print-flow completion path ---'
cat -n resources/js/orders-list-print-flow.js | sed -n '225,365p'
printf '%s\n' '--- browser and compatibility configuration ---'
git ls-files | rg '(^|/)(package(-lock)?\.json|\.browserslistrc|babel|webpack|vite|\.github/workflows|readme|README)'
rg -n -i -C 3 'AbortController|browserslist|browser support|polyfill|compatib|wp_enqueue_script' \
. --glob '!vendor/**' --glob '!node_modules/**' --glob '!*.min.js' 2>/dev/null || trueRepository: sendynl/woocommerce-plugin
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- package and webpack configuration ---'
cat package.json
cat webpack.config.js
printf '%s\n' '--- order-list script registration ---'
cat -n lib/Modules/Orders/BulkActions.php | sed -n '85,135p'
cat -n lib/Modules/Orders/Single.php | sed -n '80,110p'
printf '%s\n' '--- standalone timeout probe ---'
node <<'JS'
const POLL_TIMEOUT = 30;
let abortCount = 0;
let cleared = false;
const controller = new AbortController();
const timeoutId = setTimeout(() => {
abortCount += 1;
controller.abort();
}, POLL_TIMEOUT);
const pendingFetch = new Promise((resolve, reject) => {
controller.signal.addEventListener('abort', () => {
reject(new DOMException('The operation was aborted.', 'AbortError'));
});
});
pendingFetch.finally(() => {
clearTimeout(timeoutId);
cleared = true;
}).catch((error) => {
console.log(JSON.stringify({
name: error.name,
abortCount,
cleared,
}));
});
JSRepository: sendynl/woocommerce-plugin
Length of output: 5490
Add a timeout to ajaxPost.
Pass an AbortSignal to fetch() and abort it after a bounded interval. Clear the timer in finally so a stalled request cannot keep the print button disabled indefinitely.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@resources/js/orders-list-print-flow.js` at line 38, Update the request flow
around ajaxPost to create an AbortController, pass its signal to fetch, and
abort the request after a bounded timeout. Clear the timeout in a finally block
so both successful and failed requests release the timer and the print button
cannot remain disabled indefinitely.
1 participant
Add a per-order label print button on the order overview
Summary by CodeRabbit
New Features
Improvements