Revision on 41fe7f9d11
This commit intended to reduce the network load when reading records with
binary fields. This was made possible by enforcing the contextual key
`bin_size` to be set for records with binary fields.
However, it is causing some issues (see #22222, #22231) which are:
- write with bin_size:true generates a traceback
- read/search_read with bin_size:true breaks some images
on some views (e.g. base.view_partner_form)
Since the problems outweight the gain of these changes, we revert them.
Field widgets now have a key 'context' that let them
extend the context of the dataPoint (e.g. list).
In particular, widgets on binary fields enforce the contextual
item {bin_size: True}, so that the server gives the size of the
binary field as its value, instead of its content.
This change on binary fields reduces network load when accessing
a view with binary fields.
Here is a situation where we had a problem:
- a form view with a one2many field, which has no inline views
- the (non inline) list view has a field A, and is not editable
- the (non inline) sub form view has fields A and B, with an onchange on
B which modifies A
In that case, the user could do this:
- go to edit mode
- click on 'add' a new one2many line
- change the value of B in the form view, this changes the value of A
- click on save to close the modal form view
- click on the new o2m record to reopen the modal form view
- rechange B
=> the onchange does not work
The explanation is that when we reopen the modal, we update the known fields
information, but we had to perform a fieldviewget to fetch the list
view, so we have a full knowledge of the fields. However, the code did
not update the fields info (because it uses _.default), which means that
the onchange information contained in the form view is lost.
Note: the test system had to be adapted to more closely simulate what
actually happens. In particular, the onchange flag is no longer added
by the mock server, since it should be done by the data manager, like
'real' code. This change broke the basic model tests, which had to be
modified accordingly, by setting manually the onchange flag.
This commit introduces a new way to hide a column in a x2m list view.
The attribute 'tree_invisible' can be add in the field attrs in the x2m
view definition. This attribute can use the 'parent' key to make a reference
to the parent record (e.g. 'parent.id').
This rev. fixes several issues with the sorting of dataPoints of
type list in the BasicModel.
First, only 'static' lists (i.e. x2many lists) must be sorted
client-side, and results of search_read RPCs must remain unsorted
as they are already retrieved sorted from the server.
Second, editable and non-editable x2many lists don't behave the
same with respect to sorting (we reproduced in this commit the
behavior of saas-15). With editable lists, the edited rows are
unsorted, except if the user explicitely ask for a sort (by
clicking on a column header), until the form view is saved. With
non-editable lists (as for x2many kanban), the sort is performed
directly when a record is created or updated (as the edition is
done from a dialog).
When an onchange modifies an x2many field, the line on which the user
works remains in editable mode and the cursor is returned to the same
position.
The second argument of create command is a reference used to reactivate
the edited line.
In general (e.g. read, search_read), values for many2one fields are pairs
[id, display_name]. The default_get is an exception, it only returns the id,
so we need to manually perform a name_get afterwards. In the case of
list views, those name_get RPCs are batched.
Before this rev., the code in charge of it didn't handle the case where the
many2one value was false, so it crashed when it happened.
This rev. deals with this use case and a test case has been added.
The value for one2many fields returned by default_get may be a
list of commands, or directly a list of ids. These cases were
correctly handled. However, it seems that the value can also be
false, and when this happened, it crashed. For instance, go to
Sales > Leads (must be activated in the Settings) > in the list
view, check several leads > in 'Action', click on 'Convert to
Opportunities' > crash.
Commit 7cd2f6370 (in 10.0) recently added attribute special='cancel'
on the 'Cancel' buttons of the Settings views in Odoo. The attribute
wasn't really supported in this case by the old web framework, so
this commit also slightly adapted it, and made it reload the whole
webclient when such a button is clicked.
This commit now needs to be forwardported in saas-16, so we have to
handle the case in the new views as well. Before this rev., it
crashed, because the BasicModel tried to reload a new record (the
one of the Settings form view), so basically it performed a 'read'
RPC on a virtual ID like 'virtual_123', and the server didn't really
appreciate.
This rev. handles the case where a new record is reloaded, and
simply performs a 'default_get' instead of a read. Bonus point:
with the new views, we don't have to reload the whole webclient.
Date fields have magic grouping methods to specify how to group on
them like date:month, date:weeks, date:days for example. It needs
to be handled properly since date:month is not a valid field name
but date is.
Steps to reproduce the issue:
- Go to Sales/Dashboard
- Click on My Pipeline
- Group by "Creation Month"
Basically, this can be triggred from any view which has a search
view which defines a group using the magic date grouping methods.
In previous versions, readonly used to prevent sending the values
of the fields to the server. However, it was not working properly
for fields which have their own sub-arch in the view, as the value
used to be sent regardless of the readonly value.
In the new views, the readonly modifier is never overlooked, which
created a bug in the stock_picking_return wizard which was not
able to return products received from a po anymore. Basically the
value of the product_id field was not sent so the create crashed.
This commit introduces a new "force_save" modifier which is used
to express that we want to save the value of the field even though
it is readonly from a user interface point of view.
Some one2many fields are called with a many2many widget to unlink
records instead of delete them. But this behavior was modified with the
new views and the records were deleted anyway.
In this commit, we restore the previous behavior, and create a FORGET
command in basic model
to do that. Note that the widget='many2many' is a hack, and a better way
to accomplish this is to use a new custom option. But this would be a
fix for master (PR #18413)
Before this commit, the web client read the readonly in the modifiers,
and if no readonly was in the modifiers, it fell back to the field
property. But the field property was already taken into account in the
modifiers, so this was useless. And worse, in some case, it could cause
an issue. For example, a field with readonly=true, and readonly="0" in
the arch was considered readonly.
... when reading and writing on a record (especially inside lists).
Before this rev., the context sent when reading a record, and when
writing records inside lists (editable list views) wasn't correct.
This caused an issue in Sales > Products (variant activated) >
open a product with variants > click on variant prices > edit the
prices of a variant: the value wasn't saved correctly (because the
inverse function requires 'active_id' to be in the context and it
wasn't the case).
The reference fields haven't been correctly implemented with the new views.
This commit restores their behaviour.
Note that an small improvement has been introduced in this commit.
Before saas-16, a `name_get` was done for each record (so 80 if the field
was displayed in a full list view), which is not optimal. The calls are now
batched by model (so one `name_get` by model appearing in the field values).
When x2many fields have to be evaluated in domains (e.g. ['id',
'in', some_x2many_field]), they must be evaluated as the list of
ids in the relation (whereas in contexts, they are evaluated as a
list of commands). Before this rev., they were always evaluated as
a list of commands.
Note that this didn't work neither before the new views.
Datapoints of the BasicModel store the data fetched from the
server and changes done (not saved yet) by the user. For x2many
fields, datapoints are of type 'list' and have a 'res_ids' key
containing the ids of all records in the relation. They also have a
'data' key containing datapoints of type 'record', representing the
records in the relation that have been fetched (with a limit set to
40 or 80 depending on the subview used). So basically, the length
of res_ids may be different than the length of data.
Before this rev., the changes in x2manys were saved under the key
'_changes' as a duplication of the content of 'data', i.e. same
list of datapoints of type 'record', modulo the changes done (it
contained the new records added and didn't contained the removed
records, whereas the 'data' remained unchanged). This didn't worked
at all when there were more records than the limit in the relation.
Indeed, when this happened, the commands to send to the server
couldn't be generated correctly.
This rev. modifies the way changes are stored in those datapoints.
Now, each change is stored as an operation. An operation could be
of type 'ADD', 'REMOVE', 'REMOVE_ALL' or 'UPDATE'. As before,
'data' remains unchanged and always keeps the datapoints of records
that have been fetched (the ones that are currently displayed in
the list or kanban subview). This way, commands can now be properly
computed.
This had a nice side-effect of fixing two other bugs linked to
x2manys.
First, when used in a context (e.g. {some_key: some_x2many_field}),
the value of some_x2many_field (i.e. a list of commands) was always
[] if the field was invisible="1" in the view, because it's
subrecords weren't fetched, and the generated commands were based
on them. Now, they are generated from the list of res_ids in the
relation, so it works fine.
Second, the x2many records are supposed to be sorted client-side,
after being read, and after each edition. This was working in the
first case, but not in the second. Now it's working even after
a record edition.
This rev. also correctly sets the 'parentID' and 'static' keys in
datapoints.
Finally, we also fixed the pager in x2manys as it was badly
displayed (small less tweaks).
When creating a new record, the numeric field default value should be 0 in order
to avoid manually setting mandatory fields to 0.
This behaviour has been removed with the new views but one wants to restore it.
The `mockRead` function has also been adapted in this commit because the server
returns 0 for unset numeric fields.
The mock server had to be modified in order to test this behavior
since it used to throw an error for invalid ids, while the actual
server just ignores it and continues forth with the other ids.
Ultimately, it seems that sending the value of readonly fields
when saving is not such a good idea. Suppose that there is a
computed field displayed as readonly in a form view. This field is
computed from other records, e.g. from a one2many field which is
editable in the form view. Finally, the inverse function of the
computed field creates the one2many records. If the user adds some
records to the one2many, then saves, the readonly computed field's
value is sent to the server as well, and thus the inverse function
is executed, which isn't what we want.
This commit essentially reverts 0494d61274, except that we now take
into account the readonly modifier to determine whether or not a
value is sent to the server, and not only the readonly attribute of
the field (which can be seen as a default). Before rev. 0494d61274,
we made a difference between write and create RPCs, for an unclear
reason. We don't do that anymore: readonly fields are never sent to
the server (same behavior as before the new views).
Suppose that a default_get returns the value [[6, 0, [1]] for an
x2many field (i.e. put id 1 in the relation). A read is performed
to fetch record of the comodel whose id is 1. Suppose that there
is another x2many field of the comodel. This x2many has to be
fetched (if not empty), and processed by the BasicModel (i.e. a
dataPoint must be created for it) as well.
Before this rev., the second x2many wasn't fetched nor processed,
and it produced a traceback for example in Expenses app, when
clicking on 'Submit to manager' from an expense form view.
Suppose that we have a many2one which is set (has a value) in
a form view. Before this rev., if the user unset the value (by
erasing the input content in edit mode), and then reset the same
value, the input remained empty, and the old value wasn't actually
reset.
In some cases, the parentID was not properly set, which is a problem
when we need to evaluate the parent in a context.
Note: I also added a better error explanation in the mock server, to
make it better for the developper to see what is wrong.
Eval contexts are not easy... They should be different when sent to the
server and when evaluated in the web client. In this commit, we fix
some small issues:
- when evaluated on the web client, x2manys should be a list of ids,
when sent to the server, it should be a list of commands
- dates should be converted to string
- always add current_date in the evaluation context, as a magic key (it
was previously only available in row decoration context evaluations)
This rev. fixes the next three bugs:
- the dataPoints representing each group didn't have the parentID
attribute set
- when quick creating a record, the res_ids and count attributes
of the group, and of the parent weren't correctly updated
- when quick creating a record, the environment (view_manager)
wasn't correctly updated
The combination of thoses bugs produced the following crash. Open
a grouped kanban view (e.g. project.task), quick create a record,
open that record in form view (note that the pager was wrong, it
indicated 0/x, where x number of records), delete the record ->
the form is supposed to load the next record, but it
crashed because it couldn't get its id (trying to read id null).
When grouping on a selection field, the read_group may return a
group for value false (containing records with the selection field
unset, typically). When this happened, there was a crash because
the BasicModel didn't handle that case (it assumed that the value
of the groups was always a valid key of the selection).
... from a new record.
Steps to reproduce the issue:
- go to a multi-record view (e.g. list) of basically any model
- click on 'Create' -> it opens a form view in edit mode
- click on 'Discard' -> it goes back to the list
- click on an existing record -> it should open the selected
record in readonly.
However, it didn't display the data of the selected record, but
the default data of a new record (the result of a default_get).
This was because the changes weren't properly discarded when
reloading from a new record (it rather restored the record to its
initial state, i.e. to the result of the default_get).
The 'data' attribute of exported datapoints (using 'get') must
contain a value for each field in the view, and the default value
is false when those fields are unset. The code manipulating those
datapoints supposes that this is the case and it makes it easier
and clean.
However, it wasn't the case for x2manys in new records for which
the default value contains CREATE commands with missing fields:
in that case, those missing fields weren't added in data.
This caused a crash in the Employee form view, in edit, if the user
clicked on 'Create and Edit' of the 'Working hours' field, because
some code in field_utils assumes that if the value of a date field
isn't false, it means that it is a moment instance (but in that
particular case it was undefined).
Since the new views, the editable list view lost some behaviors. These
behaviors were indeed implemented in the form controller/renderer but
as the new editable list view does not use an inline form view, these
behaviors had to be implemented in the basic controller/renderer.
For this to work, the list editable renderer had to be changed as it
was doing work that should be done by the controller.
The problem is even more complex because the x2m fields are using a
list renderer but not a list controller. So moving code from the
renderer to the controller obviously broke the x2m fields. Right now,
the problem is solved by catching renderer events and forwarding new
ones to the form controller (which handles the x2m specifically).
The initial goal of this commit was to share the validation of records
on save. Indeed, the fields were marked as invalid in the form view but
not in the list view. Also, before the new views, the editable list
view had different behaviors if they were used for a x2m field or not.
As these behaviors are making sense, this commit tries to restore them.
Basically, what we want is:
- When a record is saved (form view save or list view line leaving),
the invalid fields are marked (in red), the names of the related fields
are notified to the user and the record is not left.
- When a record is discarded (form view discard or list view discard),
the user is asked to confirm if the record is dirty before making the
record readonly.
- When a record is discarded, if the record is a new one, then the
record should be abandoned (removed as if never existed). In the form
view this induces to go back in the history and in the list view, to
remove a row.
- For x2m fields, the notification of invalid fields is not triggered
but the user is instead asked if he wants to discard the changes made
to the row (indeed, this replaces the list "Discard" button, as non
existent for x2m lists).
- ...
Saving, discarding, marking the fields as invalid and other behaviors
are thus now shared behavior of basic views.
The management of the dirty flag has also been moved to the model
as it was handle by the controller for the form view but by the
renderer for the list view. Now this flag is directly managed in the
basic model (the model can have changes thanks to the `_changes`
property but not be dirty (this is the case for creations)).
This change however created a problem. The view manager is currently
keeping asking if there are changes to discard at each action which
might lead to leave a dirty record (appswitcher / url change / ...).
It however did not discard anything as leaving if the user is ok with
it will lead to an implicit discard. However, as the view manager might
ask for this discard multiple times by second, the controller was
marking the record as not dirty the first time but without discarding
the changes. This is more complex to do now, as the dirty state is
part of the model and that the renderer should match the model data.
To solve this problem, the view manager now actually discard changes
explicitely when asked to. Even though this had been optimized to not
cause any rerender in some cases where it is not needed, this could
cause some performance decreasing. However, this makes some cases more
logical (opening the app switcher on a dirty form view then going back
to the form view by hitting the "go back" button left the form view
untouched although the user asked to discard it). This solution will
be improved with the view manager refactoring.
This commit is also making use of the `commitChanges` system which had
been implemented for HTML fields. Indeed, these fields cannot know
about all of their changes, so when hitting the save button, we asked
those fields to commit their value. Using this system is a great way
to make the `isValid` method of x2m fields synchronous. Indeed, before
this commit, the method was sometimes asking the user if he wants to
discard an invalid line before save. That case can be handled by the
x2m `commitChanges` method: we consider that saving the lines of x2m is
an operation that has to be done before considering the save, so we ask
all the x2m fields to do so at that time. Also, the system was broken
since a recent commit: we indeed protected the changes - save order
with a mutex but unfortunately, the `commitChanges` method was part of
the save and the changes it triggered were not able to be considered
because of this mutex. This had not been detected by tests as there is
not current way to test html fields.
This commit actually fixes a few problems:
1. the _visitChildren method in the basic model was not following the
changes, only the data, so it was not correct (for example, the isDirty
method was wrong for relational data, when no other change was done)
2. new records could not be discarded, because they had no data in their
data key. What we do here is to add a savePoint, so it is safe to
restore them (it caused a crash)
3. the save method was not properly following children when it was
called with the option savePoint=true
4. when the user tried to discard a form view, in an action with only a
form view, it was redirected to the previous url (via the history_back
action), instead of simply discarding the current form view
when calling 'read' to fetch a record and 'search_read' to fetch a
list of records.
I don't know where (or if) it is used, but it seems more correct
like this (and mostly, it was the case in the old views, so we
re-introduce the same behavior).
Before this fix, default values for many2many fields (i.e. 'replace' commands)
weren't correctly handled by the model, so the default values weren't set for
those fields.
- When an one2many is displayed in a list view, the field data are not fully loaded
since only the number of record is displayed. Then when the related form view
is opened the one2many fields displayed need to be loaded.
- Optimized the fetch of x2manys without doing an useless rpc
if we don't need all the fields but only the relation records ids and adapt
the tests for this optimisation.
- Fix wrong inheritance for kanbanmany2many_tags
Before this rev., the context sent when creating (or saving) a
record was incomplete. Thus, the created record might be incorrect.
For example, go to Sales, open a team's pipeline, click on 'Create'.
The created record should belong to the selected team, thanks to
some keys specified in the context. This wasn't the case before
this rev.
The evalContext contains the current data of a record
(the value of the fields that have been fetched,
i.e. the ones referenced in the view).
However, 'id' is a particular field that should always
be in the evalContext, even if not in the view
(as we always know its value).
Before this rev., 'id' was in the evalContext if
the record already existed, but not on new records, so it crashed
on new records if 'id' was used in the context defined on a field's node.
Before this commit, all onchanges were applied if the field
was known on the model.
However it's not possible to apply onchanges on x2many not in
the view as we don't know their fieldsInfo.
This commit also use `_applyChanges` directly instead
of using `notifyChanges` to avoid the mutex which will
cause a deadlock (we are already in the mutex).
There were several issues with this widget, and the way the model
handles its requests (mainly the removal of values).
It crashed for instance on the product form view when the user
updated the routes field (field route_ids in tab inventory).
The readonly attribute of a field in its description can be
overriden in the views (e.g. with modifiers). So readonly fields
may actually be edited from views in which there is a modifiers
on their node overriding the default value.
Before this rev., readonly fields couldn't be saved, even if they
could be edited from views (with such modifiers).
For instance, it wasn't possible to change the value of the
partner_id field in a draft sale_order.
In a form view, if a one2many list view contained a many2many
field (e.g. widget many2many_tags), the commands for the embedded
many2many weren't generated for newly created one2many subrecords,
so this field couldn't be saved.
Alexandre Kühn