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

8.0 account financial report horizontal #77

Merged
merged 4 commits into from Jun 11, 2015

Conversation

hbrunn
Copy link
Member

@hbrunn hbrunn commented May 22, 2015

This is basically a rewrite to get rid of the inherited 6.0 code. So better ignore the diff and have a look at the code.

I inject a context key to determine which reports to show, so the most probable place where I might have messed up is in https://github.com/hbrunn/account-financial-reporting/blob/8.0-account_financial_report_horizontal/account_financial_report_horizontal/models/account_financial_report.py#L28 and _get_children_by_order

@pedrobaeza
Copy link
Member

@gurneyalex, can you please activate runbot here to try this module?

@StefanRijnhart
Copy link
Member

Nice! Failing tests are unrelated. The same errors occur in other PRs.

You can remove views/account_report.xml.

In README.rst, please change the description to reflect that this is no longer a 'rewrite of the financial reports from OpenERP 6.0' but instead only a cosmetic change to the existing reports.

Last remark: conventionally, in our region, the sign is reversed in the right column so that it does not usually contain negative amounts. However, this should probably not be changed to preserve consistency with the 'show debit/credit' option, which makes it clear that the amounts are actually the balances of the accounts, with its proper sign.

@hbrunn
Copy link
Member Author

hbrunn commented May 27, 2015

@StefanRijnhart thanks!

Here the updated README.rst for convenience: https://github.com/hbrunn/account-financial-reporting/blob/8.0-account_financial_report_horizontal/account_financial_report_horizontal/README.rst

If anybody knows other countries where this reports are customary, please let me know

@StefanRijnhart
Copy link
Member

Thanks for the changes, 👍 (code review, test)

@sbidoul sbidoul added this to the 8.0 milestone Jun 7, 2015
@sbidoul
Copy link
Member

sbidoul commented Jun 7, 2015

Red because of #67

@pedrobaeza
Copy link
Member

Please rebase to get green results

by reusing as much as possible functionality from the existing report
and remove obsolete file
@hbrunn hbrunn force-pushed the 8.0-account_financial_report_horizontal branch from 963bf78 to 37f409e Compare June 11, 2015 06:09
@hbrunn
Copy link
Member Author

hbrunn commented Jun 11, 2015

@pedrobaeza done

@pedrobaeza
Copy link
Member

👍 Tried on runbot

pedrobaeza added a commit that referenced this pull request Jun 11, 2015
…ontal

8.0 account financial report horizontal
@pedrobaeza pedrobaeza merged commit e3a9614 into OCA:8.0 Jun 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants