Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] web: search more in many2one: filter on ids #31232

Closed
wants to merge 1 commit into
base: saas-12.1
from

Conversation

Projects
None yet
8 participants
@aab-odoo
Copy link
Contributor

aab-odoo commented Feb 19, 2019

Let's assume a many2one field with a lot of possible values. The
user clicks on 'Search More...'. In the opened dialog, only 160
records are available (their ids have been obtained with a
name_search, with the optional text the user could have typed in
the many2one input).

Before 4cd379c, that extra domain on ids was removed automatically
as soon as the user interacted with the search view in the dialog.
This was especially useful when there were more than 160 records.
However, this was rather an happy coincidence than a designed
feature.
Let's assume a many2one field with a lot of possible values. The
user clicks on 'Search More...'. In the opened dialog, only 160
records are available (their ids have been obtained with a
name_search, with the optional text the user could have typed in
the many2one input).

Before 4cd379c, that extra domain on ids was removed automatically
as soon as the user interacted with the search view in the dialog.
This was especially useful when there were more than 160 records.
However, this was rather an happy coincidence than a designed
feature.

From 4cd379c, the ids selection was added to the initial domain
of the list, so they couldn't be removed from the domain
afterwards. The user was thus stucked with its preselected 160
records.

This rev. doesn't restore the former behavior, but rather improves
the current one, as follows:

  • when there is no text in the many2one input (i.e. no value to
    filter on), we bypass the name_search, s.t. all records are
    available in the dialog
  • when the name_search returns as many results as the limit (i.e.
    when there are potentially more records than the limit matching
    the search), we add a special filter to the search view in the
    dialog (the filter on ids), s.t. the user can remove it if he
    wants to access the remaining records.
  • when the name_search returns less results than the limit, we
    keep the current behavior as in this case, all records matching
    the search are available in the dialog
  • finally, the limit is now set to 480, to mitigate the problem

Issue reported on the saas-12.1 migration pad.

Description of the issue/feature this PR addresses:

Current behavior before PR:

Desired behavior after PR is merged:

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@robodoo robodoo added the seen 🙂 label Feb 19, 2019

@aab-odoo aab-odoo requested a review from ged-odoo Feb 19, 2019

@C3POdoo C3POdoo added the RD label Feb 19, 2019

@robodoo robodoo added the CI 🤖 label Feb 19, 2019

@aab-odoo aab-odoo force-pushed the odoo-dev:saas-12.1-searchmore-m2o-aab branch from 9119c68 Feb 21, 2019

@robodoo robodoo removed the CI 🤖 label Feb 21, 2019

@aab-odoo aab-odoo force-pushed the odoo-dev:saas-12.1-searchmore-m2o-aab branch 2 times, most recently Feb 21, 2019

@robodoo robodoo added the CI 🤖 label Feb 21, 2019

addons/web/static/src/js/fields/relational_fields.js Outdated
@@ -107,6 +107,7 @@ var FieldMany2One = AbstractField.extend({
'click': '_onClick',
}),
AUTOCOMPLETE_DELAY: 200,
SEARCH_MORE_LIMIT: 480,

This comment has been minimized.

@fmdl

fmdl Feb 21, 2019

Contributor

maybe use an ir.parameter

SEARCH_MORE_LIMIT: int(env[ir.parameter].get_paramter('SEARCH_MORE_LIMIT')) or 480

This comment has been minimized.

@Yenthe666

Yenthe666 Feb 21, 2019

Contributor

+1 would be a good moment to have it configurable.

This comment has been minimized.

@VincentSchippefilt

VincentSchippefilt Feb 21, 2019

Contributor

If it should be a parameter, then I would put it at the level of the arch of the view, not global, so that if could be change on view by view basis, and also be extensible.

@aab-odoo aab-odoo force-pushed the odoo-dev:saas-12.1-searchmore-m2o-aab branch Feb 22, 2019

@robodoo robodoo removed the CI 🤖 label Feb 22, 2019

@aab-odoo aab-odoo force-pushed the odoo-dev:saas-12.1-searchmore-m2o-aab branch Feb 22, 2019

