[FIX] spreadsheet: falsy pivot values #165976
Conversation
|
|
|
Hi :) |
dhrp-odoo
force-pushed
the
17.0-fix-falsy-aggregated-values-dhrp
branch
from
77194a7
to
f2f019b
Compare
|
@rrahir, I have revised both the commit and PR message for clarity. Regarding the test, I've attempted it previously and again now, but I am receiving |
|
|
||
| if ( | ||
| values && | ||
| Object.prototype.hasOwnProperty.call(values[0], measure) && |
There was a problem hiding this comment.
It checks if values[0] has a property called measure.
Previously, I used values[0].hasOwnProperty, but encountered an ESLint error while playing around with tests: Do not access Object.prototype method 'hasOwnProperty' from target object no-prototype-builtins.
rrahir
force-pushed
the
17.0-fix-falsy-aggregated-values-dhrp
branch
from
f2f019b
to
a702bb9
Compare
|
|
||
| if ( | ||
| values && | ||
| Object.prototype.hasOwnProperty.call(values[0], measure) && |
There was a problem hiding this comment.
Why do you need to do this? if the measure is not present, values[0][measure] will be undefined and you get out of the if statement as well. So this condition is a duplicate of the following line.
There was a problem hiding this comment.
You are correct; this line is unnecessary. It was initially added while experimenting with tests and addressing an ESLint error. Thank you for the commit related to mockRPC 😊. I took the liberty of updating the commit message as suggested by AAB.
There was a problem hiding this comment.
thanks a lot! the fix seems good :)
addons/spreadsheet/static/tests/pivots/model/pivot_plugin_test.js
Outdated
Show resolved
Hide resolved
C3POdoo
requested review from
a team,
aab-odoo and
BastienFafchamps
and removed request for
a team
|
Don't forget to also fix the hoot mock server in the forward ports (>= 17.2), as it most likely has the same issue |
dhrp-odoo
force-pushed
the
17.0-fix-falsy-aggregated-values-dhrp
branch
from
a702bb9
to
33a7dcd
Compare
`mockReadGroup` did not follow the server implementation in the case of missing groupby (did not return __domain key) and was returning `0` instead of `False` for the aggregate value when the domain did not match any records.
Previously, when entering a pivot and applying a filter domain, the pivot formula (without any domain field and domain value) would return 'FALSE' if it did not match any records. The code now checks the value and returns an empty string if it's false. Task-3888401
dhrp-odoo
force-pushed
the
17.0-fix-falsy-aggregated-values-dhrp
branch
from
33a7dcd
to
aeca219
Compare
|
@robodoo rebase-ff |
|
Merge method set to rebase and fast-forward. |
|
@robodoo r+ |
`mockReadGroup` did not follow the server implementation in the case of missing groupby (did not return __domain key) and was returning `0` instead of `False` for the aggregate value when the domain did not match any records. Part-of: #165976
|
@dhrp-odoo @rrahir this pull request has forward-port PRs awaiting action (not merged or closed): |
|
@dhrp-odoo @rrahir this pull request has forward-port PRs awaiting action (not merged or closed): |
`mockReadGroup` did not follow the server implementation in the case of missing groupby (did not return __domain key) and was returning `0` instead of `False` for the aggregate value when the domain did not match any records. Part-of: odoo#165976
Previously, when entering a pivot and applying a filter domain, the pivot formula (without any domain field and domain value) would return 'FALSE' if it did not match any records. The code now checks the value and returns an empty string if it's false. Task-3888401 closes odoo#165976 Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>
Description:
Previously, when entering a pivot and applying a filter domain, the pivot formula (without domain field and domain value in function parameter) would return 'FALSE' if it did not match any records. The code now checks the value and returns an empty string if it's 'FALSE'.
Task - 3888401
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr