openerp-expert-framework team mailing list archive

[Olivier Dony <odo@xxxxxxxxxxx>]

On 06/01/2012 03:54 PM, Thibaut DIRLIK wrote:
> 2012/6/1 Olivier Dony <odo@xxxxxxxxxxx <mailto:odo@xxxxxxxxxxx>
>     How do you guys think o2m serialization should work in the various cases?
> 
>     0) When returned by read() (server-side)
>     1) When returned as a _defaults value for a o2m field? (addons side)
>     2) When sent to create()? (client side)
>     3) When sent to write()? (client side)
>     4) When evaluated (client-side), e.g for on_change parameters or attrs?
>     5) Any other cases?
> 
> 
>     As a starting point:
> 
>     0) read() will always return real values with IDs, so keeping the current
>     format [ID1,ID2,...] sounds like the KISS option, with `False` indicating
>     no value.
>     In fact if we decided that the LINK(4) command is the default o2m command,
>     [32,34] could simple be a simplified version of [(4,32),(4,34)], so we'd be
>     using consistent formats in the various contexts.
> 
> 
> I'm fine with this. read() should only return ids.
>  
> 
> 
>     1) default_get() result is supposed to be similar to read(), so you'd be
>     tempted to say: exactly the same as for 0). Except you also need to return
>     default values that don't exist yet, and have no ID -> need to mix existing
>     values with new ones. As a result, we could say:
>     -> must always be a list of o2m commands LINK(4) or CREATE(0), and if LINK(4)
>     is the default, read()-like results would be supported too.
>     Should the RESET-LINK(6) command be accepted here too?
> 
> 
> This is partially broken today in GTK, mixing values is not possible, so you
> have to construct a dictionnary for existing ids and return a list of dicts.
> 
> What you propose seems good. I don't see where the RESET-LINK command
> could be used : if you don't want any entry, don't set a default value.

By RESET-LINK(6) I meant (6,0,[IDs]) - which is an alternative for saying:
UNLINK_ALL(5) + LINK(4) all the IDS. For the record, as documented in the
docstring of write(), we have:

- CREATE(0): (0,0,{values})
- UPDATE(1): (1,ID,{values})
- DELETE(2): (2,ID)
- UNLINK(3): (3,ID)
- LINK(4):   (4,ID)
- UNLINK-ALL(5): (5,)
- (RE)SET-LINKS(6): (6,0,[IDs])

So the 6 operation could make sense as a shortcut for a list of LINK(4)
commands, but the simplified list of IDs would be even shorter, so perhaps it's
cleaner to disallow it.


>     2) create() should only ever LINK(4) or CREATE(0) values, but other commands
>     are supported now, which I suppose is fine. If LINK(4) is the default, we'd
>     have to support bare IDs too in the command list.
> 
>     3) write() is meant to support the full range of commands (1-6), and should be
>     adapted to support bare IDs too if LINK(4) becomes a default.
>     Now, should the order of the commands matter? There are cases where it does,
>     become the ORM will respect the order when performing the operations, so a
>     CREATE(0) fail with a unique constraint violation simply because the DELETE(2)
>     that follows should have been executed before. Or perhaps the ORM should always
>     reorder the operations before performing them: [2,3,5,0,1,4,6]?
> 
> 
> This is exactly one of our problem today : we have to save between two
> entries to avoid contraints errors. It just sucks. Reorder operation is a good
> idea, delete should always happen first,

Does the suggested order [2,3,5,0,1,4,6] work for you?


>     4) For consistency we could say that the format for this case is the same as
>     the one for 1), as the intent is the same:"What are the current lines,
>     including possibly unsaved ones?"
>     This means deletions will not be returned, so if you need to know what was
>     deleted you'll have to diff against the current saved value.
>     What about partial changes and evaluating the o2m value while you're modifying
>     it (i.e. inside it)? If you are editing a o2m child record but haven't finished
>     yet, should the serialized value include some partial changes? Obviously it
>     cannot work 100%, so perhaps it should only include changes up to the last time
>     you started editing a line... but we can't really tell that, can we? (think
>     editable lists etc.)
> 
> 
> Deletion should be allowed in on_change. For example, changing a field could
> remove a line in a one2many in the same view (partial deletion : only one item).

Oh right. I think you're referring to a fifth case: the possible values addons
code may return from an on_change call. I forgot about that.
Let's make this 5) then:

0) When returned by read() (server-side)
1) When returned as a _defaults value for a o2m field? (addons side)
2) When sent to create()? (client side)
3) When sent to write()? (client side)
4) When evaluated (client-side), e.g for on_change parameters or attrs?
5) When returned by on_change calls to set o2m field (addons side)
6) Any other cases?

For 5) the current situation is quite confusing. I would imagine we need the
same values as 1) (default_get) plus the UNLINK(3) operation. The semantics of
DELETE(2) and UNLINK(3) could be the same here. Perhaps UNLINK-ALL(5) could be
supported too. What about the UPDATE(1) operation? It would work only for lines
that already have an ID, so it's a bit weird to support it, isn't it? And I
suppose if you really wanted to update a certain o2m line you could
UNLINK_ALL(5) then provide all the values again. (RE)SET-LINKS(6) should be
supported too if we allow it for 1) (default_get).

Now it becomes tricky if we allow such changes while the user is in the middle
of editing a o2m line. Imagine the return value removes the line you are
editing, or changes its position in the list, or modifies it. There's no way
the user will expect it. To avoid those confusing cases we need to set a few
extra rules:
- A. on_change() on o2m only triggers when we finish editing a line (e.g. close
popup or go to next line in editable lists)
- B. on_change() result on any field must not be allowed to alter 'parent'
record. `parent.field` is allowed in expressions to read a parent field, but it
cannot be used when returning values from on_change - only the current record
is accessible.

Sounds ok?


> The problem will be to identify non-saved lines, without ids.
> 
> For partial changes while editing a one2many, they should be considered only every
> "field change" is ok : when you hit TAB in GTK to change a field (after
> on_changer triggers, etc).
>
> This would work for editable lists too. Anyway, I don't see a clear use case
> for this.

Triggering the o2m's on_change() after each individual field change inside a
o2m line would in fact be disallowed by rule A above, to prevent those
confusing situations. And I don't see any use case requiring it either.