Merge lp:~openerp-dev/openobject-server/7.0-sequence-next-fix-csn into lp:openobject-server/7.0

Proposed by Cedric Snauwaert (OpenERP) on 2013-05-17
Status: Merged
Merged at revision: 4978
Proposed branch: lp:~openerp-dev/openobject-server/7.0-sequence-next-fix-csn
Merge into: lp:openobject-server/7.0
Diff against target: 129 lines (+49/-10)
2 files modified
openerp/addons/base/ir/ir_sequence.py (+47/-8)
openerp/addons/base/ir/ir_sequence_view.xml (+2/-2)
To merge this branch: bzr merge lp:~openerp-dev/openobject-server/7.0-sequence-next-fix-csn
Reviewer Review Type Date Requested Status
Olivier Dony (Odoo) 2013-05-17 Approve on 2013-05-17
qdp (OpenERP) 2013-05-17 Pending
Review via email: mp+164363@code.launchpad.net

Description of the change

Change in ir_sequence to have information on current_number and better warning message in case the format of prefix/suffix isn't correct

To post a comment you must log in.
4978. By Cedric Snauwaert (OpenERP) on 2013-05-17

[FIX]ir_sequence: better args definition

Olivier Dony (Odoo) (odo-openerp) wrote :

Looks good to me except the default value of number_next when passed to _alter_sequence(), that should be None rather than 0, otherwise you're forbidding resetting the sequence to 0, which is perfectly legit.

Note: the patch needs to be credited to Therp, as you reused a large part of their merge prop. We can do that when merging, but please be careful to do that at commit-level directly next time, this is important.

review: Needs Fixing
Olivier Dony (Odoo) (odo-openerp) wrote :

I see that the number_next default to None is already fixed, thanks!

review: Approve
Olivier Dony (Odoo) (odo-openerp) wrote :

Merged. I also changed the label from "Actual Next Number" back to "Next Number" because `Actual` conveys a sense of exactness while this value is really not guaranteed to be correct. It might have made sense in the original patch because both fields were displayed, but it does not here. Also improve the tooltip.

Olivier Dony (Odoo) (odo-openerp) wrote :

