Tryton - Issues

 

Issue6081

Title Allow to define sort of M2O, O2M, M2M and Reference fields when searching them
Priority feature Status resolved
Superseder Nosy List ced, nicoe, pokoli, reviewbot, roundup-bot
Type feature request Components proteus, sao, tryton, trytond
Assigned To nicoe Keywords review
Reviews 30851002, 25871002, 25981002, 31901003
View: 30851002, 25871002, 25981002, 31901003

Created on 2016-11-30.16:05:47 by nicoe, last changed by roundup-bot.

Messages
New changeset 7db95d0e97d3 by Nicolas ?vrard in branch 'default':
Add support for search_* attributes on ModelList
http://hg.tryton.org/proteus/rev/7db95d0e97d3
New changeset d468bff5895d by Nicolas ?vrard in branch 'default':
Add support for search_* attributes on relational fields
http://hg.tryton.org/sao/rev/d468bff5895d
New changeset 7331049f8836 by Nicolas ?vrard in branch 'default':
Add support for search_* attributes on relational fields
http://hg.tryton.org/trytond/rev/7331049f8836
New changeset c219609b2a1a by Nicolas ?vrard in branch 'default':
Add support for search_* attributes on relational fields
http://hg.tryton.org/tryton/rev/c219609b2a1a
review31901003 updated at https://codereview.tryton.org/31901003/#ps20001
review30851002 updated at https://codereview.tryton.org/30851002/#ps120001
review25981002 updated at https://codereview.tryton.org/25981002/#ps60001
msg37223 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2017-12-10.17:15:55
Why are the review for tryton, sao and proteus closed? Without them the trytond patch is useless.
review25871002 updated at https://codereview.tryton.org/25871002/#ps160001
review25871002 updated at https://codereview.tryton.org/25871002/#ps140001
review25871002 updated at https://codereview.tryton.org/25871002/#ps120001
review25981002 updated at https://codereview.tryton.org/25981002/#ps40001
review25871002 updated at https://codereview.tryton.org/25871002/#ps100001
review25871002 updated at https://codereview.tryton.org/25871002/#ps80001
review30851002 updated at https://codereview.tryton.org/30851002/#ps100001
New review31901003 at https://codereview.tryton.org/31901003/#ps1
review30851002 updated at https://codereview.tryton.org/30851002/#ps80001
review25981002 updated at https://codereview.tryton.org/25981002/#ps20001
New review25981002 at https://codereview.tryton.org/25981002/#ps1
msg31104 (view) Author: [hidden] (nicoe) (Tryton committer) (Tryton translator) Date: 2017-01-04.17:25:48
Here is review25981002 with the proteus patch
msg30971 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2016-12-26.16:56:22
I think proteus should also support them in the find method.
review30851002 updated at https://codereview.tryton.org/30851002/#ps60001
review25871002 updated at https://codereview.tryton.org/25871002/#ps60001
review30851002 updated at https://codereview.tryton.org/30851002/#ps40001
msg30751 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2016-12-15.09:18:20
With my last comment [1] on the review, I think search_domain is a bad idea.
I think the same behaviour can be achieved using a PYSON domain based on the id of the record or the state. We already have an example in sale_shipment_cost module for the field Sale.carrier.
So for me, we should drop the search_domain as it is a duplicate feature of the actual domain.


[1] https://tryton-rietveld.appspot.com/30851002/diff/20001/tryton/gui/window/view_form/view/form_gtk/many2one.py#newcode145
review25871002 updated at https://codereview.tryton.org/25871002/#ps40001
msg30722 (view) Author: [hidden] (nicoe) (Tryton committer) (Tryton translator) Date: 2016-12-13.18:34:15
* Cédric Krier  [2016-12-06 05:23 +0100]: 

>I have a concern about the search_domain as it is implemented because
>it could generate a constraint when creating new record form the search
>window. I think it is wrong to force value base on the search_domain
>because it should not be a constraint. Also about the UX, we could have
>weird behaviour like user clear the value of a Many2One but he can not
>add it back. So I think the search_domain should only be a default
>search filter.

Those are valid concerns and I implemented search_domain as a default
search filter by using the search_value attribute of the screen.

It works but the issue I have with it is that if the user does not
leave the field empty when making a search then the search_domain is not
used at all. But on the other hand this behavior is coherent with the
one used when updating the autocompletion, so it seems alright.
review30851002 updated at https://codereview.tryton.org/30851002/#ps20001
review25871002 updated at https://codereview.tryton.org/25871002/#ps20001
msg30581 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2016-12-06.05:23:39
I have a concern about the search_domain as it is implemented because it could generate a constraint when creating new record form the search window. I think it is wrong to force value base on the search_domain because it should not be a constraint. Also about the UX, we could have weird behaviour like user clear the value of a Many2One but he can not add it back. So I think the search_domain should only be a default search filter.
New review25871002 at https://codereview.tryton.org/25871002/#ps1
New review30851002 at https://codereview.tryton.org/30851002/#ps1
msg30560 (view) Author: [hidden] (nicoe) (Tryton committer) (Tryton translator) Date: 2016-12-02.18:06:14
Here is trytond review: https://codereview.tryton.org/25871002/
Here is tryton review: https://codereview.tryton.org/30851002/

sao will follow …
msg30529 (view) Author: [hidden] (nicoe) (Tryton committer) (Tryton translator) Date: 2016-11-30.18:40:56
* Sergi Almacellas Abellana  [2016-11-30 16:37 +0100]: 
>
>Sergi Almacellas Abellana <sergi@koolpi.com> added the comment:
>
>Indeed why aren't why implementing search_context and search_domain
>also?
>
>The first can be used for sale_ and purchase modules to show product
>quantities when searching products, but not setting the context when
>browsing the records on server side.
>
>The latter can be used to restrict the results of a select, for
>example only allow to select validated customers on sale without
>forcing a domain because the party can go to invalid after the sale
>have been encodeed.

OK let's do it too then.

According to me those two additional attributes should be merged with
their equivalent non search related attributes. For the domain, it's
easy: just AND them.

But for the context, it's a bit tricky which one should have the
priority. According to me the most local one (ie: search_context)
should have the priority.
msg30526 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2016-11-30.16:37:19
Indeed why aren't why implementing search_context and search_domain also? 

The first can be used for sale_ and purchase modules to show product quantities when searching products, but not setting the context when browsing the records on server side. 

The latter can be used to restrict the results of a select, for example only allow to select validated customers on sale without forcing a domain because the party can go to invalid after the sale have been encodeed.
msg30525 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2016-11-30.16:26:09
I think it is better to name it search_order because we will maybe one day implement search_context, search_domain etc.
msg30524 (view) Author: [hidden] (nicoe) (Tryton committer) (Tryton translator) Date: 2016-11-30.16:05:46
In issue5853 patch we're unconditionally sorting the M2O according to the proximity field.

Not only is this behavior not efficient in all the other cases, this kind of specific order when searching on a relation is a generic feature that is useful elsewhere.

Thus adding a keyword order_search on the relational fields defining the order used when searching would be a nice addition.
History
Date User Action Args
2018-04-05 12:56:29cedlinkissue5283 superseder
2018-03-21 18:03:20roundup-botsetmessages: + msg39189
2018-03-21 18:01:45roundup-botsetmessages: + msg39188
2018-03-21 18:00:01roundup-botsetmessages: + msg39187
2018-03-21 17:58:59roundup-botsetstatus: testing -> resolved
nosy: + roundup-bot
messages: + msg39186
2018-03-21 13:31:14cedsetstatus: in-progress -> testing
2018-03-15 19:12:34reviewbotsetmessages: + msg39025
2018-03-15 19:12:18reviewbotsetmessages: + msg39024
2018-03-15 19:12:15reviewbotsetmessages: + msg39023
2017-12-10 17:15:55cedsetmessages: + msg37223

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