-
-
Notifications
You must be signed in to change notification settings - Fork 794
[16.0][FIX]product_supplierinfo_for_customer: misc changes for search count. #1931
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
base: 16.0
Are you sure you want to change the base?
[16.0][FIX]product_supplierinfo_for_customer: misc changes for search count. #1931
Conversation
| @api.model | ||
| def search(self, args, offset=0, limit=None, order=None, count=False): | ||
| res = super().search(args, offset=offset, limit=limit, order=order, count=count) | ||
| res = super().search(args, offset=offset, limit=limit, order=order, count=False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO, this is not correct as you change the passed parameter.
| limit2 = limit - len(res) if limit else limit | ||
| res2 = self.env["product.customerinfo"].search( | ||
| args, offset=offset, limit=limit2, order=order, count=count | ||
| args, offset=offset, limit=limit2, order=order, count=False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But count argument is False by default...
|
@rousseldenis, The search method is called from the search_count method with the count=True argument in Odoo 16 core (https://github.com/odoo/odoo/blob/16.0/odoo/models.py#L1511). In this case, the result of the super() call will be the number of records (an integer). However, in the code, the len() function is being used on the result of the super() call (https://github.com/OCA/product-attribute/pull/1931/files#diff-6fc0b1283cb7afcdf7bf53b3e8e5e8a343f43ff4a84d4c65f68e81e756f67933L16), which will attempt to call len() on an integer object, ultimately causing an error. |
|
Hey @OCA/product-maintainers would be great if someone could have a look at this changes. |
| res = super().search(args, offset=offset, limit=limit, order=order, count=False) | ||
| if ( | ||
| self.env.context.get("customerinfo") | ||
| and self._name == "product.supplierinfo" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we not just add a condition here like
and not count
to bypass the customer info logic, if count is true?
| args, offset=offset, limit=limit2, order=order, count=count | ||
| args, offset=offset, limit=limit2, order=order, count=False | ||
| ) | ||
| res2 = res2.read(list(self.env["product.supplierinfo"]._fields.keys())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If also in case of count = true we need to consider the customer info, can we not do something like:
if count:
res += res2
else:
for result in res2:
res += self.env["product.supplierinfo"].new(result)e4cc0fd to
0479df6
Compare
0479df6 to
f65384d
Compare
baguenth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
|
ping @rousseldenis |
No description provided.