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

[medteck]

Resolves #519

[mamazu]

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]

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]

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]

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]

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]

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]

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]

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.

[medteck]

@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]

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.

[lchrusciel]

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

[mamazu]

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]

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]

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]

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]

@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]

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]

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