Adds an endpoint to get a list of countries, and provinces by country #722

medteck · 2021-12-19T13:30:11Z

Resolves #519

… for provinces by country

src/Factory/Country/CountryViewFactory.php

src/Controller/Country/ShowCountryProvincesAction.php

src/View/Country/Province/ProvinceView.php

tests/Controller/Country/ShowCountryProvincesApiTest.php

src/Resources/config/routing/country.yml

mamazu · 2021-12-20T10:50:35Z

First of all thank you for your contribution in adding those endpoints. As I see the endpoints you added provide a list of countries and a list of provinces by country code.

In this case I would suggest to not use a custom controller but rather use the default Sylius controller like sylius.controller.country and sylius.controller.province with the right repository methods. And if we need to customize the output then we can just change it in the serializer. This way we don't need to write any custom Controller/Factory or View classes.

If you want to have a look where this is already in use then have a look at the address_book configuration file:
https://github.com/Sylius/ShopApiPlugin/blob/master/src/Resources/config/routing/address_book.yml

medteck · 2021-12-21T12:16:17Z

First of all thank you for your contribution in adding those endpoints. As I see the endpoints you added provide a list of countries and a list of provinces by country code.

In this case I would suggest to not use a custom controller but rather use the default Sylius controller like sylius.controller.country and sylius.controller.province with the right repository methods. And if we need to customize the output then we can just change it in the serializer. This way we don't need to write any custom Controller/Factory or View classes.

If you want to have a look where this is already in use then have a look at the address_book configuration file: https://github.com/Sylius/ShopApiPlugin/blob/master/src/Resources/config/routing/address_book.yml

Thanks for your comment!
I was looking into the default Sylius controllers and they don't seem to provide Actions that actually return lists of countries nor provinces.
I think we will have to use custom controllers finally.

diimpp · 2021-12-21T20:38:47Z

I was looking into the default Sylius controllers and they don't seem to provide Actions that actually return lists of countries nor provinces.

Controllers for both countries and provinces are available as services sylius.controller.country and sylius.controller.provice. indexAction will return list of underline resources.
That's done automatically for each entity registered as resource. Take a look at https://docs.sylius.com/en/1.11/book/architecture/resource_layer.html

medteck · 2021-12-21T21:17:12Z

I was looking into the default Sylius controllers and they don't seem to provide Actions that actually return lists of countries nor provinces.

Controllers for both countries and provinces are available as services sylius.controller.country and sylius.controller.provice. indexAction will return list of underline resources. That's done automatically for each entity registered as resource. Take a look at https://docs.sylius.com/en/1.11/book/architecture/resource_layer.html

You're right! Thanks, I know better now. I just looked into it and tested using their controllers instead of mines.

However, I don't think sylius.controller.country:indexAction is adapted in most situations because :

The list of countries is too heavy (Every details like list of provinces by country is included)
It's paginated, while the list of countries is used on the frontend to display a list of countries. When a country is selected, a list of provinces is generated, depending to the selected country.

But... sylius.controller.country:showAction can be used as it displays the country data as well as its list of provinces, thus the data structure is similar to what we have in other endpoints.

medteck · 2021-12-21T22:24:02Z

After some more investigation, I think we should use a custom Controller instead of sylius.controller.country:showAction as well since that default controller does not return the country name attribute.

mamazu · 2021-12-30T02:53:44Z

So if I understand you correctly you have the problem that one endpoint has "too much" info and the other has too little. I think those things both can be relegated with some serialization information. This can be defined in the yaml files, see the Address and Addressbook for reference.

The other issue is that the endpoint is paginated. But that is fixed easily. In the route definition of Sylius Resources you can disable it: https://github.com/Sylius/SyliusResourceBundle/blob/0bb5ab5dd36edc2e6402d22f0431f2d600efe488/docs/index_resources.md#disabling-pagination---getting-a-simple-collection

medteck · 2022-01-04T12:24:52Z

Thank you @mamazu!
Well I was working on it and I made it work using only the Sylius default controllers. But there's just one issue :
The serialization doesn't seem to support translatable attributes. As a result, I'm not getting the Country's name because it doesn't have a "name" property.

This is the very last thing blocking me. Do you have any idea about how to fix this ?

diimpp · 2022-01-04T12:52:52Z

Just off the top of my head serialization should has option to specify getter method to get your property and if it won't work, then there is virtual method option as well.

…t list of countries + country details by code

This reverts commit fba081f

…endpoint for provinces by country" This reverts commit c4b2c4e

medteck · 2022-01-04T19:54:30Z

@mamazu There was no way to specify a getter but it worked like a charm with virtual methods.

The PR is now ready again for review.

mamazu

Looks good. Let's see if the pipeline says.

mamazu · 2022-01-04T20:14:23Z

Thanks you two for the combined effort. This solution looks much cleaner (no code is fewer things that can break). I am not sure about when the pipeline is going to run, but when they are all green I'll merge it.

…m/medteck/ShopApiPlugin

lchrusciel · 2022-01-13T14:53:42Z

Hey folks! Lack of the builds was slightly my mistake. It is good now 👍

lchrusciel

@medteck my congratulation for your first contribution 👍 I hope it won't be last.

Also, I need to 👏 for the cooperation that happen over this PR :)

tests/Controller/Checkout/CountriesApiTest.php

lchrusciel · 2022-01-13T14:55:38Z

tests/Controller/Checkout/CountriesApiTest.php

+
+
+namespace Tests\Sylius\ShopApiPlugin\Controller\Checkout;
+


missing license block

tests/Controller/Checkout/CountriesApiTest.php

Removes unnecessary line Co-authored-by: Łukasz Chruściel <lchrusciel@gmail.com>

Makes CountriesApiTest final Co-authored-by: Łukasz Chruściel <lchrusciel@gmail.com>

mamazu · 2022-02-02T20:15:28Z

So the only thing that is missing is the license block. I think that this should be part of the coding style configuration of this project so that it is the same license everywhere.

mamazu · 2022-02-18T21:17:30Z

I have added the license block to the master branch. Just rebase onto it and then run the codestyle (vendor/bin/ecs check --fix src/) this should add the license block to it. Then we can merge it.

medteck · 2022-03-01T22:29:35Z

I tried to add the license block with vendor/bin/ecs check --fix src/ as you suggested but it did not work. I tried switching between php8.0 and php7.4 but still.

Here's my output :

[WARNING] You are using YAML format in "easy-coding-standard.yml" config.                                              
           It is deprecated and will be removed in next ECS 9. Switch to PHP format as soon as possible with            
           "https://github.com/migrify/config-transformer"                                                              
                                                                                                                        

