Merge lp:~therp-nl/banking-addons/ba70-lp1295163-refactor_period_lookup into lp:banking-addons

Proposed by Stefan Rijnhart (Opener) on 2014-03-20
Status: Merged
Approved by: Holger Brunn (Therp) on 2014-04-07
Approved revision: 236
Merged at revision: 250
Proposed branch: lp:~therp-nl/banking-addons/ba70-lp1295163-refactor_period_lookup
Merge into: lp:banking-addons
Diff against target: 167 lines (+45/-42)
2 files modified
account_banking/account_banking.py (+44/-41)
account_banking/banking_import_transaction.py (+1/-1)
To merge this branch: bzr merge lp:~therp-nl/banking-addons/ba70-lp1295163-refactor_period_lookup
Reviewer Review Type Date Requested Status
Ruchir Shukla(BizzAppDev) (community) Needs Information on 2014-04-23
Lara (Therp) test 2014-03-20 Approve on 2014-04-02
Holger Brunn (Therp) code review Approve on 2014-03-31
Pedro Manuel Baeza code review Approve on 2014-03-20
Review via email: mp+211970@code.launchpad.net
To post a comment you must log in.
Pedro Manuel Baeza (pedro.baeza) wrote :

LGTM

Regards.

review: Approve (code review)
236. By Stefan Rijnhart (Opener) on 2014-03-23

[FIX] Workaround for lp:1296229, defaults functions are passed context
 as a positional argument

review: Approve (code review)
Lara (Therp) (lfreeke) :
review: Approve (test)

+ except except_osv:
59 + if date:
60 + raise

This is something not needed .
as there date field is already required field for statement and statement lines .
Can you specify in which condition it will not have value date variable?

review: Needs Information

