Clicking multiple times on 'Create' (in main list views) or 'Add an
item' (in o2m lists) might create invalid rows (i.e. rows with
required fields unset).
Creating a new record requires two sequential RPCs: a default_get
and an onchange (only if there is an onchange of one of the
record's field). When clicking twice to create a new record, that
sequence of RPCs is done twice in parallel, after unselecting the
current row (i.e. saving it if it is valid). However, in both
cases, there is no line to unselect yet (as both operations are
done basically simultaneously). When the first onchange returns,
a first new record is added to the list. When the second returns,
the second one is added and takes the edition, leaving the first
one, invalid, in the list.
This rev. ensures this doesn't happen by preventing concurrent
record creation.
... before saving a line.
Suppose that we have an editable list with one required field
(e.g. many2one) with an onchange. We add a record and select a value
for the many2one. Before the onchange returns, we click again on
'Add an item'. If we don't wait for the onchange to return before
unselecting the row (i.e. before saving it), a confirm dialog asking
if the changes can be discarded opens, because the line isn't valid
yet. This could easily be reproduced on the sale_order form view,
with some throttling.
This rev. ensures that we wait for the onchange to return before
saving the line, and thus before creating a new line.
In a slightly too naive way, we apparently tried to improve the
parse_float method while rewriting the new views. This was done by
using a regexp to match all occurences of a thousand separator.
The way the regexp was built was fine, until the thousand separator is a
'.', which, if used as a regexp, matches all characters. Also, sadly,
'.' is not so rare as a thousand separator...
When a ´default_order´ is set on an embedded view for a x2many field,
the sort was only applied server-side, after saving the record. After
adding a record in a x2many, the records were thus not correctly sorted.
We reintroduce here the client-side sort after the x2many modification.
With the new views, the attribute autocomplete on input was no
longer supported. We simply reintroduce its support in this commit.
The basic attribute values are 'on' and 'off' but one may want to use a
random string.
Note that an input with type='password' is a special case: the autocomplete
attribute should always be set to 'new-password' to remove autocompletion
(autocomplete='off' doesn't actually remove the autocompletion).
When clicking on 'Save & New' in the form view dialog, a new default record
is created. The parent was not correctly set in this record, which causes an
error if `parent` was used in context.
The user menu was not visible in portrait mode with multi company in mobile. This was caused by the company menu taking too much room. We fix the issue by making the company menu smaller (just the size of an icon)
(PR #16685, task 29394)
Add support for value 'save' to attribute 'special' for buttons in
form view. This is useful for form views in dialogs. When clicked,
it saves the form view and closes the dialog (the default 'Save'
button doesn't close the dialog). Without this, the only way to
achieve that was to define a dummy python method and call it with
<button type="object" name="dummy_method"/>.
In view dialogs (e.g. click on the external button of a many2one
in edit mode), the attribute $buttons of the form controller is
undefined (the buttons are rendered by the dialog directly). The
code that (de)activate the buttons to prevent concurrent clicks
made the assumption that $buttons exist, so it crashed when it
doesn't.
For example: open a draft quotation, click on 'Confirm sale',
before this rev., the record was reloaded twice (each RPC done to
load the data was duplicated). This was actually tested, but the
tests didn't mock well the behavior of the ViewManager, so they
couldn't detect that those RPCs were done twice.
Suppose that there is a many2many list (non editable) in a form
view. When one of the many2many subrecords is edited (from the
form view in a dialog), it is saved directly when the user clicks
on 'Save' in the dialog.
Before this rev., if the user clicked on 'Save' in the main form
view after editing a m2m subrecord, a 'write' RPC was done, whereas
there were nothing left to save (as the subrecord had already been
saved).
Suppose that we have a form view with a one2many list (non
editable) containing a record. In edit mode, the user clicks on
that record to edit it in a form view (in a dialog). In this form
view, there is an x2many field that isn't in the list. Before this
rev., it crashed when the user clicked on 'Save' in the dialog
after editing the value of the x2many.
This was because the basic model didn't use the correct fieldsInfo
in that case (it use the fieldsInfo of the list view instead of the
one of the form view, and the x2many field isn't in the list).
Also ensures that the record is correctly saved even if editing the
x2many is the only change done, as this wasn't the case before.
Closes#16576
Description of the issue/feature this PR addresses:
According to HTML good practices, when an element is used but there's no text displayed on the screen, the attribute aria-label should be used to label that element.
This is the case in the buttons to change view types (List, Kanban, etc) since there's no text displayed, but an icon instead.
This HTML attribute is specially useful for screen readers, which usually are not able to figure out a button's functionality when that button is not properly labeled.
Current behavior before PR:
Buttons are not properly labeled
Desired behavior after PR is merged:
Appearance should be the same, but the produced HTML should be of better quality, because it would fit more HTML good conventions and it whould be more accessible
Related issue:
I had already reported this issue on the branch 10.0 at PR #16030. However, I decided to report it on this branch, because I believe this branch is the right place to be reported.
When opening a modal on top of another, we expect the associated
modal-backdrop to be over this another modal (so that it is grayed).
This was not the case as bootstrap forces the modal-backdrop z-index
to a lower value than the modal's one (see fix comments in the changes
below).
In the list controller, some code to determine the active domain to
export was causing a crash. It was easy to see: just select all records
in a list view, then click on 'Export' in the sidebar.
Note that the data_export code is still quite old, and did evolve
organically. It does not conform to our new design principles, in
particular, the way component should communicate. But this is a task for
another day.
With the new views, the special attribute password was no longer
supported. We simply reintroduce in this commit proper support to
display password fields with *.
Note: we may want to display passwords with a static number of *, but
this requires some thoughts on the usability of the change. We simply
reintroduce the previous behaviour here.
The 'Expand All' feature was tested, but in the test environment
RPCs are mocked, and everything is synchronous (by default). So
even if the tests passed, the feature actually didn't work because
of a deferred (the reload deferred) that wasn't waited for.
Keys 'pivot_measures', 'pivot_col_groupby' and 'pivot_row_groupby'
can be specified in the context and are meant to be used by pivot
views to determine the initial measures and groupbys. Before this
rev., the new implementation of the pivot view simply ignored them..
Before this rev., the _mockReadGroup function only supported
grouping on one field, and if called with an array of fields of
length > 1, the remaining fields were ignored.
Grouping on several fields is now supported, and some tests had
to be adapted.
Before this commit, the save method in the basic model did not properly
react when the rpc fails. In that case, the returned deferred stayed
indefinitely in the 'pending' state. This is a big problem, because the
basic controller put that deferred in a mutex. As a result, every
subsequent operations (including field changes!) were indefinitely delayed.
To reproduce the issue, you can find any form view where some python
business logic will cause a fail in some cases. For example, create a
new inventory adjustement, click on 'Start inventory', then add a line
in the x2many with the same product as another line. Saving that record
will fail, but then, the form view is broken: no interaction will work.
Before this commit, row decorations were only computed at the initial
rendering for a list view, then never recomputed. Most of the time, it
is not an issue, but list view can be edited, and in that case, it may
be a problem. With this commit, we just override the confirmChange
method to recompute the various decorations.
Note that in bonus, decorations now works in realtime, when editing a
list view (in saas-15, they simply did not work when editing a record)
Also, we now use the same evaluation context as everything else in the
web client: the eval context from the record, not some custom handmade
context like before.
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)
Here is what could happen before this commit: open a record (let's say
project.task) in form view. Then, on the sidebar (actions), choose
Merge Selected Tasks), choose a task, and click on Merge. What happens
next is a traceback.
Here is a more precise description of what happened:
- the formview dialog for the merge action closes, which triggers a
reload for the main formview
- the merge action returned an action description for an act_window
- the action manager instantiated a new viewmanager and prepare the form
view, which is an async operation (need to load some data to display
the form)
- the previous view manager (initial one) completes its reload, which
makes it rerender the view, which triggers a DOM_updated event
- the new view manager see this event, and tries to call getScrollTop on
the current view controller, but there are no controller yet, it is
not ready.
We could argue that some of these events should not happen (for example,
if we will open a new view manager action, why do we reload the current
view manager?). But solving these issues is a more deep architectural
problem (we probably will rewrite the actionmanager/view manager layer
at some point).
It could also be argued that in any case, the method getScrollTop should
not crash (same for setScrollTop). Sadly, these methods are fragile,
because some changes in the loading behaviour of a view may break them.
So, in any case, protecting them against unexpected traceback is a
worthwile investment.
With this commit, we just slightly improve list benchmarks, to make them
go through a wider variery of code.
Since we aim to use these benchmarks to track changes in performance
through various versions, it is useful to have a good version in
saas-16.
Before doing an onchange in a x2many, the basic model tries to evaluate
if the line is valid or not. In that computation, we used the
field.required property instead of using the required modifier. This
commit had to move the modifier handling from the BasicRenderer to the
BasicModel, in order to use it in the _isX2ManyValid method.
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).
Previously, it used to reload its data when the user clicked
multiple times on save, even though there were only changed
fields during the first initial save, all the other queued
ones have no changes at all and, as such, should not trigger
additional reloads.
Previously, it was possible to click multiple times on stat
buttons while still waiting for the result of a previous button
click. This could end up with the web client in strange states
or, worse, lead to tracebacks. Because of this, we decided to
disable the buttons once one of them is clicked and restore
them afterwards.
By doing this, we encovered a place in the code where a failed
deferred would not be properly forwarded back to the caller,
which meant, in our case, that the buttons would not be
enabled back in case the action failed. This is now fixed.
Initial methods to fix this issue used to break the test below.
Because the label of the assertion this used to break was
contradictory with the value it was actually expecting, we
decided to fix it as well.
Previously, the buttons were not updated properly after discarding
from an editable list view. The user was stuck with "Discard" even
though there was nothing to discard anymore, and he could not create
a new record either since the "Create" button was invisible.
This was fixed by 3aad756. I originally did a different fix which
has since been rendered useless, but the tests I wrote are still
meaningful since they are testing a different case than the commit
which fixed the issue.
Languages like Arabic (ar_SY), Hindi(hi_IN) do not use english digits to describe dates, and moment.js format function will keep these characters. So, the server will get date strings, and will be unable to use them.
To see the problem, just type moment().locale('ar_SY').format('YYYY-MM-DD') into a browser console.
In this commit, we simply force the locale to 'en' before sending data to the server.
PR #17156
Before this commit, it was only possible to click on input to
change the value of the many2many_checkboxes widget.
For better consistency, we must allow to click on labels.
The motivation for this commit was to fix a wrong evaluation context for
x2m fields: it was using a list of res_id, which may contain virtual
ids.
For example, it could be seen in the form view for journal entries:
adding the first line (field line_ids) was fine, but clicking on 'Add an
item' a second time caused a crash. The reason was that the line_ids
was explicitely added to the context in the view, and the second time we
add a line, the web client sent a list looking like this [35,
'virtual_43'] to the server, in the nameget context.
The proper behaviour is to send a list of commands for those fields.
This means that we had to do some non trivial changes in the
getEvalContext method: we stop using the get method with raw: true. It
was nice to use the same method, but the eval context is actually
different that the values returned by the get method. That was the
motivation to introduce the 'noUnsetNumeric' flag, but now, we simply
create another method and let each method do its own work.
TLDR: eval contexts are hard...
In Kanban views, the 'Create' button must be muted (btn-default)
in grouped mode, if there is no column and if the column quick
create feature is enabled (as in this case, the first action
should be to create a column).
Before this rev, it was actually muted if there were no record, no
matter if there were columns or not.
This was because the 'count' attribute of dataPoints was used to
determine the number of columns, but this attribute represents
the number of records in the dataPoint (i.e. the sum of the number
of records in each groups).
This particular case was tested, but the test passed thanks to
another bug (we incremented the 'count' attribute in the kanban
model when creating a new group, whereas we shouldn't).
The subtotals displayed in the oe_subtotal_footer (e.g. sale.order
form view) should be right aligned. Before this rev. this wasn't
the case for monetary fields, they are now displayed 'inline-flex',
whatever the form mode (before the new views, they were
'inline-flex' only in edit mode).
Credit goes to @qsm-odoo.
Before this commit, the o_field_invalid class was not removed once an
invalid field becomes valid, until the user saves the form view (which
will rerender the full view anyway)
It is not really easy to handle discard operations. We need to handle
the discard in editable lists, and in x2manys. This is not exactly the
same code, but most of this was done by moving the discard method from
the formcontroller to the basic controller.
With this commit, the user can now discard the changes for the current
record by pressing ESC. If the record is new, it will remove it, if
not, it will ask for confirmation before.
The issue that we fix in this commit is the following: open a form view
with a o2m with one required field, and the o2m is editable. When
adding a new line, it is invalid (since the field with required), but
when pressing ENTER, the editable list tries to navigate away, which
will discard the current line (in the unselectRow method), before
completing the move_to_next_line. So, at this point, the list editable
create a new (empty) record, and we are back to square one, but with a
visible flicker and no information on what happened.
To fix this issue, we simply check that the renderer can be saved when
moving to next line. If it is not (the line is invalid), then we simply
abort the move.
Notes:
* this behaviour worked fine in list view, because the canBeSaved check
is made by the saveRecord method in listController
* it looks like the protection should be put into unselectRow, but this is
not true (at least, in the current state): in some cases, calling
unselectRow with invalid state should result in discarding the current row
(in a new line in a o2m, when the record is not dirty, for example), and
the list editable renderer does not know if this is the case or not.
However, if the unselect action comes from a navigation event, then we want
to avoid discarding the current row.
When a many2one (in an editable list) with an onchange was edited, it
could happen that a modal window asking for confirmation of the creation
of a record was opened when it should not. Here is what actually could
happen:
* user focus a many2one
* user type something
* user press ENTER (keydown + keyup)
* user press ENTER again
* the onchange comes back
* modal window is open
Because of the onchange, the _reset method was called after the
_onInputKeyup method. So, the m2o_value was not equal to the
$input.val() and the 'floating' flag was set to true (after the first ENTER).
The second ENTER caused a unselectRow in the editable list, so the input
was focusouted, and the modal is set. This is basically caused by the
asynchronous behaviour of the onchange.
We simply fix the problem by setting the floating flag back to false in
the keydown method.
When a many2one (in an editable list) with an onchange was edited, it
could happen that a modal window asking for confirmation of the creation
of a record was opened when it should not. Here is what actually could
happen:
* user focus a many2one
* user type something
* user press ENTER (keydown + keyup)
* the onchange comes back
* user press ENTER again
* modal window is open
Because of the onchange, the _reset method was called after the
_onInputKeyup method. So, the m2o_value was not equal to the $input.val() and
the 'floating' flag was set to true, which means that the _onInputFocusout
method (called indirectly by the listeditablerenderer when unselecting the row)
was forced to open a confirm dialog.
We simply fix the problem by setting the floating flag back to false in the
reset method.
Before this commit, when the user write some text in a many2one, then
press ENTER, the key event will trigger a navigation move, which will
validate the field, and move the focus to the next line. The same issue
happens when pressing TAB.
With this commit, we simply stop the propagation, and preventdefault the
event that cause the selection.
Before this commit, when editing a many2one (with an onchange) by
clicking on the small form button on the right, then changing something,
then saving the modal dialog, no onchange was triggered, because,
technically, the value of the many2one was not changed, only the values
from the record pointed by the many2one were.
The same remarks applies to many2manys.
To see this issue in odoo, you can open a product (technically, a
product.template) from the 'Catalog' menu in sales, then edit it, then
click on the small form view icon next to Internal Category. This is a
many2one (categ_id) with an onchange. In the form view dialog, change
something, for example costing method, then save. The res_id of the
internal category was not changed, but an onchange should still be
triggered.
Olivier Dony