[17.0][MIG] delivery_roulier: Migration to 17.0

[yankinmax]

Depends on:

• [x] https://github.com/OCA/delivery-carrier/pull/828
• [x] https://github.com/OCA/delivery-carrier/pull/827

[yankinmax]

@rousseldenis is this migration ok now?

[yankinmax]

I'll remove the part I've proposed. I don't have any other better solution for now.

[florian-dacosta]

I'll remove the part I've proposed. I don't have any other better solution for now.

Thanks, I'll approve then. Can you also have a look here https://github.com/OCA/delivery-carrier/pull/851 and make a cherry pick to include the v16 fix here please?

[yankinmax]

I'll remove the part I've proposed. I don't have any other better solution for now.

Thanks, I'll approve then. Can you also have a look here #851 and make a cherry pick to include the v16 fix here please?

of course, thanks

[yankinmax]

@florian-dacosta I've checked your fix in v17. Still the same issue:

_____________________________________________________________________________________ DeliveryRoulierCase.test_roulier _____________________________________________________________________________________

self = <odoo.addons.delivery_roulier.tests.test_delivery_roulier.DeliveryRoulierCase testMethod=test_roulier>

    def test_roulier(self):
        roulier.get_carriers_action_available = MagicMock(
            return_value={"test": ["get_label"]}
        )
        with patch("roulier.roulier.get") as mock_roulier:
            mock_roulier.return_value = roulier_ret
            self.picking.send_to_shipper()
            roulier_args = mock_roulier.mock_calls[0][1]
            self.assertEqual("get_label", roulier_args[1])
            roulier_payload = roulier_args[2]
            self.assertEqual(len(roulier_payload["parcels"]), 1)
>           self.assertEqual(roulier_payload["parcels"][0].get("weight"), 1.2)
E           AssertionError: 0.0 != 1.2

so weight is not properly updated in _roulier_get_parcel

[florian-dacosta]

@florian-dacosta I've checked your fix in v17. Still the same issue:

_____________________________________________________________________________________ DeliveryRoulierCase.test_roulier _____________________________________________________________________________________

self = <odoo.addons.delivery_roulier.tests.test_delivery_roulier.DeliveryRoulierCase testMethod=test_roulier>

    def test_roulier(self):
        roulier.get_carriers_action_available = MagicMock(
            return_value={"test": ["get_label"]}
        )
        with patch("roulier.roulier.get") as mock_roulier:
            mock_roulier.return_value = roulier_ret
            self.picking.send_to_shipper()
            roulier_args = mock_roulier.mock_calls[0][1]
            self.assertEqual("get_label", roulier_args[1])
            roulier_payload = roulier_args[2]
            self.assertEqual(len(roulier_payload["parcels"]), 1)
>           self.assertEqual(roulier_payload["parcels"][0].get("weight"), 1.2)
E           AssertionError: 0.0 != 1.2

so weight is not properly updated in _roulier_get_parcel

The fix was not related to the weight issue, it was about picking tracking link. Where do you see this error ? The test on this PR seem fine, am I missing something ?

[yankinmax]

ah sorry I forgot to update, wait a sec

[yankinmax]

@florian-dacosta For this error my fix was applied with put_in_pack override: https://github.com/OCA/delivery-carrier/actions/runs/9856183670/job/27212631968?pr=826#step:8:74

[florian-dacosta]

Oh, I understand. After checking the test both on v16 and v17, here is the change that did break the delivery_roulier test : https://github.com/OCA/delivery-carrier/pull/828/commits/99e3d580ad8a7bd66b554cafa09aaf5adb318561#diff-8e3509b96799ba2c6bfb09b79df02a224f2ed73f5b529a0e2fa210f105fd64d2R25 Condition was changed from if not pack.quant_ids to if pack.quant_ids I don't understand why this was changed, but this is the reason of the test failing now.

[yankinmax]

Oh, I understand. After checking the test both on v16 and v17, here is the change that did break the delivery_roulier test : 99e3d58#diff-8e3509b96799ba2c6bfb09b79df02a224f2ed73f5b529a0e2fa210f105fd64d2R25 Condition was changed from if not pack.quant_ids to if pack.quant_ids I don't understand why this was changed, but this is the reason of the test failing now.

I'll prepare a fix

[yankinmax]

@florian-dacosta is this what we need:

• [ ] https://github.com/OCA/delivery-carrier/pull/856 I feel like I'm missing something

[florian-dacosta]

@florian-dacosta is this what we need:

* [ ]  [[FIX] base_delivery_carrier_label: compute the weight by the product weight #856](https://github.com/OCA/delivery-carrier/pull/856)
  I feel like I'm missing something

I think it is. In odoo the native _compute_weight method compute weight of a pack from the stock.move.line if the context "picking_id" is set (I have no idea when/where it is set) else it is based on the linked quants. The override in base_delivery_carrier_label tries to compute the weight in case we do not have quant and we do not have the context either (so native method does not work). That is not very great... But that is what we have for now, it does seem to do the job!

[yankinmax]

@florian-dacosta your test written in base_delivery_carrier_label is correct, no doubts. But, then we should find an issue and fix that module.

[yankinmax]

@florian-dacosta I'm feeling like we can finish the job finally) Can you approve?

[yankinmax]

BTW, if we could merge this one today we can also merge one more so it can be a very fast line day :smile:

• [ ] https://github.com/OCA/delivery-carrier/pull/829

[florian-dacosta]

@rousseldenis I guess your comment is now obsolete, but if you could confirm it changing your review it would be great!

[yankinmax]

@rousseldenis can you please update your review and in case you approve trigger the merge?

[simahawk]

/ocabot migration delivery_roulier

[yankinmax]

@simahawk updated with fixup

[simahawk]

/ocabot merge nopbump

[OCA-git-bot]

Hi @simahawk. Your command failed:

Invalid options for command merge: nopbump.

Ocabot commands

• ocabot merge major|minor|patch|nobump
• ocabot rebase
• ocabot migration {MODULE_NAME}

More information

• ocabot documentation
• OCA guidelines, specially the "Version Numbers" section.

[simahawk]

/ocabot merge nobump

[OCA-git-bot]

On my way to merge this fine PR! Prepared branch 17.0-ocabot-merge-pr-826-by-simahawk-bump-nobump, awaiting test results.

[OCA-git-bot]

Congratulations, your PR was merged at f219c965b97a2fb360fc5fea9c1368807a8ab287. Thanks a lot for contributing to OCA. ❤️