@ruchir thanks for the review! The answer to your question is: the date is not specified when called as a default function.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'account_banking/account_banking.py'
2--- account_banking/account_banking.py 2014-03-15 15:43:08 +0000
3+++ account_banking/account_banking.py 2014-03-23 10:07:22 +0000
4@@ -298,17 +298,24 @@
5 def _check_company_id(self, cr, uid, ids, context=None):
6 """
7 Adapt this constraint method from the account module to reflect the
8- move of period_id to the statement line
9+ move of period_id to the statement line: also check the periods of the
10+ lines. Update the statement period if it does not have one yet.
11+ Don't call super, because its check is integrated below and
12+ it will break if a statement does not have any lines yet and
13+ therefore may not have a period.
14 """
15 for statement in self.browse(cr, uid, ids, context=context):
16+ if (statement.period_id and
17+ statement.company_id != statement.period_id.company_id):
18+ return False
19 for line in statement.line_ids:
20 if (line.period_id and
21- statement.company_id.id != line.period_id.company_id.id):
22+ statement.company_id != line.period_id.company_id):
23 return False
24 if not statement.period_id:
25 statement.write({'period_id': line.period_id.id})
26- return super(account_bank_statement, self)._check_company_id(
27- cr, uid, ids, context=context)
28+ statement.refresh()
29+ return True
30
31 # Redefine the constraint, or it still refer to the original method
32 _constraints = [
33@@ -317,13 +324,24 @@
34 ['journal_id','period_id']),
35 ]
36
37- def _get_period(self, cr, uid, date, context=None):
38- '''
39- Find matching period for date, not meant for _defaults.
40- '''
41- period_obj = self.pool.get('account.period')
42- periods = period_obj.find(cr, uid, dt=date, context=context)
43- return periods and periods[0] or False
44+ def _get_period(self, cr, uid, date=False, context=None):
45+ """
46+ Used in statement line's _defaults, so it is always triggered
47+ on installation or module upgrade even if there are no records
48+ without a value. For that reason, we need
49+ to be tolerant and allow for the situation in which no period
50+ exists for the current date (i.e. when no date is specified).
51+
52+ Cannot be used directly as a defaults method due to lp:1296229
53+ """
54+ local_ctx = dict(context or {}, account_period_prefer_normal=True)
55+ try:
56+ return self.pool.get('account.period').find(
57+ cr, uid, dt=date, context=local_ctx)[0]
58+ except except_osv:
59+ if date:
60+ raise
61+ return False
62
63 def _prepare_move(
64 self, cr, uid, st_line, st_line_number, context=None):
65@@ -368,7 +386,7 @@
66 # Take period from statement line and write to context
67 # this will be picked up by the _prepare_move* methods
68 period_id = self._get_period(
69- cr, uid, st_line.date, context=context)
70+ cr, uid, date=st_line.date, context=context)
71 localctx = context.copy()
72 localctx['period_id'] = period_id
73
74@@ -424,7 +442,8 @@
75 context=context)
76 for st in self.browse(cr, uid, noname_ids, context=context):
77 if st.journal_id.sequence_id:
78- period_id = self._get_period(cr, uid, st.date)
79+ period_id = self._get_period(
80+ cr, uid, date=st.date, context=context)
81 year = self.pool.get('account.period').browse(
82 cr, uid, period_id, context=context).fiscalyear_id.id
83 c = {'fiscalyear_id': year}
84@@ -436,8 +455,6 @@
85 return super(account_bank_statement, self).button_confirm_bank(
86 cr, uid, ids, context)
87
88-account_bank_statement()
89-
90
91 class account_voucher(orm.Model):
92 _inherit = 'account.voucher'
93@@ -446,12 +463,11 @@
94 if context is None:
95 context = {}
96 if not context.get('period_id') and context.get('move_line_ids'):
97- return self.pool.get('account.move.line').browse(
98- cr, uid , context.get('move_line_ids'), context=context)[0].period_id.id
99+ move_line = self.pool.get('account.move.line').browse(
100+ cr, uid , context.get('move_line_ids')[0], context=context)
101+ return move_line.period_id.id
102 return super(account_voucher, self)._get_period(cr, uid, context)
103
104-account_voucher()
105-
106
107 class account_bank_statement_line(orm.Model):
108 '''
109@@ -464,28 +480,15 @@
110 _inherit = 'account.bank.statement.line'
111 _description = 'Bank Transaction'
112
113- def _get_period(self, cr, uid, context=None):
114- """
115- Get a non-opening period for today or a date specified in
116- the context.
117+ def _get_period(self, cr, uid, date=False, context=None):
118+ return self.pool['account.bank.statement']._get_period(
119+ cr, uid, date=date, context=context)
120
121- Used in this model's _defaults, so it is always triggered
122- on installation or module upgrade. For that reason, we need
123- to be tolerant and allow for the situation in which no period
124- exists for the current date (i.e. when no date is specified).
125- """
126- if context is None:
127- context = {}
128- date = context.get('date', False)
129- local_ctx = dict(context)
130- local_ctx['account_period_prefer_normal'] = True
131- try:
132- return self.pool.get('account.period').find(
133- cr, uid, dt=date, context=local_ctx)[0]
134- except except_osv:
135- if date:
136- raise
137- return False
138+ def _get_period_context(self, cr, uid, context=None):
139+ """
140+ Workaround for lp:1296229, context is passed positionally
141+ """
142+ return self._get_period(cr, uid, context=context)
143
144 def _get_currency(self, cr, uid, context=None):
145 '''
146@@ -549,7 +552,7 @@
147 }
148
149 _defaults = {
150- 'period_id': _get_period,
151+ 'period_id': _get_period_context,
152 'currency': _get_currency,
153 }
154
155
156=== modified file 'account_banking/banking_import_transaction.py'
157--- account_banking/banking_import_transaction.py 2014-01-21 07:55:55 +0000
158+++ account_banking/banking_import_transaction.py 2014-03-23 10:07:22 +0000
159@@ -1576,7 +1576,7 @@
160 self.write(
161 cr, uid, [st_line.id], {
162 'period_id': self._get_period(
163- cr, uid, {'date': st_line.date})
164+ cr, uid, date=st_line.date, context=context)
165 })
166 st_line.refresh()
167 # Generate the statement number, if it is not already done

Subscribers

People subscribed via source and target branches

to status/vote changes: