The previous code used to create its own args array
for the search_read method, replacing the one that
was originally given to it.
This lead to silent errors since positional arguments
to this function were completely ignored in the case
of search_read.
I took this commit as an opportunity to improve a
bit on the current state of these rpc helpers.
Namely, the basic args/kwargs priority is the following (top-to-bottom):
- kwargs defined in the rpc call
- kwargs defined in the kwargs key of the params kwarg of the rpc call
- positional arguments are passed as is
For read_group and search_read methods, the priority is the following:
- kwargs defined in the rpc call
- kwargs defined in the params kwarg to the rpc call
- kwargs defined in the kwargs key of the params kwarg of the rpc call
- positional arguments are passed as is
For the /web/dataset/search_read controller, only the params kwarg is
supported, resulting in the following priority:
- kwargs defined in the rpc call
- kwargs defined in the params kwarg to the rpc call
- positional arguments are passed as is
If both a kwargs and a positional arg is given for the same parameter,
it will be sent as is to the server which will crash with a typical
"got multiple values for keyword argument" TypeError. However, in the
MockServer however, we give priority to kwargs over args in this case.
Please note that I also chose to remove unnecessary default values so
that the ones actually used are the ones from the server, not the ones
that were duplicating those in the rpc js file.
Widgets which manipulates their dom in _renderEdit were left out
from the previous implementation since this processing was done
only once when the widget is rendered the first time. If the
widget regenerates a new input upon reset for example, then it
would lack the idForLabel.
When enterprise is installed, the rendering of these widgets on
extra small screens is different, thus breaking these tests. We
need to force the test to run on normal screens.
Before this commit, when we moved from one line to the next, we did not
wait for the unselectRow to end. This means that if the read (from the
unselect row) completes after the default_get, the list renderer was not
in a coherent state (currentRow was set to null), which could (and did)
cause a crash.
This was found by pressing the TAB key a few times, and editing some
values.
Also, we slightly improved the logging in the mock server. Before this,
the responses from the mock server did not show from which route it came
from. But in this test, we specifically make sure that the rpcs
complete in a different order, so it was not really optimal.
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.
The FieldTimezoneMismatch was redone, but an error was done after the
last code review. With this commit, we fix the mistake (a function is
properly called) and improve the test to make sure we catch such a
regression later.
Trash icon does not take account of the option on the tree tag
Before this fix: The delete option on the tree tag is not taken
into account in the widget
Before this rev., there was a traceback when grouping a list view
by a, e.g., many2one field, and then by a selection field. It
crashed when formatting the title of the group, because the type
of the grouped field was incorrectly determined (it selected the
type of the inner grouping level, not the current one). For
instance, it tried to format a many2one like if it was a selection.
When switching between records, the active tab of the previous
record is made active on the new record.
However, before this rev., this was done without checking that
this tab is still visible on the new record (there may be an
invisible attrs that makes it invisible), so it happened that
the active tab was actually invisible.
This was for instance the case when switching between stock
pickings, as the first tab ('Operations') is sometimes hidden.
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 fetching record, `bin_size` needs to be passed in the context
to notify the server to send the `bin_size` and not the whole base64
of binary fields.
Moves back the mobile rendering part of the FieldStatus to
enterprise, alongside all other tweaks to improve the UX in mobile.
This part had been moved to community inadvertendly in the new
views branch.
We changed the do_action method recently: before, it was calling
recursively its parent, now it simply triggers up an event. We also
moved it to the 'services' mixin. However, we did not properly connect
the 'on_success' option: it is not an option for the view manager, but
for the abstract web client. As such, it should not be in the options
parameter, but it should simply be a key in the event payload.
This commit was found by solving the two following issues:
- open sale application, click on 'Inbox' icon to go to discuss,
Discuss is opened, but the navbar stays with sales application
- after installing module website_crm_partner_assign in kanban view for apps,
it does not reload...
Both of these were caused by the need to do something after a do_action,
and the deferred was not resolved at all.
The context sent when performing RPCs is a mix of several contexts
(user_context, action's context, context defined in a field's node
in an arch...).
In general, the action's context should not be mixed with the
field's context:
- for global RPCs (read a record, write a record, default_get...)
only the action's context should be given
- for RPCs triggered by fields (e.g. name_search in a m2o), only
the field's context should be sent.
The exception is for onchange RPCs, as in this case, both action
and field's contexts should be sent.
Before this rev, the action context was always sent, which might
produced tracebacks (e.g. go to Sales > My Pipeline > New > click
on Customer field > Create and Edit > boom).
Before this commit, the form dialog was open twice when you tried to
add a new record on a one2many if you were already in a modal.
The event wasn't stopped, so the first form dialog was correcly rendered
but the second one tried to load the same form view on a wrong model.
The basic model did not transfer field options to special data methods
from fields. This was mostly not an issue, except for the widgets that
actually need them, such as the status bar and the domain editor.
This was only visible when opening views with different view types,
but this is a problem in general. We should have a better mechanism to
help widgets fetching data, but for now, this is the best we can do.
The views can specify js (and css) libraries to lazy load when
they are instantiated for the first time. The previous
implementation was too naive as it always loaded the specified
libraries in parallel. However, it may happen (e.g. gantt view)
that some of those libraries depend on other ones.
This commit implements a mechanism to handle libs dependencies
between each other. For instance, specifying:
js_libs: [
['a', 'b'],
['c'],
],
will load 'a' and 'b' in parallel, but wait for them to be
loaded before loading 'c'.
The new implementation still supports the old syntax. So,
js_libs: ['a', 'b', 'c'],
will load 'a', 'b' and 'c' in parallel.
In a notebook, tabs may have an invisible attrs. The first visible
tab should be active in any case. This wasn't working at the first
rendering of the view, because the invisible attrs was evaluated
for each tab, before processing the next ones, so the it couldn't
find a next tab to activate.
This fix ensures to process all tabs (i.e. generate their DOM
element) before evaluating their invisible attrs, and trying to
find which tab to activate.
Mode 'readonly' is a concept of list and form views. When
switching from a graph view to another view, then switching back
to graph, there was a crash because 'readonly' is not a known mode
of graph (in graph, mode is used to determine the type of graph to
display (pie, chart or bar)).
This is due to commit odoo/odoo@ef6b330 which aims to reset the
mode to readonly when coming back to a form view using the
breadbrumbs.
Before this commit, concurrency issues were mostly handled by the mutex
in the basic model. That mutex makes sure that save and change
operations are properly serialized. However, this is not enough. Here
is an example:
1. we have a required field with an onchange and a current value
2. the user changes the value to an empty value. This triggers an
onchange
3. the user clicks on save, before the return of the onchange
4. the controller checks with the renderer if it can be saved. At that
point, the onchange is not yet complete, so the values stored in the
fields are still the initial ones, which are valid (non empty value
for required fields)
5. so, the controller request a save operation
6. the model add the save operation in the mutex, wait for the onchange
to complete, then apply the save operation
7. we end up with a required field with an empty value
The issue here is that the model properly saves and apply onchanges, but
the responsability to check if a record is valid (which depends on the
modifiers AND the field widgets) is the renderer's responsability (it
has to ask the question to all its sub field widgets).
So, we need to serialize 3 operations: applying the onchange, checking
if the save is valid, and saving the record. This cannot be only in the
model, so we need to add a concurrency control mechanism somewhere else.
What we do in this commit is to add a mutex on the basic controller (it
will most likely be used by the list controller as well). In this
mutex, we currently serialize the field change operation and the save
operation, but I assume some other operations will need to be handled by
the mutex.
In addition to this, we also change some asynchronous methods to return
a deferred, so we are ready for other kind of asynchronous behaviour in
the future.
When a user opens a record in a form view, the chatter is
displayed. Then, if he clicks on 'Create', the chatter is removed
from the DOM (there is no chatter in create mode). After saving
the newly created record, the chatter is displayed again, but the
same widget's instance is kept.
However, when being removed from the DOM, the event handlers bound
on elements of the chatter (e.g. 'New Message', 'Log a Note'...
buttons) are automatically unbound.
So before this rev., the buttons didn't work anymore once the form
view had switched to create mode. In this commit, we detach the
chatter's $el before updating the view, so that its handlers aren't
unbound when the old content is replaced by the new one.
Before this rev., some keys were added to the context of a record
if there were x2many fields in the form view with a context
defined on their node (containing *_view_ref keys, specifying the
fields_view to load to display the relational data). Those keys
should not be added to the main record's context, and it may cause
errors (e.g. click on the 'Procurements' stat button in the Product
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.
This commit modifies the way field widgets can specify the fields to fetch.
Before this commit, a key `fetchSubFields` was used to fetch the `display_name`
and the `color` in the many2manytag, but this behavior wasn't generic.
A new key `fieldsToFetch` has now been introduced ; it should contain the
definition of all fields to fetch.
The editable list view (and the x2manys) have been reimplemented, but
the user experience was not totally (this is an understatement)
polished. With this commit, we reintroduce a good functionality:
pressing enter on the last line of the list creates a new line.
Also, this commit removes a 'debug: true', leftover by someone with a
trigram starting by F and ending by WI.
When we load a field data with many2many_tags widget, there is no views in the
field info but instead we need to fetch the default fields,
e.g. color, display name and id.
Before this commit, all the fields with value `false` on record creation
were deleted from the given parameters for the `create` rpc.
e.g. it was impossible to untick a checkbox in the settings, the `false`
value was not saved in the created record.
hr_holiday was the only module that used a widget=toggle_boolean on a
button in a list view. That behaviour was implemented as a column
widget in the previous list view, and was not reimplemented in the new
views. The feature is useful, but this was not done properly: it is
better to use a widget on a field (that was the intent) instead of doing
a weird hack like it was done.
With this commit, we update hr_holidays to use the ToggleBoolean widget,
which is supposed to work on every views.
Also, we fix the toggleboolean widget (it was not properly rendering
tooltips, and changes were not saved in readonly list view).
Note that it as the side effect of being better from the point of rpcs:
before, the list view had to reload itself. Also, another advantage is
that models do not need to implement custom methods (such as
toggle_payslip_status) just to toggle a boolean...
Before this commit, the code that handled buttons in list view was kind
of simple, and did not reload the view after.
But clicking on action button is not only a problem for the list view,
the form view already had something in the controller doing exactly
that. With this commit, we move that code to the basic controller, and
use the method in the list view.
Currently applying a color to a kanban item applies it to the whole
card. This creates usability issues as the content is not dynamically
updated to match the chosen color.
This commit proposes to apply the color only to the header and let the
card content standard. It helps designing cards that are always usable
and readable.
In general, I think that we can safely consider that crashes are not the
intended behaviour.
In this case, viewing metadata crashed because we tried to format a date
which was not parsed (so, a string), instead of a moment object. We
simply just parse the date before, and it works as intended.
With new records, it could happen that clicking on a button in the
header was followed by actions using the wrong id. This was caused by a
field name="id" in the view (see sale order view, in the
delivery.view_order_form_with_carrier inherited view).
In that case, the id was registered in the changes list, and caused
invalid data in the record: its data.id was set to null.
David Monjoie