Account - Financial Tools

Merge lp:~camptocamp/account-financial-tools/credit_control_report_improvement_vre into lp:~account-core-editors/account-financial-tools/7.0

  1. credit_control_report_improvement_vre
  2. Merge into 7.0
Proposed by Vincent Renaville@camptocamp
Status: Rejected
Rejected by: Yannick Vaucher @ Camptocamp
Proposed branch: lp:~camptocamp/account-financial-tools/credit_control_report_improvement_vre
Merge into: lp:~account-core-editors/account-financial-tools/7.0
Diff against target: 240 lines (+124/-55)
1 file modified
account_credit_control/report/credit_control_summary.html.mako (+124/-55)
To merge this branch: bzr merge lp:~camptocamp/account-financial-tools/credit_control_report_improvement_vre
Reviewer Review Type Date Requested Status
Nicolas Bessi - Camptocamp (community) Disapprove
Leonardo Pistone code review Needs Information
Review via email: mp+201920@code.launchpad.net

Description of the change

Improvement of layout of report, on address format and add banking information related to the invoice linked

To post a comment you must log in.
Revision history for this message
Leonardo Pistone (lepistone) wrote :

Vincent,

If I get it correctly, you use credit_control_line_ids[0] because normally there is only one line. This is ok for me if it has a comment saying that.

Otherwise LGTM, thanks!

review: Needs Information (code review)
Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

 LGTM, Thanks

review: Approve (code review, no test)
Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

Sorry I have to withdraw my approve,

After proofreading the credit communication code I can not accept you patch.

A credit communication is an aggregation of credit lines per partner per policy level.

That means related credit lines can be related to many invoices that have the same level of overdue.

So the way to determine the invoice is not correct, also you should use the communication partner_id address not the one of the invoice. The adress is deternined using the function get_contact_address that will return the invoice address first if available.

Also it is possible to have credit line without invoices so you have to manage this usecase.

Regards

Nicolas

review: Needs Fixing
Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) :
review: Disapprove
Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote :

Following Nicolas comment, I reject this MP

Unmerged revisions

148. By Vincent Renaville@camptocamp

[FIX] change hard-coded address

147. By Vincent Renaville@camptocamp

[IMP] layout of report, on address format and add banking information related to the invoice linked

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'account_credit_control/report/credit_control_summary.html.mako'
2--- account_credit_control/report/credit_control_summary.html.mako 2013-11-21 15:44:48 +0000
3+++ account_credit_control/report/credit_control_summary.html.mako 2014-01-16 12:50:15 +0000
4@@ -4,17 +4,15 @@
5 <style type="text/css">
6 ${css}
7 body {
8- font-family: helvetica;
9 font-size: 12px;
10 }
11
12 .custom_text {
13- font-family: helvetica;
14 font-size: 12px;
15+ text-align:justify;
16 }
17
18 table {
19- font-family: helvetica;
20 font-size: 12px;
21 }
22
23@@ -34,7 +32,6 @@
24 text-align: center;
25 border: 1px solid lightGrey;
26 border-collapse: collapse;
27- font-family: helvetica;
28 font-size: 12px;
29 }
30
31@@ -110,6 +107,27 @@
32 .address td.name {
33 font-weight: bold;
34 }
35+.list_bank_table {
36+ text-align:center;
37+ border-collapse: collapse;
38+}
39+.list_bank_table th {
40+ background-color: #EEEEEE;
41+ text-align:left;
42+ font-size:12;
43+ font-weight:bold;
44+ padding-right:3px;
45+ padding-left:3px;
46+}
47+.list_bank_table td {
48+ text-align:left;
49+ font-size:12;
50+ padding-right:3px;
51+ padding-left:3px;
52+ padding-top:3px;
53+ padding-bottom:3px;
54+}
55+
56
57 td.amount, th.amount {
58 text-align: right;
59@@ -135,42 +153,68 @@
60 <body>
61
62 %for comm in objects :
63+ <%
64+ inv_bank = comm.credit_control_line_ids[0].invoice_id.partner_bank_id
65+ inv = comm.credit_control_line_ids[0].invoice_id
66+ %>
67 <% setLang(comm.get_contact_address().lang) %>
68 <div class="address">
69- <table class="recipient">
70- <%
71- add = comm.get_contact_address()
72- %>
73- %if comm.partner_id.id == add.id:
74- <tr><td class="name">${comm.partner_id.title and comm.partner_id.title.name or ''} ${comm.partner_id.name }</td></tr>
75- <% address_lines = comm.partner_id.contact_address.split("\n") %>
76-
77- %else:
78- <tr><td class="name">${comm.partner_id.name or ''}</td></tr>
79- <tr><td>${add.title and add.title.name or ''} ${add.name}</td></tr>
80- <% address_lines = add.contact_address.split("\n")[1:] %>
81- %endif
82- %for part in address_lines:
83- %if part:
84- <tr><td>${part}</td></tr>
85- %endif
86- %endfor
87- </table>
88- <br/>
89- <br/>
90- <br/>
91- <br/>
92-
93- </div>
94- <br/>
95- <br/>
96- <br/>
97+ %if hasattr(inv, 'commercial_partner_id'):
98+ <table class="recipient" >
99+ %if inv.partner_id.id != inv.commercial_partner_id.id:
100+ <tr><td class="name">${inv.commercial_partner_id.name or '' }</td></tr>
101+ <tr><td>${inv.partner_id.title and inv.partner_id.title.name or ''} ${inv.partner_id.name }</td></tr>
102+ %else:
103+ <tr><td class="name">${inv.partner_id.title and inv.partner_id.title.name or ''} ${inv.partner_id.name}</td></tr>
104+ %endif
105+ %for part in inv.partner_id.contact_address.split("\n")[0:]:
106+ %if part:
107+ <tr><td>${part}</td></tr>
108+ %endif
109+ %endfor
110+ </table>
111+ %else:
112+ <table class="recipient">
113+ %if inv.partner_id.parent_id:
114+ <tr><td class="name">${inv.partner_id.parent_id.name or ''}</td></tr>
115+ <tr><td>${inv.partner_id.title and inv.partner_id.title.name or ''} ${inv.partner_id.name }</td></tr>
116+ %else:
117+ <tr><td class="name">${inv.partner_id.title and inv.partner_id.title.name or ''} ${inv.partner_id.name }</td></tr>
118+ %endif
119+ %for part in inv.partner_id.contact_address.split("\n")[0:]:
120+ %if part:
121+ <tr><td>${part}</td></tr>
122+ %endif
123+ %endfor
124+ </table>
125+ %endif
126+ <br/>
127+ <br/>
128+ <br/>
129+ <br/>
130+
131+ </div>
132+ <br/>
133+ <br/>
134+ <br/>
135+ <br/>
136+ <br/>
137+ <br/>
138+ <br/>
139+ <br/>
140+ <br/>
141+ <div class="address">
142+ <table class="recipient">
143+ <tr><td style="text-align:right;">${comm.company_id.city}, ${comm.credit_control_line_ids[0].date}</td></tr>
144+ </table>
145+
146+ </div>
147 <div>
148
149 <h3 style="clear: both; padding-top: 20px;">
150 ${_('Reminder')}: ${comm.current_policy_level.name or '' }
151 </h3>
152-
153+ <br/>
154 <p>${_('Dear')},</p>
155 <p class="custom_text" width="95%">${comm.current_policy_level.custom_text.replace('\n', '<br />')}</p>
156
157@@ -203,37 +247,62 @@
158 <td class="date">${line.date_due}</td>
159 <td class="amount">${line.amount_due}</td>
160 <td class="amount">${line.balance_due}</td>
161- <td class="amount">${line.currency_id.name or comm.company_id.currency_id.name}</td>
162+ <td class="amount">${line.currency_id.name or inv.company_id.currency_id.name}</td>
163 </tr>
164 %endfor
165 </table>
166 <br/>
167 <br/>
168+ <table class="list_bank_table" width="100%" >
169+ <!-- vat value are taken back from commercial id -->
170+ <tr>
171+ <th style="width:20%;">${_("Bank")}</th>
172+ <td style="width:30%;text-align:left;">${inv_bank and inv_bank.bank_name or '-' } </td>
173+ %if inv.partner_id and inv.partner_id.vat :
174+ <th style="width:20%;">${_("Customer VAT No")}</th>
175+ <td style="width:30%;">${inv.partner_id.vat or '-'}</td>
176+ %else:
177+ <!-- conserve table's cells widths -->
178+ <td style="width:20%;"></td>
179+ <td style="width:30%;"></td>
180+ %endif
181+ </tr>
182+ <tr>
183+ <th style="width:20%;">${_("Bank account")}</th>
184+ <td style="width:50%;text-align:left;">${ inv_bank and inv_bank.acc_number or '-' }</td>
185+ </tr>
186+ <tr>
187+ <th width="20%">${_("BIC")}</th>
188+ <td style="width:30%;">${inv_bank and inv_bank.bank_bic or '-' }</td>
189+ </tr>
190+ </table>
191+ <br/>
192+ <br/>
193 <%doc>
194 <!-- uncomment to have info after summary -->
195 <p>${_('If you have any question, do not hesitate to contact us.')}</p>
196
197- <p>${comm.user_id.name} ${comm.user_id.email and '<%s>'%(comm.user_id.email) or ''}<br/>
198- ${comm.company_id.name}<br/>
199- % if comm.company_id.street:
200- ${comm.company_id.street or ''}<br/>
201-
202- % endif
203-
204- % if comm.company_id.street2:
205- ${comm.company_id.street2}<br/>
206- % endif
207- % if comm.company_id.city or comm.company_id.zip:
208- ${comm.company_id.zip or ''} ${comm.company_id.city or ''}<br/>
209- % endif
210- % if comm.company_id.country_id:
211- ${comm.company_id.state_id and ('%s, ' % comm.company_id.state_id.name) or ''} ${comm.company_id.country_id.name or ''}<br/>
212- % endif
213- % if comm.company_id.phone:
214- Phone: ${comm.company_id.phone}<br/>
215- % endif
216- % if comm.company_id.website:
217- ${comm.company_id.website or ''}<br/>
218+ <p>${inv.user_id.name} ${inv.user_id.email and '<%s>'%(inv.user_id.email) or ''}<br/>
219+ ${inv.company_id.name}<br/>
220+ % if inv.company_id.street:
221+ ${inv.company_id.street or ''}<br/>
222+
223+ % endif
224+
225+ % if inv.company_id.street2:
226+ ${inv.company_id.street2}<br/>
227+ % endif
228+ % if inv.company_id.city or inv.company_id.zip:
229+ ${inv.company_id.zip or ''} ${inv.company_id.city or ''}<br/>
230+ % endif
231+ % if inv.company_id.country_id:
232+ ${inv.company_id.state_id and ('%s, ' % inv.company_id.state_id.name) or ''} ${inv.company_id.country_id.name or ''}<br/>
233+ % endif
234+ % if inv.company_id.phone:
235+ Phone: ${inv.company_id.phone}<br/>
236+ % endif
237+ % if inv.company_id.website:
238+ ${inv.company_id.website or ''}<br/>
239 % endif
240 </%doc>
241

Subscribers

People subscribed via source and target branches