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

[Cedric Snauwaert (OpenERP)]

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

[lp:~openerp-dev/openobject-server/7.0-sequence-next-fix-csn]

4978. By Cedric Snauwaert (OpenERP) on 2013-05-17

[FIX]ir_sequence: better args definition

[Olivier Dony (Odoo) (odo-openerp)]

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)]

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

review: Approve

[Olivier Dony (Odoo) (odo-openerp)]

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)]

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)

=== modified file 'openerp/addons/base/ir/ir_sequence.py'
--- openerp/addons/base/ir/ir_sequence.py	2013-04-19 15:49:20 +0000
+++ openerp/addons/base/ir/ir_sequence.py	2013-05-17 15:48:24 +0000
@@ -54,6 +54,34 @@
     """
     _name = 'ir.sequence'
     _order = 'name'
+    
+    def _get_number_next_actual(self, cr, user, ids, field_name, arg, context=None):
+        '''Return number from ir_sequence row when no_gap implementation,
+        and number from postgres sequence when standard implementation.'''
+        res = dict.fromkeys(ids)
+        for element in self.browse(cr, user, ids, context=context):
+            if  element.implementation != 'standard':
+                res[element.id] = element.number_next
+            else:
+                # get number from postgres sequence. Cannot use
+                # currval, because that might give an error when
+                # not having used nextval before.
+                statement = (
+                    "SELECT last_value, increment_by, is_called"
+                    " FROM ir_sequence_%03d"
+                    % element.id)
+                cr.execute(statement)
+                (last_value, increment_by, is_called) = cr.fetchone()
+                if is_called:
+                    res[element.id] = last_value + increment_by
+                else:
+                    res[element.id] = last_value
+        return res
+
+    def _set_number_next_actual(self, cr, uid, id, name, value, args=None, context=None):
+        return self.write(cr, uid, id, {'number_next': value or 0}, context=context)
+
+
     _columns = {
         'name': openerp.osv.fields.char('Name', size=64, required=True),
         'code': openerp.osv.fields.selection(_code_get, 'Code', size=64),
@@ -67,6 +95,7 @@
         'prefix': openerp.osv.fields.char('Prefix', size=64, help="Prefix value of the record for the sequence"),
         'suffix': openerp.osv.fields.char('Suffix', size=64, help="Suffix value of the record for the sequence"),
         'number_next': openerp.osv.fields.integer('Next Number', required=True, help="Next number of this sequence"),
+        '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'),
         'number_increment': openerp.osv.fields.integer('Increment Number', required=True, help="The next number of the sequence will be incremented by this number"),
         '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."),
         'company_id': openerp.osv.fields.many2one('res.company', 'Company'),
@@ -121,7 +150,7 @@
         # object depends on it.
         cr.execute("DROP SEQUENCE IF EXISTS %s RESTRICT " % names)
 
-    def _alter_sequence(self, cr, id, number_increment, number_next):
+    def _alter_sequence(self, cr, id, number_increment, number_next=None):
         """ Alter a PostreSQL sequence.
 
         There is no access rights check.
@@ -129,9 +158,10 @@
         if number_increment == 0:
              raise osv.except_osv(_('Warning!'),_("Increment number must not be zero."))
         assert isinstance(id, (int, long))
-        cr.execute("""
-            ALTER SEQUENCE ir_sequence_%03d INCREMENT BY %%s RESTART WITH %%s
-            """ % id, (number_increment, number_next))
+        statement = ("ALTER SEQUENCE ir_sequence_%03d INCREMENT BY %d" % (id, number_increment))
+        if number_next is not None:
+            statement += " RESTART WITH %d" % (number_next, )
+        cr.execute(statement)
 
     def create(self, cr, uid, values, context=None):
         """ Create a sequence, in implementation == standard a fast gaps-allowed PostgreSQL sequence is used.
@@ -160,7 +190,13 @@
             n = values.get('number_next', row['number_next'])
             if row['implementation'] == 'standard':
                 if new_implementation in ('standard', None):
-                    self._alter_sequence(cr, row['id'], i, n)
+                    # Implementation has NOT changed.
+                    # Only change sequence if really requested.
+                    if row['number_next'] != n:
+                        self._alter_sequence(cr, row['id'], i, n)
+                    else:
+                        # Just in case only increment changed
+                        self._alter_sequence(cr, row['id'], i)
                 else:
                     self._drop_sequence(cr, row['id'])
             else:
@@ -200,7 +236,7 @@
         force_company = context.get('force_company')
         if not force_company:
             force_company = self.pool.get('res.users').browse(cr, uid, uid).company_id.id
-        sequences = self.read(cr, uid, seq_ids, ['company_id','implementation','number_next','prefix','suffix','padding'])
+        sequences = self.read(cr, uid, seq_ids, ['name','company_id','implementation','number_next','prefix','suffix','padding'])
         preferred_sequences = [s for s in sequences if s['company_id'] and s['company_id'][0] == force_company ]
         seq = preferred_sequences[0] if preferred_sequences else sequences[0]
         if seq['implementation'] == 'standard':
@@ -210,8 +246,11 @@
             cr.execute("SELECT number_next FROM ir_sequence WHERE id=%s FOR UPDATE NOWAIT", (seq['id'],))
             cr.execute("UPDATE ir_sequence SET number_next=number_next+number_increment WHERE id=%s ", (seq['id'],))
         d = self._interpolation_dict()
-        interpolated_prefix = self._interpolate(seq['prefix'], d)
-        interpolated_suffix = self._interpolate(seq['suffix'], d)
+        try:
+            interpolated_prefix = self._interpolate(seq['prefix'], d)
+            interpolated_suffix = self._interpolate(seq['suffix'], d)
+        except ValueError:
+            raise osv.except_osv(_('Warning'), _('Invalid prefix or suffix for sequence \'%s\'') % (seq.get('name')))
         return interpolated_prefix + '%%0%sd' % seq['padding'] % seq['number_next'] + interpolated_suffix
 
     def next_by_id(self, cr, uid, sequence_id, context=None):

=== modified file 'openerp/addons/base/ir/ir_sequence_view.xml'
--- openerp/addons/base/ir/ir_sequence_view.xml	2012-10-13 12:09:24 +0000
+++ openerp/addons/base/ir/ir_sequence_view.xml	2013-05-17 15:48:24 +0000
@@ -20,7 +20,7 @@
                             <field name="suffix"/>
                             <field name="padding"/>
                             <field name="number_increment"/>
-                            <field name="number_next"/>
+                            <field name="number_next_actual"/>
                             <field name="implementation"/>
                         </group>
                         <group col="3" string="Legend (for prefix, suffix)">
@@ -57,7 +57,7 @@
                     <field name="prefix"/>
                     <field name="padding"/>
                     <field name="company_id" groups="base.group_multi_company"/>
-                    <field name="number_next"/>
+                    <field name="number_next_actual"/>
                     <field name="number_increment"/>
                     <field name="implementation"/>
                 </tree>