Merge pull request #293 from acsone/9.0-a_t_b-perf-sbi

[9.0] account_tax_balance performance

account_tax_balance/__openerp__.py

@@ -5,10 +5,10 @@
 {
     "name": "Tax Balance",
     "summary": "Compute tax balances based on date range",
-    "version": "9.0.1.0.1",
+    "version": "9.0.1.1.0",
     "category": "Accounting & Finance",
     "website": "https://www.agilebg.com/",
-    "author": "Agile Business Group, Therp BV, Tecnativa, "
+    "author": "Agile Business Group, Therp BV, Tecnativa, ACSONE SA/NV, "
               "Odoo Community Association (OCA)",
     "license": "AGPL-3",
     "application": False,

account_tax_balance/models/account_tax.py

@@ -3,8 +3,7 @@
 # © 2016 Antonio Espinosa <antonio.espinosa@tecnativa.com>
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
 
-from openerp import models, fields, api
-from openerp.tools.safe_eval import safe_eval
+from openerp import models, fields, api, _
 
 
 class AccountTax(models.Model):
@@ -12,57 +11,27 @@ class AccountTax(models.Model):
 
     balance = fields.Float(
         string="Total Balance", compute="_compute_balance",
-        search='_search_balance')
+    )
     base_balance = fields.Float(
         string="Total Base Balance", compute="_compute_balance",
-        search='_search_base_balance')
+    )
     balance_regular = fields.Float(
         string="Balance", compute="_compute_balance",
-        search='_search_balance_regular')
+    )
     base_balance_regular = fields.Float(
         string="Base Balance", compute="_compute_balance",
-        search='_search_base_balance_regular')
+    )
     balance_refund = fields.Float(
         string="Balance Refund", compute="_compute_balance",
-        search='_search_balance_refund')
+    )
     base_balance_refund = fields.Float(
         string="Base Balance Refund", compute="_compute_balance",
-        search='_search_base_balance_refund')
-
-    def _search_balance_field(self, field, operator, value):
-        operators = {'>', '<', '>=', '<=', '!=', '=', '<>'}
-        fields = {
-            'balance', 'base_balance', 'balance_regular',
-            'base_balance_regular', 'balance_refund', 'base_balance_refund',
-        }
-        domain = []
-        if operator in operators and field in fields:
-            value = float(value) if value else 0
-            taxes = self.search([]).filtered(
-                lambda x: safe_eval(
-                    '%.2f %s %.2f' % (x[field], operator, value)))
-            domain.append(('id', 'in', taxes.ids))
-        return domain
-
-    def _search_balance(self, operator, value):
-        return self._search_balance_field('balance', operator, value)
-
-    def _search_base_balance(self, operator, value):
-        return self._search_balance_field('base_balance', operator, value)
-
-    def _search_balance_regular(self, operator, value):
-        return self._search_balance_field('balance_regular', operator, value)
-
-    def _search_base_balance_regular(self, operator, value):
-        return self._search_balance_field(
-            'base_balance_regular', operator, value)
-
-    def _search_balance_refund(self, operator, value):
-        return self._search_balance_field('balance_refund', operator, value)
-
-    def _search_base_balance_refund(self, operator, value):
-        return self._search_balance_field(
-            'base_balance_refund', operator, value)
+    )
+    has_moves = fields.Boolean(
+        string="Has balance in period",
+        compute="_compute_has_moves",
+        search="_search_has_moves",
+    )
 
     def get_context_values(self):
         context = self.env.context
@@ -73,6 +42,52 @@ class AccountTax(models.Model):
             context.get('target_move', 'posted')
         )
 
