Tryton - Issues

 

Issue8880

Title Get clients to use a better default position for new records
Priority feature Status testing
Superseder Nosy List albertca, ced, reviewbot
Type feature request Components sao, tryton, trytond
Assigned To ced Keywords review
Reviews 276501003
View: 276501003

Created on 2019-11-29.13:37:10 by ced, last changed by reviewbot.

Messages
review276501003 updated at https://codereview.tryton.org/276501003/#ps268581002
msg53753 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2019-11-30.10:39:03
The problem is that we set a default order on id (to have constant order on reload because database does not have it).
I think we could add support for empty order in the definition (which will still be converted to ASC in ModelSQL). Then the client will ignore those 'id' clauses and fallback to the default behavior. The default behavior can be improved to return -1 when there is a parent and otherwise 0 (so on main tab new record will be added on top and on xxx2many field it will be added at the bottom).
msg53750 (view) Author: [hidden] (albertca) (Tryton committer) Date: 2019-11-29.22:57:59
Missatge de Cédric Krier <bugs@tryton.org> del dia dv., 29 de nov.
2019 a les 17:16:
>
>
> Cédric Krier <cedric.krier@b2ck.com> added the comment:
>
> Moreover I prefer a solution that "magically" fix most of the existing views (like the current sale, purchase and invoice) than adding a new option that requires to review all the views and allow inconsistent behavior.
> For the special cases, it will always be possible to write an order clause that will provide the desired behavior.

I reviewed the patch and I like the fact that the position where new
records are added are part of the action (instead of the view arch).

However, I don't think this solution magically fixes most existing
views. Parties are still sorted by ID in ascending order, the same
happens with addresses, products, product suppliers or accounts, to
name a few. Almost all of those tend to have thousands of records and
thus will suffer from the same problem.

The only way I see to fix it without having to review all views, is
simply to set ('id', 'DESC') in ModelSQL, although that probably means
that all models that change the _order should be reviewed anyway.

Another possibility is to make the top/bottom choice a Seleciton field
in the Action, set "Top" as default and do not take into account the
order.
review276501003 updated at https://codereview.tryton.org/276501003/#ps260621003
msg53748 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2019-11-29.17:16:38
Moreover I prefer a solution that "magically" fix most of the existing views (like the current sale, purchase and invoice) than adding a new option that requires to review all the views and allow inconsistent behavior.
For the special cases, it will always be possible to write an order clause that will provide the desired behavior.
msg53747 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2019-11-29.16:52:44
> I'm not specially excited about the proposal. I think we can easily find cases in which you may want the oldest records to be shown at the beginning ('id', 'ASC') and in those cases the user will have a bad UX because newly created records are immediately shown at the end, causing Sao to hang for several seconds or minutes.

The client must put the record at the best place where it will be.

> I would propose to:
>
> - Make "editable" attribute accept only "1" or "0"
> - Add a "new" attribute that accepts "top" or "bottom" (and default to "top" if not set)

Indeed we should change editable to be only boolean and always rely on the order for the position.
msg53746 (view) Author: [hidden] (albertca) (Tryton committer) Date: 2019-11-29.16:50:15
I'm not specially excited about the proposal. I think we can easily find cases in which you may want the oldest records to be shown at the beginning ('id', 'ASC') and in those cases the user will have a bad UX because newly created records are immediately shown at the end, causing Sao to hang for several seconds or minutes.

I would propose to:

- Make "editable" attribute accept only "1" or "0"
- Add a "new" attribute that accepts "top" or "bottom" (and default to "top" if not set)

This way, non-editable tree views could override the default "top" behaviour if really needed.
review276501003 updated at https://codereview.tryton.org/276501003/#ps252721002
msg53736 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2019-11-29.13:37:10
By default if the editable option is not define on a tree view, the clients put new record at the bottom. This may not be the desired behavior for example many documents are ordered by descending dates (and ids) like the sale, invoice and purchase.
Also on sao, when a new record is added at the bottom when the user switch back to the list view, sao has to read all the record to the bottom to display it.

I think we could provide a better default behavior (not perfect but at leaste better) if the server send the default order when the action window does not have one and also on the definition of the xxx2many.
This way the client could search for the clause about the id and decide to put new record at the bottom or on top. This is based on the idea that we always set an order for 'id' that matches the expected behavior of creating new record. And if there are no clause for 'id' we keep the current behavior.
History
Date User Action Args
2019-12-02 14:07:10reviewbotsetmessages: + msg53789
2019-11-30 10:39:04cedsetmessages: + msg53753
2019-11-29 22:58:00albertcasetmessages: + msg53750
2019-11-29 20:08:38reviewbotsetmessages: + msg53749
2019-11-29 17:16:38cedsetmessages: + msg53748
2019-11-29 16:52:44cedsetmessages: + msg53747
2019-11-29 16:50:16albertcasetnosy: + albertca
messages: + msg53746
2019-11-29 16:34:44reviewbotsetnosy: + reviewbot
messages: + msg53745
2019-11-29 16:23:16cedsetstatus: in-progress -> testing
reviews: 276501003
keyword: + review
2019-11-29 13:37:10cedcreate

Showing 10 items. Show all history (warning: this could be VERY long)