OP-319 Added the ability to select package dimensions #28

tomkalon · 2024-08-06T13:26:26Z

Added the ability to select package dimensions

Changing the dimensions of the package is possible until the export shipment is sent.

# Conflicts: # src/Api/WebClient.php # src/EventListener/ShippingExportEventListener.php

senghe · 2024-08-13T08:24:47Z

features/changing_shipping_method_parcel_template.feature

+  Scenario: Seeing shipments to export
+    When I go to the shipping export page
+    Then I should see 1 shipments with "New" state
+    When I select parcel template
+    Then I should see that shipping export parcel template is set


This seem to be two scenarios. I can approve this, as the scenario is not so complex, but the order of keywords should be at most: Given/When/Then and after then cannot be next Given/When.

Ok 👍
80cdcfb

senghe · 2024-08-13T08:25:43Z

spec/EventListener/SelectParcelTemplateEventListenerSpec.php

+    /**
+     * @throws \Exception
+     */


We do not put @throws annotation.

Suggested change

/**

* @throws \Exception

*/

OK 👍
e161c2e

senghe · 2024-08-13T08:26:58Z

src/Api/WebClient.php

+        ShipmentInterface $shipment,
+        ?ShippingExportInterface $shippingExport = null,
+    ): array {
+        $this->shippingExport = $shippingExport;


There is no need for putting this as a class property. You can put this into the dependent methods as a parameter.

Done 👍
Commit:
7c1d315#diff-111b44afb58dab47072d0539bea02c838d1bfad316e7d5733f26853abd28a92e

senghe · 2024-08-13T08:28:15Z

src/Api/WebClient.php

        $weight = $shipment->getShippingWeight();
+        $template = 'large';


I think we should put the value into container parameter, as it should be configurable.

Done 👍
Added ability to select default parcel_template and label_type to .env file.
7c1d315#diff-111b44afb58dab47072d0539bea02c838d1bfad316e7d5733f26853abd28a92e

senghe · 2024-08-13T08:28:34Z

src/Api/WebClientInterface.php

@@ -78,7 +79,7 @@ public function getLabels(array $shipmentIds): ?string;

    public function getAuthorizedHeaderWithContentType(): array;

-    public function createShipment(ShipmentInterface $shipment): array;
+    public function createShipment(ShipmentInterface $shipment, ?ShippingExportInterface $shippingExport = null): array;


Why there can be null?

Changed 👍
I took too timid an approach.
7c1d315#diff-111b44afb58dab47072d0539bea02c838d1bfad316e7d5733f26853abd28a92e

senghe · 2024-08-13T08:31:07Z

src/Resources/config/grid/order.yml

-                route: sylius_shop_order_show
-                parameters:
-                  tokenValue: resource.tokenValue
+    templates:


What's the reason of so many lines changed? I can't read what exactly was the logic change...

Ctrl+ALt+L and changed for 4 spaces as tab.
My fault. I will remember for the future.

senghe · 2024-08-13T08:31:33Z

src/Resources/config/routes.yml

+    path: /admin/shipping_export/select_parcel_template/{id}/{template}
+    methods: [POST, PUT]
+    defaults:
+        _controller: bitbag.controller.shipping_export::selectParcelTemplate


I could create separate controller for this with __invoke() method.

senghe · 2024-08-13T08:35:14Z

tests/Application/config/packages/bitbag_sylius_shipping_export_plugin.yaml

+            classes:
+                model: Tests\BitBag\SyliusInPostPlugin\Application\src\Entity\ShippingExport
+                controller: BitBag\SyliusInPostPlugin\Controller\ShippingExportController


Is this really needed? Do you override anything in ShippingExportController::exportSingleShipmentAction() or ShippingExportController::exportAllNewShipmentsAction() ? If yes, couldn't it be changed in any other place in the business logic?

The use of the ResourceController cannot be eliminated, hence the controller override has been moved from the plugin level, to the Sylius-Stanrdard level. Installation.md and tests/Application have been completed.

ebf8ae9

senghe · 2024-08-13T08:37:05Z

For CR purposes I've changed base branch to feature/OP-374 as it's the PR dependency. The current PR can be merged only when feature/OP-374 is merged.
After the CR I've reverted back the base branch to main.

senghe

I'm approving in advance. Please do the thing I asked you with ResourceController and you're free 🖖

senghe · 2024-08-26T10:12:49Z

doc/installation.md

+use Symfony\Component\HttpFoundation\Session\Flash\FlashBagInterface;
+use Webmozart\Assert\Assert;
+
+final class ShippingExportController extends ResourceController


I would extend namespace BitBag\SyliusShippingExportPlugin\Controller\ShippingExportController and remove the public methods from the listing.

I can't extend ShippingExportController because it is a final class.

senghe · 2024-08-26T10:13:22Z

doc/installation.md

+        shipping_method:
+            classes:
+                model: App\Entity\ShippingMethod
+


Suggested change

Fixed... and few other similar more.

senghe · 2024-08-26T10:14:46Z

doc/installation.md

+                admin:
+                    json_manifest_path: '%kernel.project_dir%/public/build/admin/manifest.json'
+                shop:
+                    json_manifest_path: '%kernel.project_dir%/public/build/shop/manifest.json'
+                app.admin:
+                    json_manifest_path: '%kernel.project_dir%/public/build/app/admin/manifest.json'
+                app.shop:
+                    json_manifest_path: '%kernel.project_dir%/public/build/app/shop/manifest.json'


Suggested change

admin:

json_manifest_path: '%kernel.project_dir%/public/build/admin/manifest.json'

shop:

json_manifest_path: '%kernel.project_dir%/public/build/shop/manifest.json'

app.admin:

json_manifest_path: '%kernel.project_dir%/public/build/app/admin/manifest.json'

app.shop:

json_manifest_path: '%kernel.project_dir%/public/build/app/shop/manifest.json'

// ...

senghe · 2024-08-26T10:15:05Z

doc/installation.md

+            admin: '%kernel.project_dir%/public/build/admin'
+            shop: '%kernel.project_dir%/public/build/shop'
+            app.admin: '%kernel.project_dir%/public/build/app/admin'
+            app.shop: '%kernel.project_dir%/public/build/app/shop'


Suggested change

admin: '%kernel.project_dir%/public/build/admin'

shop: '%kernel.project_dir%/public/build/shop'

app.admin: '%kernel.project_dir%/public/build/app/admin'

app.shop: '%kernel.project_dir%/public/build/app/shop'

// ...

senghe · 2024-08-26T10:15:58Z

spec/Api/WebClientSpec.php

+
+

Suggested change

senghe · 2024-08-26T10:18:56Z

tests/Application/src/Controller/ShippingExportController.php

+use Symfony\Component\HttpFoundation\Session\Flash\FlashBagInterface;
+use Webmozart\Assert\Assert;
+
+final class ShippingExportController extends ResourceController


Please do this as I asked you in README.md comment

I can't extend ShippingExportController because it is a final class.

tomkalon added 4 commits August 6, 2024 08:28

OP-319 - Extended shipping export by a parameter parcel_template

bb6a56e

OP-319 - Added select parcel template column

c74bc5f

Merge branch 'refs/heads/feature/OP-374' into OP-319

dc08656

# Conflicts: # src/Api/WebClient.php # src/EventListener/ShippingExportEventListener.php

OP-319 - ECS fixes

fa04c2e

tomkalon force-pushed the OP-319 branch 5 times, most recently from b97d4a0 to 73101a2 Compare August 6, 2024 13:56

OP-319 - Select parcel template fixes

36bb663

tomkalon force-pushed the OP-319 branch from 73101a2 to 36bb663 Compare August 6, 2024 13:57

tomkalon added 3 commits August 7, 2024 14:02

OP-319 - Added PHPSpec tests

3cab4af

OP-319 - Extension of parcel template with courier services

dabaaca

OP-319 - Added select parcel Behat test

2ec47d1

tomkalon force-pushed the OP-319 branch from 4622111 to 2ec47d1 Compare August 8, 2024 08:58

OP-319 - Shipping-export fixes

a70f537

tomkalon force-pushed the OP-319 branch 4 times, most recently from 972f3ff to a547dff Compare August 8, 2024 11:11

tomkalon added 2 commits August 8, 2024 13:13

OP-319 - installation.md

160a5a5

OP-319 - ShippingExportEventListener fix

297dc75

tomkalon force-pushed the OP-319 branch from a547dff to 297dc75 Compare August 8, 2024 11:29

This was referenced Aug 9, 2024

Feature/parcel template #24

Closed

OP-266 - Add parcel template #26

Closed

senghe changed the base branch from main to feature/OP-374 August 13, 2024 08:20

senghe requested changes Aug 13, 2024

View reviewed changes

senghe changed the base branch from feature/OP-374 to main August 13, 2024 08:35

OP-319 - PHPSpec: PHPDoc annotation has been removed

e161c2e

tomkalon force-pushed the OP-319 branch from c2a2561 to a317a37 Compare August 13, 2024 10:50

OP-319 - Default parcel template and label type configuration

7c1d315

tomkalon force-pushed the OP-319 branch from a317a37 to 7c1d315 Compare August 13, 2024 11:01

tomkalon added 3 commits August 13, 2024 13:07

OP-319 - Behat feature fixed

80cdcfb

OP-319 - The way the controller is overwritten has been changed

ebf8ae9

OP-319 - Lexical error has been fixed

0198a0e

senghe approved these changes Aug 26, 2024

View reviewed changes

OP-319 - installation.md and code review fixes

fe10a65

tomkalon force-pushed the OP-319 branch 3 times, most recently from c1c3d84 to fe10a65 Compare September 2, 2024 11:09

senghe merged commit 2047a12 into BitBagCommerce:main Sep 5, 2024
28 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OP-319 Added the ability to select package dimensions #28

OP-319 Added the ability to select package dimensions #28

tomkalon commented Aug 6, 2024

senghe Aug 13, 2024

tomkalon Aug 13, 2024 •

edited

Loading

senghe Aug 13, 2024

tomkalon Aug 13, 2024 •

edited

Loading

senghe Aug 13, 2024

tomkalon Aug 13, 2024 •

edited

Loading

senghe Aug 13, 2024

tomkalon Aug 13, 2024 •

edited

Loading

senghe Aug 13, 2024

tomkalon Aug 13, 2024 •

edited

Loading

senghe Aug 13, 2024

tomkalon Aug 13, 2024

senghe Aug 13, 2024

tomkalon Aug 13, 2024

senghe Aug 13, 2024

tomkalon Aug 13, 2024

senghe commented Aug 13, 2024

senghe left a comment

senghe Aug 26, 2024

tomkalon Aug 27, 2024

senghe Aug 26, 2024

tomkalon Aug 27, 2024

senghe Aug 26, 2024

tomkalon Aug 27, 2024

senghe Aug 26, 2024

tomkalon Aug 27, 2024

senghe Aug 26, 2024

tomkalon Aug 27, 2024

senghe Aug 26, 2024

tomkalon Aug 27, 2024

		$weight = $shipment->getShippingWeight();
		$template = 'large';

OP-319 Added the ability to select package dimensions #28

OP-319 Added the ability to select package dimensions #28

Conversation

tomkalon commented Aug 6, 2024

Choose a reason for hiding this comment

tomkalon Aug 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomkalon Aug 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomkalon Aug 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomkalon Aug 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomkalon Aug 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

senghe commented Aug 13, 2024

senghe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomkalon Aug 13, 2024 •

edited

Loading

tomkalon Aug 13, 2024 •

edited

Loading

tomkalon Aug 13, 2024 •

edited

Loading

tomkalon Aug 13, 2024 •

edited

Loading

tomkalon Aug 13, 2024 •

edited

Loading