From 75995f5965b36b07dcce67375fd0031b32b766ef Mon Sep 17 00:00:00 2001 From: jem-odoo Date: Tue, 12 Dec 2017 09:29:35 +0100 Subject: [PATCH] [IMP] sale,sale_*: correct use of expense_policy A product can be sell or expense, but for some cases the product configuration make both flow incompatible. Indeed, when expensing the delivered qty was computed based on the AAL total amount, and manually editable. Before the delivered qty refactoring, reexpensing erased the manual value. This came from the fact product expense policy is wrongly used: the delivered qty of expensable product is not manually editable, which is a problem for milestones (and other consumable product when stock is no installed). The new refactoring of deli qty API force the expense line to be completely readonly, but the problem is not completely solved; we still can't sell properly a expensable product. To do so, the SO line must be flag as coming from an expense or vendor bill, and the reinvoice product policy should only be used to calculate the price of new SO line. This commit: 1) reinforces the readonly caracter of expense SO line, by making the expense policy independent of the computation method for delivered qty on SOL. 2) prevents generate picking when updating ordered quantity on a expense SOL with a stockable product. 3) allows service to sell a task with zero hours as ordered quantity. (it still prevents task/project creation for expense SOL). 4) does not increment anymore an existing line when expensing product at sales price and invoicing at 'delivered qty'. The only line to be incremented will be the expense line (and not the native SOL). --- addons/hr_expense/tests/common.py | 5 +++ addons/sale/models/analytic.py | 14 +++---- addons/sale/models/product_template.py | 2 +- addons/sale/models/sale.py | 17 +++----- addons/sale_expense/tests/test_reinvoice.py | 44 +++++++++++++++++---- addons/sale_mrp/models/sale_mrp.py | 2 +- addons/sale_stock/models/sale_order.py | 15 ++++--- addons/sale_timesheet/models/project.py | 12 +++++- addons/sale_timesheet/models/sale_order.py | 14 +++---- 9 files changed, 80 insertions(+), 45 deletions(-) diff --git a/addons/hr_expense/tests/common.py b/addons/hr_expense/tests/common.py index c7b7726bd4c..71a4d33e757 100644 --- a/addons/hr_expense/tests/common.py +++ b/addons/hr_expense/tests/common.py @@ -51,3 +51,8 @@ class TestExpenseCommon(TestAccountNoChartCommon): 'employee_id': cls.employee.id, 'journal_id': cls.journal.id, }) + cls.expense_sheet2 = cls.env['hr.expense.sheet'].create({ + 'name': 'Second Expense for Johnny Employee', + 'employee_id': cls.employee.id, + 'journal_id': cls.journal.id, + }) diff --git a/addons/sale/models/analytic.py b/addons/sale/models/analytic.py index b0a0a2b09fe..4eeebd2f4c6 100644 --- a/addons/sale/models/analytic.py +++ b/addons/sale/models/analytic.py @@ -28,9 +28,11 @@ class AccountAnalyticLine(models.Model): result = super(AccountAnalyticLine, self).write(values) if 'so_line' not in values: # allow to force a False value for so_line self.sudo().filtered(lambda aal: not aal.so_line and aal.product_id and aal.product_id.expense_policy != 'no')._sale_determine_order_line() + return result - # NOTE JEM: thoses method are used in vendor bills to reinvoice at cost (see test `test_cost_invoicing`) - # some cleaning are still necessary + # ---------------------------------------------------------- + # Vendor Bill / Expense : determine the Sale Order to reinvoice + # ---------------------------------------------------------- @api.multi def _sale_get_invoice_price(self, order): @@ -75,7 +77,7 @@ class AccountAnalyticLine(models.Model): 'product_id': self.product_id.id, 'product_uom': self.product_uom_id.id, 'product_uom_qty': 0.0, - 'qty_delivered': self.unit_amount, + 'is_expense': True, } @api.multi @@ -107,14 +109,14 @@ class AccountAnalyticLine(models.Model): if sale_order.state != 'sale': raise UserError(_('The Sales Order %s linked to the Analytic Account must be validated before registering expenses.') % sale_order.name) - price = analytic_line._sale_get_invoice_price(sale_order) so_line = None + price = analytic_line._sale_get_invoice_price(sale_order) if analytic_line.product_id.expense_policy == 'sales_price' and analytic_line.product_id.invoice_policy == 'delivery': so_line = self.env['sale.order.line'].search([ ('order_id', '=', sale_order.id), ('price_unit', '=', price), ('product_id', '=', self.product_id.id), - ('qty_delivered_method', '=', 'analytic'), + ('is_expense', '=', True), ], limit=1) if not so_line: @@ -122,8 +124,6 @@ class AccountAnalyticLine(models.Model): so_line_values = analytic_line._sale_prepare_sale_order_line_values(sale_order, price) so_line = self.env['sale.order.line'].create(so_line_values) so_line._compute_tax_id() - else: - so_line.write({'qty_delivered': so_line.qty_delivered + analytic_line.unit_amount}) if so_line: # if so line found or created, then update AAL (this will trigger the recomputation of qty delivered on SO line) value_to_write.setdefault(so_line.id, self.env['account.analytic.line']) diff --git a/addons/sale/models/product_template.py b/addons/sale/models/product_template.py index c4e825dcdb7..e1ffda478ab 100644 --- a/addons/sale/models/product_template.py +++ b/addons/sale/models/product_template.py @@ -17,7 +17,7 @@ class ProductTemplate(models.Model): sale_line_warn_msg = fields.Text('Message for Sales Order Line') expense_policy = fields.Selection( [('no', 'No'), ('cost', 'At cost'), ('sales_price', 'Sales price')], - string='Re-Invoice Expenses', + string='Re-Invoice Policy', default='no') @api.multi diff --git a/addons/sale/models/sale.py b/addons/sale/models/sale.py index 9fbd43b081f..21b664352f6 100644 --- a/addons/sale/models/sale.py +++ b/addons/sale/models/sale.py @@ -892,6 +892,7 @@ class SaleOrderLine(models.Model): order_partner_id = fields.Many2one(related='order_id.partner_id', store=True, string='Customer') analytic_tag_ids = fields.Many2many('account.analytic.tag', string='Analytic Tags') analytic_line_ids = fields.One2many('account.analytic.line', 'so_line', string="Analytic lines") + is_expense = fields.Boolean('Is expense', help="Is true if the sales order line comes from an expense or a vendor bills") is_downpayment = fields.Boolean( string="Is a down payment", help="Down payments are made when creating invoices from a sales order." " They are not copied when duplicating a sales order.") @@ -912,7 +913,7 @@ class SaleOrderLine(models.Model): # TODO: remove layout_category_sequence in master or make it work properly @api.multi - @api.depends('state', 'product_id.expense_policy') + @api.depends('state', 'is_expense') def _compute_qty_delivered_method(self): """ Sale module compute delivered qty for product [('type', 'in', ['consu']), ('service_type', '=', 'manual')] - consu + expense_policy : analytic (sum of analytic unit_amount) @@ -923,16 +924,10 @@ class SaleOrderLine(models.Model): and sale_timesheet implements the behavior of 'service' + service_type=timesheet. """ for line in self: - if line.product_id.type == 'consu': - if line.product_id.expense_policy == 'no': - line.qty_delivered_method = 'manual' - else: - line.qty_delivered_method = 'analytic' - else: # NOTE: line.product_id.type == 'service' and line.product_id.service_type == 'manual': - if line.product_id.expense_policy == 'no': - line.qty_delivered_method = 'manual' # ultimate fallback - else: - line.qty_delivered_method = 'analytic' + if line.is_expense: + line.qty_delivered_method = 'analytic' + else: # service and consu + line.qty_delivered_method = 'manual' @api.multi @api.depends('qty_delivered_method', 'qty_delivered_manual', 'analytic_line_ids.so_line', 'analytic_line_ids.unit_amount', 'analytic_line_ids.product_uom_id') diff --git a/addons/sale_expense/tests/test_reinvoice.py b/addons/sale_expense/tests/test_reinvoice.py index c1b1400b710..f0c028d69e6 100644 --- a/addons/sale_expense/tests/test_reinvoice.py +++ b/addons/sale_expense/tests/test_reinvoice.py @@ -76,6 +76,7 @@ class TestReInvoice(TestExpenseCommon, TestCommonSaleNoChart): self.assertEquals(sale_order_line.qty_delivered, 0, "Exising SO line should not be impacted by reinvoicing product at cost") sol_ordered = self.sale_order.order_line.filtered(lambda sol: sol.product_id == self.product_ordered_cost and sol != sale_order_line) + self.assertTrue(sol_ordered.is_expense, "The new SOL should be flag as coming from an expense") self.assertEquals(sol_ordered.qty_delivered_method, 'analytic', 'Delivered quantity of expense SO line should be computed by analytic amount') self.assertTrue(sol_ordered, "A new line with ordered expense should have been created on expense report posting") self.assertEquals(sol_ordered.price_unit, expense1.unit_amount, "The unit price of new SO line should be the one from the expense (at cost)") @@ -83,6 +84,7 @@ class TestReInvoice(TestExpenseCommon, TestCommonSaleNoChart): self.assertEquals(sol_ordered.qty_delivered, expense1.quantity, "The delivered quantity of new SO line should be the one from the expense") sol_deliver = self.sale_order.order_line.filtered(lambda sol: sol.product_id == self.product_deliver_cost and sol != sale_order_line) + self.assertTrue(sol_deliver.is_expense, "The new SOL should be flag as coming from an expense") self.assertEquals(sol_deliver.qty_delivered_method, 'analytic', 'Delivered quantity of expense SO line should be computed by analytic amount') self.assertTrue(sol_deliver, "A new line with delivered expense should have been created on expense report posting") self.assertEquals(sol_deliver.price_unit, expense2.unit_amount, "The unit price of new SO line should be the one from the expense (at cost)") @@ -118,15 +120,18 @@ class TestReInvoice(TestExpenseCommon, TestCommonSaleNoChart): self.assertEquals(len(self.sale_order.order_line), 1, "A new So line should have been created on expense report posting") sol_ordered = self.sale_order.order_line.filtered(lambda sol: sol.product_id == expense1.product_id) + self.assertTrue(sol_ordered.is_expense, "The new SOL should be flag as coming from an expense") self.assertTrue(sol_ordered, "A new line with ordered expense should have been created on expense report posting") self.assertEquals(sol_ordered.price_unit, 10, "The unit price of new SO line should be the one from the expense (at sales price)") self.assertEquals(sol_ordered.product_uom_qty, 0, "The ordered quantity of new SO line should be zero") self.assertEquals(sol_ordered.qty_delivered, expense1.quantity, "The delivered quantity of new SO line should be the one from the expense") def test_sales_price_delivered(self): - """ Test invoicing expenses at sales price for product based on delivered quantities. Check the existing SO line is incremented. """ + """ Test invoicing expenses at sales price for product based on delivered quantities. Check the existing SO line is not incremented, but a new + expense SOL will be created and incremented. + """ # create SO line and confirm SO (with only one line) - sale_order_line = self.env['sale.order.line'].create({ + sale_order_line1 = self.env['sale.order.line'].create({ 'name': self.product_deliver_sales_price.name, 'product_id': self.product_deliver_sales_price.id, 'product_uom_qty': 2, @@ -134,7 +139,7 @@ class TestReInvoice(TestExpenseCommon, TestCommonSaleNoChart): 'price_unit': self.product_deliver_sales_price.list_price, 'order_id': self.sale_order.id, }) - sale_order_line.product_id_change() + sale_order_line1.product_id_change() self.sale_order._compute_tax_id() self.sale_order.action_confirm() @@ -157,11 +162,36 @@ class TestReInvoice(TestExpenseCommon, TestCommonSaleNoChart): self.expense_sheet.approve_expense_sheets() self.expense_sheet.action_sheet_move_create() - self.assertEquals(len(self.sale_order.order_line), 1, "No SO line should have been created (or removed) on expense report posting") + self.assertEquals(len(self.sale_order.order_line), 2, "A new expense SOL should have been created.") - self.assertEquals(sale_order_line.price_unit, 10, "The unit price of SO line should be the same") - self.assertEquals(sale_order_line.product_uom_qty, 2, "The ordered quantity of new SO line should be zero") - self.assertEquals(sale_order_line.qty_delivered, expense1.quantity, "The delivered quantity of SO line should have been incremented") + sale_order_line2 = self.sale_order.order_line.filtered(lambda sol: sol != sale_order_line1) + self.assertTrue(sale_order_line2.is_expense, "The new SOL should be flag as coming from an expense") + self.assertEquals(sale_order_line2.price_unit, 10, "The unit price of SO line should be the same") + self.assertEquals(sale_order_line2.product_uom_qty, 0, "The ordered quantity of new SO line should be zero") + self.assertEquals(sale_order_line2.qty_delivered, expense1.quantity, "The delivered quantity of SO line should have be the same of the expense") + + # create 2nd expense lines + expense2 = self.env['hr.expense'].create({ + 'name': 'Expense for delivered product at sales price', + 'employee_id': self.employee.id, + 'product_id': self.product_deliver_sales_price.id, + 'unit_amount': 15, + 'quantity': 3, + 'sheet_id': self.expense_sheet2.id, + 'sale_order_id': self.sale_order.id, + 'analytic_account_id': self.sale_order.analytic_account_id.id, + }) + expense2._onchange_product_id() + + # approve and generate entries + self.expense_sheet2.approve_expense_sheets() + self.expense_sheet2.action_sheet_move_create() + + self.assertEquals(len(self.sale_order.order_line), 2, "No new expense SOL should have been created.") + + self.assertEquals(sale_order_line2.price_unit, 10, "The unit price of SO line should be the same") + self.assertEquals(sale_order_line2.product_uom_qty, 0, "The ordered quantity of new SO line should remains zero") + self.assertEquals(sale_order_line2.qty_delivered, expense1.quantity + expense2.quantity, "The delivered quantity of SO line should have been incremented") def test_no_expense(self): """ Test invoicing expenses with no policy. Check nothing happen. """ diff --git a/addons/sale_mrp/models/sale_mrp.py b/addons/sale_mrp/models/sale_mrp.py index 4ccdd92410d..596fbba50ec 100644 --- a/addons/sale_mrp/models/sale_mrp.py +++ b/addons/sale_mrp/models/sale_mrp.py @@ -13,7 +13,7 @@ class SaleOrderLine(models.Model): super(SaleOrderLine, self)._compute_qty_delivered() for line in self: - if line.qty_delivered_method == 'stock_move' and line.product_id.expense_policy == 'no': + if line.qty_delivered_method == 'stock_move': # In the case of a kit, we need to check if all components are shipped. Since the BOM might # have changed, we don't compute the quantities but verify the move state. bom = self.env['mrp.bom']._bom_find(product=line.product_id) diff --git a/addons/sale_stock/models/sale_order.py b/addons/sale_stock/models/sale_order.py index 566c8a24e1e..9b5dbb16b05 100644 --- a/addons/sale_stock/models/sale_order.py +++ b/addons/sale_stock/models/sale_order.py @@ -94,15 +94,15 @@ class SaleOrderLine(models.Model): @api.multi @api.depends('product_id.type') def _compute_qty_delivered_method(self): - """ Stock module compute delivered qty for product [('type', 'in', ['consu', 'product'])] """ + """ Stock module compute delivered qty for product [('type', 'in', ['consu', 'product'])] + For SO line coming from expense, no picking should be generate: we don't manage stock for + thoses lines, even if the product is a stockable. + """ super(SaleOrderLine, self)._compute_qty_delivered_method() for line in self: - if line.product_id.type in ['consu', 'product']: - if line.product_id.expense_policy == 'no': - line.qty_delivered_method = 'stock_move' - else: - line.qty_delivered_method = 'analytic' + if not line.is_expense and line.product_id.type in ['consu', 'product']: + line.qty_delivered_method = 'stock_move' @api.multi @api.depends('move_ids.state', 'move_ids.scrapped', 'move_ids.product_uom_qty', 'move_ids.product_uom') @@ -133,13 +133,12 @@ class SaleOrderLine(models.Model): if 'product_uom_qty' in values: precision = self.env['decimal.precision'].precision_get('Product Unit of Measure') lines = self.filtered( - lambda r: r.state == 'sale' and float_compare(r.product_uom_qty, values['product_uom_qty'], precision_digits=precision) == -1) + lambda r: r.state == 'sale' and not r.is_expense and float_compare(r.product_uom_qty, values['product_uom_qty'], precision_digits=precision) == -1) res = super(SaleOrderLine, self).write(values) if lines: lines._action_launch_procurement_rule() return res - @api.depends('order_id.state') def _compute_invoice_status(self): super(SaleOrderLine, self)._compute_invoice_status() diff --git a/addons/sale_timesheet/models/project.py b/addons/sale_timesheet/models/project.py index 3244e4990c0..facfdb04ce4 100644 --- a/addons/sale_timesheet/models/project.py +++ b/addons/sale_timesheet/models/project.py @@ -8,7 +8,7 @@ from odoo.exceptions import ValidationError class Project(models.Model): _inherit = 'project.project' - sale_line_id = fields.Many2one('sale.order.line', 'Sales Order Line', readonly=True, help="Sale order line from which the project has been created. Used for tracability.") + sale_line_id = fields.Many2one('sale.order.line', 'Sales Order Line', domain=[('is_expense', '=', False)], readonly=True, help="Sale order line from which the project has been created. Used for tracability.") @api.multi def action_view_timesheet(self): @@ -56,7 +56,15 @@ class Project(models.Model): class ProjectTask(models.Model): _inherit = "project.task" - sale_line_id = fields.Many2one('sale.order.line', 'Sales Order Item', domain="[('is_service', '=', True), ('order_partner_id', '=', partner_id)]") + sale_line_id = fields.Many2one('sale.order.line', 'Sales Order Item', domain="[('is_service', '=', True), ('order_partner_id', '=', partner_id), ('is_expense', '=', False)]") + + @api.multi + @api.constrains('sale_line_id') + def _check_sale_line_type(self): + for task in self: + if task.sale_line_id: + if not task.sale_line_id.is_service or task.sale_line_id.is_expense: + raise ValidationError(_("The Sales order line should be one selling a service, and no coming from expense.")) @api.model def create(self, values): diff --git a/addons/sale_timesheet/models/sale_order.py b/addons/sale_timesheet/models/sale_order.py index 1036591ef86..335a6b657fe 100644 --- a/addons/sale_timesheet/models/sale_order.py +++ b/addons/sale_timesheet/models/sale_order.py @@ -139,6 +139,7 @@ class SaleOrderLine(models.Model): qty_delivered_method = fields.Selection(selection_add=[('timesheet', 'Timesheets')]) task_id = fields.Many2one('project.task', 'Task', help="Task generated by the sales order item") is_service = fields.Boolean("Is a Service", compute='_compute_is_service', store=True, help="Sales Order item should generate a task and/or a project, depending on the product settings.") + analytic_line_ids = fields.One2many(domain=[('project_id', '=', False)]) # only analytic lines, not timesheets (since this field determine if SO line came from expense) @api.multi @api.depends('product_id.type', 'product_id.service_type') @@ -146,11 +147,8 @@ class SaleOrderLine(models.Model): """ Sale Timesheet module compute delivered qty for product [('type', 'in', ['service']), ('service_type', '=', 'timesheet')] """ super(SaleOrderLine, self)._compute_qty_delivered_method() for line in self: - if line.product_id.type == 'service' and line.product_id.service_type == 'timesheet': - if line.product_id.expense_policy == 'no': - line.qty_delivered_method = 'timesheet' - else: - line.qty_delivered_method = 'analytic' + if not line.is_expense and line.product_id.type == 'service' and line.product_id.service_type == 'timesheet': + line.qty_delivered_method = 'timesheet' @api.multi @api.depends('analytic_line_ids.project_id') @@ -185,9 +183,9 @@ class SaleOrderLine(models.Model): @api.model def create(self, values): line = super(SaleOrderLine, self).create(values) - precision = self.env['decimal.precision'].precision_get('Product Unit of Measure') - # check ordered quantity to avoid create project/task when expensing service products - if line.state == 'sale' and not float_is_zero(line.product_uom_qty, precision_digits=precision): + # Do not generate task/project when expense SO line, but allow + # generate task with hours=0. + if line.state == 'sale' and not line.is_expense: line._timesheet_service_generation() return line