[IMP] purchase_sale_inter_company: apply customer specific changes and improve with latest changes from v14 #504
Conversation
This reverts commit d3b6cac.
Improve with latest changes from v14
Create one SO from PO, if exists -> modify existing
yankinmax
force-pushed
the
enea-purchase_sale_inter_company
branch
from
eddf581
to
594a403
Compare
gurneyalex
added
the
no stale
| po_partner = purchase_order.partner_id | ||
| # vendor can be a member of group company or it's child | ||
| # dest_company will be anyway member of group company | ||
| dest_company = ( | ||
| purchase_order.partner_id.commercial_partner_id.ref_company_ids | ||
| po_partner.ref_company_ids or po_partner.parent_id.ref_company_ids | ||
| ) | ||
| intercompany_user = dest_company.intercompany_sale_user_id | ||
| if not intercompany_user: | ||
| intercompany_user = self.env.user |
There was a problem hiding this comment.
These lines may return a res.users recordset as intercompany_user, but at line 70 we have intercompany_user.id which will raise an error if there's more than 1 record. Please fix this.
| # if len(auto_sale_order_id) > 1: | ||
| # raise UserError( | ||
| # _( | ||
| # "You can have only one auto-generated sale order " | ||
| # "but following orders were found '%s' " | ||
| # ) | ||
| # % ",".join([order.display_name for order in auto_sale_order_id]) | ||
| # ) |
There was a problem hiding this comment.
It's standard OCA flow for this module.
The client refuses it, so I commented for future self to understand.
| auto_sale_order_id = ( | ||
| self.env["sale.order"] | ||
| .sudo() | ||
| .search([("auto_purchase_order_id", "=", purchase_order.id)]) | ||
| ) |
There was a problem hiding this comment.
This search() may return a sale.order recordset, but at line 67 we have auto_sale_order_id.state which will raise an error if there's more than 1 SO. Please fix this.
| auto_sale_order_id = ( | ||
| self.env["sale.order"] | ||
| .sudo() | ||
| .search([("auto_purchase_order_id", "=", purchase_order.id)]) | ||
| ) |
There was a problem hiding this comment.
This search() may return a sale.order recordset, but at line 28 we have auto_sale_order_id.display_name which will raise an error if there's more than 1 SO. Please fix this.
| raise UserError( | ||
| _( | ||
| "There is more than one pricelist for this vendor " | ||
| "with the same currency" | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Why do you raise this error instead of just fetching 1 of the many pricelists?
There was a problem hiding this comment.
customer request was to have only one pricelist for vendor company with the same currency
it's a signal to check and remove not needed one
| def _prepare_sale_order_line_data(self, purchase_line, dest_company, sale_order): | ||
| def _prepare_sale_order_line_data(self, purchase_line, sale_order): |
There was a problem hiding this comment.
Changing a method signature can be disruptive. Have you checked it usage in other modules?
There was a problem hiding this comment.
this method (I mean with such parameters) is only used here for this customer
There was a problem hiding this comment.
This is the exact change we do in OCA module to have customer's expected workflow.
|
@SilvioC2C thanks for your review. |
|
@yankinmax OCA modules should not contain customer-specific needs, they should be generic enough to be included in public repos. If these changes are customer-specific, they should be made directly in the customer's project. Besides, these comments are about bugs that break the execution of Odoo itself, so they should be fixed regardless:
|
…xists -> modify existing
|
@SilvioC2C I agree with you. It was a difficult decision we've taken with @gurneyalex |
|
@SilvioC2C, is it ok now ? |
legalsylvain
added
needs review
and removed
no stale
|
@legalsylvain unfortunately no, there still are bugs in this code and it still includes customer-specific behavior 😅 |
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
github-actions
bot
added
the
stale
This PR returns customer specific changes introduced in these commits:
This PR is opened as a replacement of the closed one: #445