+    def _account_tax_ids_with_moves(self):
+        """ Return all account.tax ids for which there is at least
+        one account.move.line in the context period
+        for the user company.
+
+        Caveat: this ignores record rules and ACL but it is good
+        enough for filtering taxes with activity during the period.
+        """
+        req = """
+            SELECT id
+            FROM account_tax at
+            WHERE
+            company_id = %s AND
+            EXISTS (
+              SELECT 1 FROM account_move_Line aml
+              WHERE
+                date >= %s AND
+                date <= %s AND
+                company_id = %s AND (
+                  tax_line_id = at.id OR
+                  EXISTS (
+                    SELECT 1 FROM account_move_line_account_tax_rel
+                    WHERE account_move_line_id = aml.id AND
+                      account_tax_id = at.id
+                  )
+                )
+            )
+        """
+        from_date, to_date, company_id, target_move = self.get_context_values()
+        self.env.cr.execute(
+            req, (company_id, from_date, to_date, company_id))
+        return [r[0] for r in self.env.cr.fetchall()]
+
+    @api.multi
+    def _compute_has_moves(self):
+        ids_with_moves = set(self._account_tax_ids_with_moves())
+        for tax in self:
+            tax.has_moves = tax.id in ids_with_moves
+
+    @api.model
+    def _search_has_moves(self, operator, value):
+        if operator != '=' or not value:
+            raise ValueError(_("Unsupported search operator"))
+        ids_with_moves = self._account_tax_ids_with_moves()
+        return [('id', 'in', ids_with_moves)]
+
     def _compute_balance(self):
         for tax in self:
             tax.balance_regular = tax.compute_balance(
@@ -112,13 +127,14 @@ class AccountTax(models.Model):
 
     def compute_balance(self, tax_or_base='tax', move_type=None):
         self.ensure_one()
-        move_lines = self.get_move_lines_domain(
+        domain = self.get_move_lines_domain(
             tax_or_base=tax_or_base, move_type=move_type)
         # balance is debit - credit whereas on tax return you want to see what
         # vat has to be paid so:
         # VAT on sales (credit) - VAT on purchases (debit).
-        total = -sum([l.balance for l in move_lines])
-        return total
+        balance = self.env['account.move.line'].\
+            read_group(domain, ['balance'], [])[0]['balance']
+        return balance and -balance or 0
 
     def get_balance_domain(self, state_list, type_list):
         domain = [
@@ -139,7 +155,6 @@ class AccountTax(models.Model):
         return domain
 
     def get_move_lines_domain(self, tax_or_base='tax', move_type=None):
-        move_line_model = self.env['account.move.line']
         from_date, to_date, company_id, target_move = self.get_context_values()
         state_list = self.get_target_state_list(target_move)
         type_list = self.get_target_type_list(move_type)
@@ -152,16 +167,15 @@ class AccountTax(models.Model):
             balance_domain = self.get_base_balance_domain(
                 state_list, type_list)
         domain.extend(balance_domain)
-        return move_line_model.search(domain)
+        return domain
 
     def get_lines_action(self, tax_or_base='tax', move_type=None):
-        move_lines = self.get_move_lines_domain(
+        domain = self.get_move_lines_domain(
             tax_or_base=tax_or_base, move_type=move_type)
-        move_line_ids = [l.id for l in move_lines]
         action = self.env.ref('account.action_account_moves_all_tree')
         vals = action.read()[0]
         vals['context'] = {}
-        vals['domain'] = [('id', 'in', move_line_ids)]
+        vals['domain'] = domain
         return vals
 
     @api.multi

account_tax_balance/tests/test_account_tax_balance.py

@@ -96,17 +96,22 @@ class TestAccountTaxBalance(TransactionCase):
         self.assertEqual(
             action['xml_id'], 'account_tax_balance.action_tax_balances_tree')
 
+        # exercise search has_moves = True
+        taxes = self.env['account.tax'].search([('has_moves', '=', True)])
+        self.assertEqual(len(taxes), 1)
+        self.assertEqual(taxes[0].name, u"Tax 10.0%")
+
         # testing buttons
         tax_action = tax.view_tax_lines()
         base_action = tax.view_base_lines()
-        self.assertTrue(
-            tax_action['domain'][0][2][0] in
-            [l.id for l in invoice.move_id.line_ids])
+        tax_action_move_lines = self.env['account.move.line'].\
+            search(tax_action['domain'])
+        self.assertTrue(invoice.move_id.line_ids & tax_action_move_lines)
         self.assertEqual(
             tax_action['xml_id'], 'account.action_account_moves_all_tree')
-        self.assertTrue(
-            base_action['domain'][0][2][0] in
-            [l.id for l in invoice.move_id.line_ids])
+        base_action_move_lines = self.env['account.move.line'].\
+            search(base_action['domain'])
+        self.assertTrue(invoice.move_id.line_ids & base_action_move_lines)
         self.assertEqual(
             base_action['xml_id'], 'account.action_account_moves_all_tree')
 

account_tax_balance/views/account_tax_view.xml

@@ -60,10 +60,7 @@
     <field name="res_model">account.tax</field>
     <field name="view_type">form</field>
     <field name="view_mode">tree</field>
-    <field name="domain">[
-        '|', '|', ('base_balance', '!=', 0), ('base_balance_regular', '!=', 0),
-        '|', '|', ('base_balance_refund', '!=', 0), ('balance', '!=', 0),
-        '|', ('balance_regular', '!=', 0), ('balance_refund', '!=', 0)]</field>
+    <field name="domain">[('has_moves', '=', True)]</field>
     <field name="view_id" ref="view_tax_tree_balance"/>
     <field name="search_view_id" ref="view_tax_search_balance"/>
 </record>