Additionally, the code had to be fixed to:
- provide a proper default value for number_next_actual during creation (0 is not a valid default)
- avoid crashing during creation because the new sequence is altered before being actually created (the ir.sequence ID is used to name the sequence, so the ir.sequence record needs to be created before the PG sequence)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'openerp/addons/base/ir/ir_sequence.py'
2--- openerp/addons/base/ir/ir_sequence.py 2013-04-19 15:49:20 +0000
3+++ openerp/addons/base/ir/ir_sequence.py 2013-05-17 15:48:24 +0000
4@@ -54,6 +54,34 @@
5 """
6 _name = 'ir.sequence'
7 _order = 'name'
8+
9+ def _get_number_next_actual(self, cr, user, ids, field_name, arg, context=None):
10+ '''Return number from ir_sequence row when no_gap implementation,
11+ and number from postgres sequence when standard implementation.'''
12+ res = dict.fromkeys(ids)
13+ for element in self.browse(cr, user, ids, context=context):
14+ if element.implementation != 'standard':
15+ res[element.id] = element.number_next
16+ else:
17+ # get number from postgres sequence. Cannot use
18+ # currval, because that might give an error when
19+ # not having used nextval before.
20+ statement = (
21+ "SELECT last_value, increment_by, is_called"
22+ " FROM ir_sequence_%03d"
23+ % element.id)
24+ cr.execute(statement)
25+ (last_value, increment_by, is_called) = cr.fetchone()
26+ if is_called:
27+ res[element.id] = last_value + increment_by
28+ else:
29+ res[element.id] = last_value
30+ return res
31+
32+ def _set_number_next_actual(self, cr, uid, id, name, value, args=None, context=None):
33+ return self.write(cr, uid, id, {'number_next': value or 0}, context=context)
34+
35+
36 _columns = {
37 'name': openerp.osv.fields.char('Name', size=64, required=True),
38 'code': openerp.osv.fields.selection(_code_get, 'Code', size=64),
39@@ -67,6 +95,7 @@
40 'prefix': openerp.osv.fields.char('Prefix', size=64, help="Prefix value of the record for the sequence"),
41 'suffix': openerp.osv.fields.char('Suffix', size=64, help="Suffix value of the record for the sequence"),
42 'number_next': openerp.osv.fields.integer('Next Number', required=True, help="Next number of this sequence"),
43+ 'number_next_actual': openerp.osv.fields.function(_get_number_next_actual, fnct_inv=_set_number_next_actual, type='integer', required=True, string='Actual Next Number', help='Next number that will be used. Number is only informative and can differ from real number'),
44 'number_increment': openerp.osv.fields.integer('Increment Number', required=True, help="The next number of the sequence will be incremented by this number"),
45 'padding' : openerp.osv.fields.integer('Number Padding', required=True, help="OpenERP will automatically adds some '0' on the left of the 'Next Number' to get the required padding size."),
46 'company_id': openerp.osv.fields.many2one('res.company', 'Company'),
47@@ -121,7 +150,7 @@
48 # object depends on it.
49 cr.execute("DROP SEQUENCE IF EXISTS %s RESTRICT " % names)
50
51- def _alter_sequence(self, cr, id, number_increment, number_next):
52+ def _alter_sequence(self, cr, id, number_increment, number_next=None):
53 """ Alter a PostreSQL sequence.
54
55 There is no access rights check.
56@@ -129,9 +158,10 @@
57 if number_increment == 0:
58 raise osv.except_osv(_('Warning!'),_("Increment number must not be zero."))
59 assert isinstance(id, (int, long))
60- cr.execute("""
61- ALTER SEQUENCE ir_sequence_%03d INCREMENT BY %%s RESTART WITH %%s
62- """ % id, (number_increment, number_next))
63+ statement = ("ALTER SEQUENCE ir_sequence_%03d INCREMENT BY %d" % (id, number_increment))
64+ if number_next is not None:
65+ statement += " RESTART WITH %d" % (number_next, )
66+ cr.execute(statement)
67
68 def create(self, cr, uid, values, context=None):
69 """ Create a sequence, in implementation == standard a fast gaps-allowed PostgreSQL sequence is used.
70@@ -160,7 +190,13 @@
71 n = values.get('number_next', row['number_next'])
72 if row['implementation'] == 'standard':
73 if new_implementation in ('standard', None):
74- self._alter_sequence(cr, row['id'], i, n)
75+ # Implementation has NOT changed.
76+ # Only change sequence if really requested.
77+ if row['number_next'] != n:
78+ self._alter_sequence(cr, row['id'], i, n)
79+ else:
80+ # Just in case only increment changed
81+ self._alter_sequence(cr, row['id'], i)
82 else:
83 self._drop_sequence(cr, row['id'])
84 else:
85@@ -200,7 +236,7 @@
86 force_company = context.get('force_company')
87 if not force_company:
88 force_company = self.pool.get('res.users').browse(cr, uid, uid).company_id.id
89- sequences = self.read(cr, uid, seq_ids, ['company_id','implementation','number_next','prefix','suffix','padding'])
90+ sequences = self.read(cr, uid, seq_ids, ['name','company_id','implementation','number_next','prefix','suffix','padding'])
91 preferred_sequences = [s for s in sequences if s['company_id'] and s['company_id'][0] == force_company ]
92 seq = preferred_sequences[0] if preferred_sequences else sequences[0]
93 if seq['implementation'] == 'standard':
94@@ -210,8 +246,11 @@
95 cr.execute("SELECT number_next FROM ir_sequence WHERE id=%s FOR UPDATE NOWAIT", (seq['id'],))
96 cr.execute("UPDATE ir_sequence SET number_next=number_next+number_increment WHERE id=%s ", (seq['id'],))
97 d = self._interpolation_dict()
98- interpolated_prefix = self._interpolate(seq['prefix'], d)
99- interpolated_suffix = self._interpolate(seq['suffix'], d)
100+ try:
101+ interpolated_prefix = self._interpolate(seq['prefix'], d)
102+ interpolated_suffix = self._interpolate(seq['suffix'], d)
103+ except ValueError:
104+ raise osv.except_osv(_('Warning'), _('Invalid prefix or suffix for sequence \'%s\'') % (seq.get('name')))
105 return interpolated_prefix + '%%0%sd' % seq['padding'] % seq['number_next'] + interpolated_suffix
106
107 def next_by_id(self, cr, uid, sequence_id, context=None):
108
109=== modified file 'openerp/addons/base/ir/ir_sequence_view.xml'
110--- openerp/addons/base/ir/ir_sequence_view.xml 2012-10-13 12:09:24 +0000
111+++ openerp/addons/base/ir/ir_sequence_view.xml 2013-05-17 15:48:24 +0000
112@@ -20,7 +20,7 @@
113 <field name="suffix"/>
114 <field name="padding"/>
115 <field name="number_increment"/>
116- <field name="number_next"/>
117+ <field name="number_next_actual"/>
118 <field name="implementation"/>
119 </group>
120 <group col="3" string="Legend (for prefix, suffix)">
121@@ -57,7 +57,7 @@
122 <field name="prefix"/>
123 <field name="padding"/>
124 <field name="company_id" groups="base.group_multi_company"/>
125- <field name="number_next"/>
126+ <field name="number_next_actual"/>
127 <field name="number_increment"/>
128 <field name="implementation"/>
129 </tree>