Prestashop: Migration - Create order bugs

[Robin-Fischer-PS]

What's left

• #17179

Done / cannot reproduce

Describe the bug
Following the merge of https://github.com/PrestaShop/PrestaShop/pull/16207, here is the list of the remaining bugs on this page :

1. Voucher with gift product does not work : The gift should appear in the cart, with price replaced by "Gift", but nothing happens. However, once the order is created, the gift is in the order.

➡️ PR https://github.com/PrestaShop/PrestaShop/pull/17179

1. The combinations of product with combination are no longer loaded immediately on search :
   https://drive.google.com/file/d/1gWseCzlCvy7I4e1ijpRcnlafCaquZD1R/view?usp=sharing

➡️ fixed by https://github.com/PrestaShop/PrestaShop/pull/17465 ✅

1. When adding a "Free Shipping" voucher on an order (NOT created by checking the "Free Shipping" on the page), and then, checking the "Free Shipping" option on the order, the Shipping price is deducted twice.
   See this 1.7.7.x invoice :
   Invoice two free ship 177.pdf
   And the same action on 1.7.6 (free shipping is only deducted once) :
   Invoice two free ship 176.pdf

➡️ we failed to reproduce it

1. file name is not displayed in migrated when adding file for a custom product
   screenshot 1.7.7 :
   
   in legacy 1.7.6 :
   

Please note that there might still be some bugs out there...

Additional information
PrestaShop version: 1.7.7.x
PHP version: N/A

[matks]

For the record: "gift products" display is actually a totally different listing display
so this bug is actually a not-implemented feature that we did not know about 😛

So it'll take some time to do

[zuk3975]

Voucher gifts product fix #17179

[Robin-Fischer-PS]

I changed the status to "In Progress" because the PR linked will only fix one bug, not the second one.

[tdavidsonas88]

@Robin-Fischer-PS not able to replicate the bug with the latest 1.7.7.x branch: When adding a "Free Shipping" voucher on an order (NOT created by checking the "Free Shipping" on the page), and then, checking the "Free Shipping" option on the order, the Shipping price is deducted twice.

[tdavidsonas88]

@matks done some research on bug 'file name is not displayed in migrated when adding file for a custom product'.

1. this bug does not repeat in other migrated pages like: admin-dev/index.php/sell/attachments/new/ (it does not repeat in pages which are rendered from FormType)
2. I was confused that in our migrated pages we have this bug but on prestashop-ui-kit it works correctly: https://cdn.rawgit.com/PrestaShop/prestashop-ui-kit/master/index.html#forms (scroll to the bottom of the section to try)
3. The code is very similar in prestshop-ui-kit and the migrated pages:

         <div class="custom-file">
              <input type="file" class="custom-file-input" id="file1">
              <label class="custom-file-label" for="file1">Choose file...</label>
         </div>

and the migrated page (cart.html.twig):

<script id="js-product-custom-file-template" type="text/template">
  <div class="row mt-3">
    <div class="col">
      {{ requiredFieldMarkTemplate }}
      <label for="" class="js-product-custom-input-label"></label>
      <div class="custom-file">
        <input name="" type="file" class="custom-file-input">
        <label class="custom-file-label">
          {{ 'Choose file(s)'|trans({}, 'Admin.Actions') }}
        </label>
      </div>
    </div>
  </div>
</script>

https://github.com/PrestaShop/prestashop-ui-kit/blob/ae6114aef125bbfbdad69a4baf355d3a11672ab6/index.html#L1926-L1929

1. However if I copy the code from prestashop-ui-kit to be exactly the same it is still not rendering like expected.
2. This bug is time consuming because everytime I need to test the changes - have to find customer / find product and only then can test file upload.

[matks]

@tdavidsonas88

1. I was confused that in our migrated pages we have this bug but on prestashop-ui-kit it works correctly: (scroll to the bottom of the section to try)

Warning: https://cdn.rawgit.com/PrestaShop/prestashop-ui-kit/master/index.html#forms might not be up-to-date

It looks like we better ask for help the UI Kit Manager @NeOMakinG

[Robin-Fischer-PS]

@tdavidsonas88 Here is a screenrecord for reproducing this bug :
When adding a "Free Shipping" voucher on an order (NOT created by checking the "Free Shipping" on the page), and then, checking the "Free Shipping" option on the order, the Shipping price is deducted twice.

