From cb19fac5506a8bcbcb51f077552f0277148b0a61 Mon Sep 17 00:00:00 2001
From: jcoux <julien.coux@camptocamp.com>
Date: Fri, 22 Jul 2016 15:23:38 +0200
Subject: [PATCH] Review of OCA General Ledger Report PDF

---
 .../report/__init__.py                        |   1 +
 .../report/general_ledger.py                  | 139 ++++++++++++------
 .../report/templates/general_ledger.xml       |  80 ++++++----
 .../wizard/general_ledger_wizard.py           |  22 +--
 4 files changed, 153 insertions(+), 89 deletions(-)

diff --git a/account_financial_report_qweb/report/__init__.py b/account_financial_report_qweb/report/__init__.py
index 5aa21807..eaa9c1ca 100644
--- a/account_financial_report_qweb/report/__init__.py
+++ b/account_financial_report_qweb/report/__init__.py
@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 # © 2015 Yannick Vaucher (Camptocamp)
 # © 2016 Damien Crier (Camptocamp)
+# © 2016 Julien Coux (Camptocamp)
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).-
 
 from . import aged_partner_balance
diff --git a/account_financial_report_qweb/report/general_ledger.py b/account_financial_report_qweb/report/general_ledger.py
index 120bbf7a..c2bb79ab 100644
--- a/account_financial_report_qweb/report/general_ledger.py
+++ b/account_financial_report_qweb/report/general_ledger.py
@@ -8,10 +8,22 @@ from openerp import models, fields, api
 class GeneralLedgerReport(models.TransientModel):
     """ Here, we just define class fields.
     For methods, go more bottom at this file.
+
+    The class hierarchy is :
+    * GeneralLedgerReport
+    ** GeneralLedgerReportAccount
+    *** GeneralLedgerReportMoveLine
+            For non receivable/payable accounts
+            For receivable/payable centralized accounts
+    *** GeneralLedgerReportPartner
+            For receivable/payable and not centralized accounts
+    **** GeneralLedgerReportMoveLine
+            For receivable/payable and not centralized accounts
     """
 
     _name = 'report_general_ledger_qweb'
 
+    # Filters fields, used for data computation
     date_from = fields.Date()
     date_to = fields.Date()
     fy_start_date = fields.Date()
@@ -20,14 +32,17 @@ class GeneralLedgerReport(models.TransientModel):
     company_id = fields.Many2one(comodel_name='res.company')
     filter_account_ids = fields.Many2many(comodel_name='account.account')
     filter_partner_ids = fields.Many2many(comodel_name='res.partner')
-    has_second_currency = fields.Boolean()
     centralize = fields.Boolean()
+
+    # Flag fields, used for report display
+    has_second_currency = fields.Boolean()
     show_cost_center = fields.Boolean(
         default=lambda self: self.env.user.has_group(
             'analytic.group_analytic_accounting'
         )
     )
 
+    # Data fields, used to browse report data
     account_ids = fields.One2many(
         comodel_name='report_general_ledger_qweb_account',
         inverse_name='report_id'
@@ -44,10 +59,14 @@ class GeneralLedgerReportAccount(models.TransientModel):
         ondelete='cascade',
         index=True
     )
+
+    # Data fields, used to keep link with real object
     account_id = fields.Many2one(
         'account.account',
         index=True
     )
+
+    # Data fields, used for report display
     code = fields.Char()
     name = fields.Char()
     initial_debit = fields.Float(digits=(16, 2))
@@ -56,8 +75,11 @@ class GeneralLedgerReportAccount(models.TransientModel):
     final_debit = fields.Float(digits=(16, 2))
     final_credit = fields.Float(digits=(16, 2))
     final_balance = fields.Float(digits=(16, 2))
+
+    # Flag fields, used for report display and for data computation
     is_partner_account = fields.Boolean()
 
+    # Data fields, used to browse report data
     move_line_ids = fields.One2many(
         comodel_name='report_general_ledger_qweb_move_line',
         inverse_name='report_account_id'
@@ -77,10 +99,14 @@ class GeneralLedgerReportPartner(models.TransientModel):
         ondelete='cascade',
         index=True
     )
+
+    # Data fields, used to keep link with real object
     partner_id = fields.Many2one(
         'res.partner',
         index=True
     )
+
+    # Data fields, used for report display
     name = fields.Char()
     initial_debit = fields.Float(digits=(16, 2))
     initial_credit = fields.Float(digits=(16, 2))
@@ -89,6 +115,7 @@ class GeneralLedgerReportPartner(models.TransientModel):
     final_credit = fields.Float(digits=(16, 2))
     final_balance = fields.Float(digits=(16, 2))
 
+    # Data fields, used to browse report data
     move_line_ids = fields.One2many(
         comodel_name='report_general_ledger_qweb_move_line',
         inverse_name='report_partner_id'
@@ -96,14 +123,15 @@ class GeneralLedgerReportPartner(models.TransientModel):
 
     @api.model
     def _generate_order_by(self, order_spec, query):
+        """Custom order to display "No partner allocated" at last position."""
         return """
 ORDER BY
-CASE
-    WHEN "report_general_ledger_qweb_partner"."partner_id" IS NOT NULL
-    THEN 0
-    ELSE 1
+    CASE
+        WHEN "report_general_ledger_qweb_partner"."partner_id" IS NOT NULL
+        THEN 0
+        ELSE 1
     END,
-"report_general_ledger_qweb_partner"."name"
+    "report_general_ledger_qweb_partner"."name"
         """
 
 
@@ -121,7 +149,11 @@ class GeneralLedgerReportMoveLine(models.TransientModel):
         ondelete='cascade',
         index=True
     )
+
+    # Data fields, used to keep link with real object
     move_line_id = fields.Many2one('account.move.line')
+
+    # Data fields, used for report display
     date = fields.Date()
     entry = fields.Char()
     journal = fields.Char()
@@ -138,57 +170,46 @@ class GeneralLedgerReportMoveLine(models.TransientModel):
 
 
 class GeneralLedgerReportCompute(models.TransientModel):
+    """ Here, we just define methods.
+    For class fields, go more top at this file.
+    """
 
     _inherit = 'report_general_ledger_qweb'
 
-    @api.model
-    def print_report(self):
-        self.ensure_one()
-        self.compute_data_for_report()
-        return {
-            'type': 'ir.actions.report.xml',
-            'report_name':
-                'account_financial_report_qweb.report_general_ledger_qweb',
-            'datas': {'ids': [self.id]},
-        }
-
-    @api.model
-    def print_report_xlsx(self):
+    @api.multi
+    def print_report(self, xlsx_report=False):
         self.ensure_one()
         self.compute_data_for_report()
-
+        if xlsx_report:
+            report_name = 'ledger.report.wizard.xlsx'
+        else:
+            report_name = 'account_financial_report_qweb.' \
+                          'report_general_ledger_qweb'
         return {
             'type': 'ir.actions.report.xml',
-            'name': 'export xlsx general ledger',
-            'model': self._name,
-            'report_name': 'ledger.report.wizard.xlsx',
-            'report_type': 'xlsx',
-            'context': self.env.context,
+            'report_name': report_name,
             'datas': {'ids': [self.id]},
         }
 
     @api.multi
-    def _print_report_xlsx(self, data):
-        return
-
-
-    @api.model
     def compute_data_for_report(self):
         self.ensure_one()
-
-        self.inject_account_values()
-        self.inject_partner_values()
-        self.inject_line_not_centralized_values()
-        self.inject_line_not_centralized_values(is_account_line=False,
-                                                is_partner_line=True)
-        self.inject_line_not_centralized_values(is_account_line=False,
-                                                is_partner_line=True,
-                                                only_empty_partner_line=True)
+        # Compute report data
+        self._inject_account_values()
+        self._inject_partner_values()
+        self._inject_line_not_centralized_values()
+        self._inject_line_not_centralized_values(is_account_line=False,
+                                                 is_partner_line=True)
+        self._inject_line_not_centralized_values(is_account_line=False,
+                                                 is_partner_line=True,
+                                                 only_empty_partner_line=True)
         if self.centralize:
-            self.inject_line_centralized_values()
-        self.compute_has_second_currency()
+            self._inject_line_centralized_values()
+        # Compute display flag
+        self._compute_has_second_currency()
 
-    def inject_account_values(self):
+    def _inject_account_values(self):
+        """Inject report values for report_general_ledger_qweb_account."""
         subquery_sum_amounts = """
         SELECT
             a.id AS account_id,
@@ -332,7 +353,11 @@ AND
         )
         self.env.cr.execute(query_inject_account, query_inject_account_params)
 
-    def inject_partner_values(self):
+    def _inject_partner_values(self):
+        """ Inject report values for report_general_ledger_qweb_partner.
+
+        Only for "partner" accounts (payable and receivable).
+        """
         subquery_sum_amounts = """
             SELECT
                 ap.account_id AS account_id,
@@ -494,10 +519,21 @@ AND
         print query_inject_partner_params
         self.env.cr.execute(query_inject_partner, query_inject_partner_params)
 
-    def inject_line_not_centralized_values(self,
-                                           is_account_line=True,
-                                           is_partner_line=False,
-                                           only_empty_partner_line=False):
+    def _inject_line_not_centralized_values(self,
+                                            is_account_line=True,
+                                            is_partner_line=False,
+                                            only_empty_partner_line=False):
+        """ Inject report values for report_general_ledger_qweb_move_line.
+
+        If centralized option have been chosen,
+        only non centralized accounts are computed.
+
+        In function of `is_account_line` and `is_partner_line` values,
+        the move_line link is made either with account or either with partner.
+
+        The "only_empty_partner_line" value is used
+        to compute data without partner.
+        """
         query_inject_move_line = """
 INSERT INTO
     report_general_ledger_qweb_move_line
@@ -692,7 +728,11 @@ ORDER BY
              self.date_to,)
         )
 
-    def inject_line_centralized_values(self):
+    def _inject_line_centralized_values(self):
+        """ Inject report values for report_general_ledger_qweb_move_line.
+
+        Only centralized accounts are computed.
+        """
         query_inject_move_line_centralized = """
 WITH
     move_lines AS
@@ -780,7 +820,8 @@ ORDER BY
              self.id,)
         )
 
-    def compute_has_second_currency(self):
+    def _compute_has_second_currency(self):
+        """ Compute "has_second_currency" flag which will used for display."""
         query_update_has_second_currency = """
 UPDATE
     report_general_ledger_qweb
diff --git a/account_financial_report_qweb/report/templates/general_ledger.xml b/account_financial_report_qweb/report/templates/general_ledger.xml
index fe19bb16..c5ca6ea7 100644
--- a/account_financial_report_qweb/report/templates/general_ledger.xml
+++ b/account_financial_report_qweb/report/templates/general_ledger.xml
@@ -4,59 +4,51 @@
     <template id="account_financial_report_qweb.report_general_ledger_qweb">
         <t t-call="report.html_container">
             <t t-foreach="docs" t-as="o">
+                <!-- Saved flag fields into variables, used to define columns display -->
                 <t t-set="show_cost_center" t-value="o.show_cost_center"/>
                 <t t-set="has_second_currency" t-value="o.has_second_currency"/>
 
                 <t t-call="account_financial_report_qweb.internal_layout">
+                    <!-- Defines global variables used by internal layout -->
                     <t t-set="title" t-value='"General Ledger"'/>
                     <t t-set="company_name" t-value="o.company_id.name"/>
+
                     <div class="page">
-                        <div class="act_as_table data_table" style="width: 1140px !important;">
-                            <div class="act_as_row labels">
-                                <div class="act_as_cell">Date range filter</div>
-                                <div class="act_as_cell">Target moves filter</div>
-                                <div class="act_as_cell">Account balance at 0 filter</div>
-                                <div class="act_as_cell">Centralize filter</div>
-                            </div>
-                            <div class="act_as_row">
-                                <div class="act_as_cell">
-                                    From: <span t-field="o.date_from"/> To: <span t-field="o.date_to"/>
-                                </div>
-                                <div class="act_as_cell">
-                                    <t t-if="o.only_posted_moves">All posted entries</t>
-                                    <t t-if="not o.only_posted_moves">All entries</t>
-                                </div>
-                                <div class="act_as_cell">
-                                    <t t-if="o.hide_account_balance_at_0">Hide</t>
-                                    <t t-if="not o.hide_account_balance_at_0">Show</t>
-                                </div>
-                                <div class="act_as_cell">
-                                    <t t-if="o.centralize">Yes</t>
-                                    <t t-if="not o.centralize">No</t>
-                                </div>
-                            </div>
-                        </div>
+                        <!-- Display filters -->
+                        <t t-call="account_financial_report_qweb.report_general_ledger_qweb_filters"/>
+
                         <t t-foreach="o.account_ids" t-as="account">
                             <div class="page_break">
+                                <!-- Display account header -->
                                 <div class="act_as_table list_table" style="margin-top: 10px;"/>
                                 <div class="act_as_caption account_title" style="width: 1141px !important;">
                                     <span t-field="account.code"/> - <span t-field="account.name"/>
                                 </div>
+
+                                <!-- Display account move lines -->
                                 <t t-if="account.move_line_ids">
                                     <t t-call="account_financial_report_qweb.report_general_ledger_qweb_lines">
                                         <t t-set="account_or_partner_object" t-value="account"/>
                                     </t>
                                 </t>
+
                                 <t t-if="not account.move_line_ids">
+
+                                    <!-- Display account partners -->
                                     <t t-if="account.is_partner_account">
                                         <t t-foreach="account.partner_ids" t-as="partner">
                                             <div class="page_break">
+                                                <!-- Display partner header -->
                                                 <div class="act_as_caption account_title">
                                                     <span t-field="partner.name"/>
                                                 </div>
+
+                                                <!-- Display partner move lines -->
                                                 <t t-call="account_financial_report_qweb.report_general_ledger_qweb_lines">
                                                     <t t-set="account_or_partner_object" t-value="partner"/>
                                                 </t>
+
+                                                <!-- Display partner footer -->
                                                 <t t-call="account_financial_report_qweb.report_general_ledger_qweb_ending_cumul">
                                                     <t t-set="account_or_partner_object" t-value="partner"/>
                                                     <t t-set="type" t-value='"partner_type"'/>
@@ -65,6 +57,8 @@
                                         </t>
                                     </t>
                                 </t>
+
+                                <!-- Display account footer -->
                                 <t t-call="account_financial_report_qweb.report_general_ledger_qweb_ending_cumul">
                                     <t t-set="account_or_partner_object" t-value="account"/>
                                     <t t-set="type" t-value='"account_type"'/>
@@ -77,8 +71,38 @@
         </t>
     </template>
 
+    <template id="account_financial_report_qweb.report_general_ledger_qweb_filters">
+        <div class="act_as_table data_table" style="width: 1140px !important;">
+            <div class="act_as_row labels">
+                <div class="act_as_cell">Date range filter</div>
+                <div class="act_as_cell">Target moves filter</div>
+                <div class="act_as_cell">Account balance at 0 filter</div>
+                <div class="act_as_cell">Centralize filter</div>
+            </div>
+            <div class="act_as_row">
+                <div class="act_as_cell">
+                    From: <span t-field="o.date_from"/> To: <span t-field="o.date_to"/>
+                </div>
+                <div class="act_as_cell">
+                    <t t-if="o.only_posted_moves">All posted entries</t>
+                    <t t-if="not o.only_posted_moves">All entries</t>
+                </div>
+                <div class="act_as_cell">
+                    <t t-if="o.hide_account_balance_at_0">Hide</t>
+                    <t t-if="not o.hide_account_balance_at_0">Show</t>
+                </div>
+                <div class="act_as_cell">
+                    <t t-if="o.centralize">Yes</t>
+                    <t t-if="not o.centralize">No</t>
+                </div>
+            </div>
+        </div>
+    </template>
+
     <template id="account_financial_report_qweb.report_general_ledger_qweb_lines">
         <div class="act_as_table data_table" style="width: 1140px !important;">
+
+            <!-- Display table headers for lines -->
             <div class="act_as_thead">
                 <div class="act_as_row labels">
                     <!--## date-->
@@ -113,7 +137,8 @@
                     </t>
                 </div>
             </div>
-            <!-- # initial_balance line -->
+
+            <!-- Display first line with initial balance -->
             <div class="act_as_row lines">
                 <!--## date-->
                 <div class="act_as_cell"></div>
@@ -146,6 +171,8 @@
                     <div class="act_as_cell"></div>
                 </t>
             </div>
+
+            <!-- Display each lines -->
             <t t-foreach="account_or_partner_object.move_line_ids" t-as="line">
                 <!-- # lines or centralized lines -->
                 <div class="act_as_row lines">
@@ -191,6 +218,7 @@
     </template>
 
     <template id="account_financial_report_qweb.report_general_ledger_qweb_ending_cumul">
+        <!-- Display ending balance line for account or partner -->
         <div class="act_as_table list_table" style="width: 1141px !important;">
             <div class="act_as_row labels" style="font-weight: bold;">
                 <!--## date-->
diff --git a/account_financial_report_qweb/wizard/general_ledger_wizard.py b/account_financial_report_qweb/wizard/general_ledger_wizard.py
index 3f9c7947..0785409a 100644
--- a/account_financial_report_qweb/wizard/general_ledger_wizard.py
+++ b/account_financial_report_qweb/wizard/general_ledger_wizard.py
@@ -81,22 +81,16 @@ class GeneralLedgerReportWizard(models.TransientModel):
 
     @api.multi
     def button_export_pdf(self):
-        model = self.env['report_general_ledger_qweb']
-        report = model.create({
-            'date_from': self.date_from,
-            'date_to': self.date_to,
-            'only_posted_moves': self.target_move == 'posted',
-            'hide_account_balance_at_0': self.hide_account_balance_at_0,
-            'company_id': self.company_id.id,
-            'filter_account_ids': [(6, 0, self.account_ids.ids)],
-            'filter_partner_ids': [(6, 0, self.partner_ids.ids)],
-            'centralize': self.centralize,
-            'fy_start_date': self.fy_start_date,
-        })
-        return report.print_report()
+        self.ensure_one()
+        return self._export()
 
     @api.multi
     def button_export_xlsx(self):
+        self.ensure_one()
+        return self._export(xlsx_report=True)
+
+    def _export(self, xlsx_report=False):
+        """Default export is PDF."""
         model = self.env['report_general_ledger_qweb']
         report = model.create({
             'date_from': self.date_from,
@@ -109,4 +103,4 @@ class GeneralLedgerReportWizard(models.TransientModel):
             'centralize': self.centralize,
             'fy_start_date': self.fy_start_date,
         })
-        return report.print_report_xlsx()
+        return report.print_report(xlsx_report)