PHP Notice:  Undefined index: serviceKeywords in /media/medteck/Hard-drive/OpenSource/ShopApiPlugin/vendor/symplify/easy-coding-standard/src/Yaml/CheckerServiceParametersShifter.php on line 65
PHP Stack trace:
PHP   1. {main}() /media/medteck/Hard-drive/OpenSource/ShopApiPlugin/vendor/bin/ecs:0
PHP   2. include() /media/medteck/Hard-drive/OpenSource/ShopApiPlugin/vendor/bin/ecs:92
PHP   3. Symplify\EasyCodingStandard\HttpKernel\EasyCodingStandardKernel->boot() /media/medteck/Hard-drive/OpenSource/ShopApiPlugin/vendor/symplify/easy-coding-standard/bin/ecs:77
PHP   4. Symplify\EasyCodingStandard\HttpKernel\EasyCodingStandardKernel->preBoot() /media/medteck/Hard-drive/OpenSource/ShopApiPlugin/vendor/symfony/http-kernel/Kernel.php:128
PHP   5. Symplify\EasyCodingStandard\HttpKernel\EasyCodingStandardKernel->initializeContainer() /media/medteck/Hard-drive/OpenSource/ShopApiPlugin/vendor/symfony/http-kernel/Kernel.php:789
PHP   6. Symplify\EasyCodingStandard\HttpKernel\EasyCodingStandardKernel->buildContainer() /media/medteck/Hard-drive/OpenSource/ShopApiPlugin/vendor/symfony/http-kernel/Kernel.php:547
PHP   7. Symplify\EasyCodingStandard\HttpKernel\EasyCodingStandardKernel->getContainerLoader() /media/medteck/Hard-drive/OpenSource/ShopApiPlugin/vendor/symfony/http-kernel/Kernel.php:651
PHP   8. Symplify\EasyCodingStandard\DependencyInjection\DelegatingLoaderFactory->createFromContainerBuilderAndKernel() /media/medteck/Hard-drive/OpenSource/ShopApiPlugin/vendor/symplify/easy-coding-standard/src/HttpKernel/EasyCodingStandardKernel.php:73
PHP   9. Symplify\EasyCodingStandard\DependencyInjection\DelegatingLoaderFactory->createFromContainerBuilderAndFileLocator() /media/medteck/Hard-drive/OpenSource/ShopApiPlugin/vendor/symplify/easy-coding-standard/src/DependencyInjection/DelegatingLoaderFactory.php:25
PHP  10. Symplify\EasyCodingStandard\Yaml\FileLoader\CheckerTolerantYamlFileLoader->__construct() /media/medteck/Hard-drive/OpenSource/ShopApiPlugin/vendor/symplify/easy-coding-standard/src/DependencyInjection/DelegatingLoaderFactory.php:48
PHP  11. Symplify\EasyCodingStandard\Yaml\CheckerServiceParametersShifter->__construct() /media/medteck/Hard-drive/OpenSource/ShopApiPlugin/vendor/symplify/easy-coding-standard/src/Yaml/FileLoader/CheckerTolerantYamlFileLoader.php:21

                                                                                                                        
 [ERROR] in_array() expects parameter 2 to be array, null given

Thus, I don't see any license block in other files.

mamazu · 2022-03-02T15:18:46Z

Yeah, the ECS package is broken. (Because the Yaml Interpreter it is based on changed). But what you can do is do the following:
Go to vendor/symplify/easy-coding-standard/src/Yaml/CheckerServiceParametersShifter.php to line 65 and replace the lines:

        $serviceKeywordsProperty = (new ReflectionClass(YamlFileLoader::class))
            ->getStaticProperties()['serviceKeywords'];

with

 $serviceKeywordsProperty = ['services'];

This should fix it as long as you don't update it. :D But in the end we need to migrate to php configuration for the ECS anyways.

medteck · 2022-03-05T14:29:27Z

@mamazu
New error:

[ERROR] Checker class "SlevomatCodingStandard\Sniffs\Commenting\ForbiddenAnnotationsSniff" in configuration was not    
         found in                                                                                                       
         /media/medteck/Hard-drive/OpenSource/ShopApiPlugin/vendor/sylius-labs/coding-standard/easy-coding-standard.yml 
         (which is being imported from "/media/medteck/Hard-drive/OpenSource/ShopApiPlugin/easy-coding-standard.yml").

mamazu · 2022-03-06T19:00:00Z

Are those the most recent dependencies, that's the only issue I have got. If they are can you maybe provide me with the composer.lock file so that I can reproduce the issue?

mamazu · 2022-06-29T20:09:34Z

Look like the pipeline is green now. I will merge it in the next few days.

medteck added 2 commits December 19, 2021 13:12

Adds an endpoint to get A list of all countries, and another endpoint…

c4b2c4e

… for provinces by country

Adds the expected response from for all countries request

fba081f

medteck requested a review from a team as a code owner December 19, 2021 13:30

diimpp requested changes Dec 20, 2021

View reviewed changes

wip: Replacing Country & Province Controller by Sylius Defaults

65ea84e

medteck added 3 commits January 4, 2022 15:10

Adds /checkout/countries & /checkout/countries/{code} endpoints to ge…

f1bc0c9

…t list of countries + country details by code

Revert "Adds the expected response from for all countries request"

3fe56e1

This reverts commit fba081f

Revert "Adds an endpoint to get A list of all countries, and another …

1a7c8bd

…endpoint for provinces by country" This reverts commit c4b2c4e

mamazu approved these changes Jan 4, 2022

View reviewed changes

Merge branch 'feat/countries-provinces-endpoint' of https://github.co…

274f9b7

…m/medteck/ShopApiPlugin

medteck force-pushed the master branch from 3a11248 to 274f9b7 Compare January 8, 2022 14:20

lchrusciel closed this Jan 13, 2022

lchrusciel reopened this Jan 13, 2022

lchrusciel approved these changes Jan 13, 2022

View reviewed changes

medteck and others added 4 commits January 13, 2022 18:34

Update tests/Controller/Checkout/CountriesApiTest.php

5490e96

Removes unnecessary line Co-authored-by: Łukasz Chruściel <lchrusciel@gmail.com>

Update tests/Controller/Checkout/CountriesApiTest.php

c65de23

Makes CountriesApiTest final Co-authored-by: Łukasz Chruściel <lchrusciel@gmail.com>

Adds strict types declaration to CountriesApiTest

486e3f6

Merge branch 'master' of github.com:medteck/ShopApiPlugin

474517a

Merge branch 'master' into master

6d3f018

Merge branch 'master' into master

b02d7a1

lchrusciel merged commit 6839630 into Sylius:master Jul 1, 2022

mamazu mentioned this pull request Jul 1, 2022

WIP: country route #562

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds an endpoint to get a list of countries, and provinces by country #722

Adds an endpoint to get a list of countries, and provinces by country #722

medteck commented Dec 19, 2021

mamazu commented Dec 20, 2021

medteck commented Dec 21, 2021

diimpp commented Dec 21, 2021 •

edited

Loading

medteck commented Dec 21, 2021 •

edited

Loading

medteck commented Dec 21, 2021

mamazu commented Dec 30, 2021

medteck commented Jan 4, 2022 •

edited

Loading

diimpp commented Jan 4, 2022

medteck commented Jan 4, 2022

mamazu left a comment

mamazu commented Jan 4, 2022

lchrusciel commented Jan 13, 2022

lchrusciel left a comment

lchrusciel Jan 13, 2022

mamazu commented Feb 2, 2022

mamazu commented Feb 18, 2022

medteck commented Mar 1, 2022 •

edited

Loading

mamazu commented Mar 2, 2022

medteck commented Mar 5, 2022

mamazu commented Mar 6, 2022

mamazu commented Jun 29, 2022



		namespace Tests\Sylius\ShopApiPlugin\Controller\Checkout;

Adds an endpoint to get a list of countries, and provinces by country #722

Adds an endpoint to get a list of countries, and provinces by country #722

Conversation

medteck commented Dec 19, 2021

mamazu commented Dec 20, 2021

medteck commented Dec 21, 2021

diimpp commented Dec 21, 2021 • edited Loading

medteck commented Dec 21, 2021 • edited Loading

medteck commented Dec 21, 2021

mamazu commented Dec 30, 2021

medteck commented Jan 4, 2022 • edited Loading

diimpp commented Jan 4, 2022

medteck commented Jan 4, 2022

mamazu left a comment

Choose a reason for hiding this comment

mamazu commented Jan 4, 2022

lchrusciel commented Jan 13, 2022

lchrusciel left a comment

Choose a reason for hiding this comment

lchrusciel Jan 13, 2022

Choose a reason for hiding this comment

mamazu commented Feb 2, 2022

mamazu commented Feb 18, 2022

medteck commented Mar 1, 2022 • edited Loading

mamazu commented Mar 2, 2022

medteck commented Mar 5, 2022

mamazu commented Mar 6, 2022

mamazu commented Jun 29, 2022

diimpp commented Dec 21, 2021 •

edited

Loading

medteck commented Dec 21, 2021 •

edited

Loading

medteck commented Jan 4, 2022 •

edited

Loading

medteck commented Mar 1, 2022 •

edited

Loading