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

[IMP] mis_builder: display blank cells instead of 0.0 when there is no data #169

Merged
merged 9 commits into from
Mar 22, 2016

Conversation

ThomasBinsfeld
Copy link

In KPI values computed on accounts where there are no moves during the period, show a blank instead of 0.0. This is nicer for the eye, and sometimes mandatory in some kind of reports.

This is implemented with an AccountingNone singleton which is a null value that "dissolves" in basic arithmetic operations with non-None values.

…h the debit and the credit are zero and balances among which debit and credit nullify
@oca-clabot
Copy link

Hey @ThomasBinsfeld, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/website.cla
Here is a list of the users:

Appreciation of efforts,
OCA CLAbot

@sbidoul sbidoul changed the title [ADD] AccountingNone (singleton) [IMP] mis_builder: do not display null values Mar 7, 2016
@sbidoul
Copy link
Member

sbidoul commented Mar 7, 2016

👍 (code review and functional test)

@ThomasBinsfeld can you please fill and send your individual CLA as explained here https://odoo-community.org/page/cla

@sbidoul
Copy link
Member

sbidoul commented Mar 7, 2016

@jbeficent @alexis-via you may want to look into this one

@ThomasBinsfeld
Copy link
Author

@sbidoul ICLA is signed and sent 📧

@alexis-via
Copy link
Contributor

yes, better to put 0.0 instead of bank field for those kind of reports which are mainly financial 👍

@sbidoul
Copy link
Member

sbidoul commented Mar 11, 2016

@alexis-via I'm not sure about your last sentence. To be clear:

  • before this PR, KPI with no move lines in the period are displayed 0.0
  • with this PR: KPI with no move lines in the period are displayed as an empty cell

This is important because:

  • it distinguishes clearly KPI with no activity from KPI which have a 0.0 ending balances (for instance)
  • in some cases (eg Luxemburg offical reporting) this distinction is mandatory

@adrienpeiffer
Copy link
Contributor

👍 (Functionnal test & Code review)

@sbidoul sbidoul changed the title [IMP] mis_builder: do not display null values [IMP] mis_builder: display blank cells instead of 0.0 when there is no data Mar 11, 2016
@laetitia-gangloff
Copy link

Is it normal than comparison doesn't work with AccountNone value ? (compare with)

Before :
image

After :
image

@sbidoul
Copy link
Member

sbidoul commented Mar 13, 2016

@laetitia-gangloff good catch

@oca-clabot
Copy link

Hey @ThomasBinsfeld,
We acknowledge that the following users have signed our Contributor License Agreement:

Appreciation of efforts,
OCA CLAbot

@sbidoul
Copy link
Member

sbidoul commented Mar 16, 2016

@laetitia-gangloff it should be ok, now.

@sbidoul sbidoul mentioned this pull request Mar 16, 2016
4 tasks
@laetitia-gangloff
Copy link

It seems not ok in all cases (percentage) :

image

I don't know if it is ok to get +0 if there is no move (difference)?

image

@sbidoul
Copy link
Member

sbidoul commented Mar 16, 2016

@laetitia-gangloff p1 vs p3 is blank, that is normal because (-4960 - 0)/abs(0) does not make sense. p3 vs p1 is (0-(-4960))/abs(4960) = 100%. So that part is ok.

For the second part, we can hide the +0 indeed. I'll look into it.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.6%) to 68.484% when pulling 266d055 on ThomasBinsfeld:8.0_add_AccountingNone into 631e6d9 on OCA:8.0.

@sbidoul
Copy link
Member

sbidoul commented Mar 22, 2016

@laetitia-gangloff 0 comparisons now show blank too.

@laetitia-gangloff
Copy link

👍

@sbidoul
Copy link
Member

sbidoul commented Mar 22, 2016

Ok, ready to merge.

@adrienpeiffer
Copy link
Contributor

👍 Again ! Thanks !

@pedrobaeza
Copy link
Member

What is the added value of using AccountingNone object instead of embedding the logic? I see that at the end, you have to add the clause if x is AccoutingNone in some if clauses.

@sbidoul
Copy link
Member

sbidoul commented Mar 22, 2016

What is the added value of using AccountingNone object instead of embedding the logic?

@pedrobaeza because the logic would be complex and would need to be repeated in user-entered kpi expressions.

The tests for AccountingNone are at the "boundaries" only (when rendering or returning the value to clients).

@pedrobaeza
Copy link
Member

OK then

👍

pedrobaeza added a commit that referenced this pull request Mar 22, 2016
[IMP] mis_builder: display blank cells instead of 0.0 when there is no data
@pedrobaeza pedrobaeza merged commit f69d436 into OCA:8.0 Mar 22, 2016
rconjour pushed a commit to EssentNovaTeam/account-financial-reporting that referenced this pull request Mar 16, 2020
…lidate-ape

[8.0] Port account move batch validate to 8.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants