> 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' expression - and any field that has change_default=True will trigger an on_change-like call to ir.values.get() with 'field=value' as the condition.