openerp-expert-framework team mailing list archive

[Thibaut DIRLIK <thibaut.dirlik@xxxxxxxxx>]

Hi !

2012/6/4 Olivier Dony <odo@xxxxxxxxxxx>

[snip]


>  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.
>

I think we should Keep It Simple. If a simple list of ids is enought, why
would be
make it harder to read ? I think DELETE, UNLINK, LINK and SET-LINKS should
support a list of ids, instead of repeating the tuple multiple times.


>
>
> >     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?
>

Yes, it does. Seems goods to me.


>
>
> >     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 B point should be disabled only on o2m and allowed on others fields, it
could be useful.