addons/web/static/src/js/views/control_panel/control_panel_view.js Outdated
@@ -43,8 +43,12 @@ var ControlPanelView = Factory.extend({
* breadcrumbs won't be rendered
* @param {boolean} [params.withSearchBar=true] if set to false, no default
* search bar will be rendered
* @param {boolean} [activateDefaultFavorite=false] determine if the default
* custom filters can be activated
* @param {boolean} [params.activateDefaultFavorite=false] determine if the

This comment has been minimized.

@VincentSchippefilt

VincentSchippefilt Feb 22, 2019

Contributor

default is undefined, not false

@aab-odoo aab-odoo force-pushed the odoo-dev:saas-12.1-searchmore-m2o-aab branch Feb 22, 2019

@VincentSchippefilt

This comment has been minimized.

Copy link
Contributor

VincentSchippefilt commented Feb 22, 2019

robodoo r+

@robodoo robodoo added the r+ 👌 label Feb 22, 2019

@VincentSchippefilt

This comment has been minimized.

Copy link
Contributor

VincentSchippefilt commented Feb 22, 2019

robodoo retry

@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Feb 22, 2019

I'm sorry, @VincentSchippefilt. Retry makes no sense when the PR is not in error.

[FIX] web: search more in many2one: filter on ids
Let's assume a many2one field with a lot of possible values. The
user clicks on 'Search More...'. In the opened dialog, only 160
records are available (their ids have been obtained with a
name_search, with the optional text the user could have typed in
the many2one input).

Before 4cd379c, that extra domain on ids was removed automatically
as soon as the user interacted with the search view in the dialog.
This was especially useful when there were more than 160 records.
However, this was rather an happy coincidence than a designed
feature.

From 4cd379c, the ids selection was added to the initial domain
of the list, so they couldn't be removed from the domain
afterwards. The user was thus stucked with its preselected 160
records.

This rev. doesn't restore the former behavior, but rather improves
the current one, as follows:
 - when there is no text in the many2one input (i.e. no value to
   filter on), we bypass the name_search, s.t. all records are
   available in the dialog
 - when there is some text in the input, we perform a name_search (as
   before) to get a list of record ids, and we add a special filter
   to the search view in the dialog (the filter on those ids), s.t.
   the user can remove it if he wants to access the remaining records.
 - finally, the limit is now set to 320, to mitigate the problem

Issue reported on the saas-12.1 migration pad.

@aab-odoo aab-odoo force-pushed the odoo-dev:saas-12.1-searchmore-m2o-aab branch to 70bf80d Feb 26, 2019

@robodoo robodoo added CI 🤖 and removed r+ 👌 labels Feb 26, 2019

@aab-odoo

This comment has been minimized.

Copy link
Contributor Author

aab-odoo commented Feb 26, 2019

robodoo delegate=Polymorphe57

@Polymorphe57

This comment has been minimized.

Copy link
Contributor

Polymorphe57 commented Feb 26, 2019

robodoo r+

@robodoo robodoo added the r+ 👌 label Feb 26, 2019

robodoo pushed a commit that referenced this pull request Feb 26, 2019

[FIX] web: search more in many2one: filter on ids
Let's assume a many2one field with a lot of possible values. The
user clicks on 'Search More...'. In the opened dialog, only 160
records are available (their ids have been obtained with a
name_search, with the optional text the user could have typed in
the many2one input).

Before 4cd379c, that extra domain on ids was removed automatically
as soon as the user interacted with the search view in the dialog.
This was especially useful when there were more than 160 records.
However, this was rather an happy coincidence than a designed
feature.

From 4cd379c, the ids selection was added to the initial domain
of the list, so they couldn't be removed from the domain
afterwards. The user was thus stucked with its preselected 160
records.

This rev. doesn't restore the former behavior, but rather improves
the current one, as follows:
 - when there is no text in the many2one input (i.e. no value to
   filter on), we bypass the name_search, s.t. all records are
   available in the dialog
 - when there is some text in the input, we perform a name_search (as
   before) to get a list of record ids, and we add a special filter
   to the search view in the dialog (the filter on those ids), s.t.
   the user can remove it if he wants to access the remaining records.
 - finally, the limit is now set to 320, to mitigate the problem

Issue reported on the saas-12.1 migration pad.

closes #31232

@robodoo robodoo added merging 👷 and removed merging 👷 labels Feb 26, 2019

@robodoo robodoo added error 🙅 and removed r+ 👌 CI 🤖 labels Feb 26, 2019

@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Feb 26, 2019

Staging failed: ci/runbot on f7d2d3c7a4133708d65fb46d63b84f8139c20a78 (view more at http://runbot.odoo.com/runbot/build/469403)

@d-fence

This comment has been minimized.

Copy link
Contributor

d-fence commented Feb 27, 2019

@robodo retry

@d-fence

This comment has been minimized.

Copy link
Contributor

d-fence commented Feb 27, 2019

@robodoo retry
😃

robodoo pushed a commit that referenced this pull request Feb 27, 2019

[FIX] web: search more in many2one: filter on ids
Let's assume a many2one field with a lot of possible values. The
user clicks on 'Search More...'. In the opened dialog, only 160
records are available (their ids have been obtained with a
name_search, with the optional text the user could have typed in
the many2one input).

Before 4cd379c, that extra domain on ids was removed automatically
as soon as the user interacted with the search view in the dialog.
This was especially useful when there were more than 160 records.
However, this was rather an happy coincidence than a designed
feature.

From 4cd379c, the ids selection was added to the initial domain
of the list, so they couldn't be removed from the domain
afterwards. The user was thus stucked with its preselected 160
records.

This rev. doesn't restore the former behavior, but rather improves
the current one, as follows:
 - when there is no text in the many2one input (i.e. no value to
   filter on), we bypass the name_search, s.t. all records are
   available in the dialog
 - when there is some text in the input, we perform a name_search (as
   before) to get a list of record ids, and we add a special filter
   to the search view in the dialog (the filter on those ids), s.t.
   the user can remove it if he wants to access the remaining records.
 - finally, the limit is now set to 320, to mitigate the problem

Issue reported on the saas-12.1 migration pad.

closes #31232
@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Feb 27, 2019

Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.