Merge lp:~openerp-dev/openobject-server/trunk-cleanup-ir-values into lp:openobject-server

[Olivier Dony (Odoo)]

= Summary =

While looking for a fix for bug 817430, we figured it was time to start improving our ir.values internal model.

This internal model has always been a mess. Lately it is still used for two things: storing user-defined default values, and binding actions to certain 'action slots' in the UI (mostly wizards, reports, and related links visible in the sidebar for each model).
The model definition is totally opaque and cryptic (column names like key2, key, object, ..), and their API is not better, with two generic methods (get/set), just as opaque and cryptic.

This branch is a first step towards a clean refactoring, requiring minimal backend and API change while at the same time providing an implicit fix for 817430 and preparing for a better future

== Proposed fix ==

Ideally, we should have separate models with their own clean and specific API: one for storing user defaults, and one for storing action bindings (for example: ir.user.defaults and ir.actions.bindings)
This could be the next step.

Until then, this branch attempts to preserve the current API for perfect backwards-compatibility, does not require any migration, but introduces two separate APIs and underlying implementations for the two aspects: default vs. action bindings, and fixes bug 817430 in the process.

== Implementation notes ==

The main changes are:
- obsolete columns have been removed totally from ir.values (the meta* stuff, and the object column)
- existing columns have been documented, but preserved
- the set() and get() methods are deprecated, and internally delegate their tasks to the new set_action/get_actions and set_default/get_defaults.
- the API is now documented
- the administration UI for ir.values has been split in 2: defaults and action bindings now have their own menus and views

== Tests ==

- the existing YAML tests for ir.values have been extended to cover smoke-testing for both defaults and action bindings
- the tests do not use the newer API directly yet, but they do so indirectly because the old API relies on the new implementation now

== Next steps ==

1. The rest of the server code (and clients) should be adapted progressively to use the new API, but that does not need to happen immediately.
2. For next version we should split everything properly into separate models. There is already an rudimentary and unused "ir.defaults" model lying around, but it's not clean enough - should be removed/refactored in the process.

[xrg (xrg)]

On Friday 02 September 2011, you wrote:
> Olivier Dony (OpenERP) has proposed merging
> lp:~openerp-dev/openobject-server/trunk-cleanup-ir-values into
> lp:openobject-server.
>
Right. This was mostly needed.

Note, that a shortcoming of the current implementation (with the "key" field)
is that we are allowed to base our defaults only on one other field of the ORM
model. This makes defaults unusable in a range of situations, meaning that we
need to write our own pythonic default_get() there.

You should consider a little more flexible design for the 'ir.default.values'
model to come.

--
Say NO to spam and viruses. Stop using Microsoft Windows!

[Xavier (Open ERP) (xmo-deactivatedaccount)]

I find it a bit strange that the `key2` column is defined as 128c wide, and the string put into it @449 of the diff is cut off at 200 characters. Not only do they mismatch, there's not even a warning. Would it not be better to have the same limit on both, and maybe to remove the limit altogether? (it won't bother postgres, though I'm not sure the ORM can do that).

I also think it would be better to raise some sort of error if the value tentatively set is wider than the field, instead of just removing everything which sticks out silently and potentially storing an invalid condition.

The `object` key seems completely unused (it's set to `False` in set_default and to `True` in set_action, and it does not seem used at all in the code, is there really a need to keep setting it in default values & al? Can't it just be made into a function field writing to nothing and reading from itself being a default or an action?

The `get_defaults` documentation talks about priority and links to `set_defaults`, which reads like `set_defaults` will talk further about priority setting, but that does not seem to be the case. It should also indicate that setting a default for a given company has a higher priority than for none. And the precedence between user_id and company_id priorities is not explained. From my reading, it's `Nothing < company_id < user_id < (user_id, company_id)`. Is that correct?

It's also unclear what the condition is supposed to be exactly: a domain? Or a Python expression? Or some sort of weird custom format? From the get_defaults code (diff line 482) it looks like it's just some sort of tag, with a strict identity match, is that it?

[lp:~openerp-dev/openobject-server/trunk-cleanup-ir-values]

3565. By Olivier Dony (Odoo) on 2011-09-02

[FIX] ir.values: get_defaults() should cut the condition at 200 chars to match the conditions that were set

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

> You should consider a little more flexible design for the 'ir.default.values'
> model to come.

Yup, we could imagine a more flexible model, but I'm not sure how much more. Mostly when it comes to multi-valued triggers, this is best foreseen by the module itself via default_get or some on_change, isn't it? Any more specific ideas?

BTW, for those who don't know, the current system is very basic: it lets users save default values for each field, optionally triggered by a certain value of onanother field (like the zip code for an address, or the partner on an invoice).

> I find it a bit strange that the `key2` column is defined as 128c wide, and
> the string put into it @449 of the diff is cut off at 200 characters. Not only
> do they mismatch, there's not even a warning. Would it not be better to have
> the same limit on both, and maybe to remove the limit altogether? (it won't
> bother postgres, though I'm not sure the ORM can do that).

That's indeed one of the consequences of the current model. For default values key2 can hold a string of the form "field=value", which acts as an opaque qualifier for the key, as far as the server-side is concerned. Instead of making key2 a TEXT, the original implementation arbitrarily decided to cut off the conditions at 200 chars IN and OUT - effectively causing collisions for larger values - though I admit it does not occur often.
Reminds me the 200c limit needs to be enforced in get_defaults() too, fixed that!

Raising exceptions won't help unless we also fix the client side to make sure it handles these errors properly - and my point is not to waste time on this right now, we'll do it when we properly revamp the whole thing.

> The `object` key seems completely unused (it's set to `False` in set_default
> and to `True` in set_action, and it does not seem used at all in the code, is
> there really a need to keep setting it in default values & al? Can't it just
> be made into a function field writing to nothing and reading from itself being
> a default or an action?

Actually that's right, we don't actually need to care about it anymore, as the behavior is now fixed
in both cases. I'll drop it, this will further simplify the view and code.

> The `get_defaults` documentation talks about priority and links to
> `set_defaults`, which reads like `set_defaults` will talk further about
> priority setting, but that does not seem to be the case. It should also
> indicate that setting a default for a given company has a higher priority than
> for none. And the precedence between user_id and company_id priorities is not
> explained. From my reading, it's `Nothing < company_id < user_id < (user_id,
> company_id)`. Is that correct?

Correct, will fix the doc to mention it - this will probably stay true in the future API.

> It's also unclear what the condition is supposed to be exactly: a domain? Or a
> Python expression? Or some sort of weird custom format? From the get_defaults
> code (diff line 482) it looks like it's just some sort of tag, with a strict
> identity match, is that it?

It's actually left to the caller to use it as they wish, but in practice the clients use it to store a 'field=value' expres...

[Thibaut DIRLIK (Logica) (thibaut-dirlik)]

I'm glad to see some improvements and documentation about this! I made a bug request some days ago about the possibility to add a domain on ir.values (or on "actions bindings" :) to show/hide reports or actions based on the object they will be called on.

For example, this would let developpers do some reports like "Order Sumarry" once the order is done. This is a stupid example, juste to show my point.

It could be the good time to make this !

Anyway, thanks for your work Olivier, nice to see things evolve in the good direction.

[lp:~openerp-dev/openobject-server/trunk-cleanup-ir-values]

3566. By Olivier Dony (Odoo) on 2011-09-05

[MERGE] lastest trunk

3567. By Olivier Dony (Odoo) on 2011-09-05

[IMP] ir.values: improve security: users can only write to their personal defaults

Administrator access is required to set defaults for
everybody, as well as to alter the action bindings.

3568. By Olivier Dony (Odoo) on 2011-09-05

[IMP] ir.values: improved defaults doc + removed `object` column completely

The `object` column actually directly depended on the value of
the `key` column, so it can be totally removed with no side-effects.
Docstrings updated following review comments.

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

Thanks for the feedback so far.

I've updated the docstrings for {get,set}_default to explain the priorities, and removed the `object` column that was in fact useless now, as noted by Xavier.

I forgot to mention that this branch originally stems from bug 817430 - it's normally a low-risk fix for the bug, requiring minimal backend and API change, but preparing for a better future.

[Naresh(OpenERP) (nch-openerp)]

Hello Olivier,

The improvements are good but it does not fix the bug that led the improvement. It still returns the wrong set of records.

Thanks,

[lp:~openerp-dev/openobject-server/trunk-cleanup-ir-values]

3569. By Olivier Dony (Odoo) on 2011-09-08

[MERGE] latest trunk

3570. By Olivier Dony (Odoo) on 2011-09-08

[FIX] ir.values.get_defaults: typo in SQL query - spotted by Naresh, thanks!

3571. By Vo Minh Thu on 2011-10-03

[MERGE] merged trunk.

3572. By Vo Minh Thu on 2011-10-03

[FIX] ir_values: missing trimming of the condition (as done elsewhere).

=== modified file 'openerp/addons/base/ir/ir.xml'
--- openerp/addons/base/ir/ir.xml	2011-10-02 15:44:57 +0000
+++ openerp/addons/base/ir/ir.xml	2011-10-03 15:51:44 +0000
@@ -7,44 +7,50 @@
             <field name="name">ir.values.form.action</field>
             <field name="model">ir.values</field>
             <field name="type">form</field>
-            <field name="priority">20</field>
             <field name="arch" type="xml">
-                <form string="Connect Events to Actions">
-                   <field name="name" required="1"/>
+                <form string="Action Bindings">
+                   <field name="name"/>
                    <newline/>
                     <group col="2" colspan="2">
-                        <separator string="Action Source" colspan="2"/>
+                        <separator string="Action Binding" colspan="2"/>
                         <field name="model_id" on_change="onchange_object_id(model_id)"/>
+                        <field name="model"/>
                         <field name="res_id"/>
-                        <field name="key2" required="1"/>
+                        <field name="key2"/>
                     </group>
                     <group col="2" colspan="2">
-                        <separator string="Action To Launch" colspan="2"/>
+                        <separator string="Action" colspan="2"/>
                         <field name="action_id" on_change="onchange_action_id(action_id)"/>
-
-                        <field name="object" readonly="1"/>
-
-                    </group>
-                    <group col="2" colspan="2">
-                        <separator string="Values for Event Type" colspan="2"/>
-                        <label string="client_action_multi, client_action_relate" colspan="2"/>
-                        <label string="tree_but_open, client_print_multi"  colspan="2"/>
-                    </group>
-                    <group col="2" colspan="2">
-                        <separator colspan="2" string="Value"/>
-                         <field name="value_unpickle" nolabel="1" colspan="4"/>
-                    </group>
-                    <group col="2" colspan="2">
-                        <separator colspan="2" string="Metadata"/>
-                        <field name="meta_unpickle" nolabel="1"/>
-                    </group>
-                    <group col="2" colspan="2">
-                    <separator colspan="2" string=""/>
-                    <field name="user_id"/>
-                    <field name="company_id" groups="base.group_multi_company"/>
-                    </group>
-              </form>
-
+                        <field name="value_unpickle" colspan="4" string="Action Reference"/>
+                    </group>
+                </form>
+            </field>
+        </record>
+
+        <record id="values_view_form_defaults" model="ir.ui.view">
+            <field name="name">ir.values.form.defaults</field>
+            <field name="model">ir.values</field>
+            <field name="type">form</field>
+            <field name="arch" type="xml">
+                <form string="User-defined Defaults">
+                   <field name="name"/>
+                   <newline/>
+                    <group col="2" colspan="2">
+                        <separator string="Model" colspan="2"/>
+                        <field name="model_id" on_change="onchange_object_id(model_id)"/>
+                        <field name="model"/>
+                        <field name="key2" string="Condition"/>
+                    </group>
+                    <group col="2" colspan="2">
+                        <separator string="Default Value" colspan="2"/>
+                        <field name="value_unpickle" colspan="4" nolabel="1"/>
+                    </group>
+                    <group col="2" colspan="2">
+                        <separator colspan="2" string="Default Value Scope"/>
+                        <field name="user_id"/>
+                        <field name="company_id" groups="base.group_multi_company"/>
+                    </group>
+                </form>
             </field>
         </record>
 
@@ -53,10 +59,9 @@
             <field name="model">ir.values</field>
             <field name="type">tree</field>
             <field name="arch" type="xml">
-                <tree string="Client Actions">
+                <tree string="Action Bindings/Defaults">
                     <field name="name"/>
                     <field name="model"/>
-                    <field name="action_id"/>
                     <field name="key2"/>
                 </tree>
             </field>
@@ -73,7 +78,7 @@
                     <field name="key2"/>
                     <newline/>
                     <group expand="0" string="Group By...">
-                        <filter string="Object" icon="terp-stock_align_left_24" domain="[]" context="{'group_by':'model'}"/>
+                        <filter string="Model" icon="terp-stock_align_left_24" domain="[]" context="{'group_by':'model'}"/>
                         <filter string="Type" icon="terp-stock_symbol-selection" domain="[]" context="{'group_by':'key2'}"/>
                     </group>
                 </search>
@@ -81,23 +86,21 @@
         </record>
 
         <record id="act_values_form_action" model="ir.actions.act_window">
-            <field name="name">Client Events</field>
+            <field name="name">Action Bindings</field>
             <field name="type">ir.actions.act_window</field>
             <field name="res_model">ir.values</field>
             <field name="view_type">form</field>
             <field name="view_mode">tree,form</field>
             <field name="search_view_id" ref="values_view_search_action"/>
             <field name="domain">[('key','=','action')]</field>
-            <field name="context">{'read':'default','default_object':1}</field>
+            <field name="context">{'default_key':'action'}</field>
         </record>
-
         <record model="ir.actions.act_window.view" id="action_values_tree_view">
             <field name="sequence" eval="1"/>
             <field name="view_mode">tree</field>
             <field name="view_id" ref="values_view_tree_action"/>
             <field name="act_window_id" ref="act_values_form_action"/>
         </record>
-
         <record model="ir.actions.act_window.view" id="action_values_form_view">
             <field name="sequence" eval="2"/>
             <field name="view_mode">form</field>
@@ -105,54 +108,27 @@
             <field name="act_window_id" ref="act_values_form_action"/>
         </record>
 
-
-
-
-        <!-- Values -->
-
-        <record id="values_view_form" model="ir.ui.view">
-            <field name="name">ir.values.form</field>
-            <field name="model">ir.values</field>
-            <field name="type">form</field>
-            <field name="arch" type="xml">
-                <form string="Values">
-                    <field name="name" select="1"/>
-                    <field name="model" select="1"/>
-                    <field name="key" select="1"/>
-                    <field name="key2" select="2"/>
-                    <field name="object" select="2"/>
-                    <field name="res_id"/>
-                    <field name="user_id" select="2"/>
-                    <field name="company_id" select="2"/>
-                    <field name="value_unpickle"/>
-                    <field name="meta_unpickle"/>
-                </form>
-            </field>
-        </record>
-
-        <record id="values_view_tree" model="ir.ui.view">
-            <field name="name">ir.values.tree</field>
-            <field name="model">ir.values</field>
-            <field name="type">tree</field>
-            <field name="arch" type="xml">
-                <tree string="Values">
-                    <field name="name"/>
-                    <field name="model"/>
-                    <field name="key"/>
-                    <field name="key2"/>
-                    <field name="user_id"/>
-                    <field name="company_id"/>
-                </tree>
-            </field>
-        </record>
-
-        <record id="act_values_form" model="ir.actions.act_window">
-            <field name="name">Client Actions Connections</field>
+        <record id="act_values_form_defaults" model="ir.actions.act_window">
+            <field name="name">User-defined Defaults</field>
             <field name="type">ir.actions.act_window</field>
             <field name="res_model">ir.values</field>
             <field name="view_type">form</field>
-            <field name="view_id" ref="values_view_tree"/>
-            <field name="context">{'read':'default'}</field>
+            <field name="view_mode">tree,form</field>
+            <field name="search_view_id" ref="values_view_search_action"/>
+            <field name="domain">[('key','=','default')]</field>
+            <field name="context">{'default_key':'default','default_key2':''}</field>
+        </record>
+        <record model="ir.actions.act_window.view" id="action_values_defaults_tree_view">
+            <field name="sequence" eval="1"/>
+            <field name="view_mode">tree</field>
+            <field name="view_id" ref="values_view_tree_action"/>
+            <field name="act_window_id" ref="act_values_form_defaults"/>
+        </record>
+        <record model="ir.actions.act_window.view" id="action_values_defaults_form_view">
+            <field name="sequence" eval="2"/>
+            <field name="view_mode">form</field>
+            <field name="view_id" ref="values_view_form_defaults"/>
+            <field name="act_window_id" ref="act_values_form_defaults"/>
         </record>
 
         <!-- Sequences -->
@@ -344,6 +320,7 @@
         <menuitem id="next_id_6" name="Actions" parent="base.next_id_4" sequence="1"/>
         <menuitem action="ir_sequence_actions" id="menu_ir_sequence_actions" parent="next_id_6"/>
         <menuitem action="act_values_form_action" id="menu_values_form_action" parent="next_id_6"/>
+        <menuitem action="act_values_form_defaults" id="menu_values_form_defaults" parent="next_id_6"/>
 
         <!--Filters form view-->
 

=== modified file 'openerp/addons/base/ir/ir_values.py'
--- openerp/addons/base/ir/ir_values.py	2011-07-22 14:46:06 +0000
+++ openerp/addons/base/ir/ir_values.py	2011-10-03 15:51:44 +0000
@@ -28,19 +28,89 @@
     'report_sxw_content', 'report_rml_content', 'report_sxw', 'report_rml',
     'report_sxw_content_data', 'report_rml_content_data', 'search_view', ))
 
+#: Possible slots to bind an action to with :meth:`~.set_action`
+ACTION_SLOTS = [
+                "client_action_multi",  # sidebar wizard action
+                "client_print_multi",   # sidebar report printing button
+                "client_action_relate", # sidebar related link
+                "tree_but_open",        # double-click on item in tree view
+                "tree_but_action",      # deprecated: same as tree_but_open
+               ]
+
+
 class ir_values(osv.osv):
+    """Holds internal model-specific action bindings and user-defined default
+       field values. definitions. This is a legacy internal model, mixing
+       two different concepts, and will likely be updated or replaced in a
+       future version by cleaner, separate models. You should not depend
+       explicitly on it.
+
+       The purpose of each ``ir.values`` entry depends on its type, defined
+       by the ``key`` column:
+
+        * 'default': user-defined default values, used when creating new
+          records of this model:
+        * 'action': binding of an action to a particular *action slot* of
+          this model, making the action easily available in the user
+          interface for this model.
+
+       The ``key2`` column acts as a qualifier, further refining the type
+       of the entry. The possible values are:
+
+        * for 'default' entries: an optional condition restricting the
+          cases where this particular default value will be applicable,
+          or ``False`` for no condition
+        * for 'action' entries: the ``key2`` qualifier is one of the available
+          action slots, defining how this action can be invoked:
+
+            * ``'client_print_multi'`` for report printing actions that will
+              be available on views displaying items from this model
+            * ``'client_action_multi'`` for assistants (wizards) actions
+              that will be available in views displaying objects of this model
+            * ``'client_action_relate'`` for links towards related documents
+              that should be available in views displaying objects of this model
+            * ``'tree_but_open'`` for actions that will be triggered when
+              double-clicking an item from this model in a hierarchical tree view
+
+       Each entry is specific to a model (``model`` column), and for ``'actions'``
+       type, may even be made specific to a given record of that model when the
+       ``res_id`` column contains a record ID (``False`` means it's global for
+       all records).
+
+       The content of the entry is defined by the ``value`` column, which may either
+       contain an arbitrary value, or a reference string defining the action that
+       should be executed.
+
+       .. rubric:: Usage: default values
+       
+       The ``'default'`` entries are usually defined manually by the
+       users, and set by their UI clients calling :meth:`~.set_default`.
+       These default values are then automatically used by the
+       ORM every time a new record is about to be created, i.e. when
+       :meth:`~openerp.osv.osv.osv.default_get`
+       or :meth:`~openerp.osv.osv.osv.create` are called.
+
+       .. rubric:: Usage: action bindings
+
+       Business applications will usually bind their actions during
+       installation, and OpenERP UI clients will apply them as defined,
+       based on the list of actions included in the result of
+       :meth:`~openerp.osv.osv.osv.fields_view_get`,
+       or directly returned by explicit calls to :meth:`~.get_actions`.
+    """
     _name = 'ir.values'
 
     def _value_unpickle(self, cursor, user, ids, name, arg, context=None):
         res = {}
-        for report in self.browse(cursor, user, ids, context=context):
-            value = report[name[:-9]]
-            if not report.object and value:
+        for record in self.browse(cursor, user, ids, context=context):
+            value = record[name[:-9]]
+            if record.key == 'default' and value:
+                # default values are pickled on the fly
                 try:
                     value = str(pickle.loads(value))
-                except:
+                except Exception:
                     pass
-            res[report.id] = value
+            res[record.id] = value
         return res
 
     def _value_pickle(self, cursor, user, id, name, value, arg, context=None):
@@ -49,18 +119,20 @@
         ctx = context.copy()
         if self.CONCURRENCY_CHECK_FIELD in ctx:
             del ctx[self.CONCURRENCY_CHECK_FIELD]
-        if not self.browse(cursor, user, id, context=context).object:
+        record = self.browse(cursor, user, id, context=context)
+        if record.key == 'default':
+            # default values are pickled on the fly
             value = pickle.dumps(value)
         self.write(cursor, user, id, {name[:-9]: value}, context=ctx)
 
-    def onchange_object_id(self, cr, uid, ids, object_id, context={}):
+    def onchange_object_id(self, cr, uid, ids, object_id, context=None):
         if not object_id: return {}
         act = self.pool.get('ir.model').browse(cr, uid, object_id, context=context)
         return {
                 'value': {'model': act.model}
         }
 
-    def onchange_action_id(self, cr, uid, ids, action_id, context={}):
+    def onchange_action_id(self, cr, uid, ids, action_id, context=None):
         if not action_id: return {}
         act = self.pool.get('ir.actions.actions').browse(cr, uid, action_id, context=context)
         return {
@@ -68,32 +140,47 @@
         }
 
     _columns = {
-        'name': fields.char('Name', size=128),
-        'model_id': fields.many2one('ir.model', 'Object', size=128,
-            help="This field is not used, it only helps you to select a good model."),
-        'model': fields.char('Object Name', size=128, select=True),
-        'action_id': fields.many2one('ir.actions.actions', 'Action',
-            help="This field is not used, it only helps you to select the right action."),
-        'value': fields.text('Value'),
+        'name': fields.char('Name', size=128, required=True),
+        'model': fields.char('Model Name', size=128, select=True, required=True,
+                             help="Model to which this entry applies"),
+
+        # TODO: model_id and action_id should be read-write function fields
+        'model_id': fields.many2one('ir.model', 'Model (change only)', size=128,
+                                    help="Model to which this entry applies - "
+                                         "helper field for setting a model, will "
+                                         "automatically set the correct model name"),
+        'action_id': fields.many2one('ir.actions.actions', 'Action (change only)',
+                                     help="Action bound to this entry - "
+                                         "helper field for binding an action, will "
+                                         "automatically set the correct reference"),
+
+        'value': fields.text('Value', help="Default value (pickled) or reference to an action"),
         'value_unpickle': fields.function(_value_unpickle, fnct_inv=_value_pickle,
-            method=True, type='text', string='Value'),
-        'object': fields.boolean('Is Object'),
-        'key': fields.selection([('action','Action'),('default','Default')], 'Type', size=128, select=True),
-        'key2' : fields.char('Event Type', size=128, select=True, help="The kind of action or button on the client side "
-                                                                       "that will trigger the action. One of: "
-                                                                       "client_action_multi, client_action_relate, tree_but_open, "
-                                                                       "client_print_multi"),
-        'meta': fields.text('Meta Datas'),
-        'meta_unpickle': fields.function(_value_unpickle, fnct_inv=_value_pickle,
-            method=True, type='text', string='Metadata'),
-        'res_id': fields.integer('Object ID', help="Keep 0 if the action must appear on all resources.", select=True),
-        'user_id': fields.many2one('res.users', 'User', ondelete='cascade', select=True),
-        'company_id': fields.many2one('res.company', 'Company', select=True)
+                                          type='text',
+                                          string='Default value or action reference'),
+        'key': fields.selection([('action','Action'),('default','Default')],
+                                'Type', size=128, select=True, required=True,
+                                help="- Action: an action attached to one slot of the given model\n"
+                                     "- Default: a default value for a model field"),
+        'key2' : fields.char('Qualifier', size=128, select=True,
+                             help="For actions, one of the possible action slots: \n"
+                                  "  - client_action_multi\n"
+                                  "  - client_print_multi\n"
+                                  "  - client_action_relate\n"
+                                  "  - tree_but_open\n"
+                                  "For defaults, an optional condition"
+                             ,),
+        'res_id': fields.integer('Record ID', select=True,
+                                 help="Database identifier of the record to which this applies. "
+                                      "0 = for all records"),
+        'user_id': fields.many2one('res.users', 'User', ondelete='cascade', select=True,
+                                   help="If set, action binding only applies for this user."),
+        'company_id': fields.many2one('res.company', 'Company', ondelete='cascade', select=True,
+                                      help="If set, action binding only applies for this company")
     }
     _defaults = {
-        'key': lambda *a: 'action',
-        'key2': lambda *a: 'tree_but_open',
-        'company_id': lambda *a: False
+        'key': 'action',
+        'key2': 'tree_but_open',
     }
 
     def _auto_init(self, cr, context=None):
@@ -102,140 +189,271 @@
         if not cr.fetchone():
             cr.execute('CREATE INDEX ir_values_key_model_key2_res_id_user_id_idx ON ir_values (key, model, key2, res_id, user_id)')
 
-    def set(self, cr, uid, key, key2, name, models, value, replace=True, isobject=False, meta=False, preserve_user=False, company=False):
+    def set_default(self, cr, uid, model, field_name, value, for_all_users=True, company_id=False, condition=False):
+        """Defines a default value for the given model and field_name. Any previous
+           default for the same scope (model, field_name, value, for_all_users, company_id, condition)
+           will be replaced and lost in the process.
+
+           Defaults can be later retrieved via :meth:`~.get_defaults`, which will return
+           the highest priority default for any given field. Defaults that are more specific
+           have a higher priority, in the following order (highest to lowest):
+
+               * specific to user and company
+               * specific to user only
+               * specific to company only
+               * global to everyone
+
+           :param string model: model name
+           :param string field_name: field name to which the default applies
+           :param value: the default field value to set
+           :type value: any serializable Python value
+           :param bool for_all_users: whether the default should apply to everybody or only
+                                      the user calling the method
+           :param int company_id: optional ID of the company to which the default should
+                                  apply. If omitted, the default will be global. If True
+                                  is passed, the current user's company will be used.
+           :param string condition: optional condition specification that can be used to
+                                    restrict the applicability of the default values
+                                    (e.g. based on another field's value). This is an
+                                    opaque string as far as the API is concerned, but client
+                                    stacks typically use single-field conditions in the
+                                    form ``'key=stringified_value'``.
+                                    (Currently, the condition is trimmed to 200 characters,
+                                    so values that share the same first 200 characters always
+                                    match)
+           :return: id of the newly created ir.values entry
+        """
         if isinstance(value, unicode):
             value = value.encode('utf8')
-        if not isobject:
-            value = pickle.dumps(value)
-        if meta:
-            meta = pickle.dumps(meta)
-        assert isinstance(models, (list, tuple)), models
-        ids_res = []
-        for model in models:
+        if company_id is True:
+            # should be company-specific, need to get company id
+            user = self.pool.get('res.users').browse(cr, uid, uid)
+            company_id = user.company_id.id
+
+        # remove existing defaults for the same scope
+        search_criteria = [
+            ('key', '=', 'default'),
+            ('key2', '=', condition and condition[:200]),
+            ('model', '=', model),
+            ('name', '=', field_name),
+            ('user_id', '=', False if for_all_users else uid),
+            ('company_id','=', company_id)
+            ]
+        self.unlink(cr, uid, self.search(cr, uid, search_criteria))
+
+        return self.create(cr, uid, {
+            'name': field_name,
+            'value': pickle.dumps(value),
+            'model': model,
+            'key': 'default',
+            'key2': condition and condition[:200],
+            'user_id': False if for_all_users else uid,
+            'company_id': company_id,
+        })
+
+    def get_defaults(self, cr, uid, model, condition=False):
+        """Returns any default values that are defined for the current model and user,
+           (and match ``condition``, if specified), previously registered via
+           :meth:`~.set_default`.
+
+           Defaults are global to a model, not field-specific, but an optional
+           ``condition`` can be provided to restrict matching default values
+           to those that were defined for the same condition (usually based
+           on another field's value).
+
+           Default values also have priorities depending on whom they apply
+           to: only the highest priority value will be returned for any
+           field. See :meth:`~.set_default` for more details.
+
+           :param string model: model name
+           :param string condition: optional condition specification that can be used to
+                                    restrict the applicability of the default values
+                                    (e.g. based on another field's value). This is an
+                                    opaque string as far as the API is concerned, but client
+                                    stacks typically use single-field conditions in the
+                                    form ``'key=stringified_value'``.
+                                    (Currently, the condition is trimmed to 200 characters,
+                                    so values that share the same first 200 characters always
+                                    match)
+           :return: list of default values tuples of the form ``(id, field_name, value)``
+                    (``id`` is the ID of the default entry, usually irrelevant)
+        """
+        # use a direct SQL query for performance reasons,
+        # this is called very often
+        query = """SELECT v.id, v.name, v.value FROM ir_values v
+                      LEFT JOIN res_users u ON (v.user_id = u.id)
+                   WHERE v.key = %%s AND v.model = %%s
+                      AND (v.user_id = %%s OR v.user_id IS NULL)
+                      AND (v.company_id IS NULL OR
+                           v.company_id =
+                             (SELECT company_id from res_users where id = %%s)
+                          )
+                      %s
+                   ORDER BY v.user_id, u.company_id"""
+        query = query % ('AND v.key2 = %s' if condition else '')
+        params = ('default', model, uid, uid)
+        if condition:
+            params += (condition[:200],)
+        cr.execute(query, params)
+
+        # keep only the highest priority default for each field
+        defaults = {}
+        for row in cr.dictfetchall():
+            defaults.setdefault(row['name'],
+                (row['id'], row['name'], pickle.loads(row['value'].encode('utf-8'))))
+        return defaults.values()
+
+    def set_action(self, cr, uid, name, action_slot, model, action, res_id=False):
+        """Binds an the given action to the given model's action slot - for later
+           retrieval via :meth:`~.get_actions`. Any existing binding of the same action
+           to the same slot is first removed, allowing an update of the action's name.
+           See the class description for more details about the various action
+           slots: :class:`~ir_values`.
+
+           :param string name: action label, usually displayed by UI client
+           :param string action_slot: the action slot to which the action should be
+                                      bound to - one of ``client_action_multi``,
+                                      ``client_print_multi``, ``client_action_relate``,
+                                      ``tree_but_open``.
+           :param string model: model name
+           :param string action: action reference, in the form ``'model,id'``
+           :param int res_id: optional record id - will bind the action only to a
+                              specific record of the model, not all records.
+           :return: id of the newly created ir.values entry
+        """
+        assert isinstance(action, basestring) and ',' in action, \
+               'Action definition must be an action reference, e.g. "ir.actions.act_window,42"'
+        assert action_slot in ACTION_SLOTS, \
+               'Action slot (%s) must be one of: %r' % (action_slot, ACTION_SLOTS)
+
+        # remove existing action definition of same slot and value
+        search_criteria = [
+            ('key', '=', 'action'),
+            ('key2', '=', action_slot),
+            ('model', '=', model),
+            ('res_id', '=', res_id or 0), # int field -> NULL == 0
+            ('value', '=', action),
+            ]
+        self.unlink(cr, uid, self.search(cr, uid, search_criteria))
+
+        return self.create(cr, uid, {
+            'key': 'action',
+            'key2': action_slot,
+            'model': model,
+            'res_id': res_id,
+            'name': name,
+            'value': action,
+        })
+
+    def get_actions(self, cr, uid, action_slot, model, res_id=False, context=None):
+        """Retrieves the list of actions bound to the given model's action slot.
+           See the class description for more details about the various action
+           slots: :class:`~.ir_values`.
+
+           :param string action_slot: the action slot to which the actions should be
+                                      bound to - one of ``client_action_multi``,
+                                      ``client_print_multi``, ``client_action_relate``,
+                                      ``tree_but_open``.
+           :param string model: model name
+           :param int res_id: optional record id - will bind the action only to a
+                              specific record of the model, not all records.
+           :return: list of action tuples of the form ``(id, name, action_def)``,
+                    where ``id`` is the ID of the default entry, ``name`` is the
+                    action label, and ``action_def`` is a dict containing the
+                    action definition as obtained by calling
+                    :meth:`~openerp.osv.osv.osv.read` on the action record.
+        """
+        assert action_slot in ACTION_SLOTS, 'Illegal action slot value: %s' % action_slot
+        # use a direct SQL query for performance reasons,
+        # this is called very often
+        query = """SELECT v.id, v.name, v.value FROM ir_values v
+                   WHERE v.key = %s AND v.key2 = %s
+                        AND v.model = %s
+                        AND (v.res_id = %s
+                             OR v.res_id IS NULL
+                             OR v.res_id = 0)
+                   ORDER BY v.id"""
+        cr.execute(query, ('action', action_slot, model, res_id or None))
+        results = {}
+        for action in cr.dictfetchall():
+            action_model,id = action['value'].split(',')
+            fields = [
+                    field
+                    for field in self.pool.get(action_model)._all_columns
+                    if field not in EXCLUDED_FIELDS]
+            # FIXME: needs cleanup
+            try:
+                action_def = self.pool.get(action_model).read(cr, uid, int(id), fields, context)
+                if action_def:
+                    if action_model in ('ir.actions.report.xml','ir.actions.act_window',
+                                        'ir.actions.wizard'):
+                        groups = action_def.get('groups_id')
+                        if groups:
+                            cr.execute('SELECT 1 FROM res_groups_users_rel WHERE gid IN %s AND uid=%s',
+                                       (tuple(groups), uid))
+                            if not cr.fetchone():
+                                if action['name'] == 'Menuitem':
+                                    raise osv.except_osv('Error !',
+                                                         'You do not have the permission to perform this operation !!!')
+                                continue
+                # keep only the first action registered for each action name
+                results[action['name']] = (action['id'], action['name'], action_def)
+            except except_orm, e:
+                continue
+        return results.values()
+
+    def _map_legacy_model_list(self, model_list, map_fn, merge_results=False):
+        """Apply map_fn to the various models passed, according to
+           legacy way to specify models/records.
+        """
+        assert isinstance(model_list, (list, tuple)), \
+            "model_list should be in the form [model,..] or [(model,res_id), ..]"
+        results = []
+        for model in model_list:
+            res_id = False
             if isinstance(model, (list, tuple)):
-                model,res_id = model
+                model, res_id = model
+            result = map_fn(model, res_id)
+            # some of the functions return one result at a time (tuple or id)
+            # and some return a list of many of them - care for both
+            if merge_results:
+                results.extend(result)
             else:
-                res_id = False
-            if replace:
-                search_criteria = [
-                    ('key', '=', key),
-                    ('key2', '=', key2),
-                    ('model', '=', model),
-                    ('res_id', '=', res_id),
-                    ('user_id', '=', preserve_user and uid)
-                ]
-                if key in ('meta', 'default'):
-                    search_criteria.append(('name', '=', name))
-                else:
-                    search_criteria.append(('value', '=', value))
-
-                self.unlink(cr, uid, self.search(cr, uid, search_criteria))
-            vals = {
-                'name': name,
-                'value': value,
-                'model': model,
-                'object': isobject,
-                'key': key,
-                'key2': key2 and key2[:200],
-                'meta': meta,
-                'user_id': preserve_user and uid,
-            }
-            if company:
-                cid = self.pool.get('res.users').browse(cr, uid, uid, context={}).company_id.id
-                vals['company_id']=cid
-            if res_id:
-                vals['res_id']= res_id
-            ids_res.append(self.create(cr, uid, vals))
-        return ids_res
+                results.append(result)
+        return results
+
+    # Backards-compatibility adapter layer to retrofit into split API
+    def set(self, cr, uid, key, key2, name, models, value, replace=True, isobject=False, meta=False, preserve_user=False, company=False):
+        """Deprecated legacy method to set default values and bind actions to models' action slots.
+           Now dispatches to the newer API methods according to the value of ``key``: :meth:`~.set_default`
+           (``key=='default'``) or :meth:`~.set_action` (``key == 'action'``).
+
+          :deprecated: As of v6.1, ``set_default()`` or ``set_action()`` should be used directly.
+        """
+        assert key in ['default', 'action'], "ir.values entry keys must be in ['default','action']"
+        if key == 'default':
+            def do_set(model,res_id):
+                return self.set_default(cr, uid, model, field_name=name, value=value,
+                                        for_all_users=(not preserve_user), company_id=company,
+                                        condition=key2)
+        elif key == 'action':
+            def do_set(model,res_id):
+                return self.set_action(cr, uid, name, action_slot=key2, model=model, action=value, res_id=res_id)
+        return self._map_legacy_model_list(models, do_set)
 
     def get(self, cr, uid, key, key2, models, meta=False, context=None, res_id_req=False, without_user=True, key2_req=True):
-        if context is None:
-            context = {}
-        result = []
-        assert isinstance(models, (list, tuple)), models
-
-        for m in models:
-            if isinstance(m, (list, tuple)):
-                m, res_id = m
-            else:
-                res_id = False
-
-            where = ['key=%s','model=%s']
-            params = [key, str(m)]
-            if key2:
-                where.append('key2=%s')
-                params.append(key2[:200])
-            elif key2_req and not meta:
-                where.append('key2 is null')
-            if res_id_req and (models[-1][0] == m):
-                if res_id:
-                    where.append('res_id=%s')
-                    params.append(res_id)
-                else:
-                    where.append('(res_id is NULL)')
-            elif res_id:
-                if (models[-1][0]==m):
-                    where.append('(res_id=%s or (res_id is null))')
-                    params.append(res_id)
-                else:
-                    where.append('res_id=%s')
-                    params.append(res_id)
-            order = 'id'
-            if key == 'default':
-                # Make sure we get first the values for specific users, then
-                # the global values. The map/filter below will retain the first
-                # value for any given name. The 'order by' will put the null
-                # values last; this may be postgres specific (it is the
-                # behavior in postgres at least since 8.2).
-                order = 'user_id'
-            where.append('(user_id=%s or (user_id IS NULL)) order by '+ order)
-            params.append(uid)
-            clause = ' and '.join(where)
-            cr.execute('select id,name,value,object,meta, key from ir_values where ' + clause, params)
-            result = cr.fetchall()
-            if result:
-                break
-
-        if not result:
-            return []
-
-        def _result_get(x, keys):
-            if x[1] in keys:
-                return False
-            keys.append(x[1])
-            if x[3]:
-                model,id = x[2].split(',')
-                # FIXME: It might be a good idea to opt-in that kind of stuff
-                # FIXME: instead of arbitrarily removing random fields
-                fields = [
-                    field
-                    for field in self.pool.get(model).fields_get_keys(cr, uid)
-                    if field not in EXCLUDED_FIELDS]
-
-                try:
-                    datas = self.pool.get(model).read(cr, uid, [int(id)], fields, context)
-                except except_orm, e:
-                    return False
-                datas = datas and datas[0]
-                if not datas:
-                    return False
-            else:
-                datas = pickle.loads(x[2].encode('utf-8'))
-            if meta:
-                return (x[0], x[1], datas, pickle.loads(x[4]))
-            return (x[0], x[1], datas)
-        keys = []
-        res = filter(None, map(lambda x: _result_get(x, keys), result))
-        res2 = res[:]
-        for r in res:
-            if isinstance(r[2], dict) and r[2].get('type') in ('ir.actions.report.xml','ir.actions.act_window','ir.actions.wizard'):
-                groups = r[2].get('groups_id')
-                if groups:
-                    cr.execute('SELECT COUNT(1) FROM res_groups_users_rel WHERE gid IN %s AND uid=%s',(tuple(groups), uid))
-                    cnt = cr.fetchone()[0]
-                    if not cnt:
-                        res2.remove(r)
-                    if r[1] == 'Menuitem' and not res2:
-                        raise osv.except_osv('Error !','You do not have the permission to perform this operation !!!')
-        return res2
-ir_values()
+        """Deprecated legacy method to get the list of default values or actions bound to models' action slots.
+           Now dispatches to the newer API methods according to the value of ``key``: :meth:`~.get_defaults`
+           (``key=='default'``) or :meth:`~.get_actions` (``key == 'action'``)
+
+          :deprecated: As of v6.1, ``get_defaults()`` or ``get_actions()`` should be used directly.
+
+        """
+        assert key in ['default', 'action'], "ir.values entry keys must be in ['default','action']"
+        if key == 'default':
+            def do_get(model,res_id):
+                return self.get_defaults(cr, uid, model, condition=key2)
+        elif key == 'action':
+            def do_get(model,res_id):
+                return self.get_actions(cr, uid, action_slot=key2, model=model, res_id=res_id, context=context)
+        return self._map_legacy_model_list(models, do_get, merge_results=True)

=== modified file 'openerp/addons/base/security/base_security.xml'
--- openerp/addons/base/security/base_security.xml	2011-09-19 11:47:17 +0000
+++ openerp/addons/base/security/base_security.xml	2011-10-03 15:51:44 +0000
@@ -69,6 +69,13 @@
             <field name="domain_force">[('company_id','child_of',[user.company_id.id])]</field>
         </record>
 
+    <record model="ir.rule" id="ir_values_default_rule">
+        <field name="name">Defaults: alter personal values only</field>
+        <field name="model_id" ref="model_ir_values"/>
+        <field name="domain_force">[('key','=','default'),('user_id','=',user.id)]</field>
+        <field name="perm_read" eval="False"/>
+    </record>
+
     </data>
 </openerp>
 

=== modified file 'openerp/addons/base/security/ir.model.access.csv'
--- openerp/addons/base/security/ir.model.access.csv	2011-09-19 21:01:21 +0000
+++ openerp/addons/base/security/ir.model.access.csv	2011-10-03 15:51:44 +0000
@@ -38,8 +38,7 @@
 "access_ir_ui_view_custom_group_user","ir_ui_view_custom_group_user","model_ir_ui_view_custom",,1,0,0,0
 "access_ir_ui_view_custom_group_system","ir_ui_view_custom_group_system","model_ir_ui_view_custom","group_system",1,1,1,1
 "access_ir_ui_view_sc_group_user","ir_ui_view_sc group_user","model_ir_ui_view_sc",,1,1,1,1
-"access_ir_values_group_erp_manager","ir_values group_erp_manager","model_ir_values","group_erp_manager",1,1,1,1
-"access_ir_values_group_all","ir_values group_all","model_ir_values",,1,0,1,0
+"access_ir_values_group_all","ir_values group_all","model_ir_values",,1,1,1,1
 "access_res_company_group_erp_manager","res_company group_erp_manager","model_res_company","group_erp_manager",1,1,1,1
 "access_res_company_group_user","res_company group_user","model_res_company",,1,0,0,0
 "access_res_country_group_all","res_country group_user_all","model_res_country",,1,0,0,0

=== modified file 'openerp/addons/base/test/test_ir_values.yml'
--- openerp/addons/base/test/test_ir_values.yml	2011-05-23 12:09:40 +0000
+++ openerp/addons/base/test/test_ir_values.yml	2011-10-03 15:51:44 +0000
@@ -2,24 +2,71 @@
     Create some default value for some (non-existing) model, for all users.
 -
     !python {model: ir.values }: |
-        self.set(cr, uid, 'default', False, 'my_test_ir_value',['unexisting_model'], 'global value')
+        self.set(cr, uid, 'default', False, 'my_test_field',['unexisting_model'], 'global value')
 -
     Retrieve it.
 -
     !python {model: ir.values }: |
         # d is a list of triple (id, name, value)
         d = self.get(cr, uid, 'default', False, ['unexisting_model'])
-        assert d[0][1] == u'my_test_ir_value', "Can't retrieve the created default value."
+        assert d[0][1] == 'my_test_field', "Can't retrieve the created default value."
         assert d[0][2] == 'global value', "Can't retrieve the created default value."
 -
     Do it again but for a specific user.
 -
     !python {model: ir.values }: |
-        self.set(cr, uid, 'default', False, 'my_test_ir_value',['unexisting_model'], 'specific value', preserve_user=True)
+        self.set(cr, uid, 'default', False, 'my_test_field',['unexisting_model'], 'specific value', preserve_user=True)
 -
     Retrieve it and check it is the one for the current user.
 -
     !python {model: ir.values }: |
         d = self.get(cr, uid, 'default', False, ['unexisting_model'])
-        assert d[0][1] == u'my_test_ir_value', "Can't retrieve the created default value."
+        assert len(d) == 1, "Only one default must be returned per field"
+        assert d[0][1] == 'my_test_field', "Can't retrieve the created default value."
         assert d[0][2] == 'specific value', "Can't retrieve the created default value."
+-
+    Create some action bindings for a non-existing model
+-
+    !python {model: ir.values }: |
+        self.set(cr, uid, 'action', 'tree_but_open', 'OnDblClick Action', ['unexisting_model'], 'ir.actions.act_window,10', isobject=True)
+        self.set(cr, uid, 'action', 'tree_but_open', 'OnDblClick Action 2', ['unexisting_model'], 'ir.actions.act_window,11', isobject=True)
+        self.set(cr, uid, 'action', 'client_action_multi', 'Side Wizard', ['unexisting_model'], 'ir.actions.act_window,12', isobject=True)
+        self.set(cr, uid, 'action', 'client_print_multi', 'Nice Report', ['unexisting_model'], 'ir.actions.report.xml,2', isobject=True)
+        self.set(cr, uid, 'action', 'client_action_relate', 'Related Stuff', ['unexisting_model'], 'ir.actions.act_window,14', isobject=True)
+-
+    Replace one action binding to set a new name
+-
+    !python {model: ir.values }: |
+        self.set(cr, uid, 'action', 'tree_but_open', 'OnDblClick Action New', ['unexisting_model'], 'ir.actions.act_window,10', isobject=True)
+-
+    Retrieve the action bindings and check they're correct
+-
+    !python {model: ir.values }: |
+        actions = self.get(cr, uid, 'action', 'tree_but_open', ['unexisting_model'])
+        assert len(actions) == 2, "Mismatching number of bound actions"
+        #first action
+        assert len(actions[0]) == 3, "Malformed action definition"
+        assert actions[0][1] == 'OnDblClick Action 2', 'Bound action does not match definition'
+        assert isinstance(actions[0][2], dict) and actions[0][2]['id'] == 11, 'Bound action does not match definition'
+        #second action - this ones comes last because it was re-created with a different name
+        assert len(actions[1]) == 3, "Malformed action definition"
+        assert actions[1][1] == 'OnDblClick Action New', 'Re-Registering an action should replace it'
+        assert isinstance(actions[1][2], dict) and actions[1][2]['id'] == 10, 'Bound action does not match definition'
+
+        actions = self.get(cr, uid, 'action', 'client_action_multi', ['unexisting_model'])
+        assert len(actions) == 1, "Mismatching number of bound actions"
+        assert len(actions[0]) == 3, "Malformed action definition"
+        assert actions[0][1] == 'Side Wizard', 'Bound action does not match definition'
+        assert isinstance(actions[0][2], dict) and actions[0][2]['id'] == 12, 'Bound action does not match definition'
+
+        actions = self.get(cr, uid, 'action', 'client_print_multi', ['unexisting_model'])
+        assert len(actions) == 1, "Mismatching number of bound actions"
+        assert len(actions[0]) == 3, "Malformed action definition"
+        assert actions[0][1] == 'Nice Report', 'Bound action does not match definition'
+        assert isinstance(actions[0][2], dict) and actions[0][2]['id'] == 2, 'Bound action does not match definition'
+
+        actions = self.get(cr, uid, 'action', 'client_action_relate', ['unexisting_model'])
+        assert len(actions) == 1, "Mismatching number of bound actions"
+        assert len(actions[0]) == 3, "Malformed action definition"
+        assert actions[0][1] == 'Related Stuff', 'Bound action does not match definition'
+        assert isinstance(actions[0][2], dict) and actions[0][2]['id'] == 14, 'Bound action does not match definition'