[17.0][ADD] delivery_sendcloud_oca #852
Conversation
ByteMeAsap
force-pushed
the
17.0-add-delivery_sendcloud
branch
from
51c3c10 to
efd86b3
Compare
There was a problem hiding this comment.
LGTM! This module was moved and migrated from https://github.com/onesteinbv/addons-sendcloud. We (Onestein) have given permission to transition this to the OCA
|
@ByteMeAsap Could you rename this to Thanks |
ByteMeAsap
force-pushed
the
17.0-add-delivery_sendcloud
branch
from
efd86b3 to
f9f5490
Compare
ByteMeAsap
changed the title
ByteMeAsap
force-pushed
the
17.0-add-delivery_sendcloud
branch
2 times, most recently
from
c909c4c to
7160fea
Compare
ByteMeAsap
force-pushed
the
17.0-add-delivery_sendcloud
branch
2 times, most recently
from
ab46bed to
fb0ba77
Compare
@rousseldenis , Yes , I have renamed the module. Can you help me with how do I go about resolving conflicts as there have been changes in the 17.0 branch since I had raised the PR a while back? |
ByteMeAsap
force-pushed
the
17.0-add-delivery_sendcloud
branch
2 times, most recently
from
17824cb to
3b5536f
Compare
ByteMeAsap
force-pushed
the
17.0-add-delivery_sendcloud
branch
3 times, most recently
from
a6364c1 to
4f240b0
Compare
|
@OCA/delivery-carrier-maintainers, @rousseldenis , can we have this merged? |
ByteMeAsap
force-pushed
the
17.0-add-delivery_sendcloud
branch
from
4f240b0 to
63376d2
Compare
|
Can we please have this merged ?? |
|
|
||
| @api.depends("sendcloud_service_point_input", "delivery_type") | ||
| def _compute_sendcloud_service_point_required(self): | ||
| for carrier in self: |
There was a problem hiding this comment.
self.update({"sendcloud_service_point_required": False})
There was a problem hiding this comment.
@rousseldenis , I have updated the function
| @api.depends() | ||
| def _compute_sendcloud_country_ids(self): | ||
| for carrier in self: | ||
| countries = self.env["sendcloud.shipping.method.country"].search( |
There was a problem hiding this comment.
Do a search before the loop and then filter in it to improve performances.
|
|
||
| @api.onchange("delivery_type") | ||
| def _onchange_sendcloud_delivery_type(self): | ||
| self._sendcloud_set_countries() |
There was a problem hiding this comment.
Is this really to be done in an onchange ? This can be bad from a UX point of view.
There was a problem hiding this comment.
I have updated to set the countries on write
| deleted_parcels.append(parcel_code) | ||
| elif ( | ||
| res.get("status") == "failed" | ||
| and res.get("message") == "This shipment is already being cancelled." |
There was a problem hiding this comment.
Could not this be translated ?
There was a problem hiding this comment.
No, this cannot be translated
| elif res.get("error", {}).get("code") == 404: | ||
| deleted_parcels.append(parcel_code) # ignore "Not Found" error | ||
| elif res.get("error"): | ||
| raise ValidationError(_("Sendcloud: %s") % res["error"].get("message")) |
There was a problem hiding this comment.
Use translation function arguments instead.
| def _compute_can_generate_return(self): | ||
| res = super()._compute_can_generate_return() | ||
| for carrier in self.filtered(lambda c: c.delivery_type == "sendcloud"): | ||
| carrier.can_generate_return = True |
There was a problem hiding this comment.
you can use an update() on recordset instead.
| ): | ||
| record.country_ids = record._sendcloud_get_countries_from_cache() | ||
|
|
||
| def _sendcloud_get_countries_from_cache(self): |
There was a problem hiding this comment.
Maybe the method name deos not reflect the current behavior.
| if existing.sendcloud_code not in existing_shipping_methods_map: | ||
| existing_shipping_methods_map[existing.sendcloud_code] = self.env[ | ||
| "delivery.carrier" | ||
| ] |
There was a problem hiding this comment.
self.env["delivery.carrier"].browse() is semantically more correct.
| shipping_method_country.product_id = item.product_id | ||
| shipping_method_country.enable_price_custom = item.enable_price_custom | ||
| else: | ||
| self.env["sendcloud.shipping.method.country.custom"].create( |
There was a problem hiding this comment.
Build a list of create values in the loop, then call create() once.
ByteMeAsap
force-pushed
the
17.0-add-delivery_sendcloud
branch
from
63376d2 to
d809bbd
Compare
Updated requirements.txt file and changed qty_done to quantity_product_uom for stock move lines Updates Updated test scripts
ByteMeAsap
force-pushed
the
17.0-add-delivery_sendcloud
branch
from
d809bbd to
60f7cef
Compare
|
@rousseldenis , please review |
delivery_sendcloud_oca module provides sendcloud shipping integration with Odoo
This module mostly implements what's described in https://docs.sendcloud.sc/api/v2/shipping/
Full documentation for developers is in https://docs.sendcloud.sc/.
This module works for the Community Edition as well as the Enterprise Edition.