https://drive.google.com/file/d/1sXSULMDGfz1TMAAqY0noKBKomAXnT41W/view?usp=sharing

On the record, the shipping price is deduced two times, giving a total price of 5.88€ instead of 14.28€ (the price without shipping).
This bug is also here in Legacy 177 but was not present in Legacy 176.

[tdavidsonas88]

Thank you @Robin-Fischer-PS, I take this issue now.

[tdavidsonas88]

The easiest suggestion to solve this bug is to disable Free Shipping (Yes/No) toggle button if the user already added the discount voucher with free shipping. If you have a better offer please let me know.

[matks]

I like this solution (simple and efficient), but we need PM confirmation. @MatShir ?

[MatShir]

Edit:
I recommended disabling the input and putting it as uneditable, I find it better rather hide the fields.
Like in the screenshot below:

@tdavidsonas88 Do you think you will be able to do it easily? So if I deleted a cart rule with free shipping in a new order, it will enable the toggle and vice versa.

[tdavidsonas88]

@tdavidsonas88 Do you think you will be able to do it easily? So if I deleted a cart with free shipping in a new order, it will enable the toggle and vice versa.

@MatShir Plan to submit solution PR today. There is some complexity because it is javascript code. For deleting cart for already created order case is one I have not thought about initially so let me try it out

[tdavidsonas88]

@matks In the middle of implementing the disable solution found out it is not that simple and many .js files need to be changed. Not even started to implement vice versa solution for not to allow the voucher with free shipping added if free shipping is already enabled. Now it seems to me that throwing exception from the backend could be easier - just would need some creative ideas how to handle exception and show a nice message to the user. I think this solution is more beautiful for the user to interact but not the simplest to develop

[tdavidsonas88]

Looks like was able to finish one part of the solution today: for disabling the toggle. However would like to do more testing tomorrow and also finish the part with not allowing to add "Free Shipping" if there is cart rule with the free shipping already. Plan to provide PR tomorrow.

[matks]

Looks like was able to finish one part of the solution today: for disabling the toggle. However would like to do more testing tomorrow and also finish the part with not allowing to add "Free Shipping" if there is cart rule with the free shipping already. Plan to provide PR tomorrow.

I'm actually worried it took 1 day, in my mind it's not supposed to be too hard 😅 . Can you push a draft PR with your code so we can check it ?

[tdavidsonas88]

Looks like was able to finish one part of the solution today: for disabling the toggle. However would like to do more testing tomorrow and also finish the part with not allowing to add "Free Shipping" if there is cart rule with the free shipping already. Plan to provide PR tomorrow.

I'm actually worried it took 1 day, in my mind it's not supposed to be too hard . Can you push a draft PR with your code so we can check it ?

@matks Already created a normal PR, finished this morning. It took a bit more 1 hour less than one day. Your guidance helped me a lot :slightly_smiling_face:

[tdavidsonas88]

So if I deleted a cart with free shipping in a new order, it will enable the toggle and vice versa.

@MatShir Well if you delete the cart then it is not possible to create new order from the deleted cart. And if you create the order with the cart then it is not allowed to delete the cart. Maybe I don't understand something?

[MatShir]

Hi @tdavidsonas88, sorry I meant the cart rule

[matks]

It seems latest issue has been moved to https://github.com/PrestaShop/PrestaShop/issues/17875

@Robin-Fischer-PS @MatShir can I close this issue now ?

[matks]

So it seems the last bug in this issue is the counterpart of https://github.com/PrestaShop/PrestaShop/issues/17875 but in BO 😄

[Robin-Fischer-PS]

FYI @matks , we also need https://github.com/PrestaShop/PrestaShop/pull/17179 to finish this issue :)

[matks]

As discussed here we await for the specs for the behavior of the "free gift" cart rule in order to avoid additionnal back-and-forths with QA

[colinegin]

I thought this issue was meant to fix the last bug that was supposed to be fixed by Tadas.
IMO, the "free gift cart rule" behaviours are to be managed in this issue : https://github.com/PrestaShop/PrestaShop/issues/18119.
I'm currently updating the issue with the specs.

[colinegin]

After discussing with @matks, we've decided we can close this issue as most of the work has been done. There are 2 bugs left related to this issue, issues have been added to the 1.7.7 kanban:

• bug on the shipping price which is deducted two times (both on FO & BO) --> follow issue #17875
• bug on gift products added to manually created orders --> follow issue #18119

FYI @Robin-Fischer-PS