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.

Sign up for GitHub

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

Jump to bottom

[9.0][ADD] Account Journal Report #298

Merged
merged 1 commit into from
Jun 9, 2017
Merged

[9.0][ADD] Account Journal Report #298

merged 1 commit into from
Jun 9, 2017

Conversation

MiquelRForgeFlow
Copy link
Contributor

Journal Report

Adds the following report in PDF and XLSX format:

  • Journal Ledger. It groups the moves by their entries.

cc: @pedrobaeza @gboros-rgbconsulting @jbeficent

@MiquelRForgeFlow MiquelRForgeFlow force-pushed the 9.0-add-account_journal_report branch 5 times, most recently from 5eb1b8e to cbe72ab Compare April 19, 2017 10:20
@MiquelRForgeFlow MiquelRForgeFlow force-pushed the 9.0-add-account_journal_report branch 2 times, most recently from b4a1c74 to ca862cc Compare April 28, 2017 11:10
pedrobaeza
pedrobaeza requested changes Apr 28, 2017
View reviewed changes
account_journal_report/README.rst Outdated

#. Go to *Invoicing* or *Accounting* menu
#. Press 'Reporting > PDF Reports > Journal Ledger'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Include a line for indicating that pressing button "Export", you'll get the report on Excel.

account_journal_report/README.rst Outdated
Contributors
------------

* Jordi Esteve <jesteve@zikzakmedia.com>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this one, as the code has nothing to do with this original contributor now.

account_journal_report/__init__.py Outdated
@@ -0,0 +1,14 @@
# -*- coding: utf-8 -*-
# Copyright 2009 Zikzakmedia S.L. (http://zikzakmedia.com)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file doesn't need this overloaded header.

account_journal_report/__openerp__.py Outdated
@@ -0,0 +1,32 @@
# -*- coding: utf-8 -*-
# Copyright 2009 Zikzakmedia S.L. - Jordi Esteve
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this copyright.

account_journal_report/__openerp__.py Outdated
"report_xlsx",
],
'license': "AGPL-3",
'author': "Zikzakmedia SL, "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the first author as stated.

account_journal_report/report/journal_ledger_report.py Outdated
[('date', '<=', date_end),
('date', '>=', date_start),
('journal_id', 'in', journal_ids),
('state', '<>', 'draft')],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/<>/!= (the other operator is deprecated)

account_journal_report/tests/test_account_journal_report.py Outdated
self.company = self.env.ref('base.main_company')
self.g_account_user = self.env.ref('account.group_account_user')

self.user = self._create_user('user_1', [self.g_account_user],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For assuring the test, you must provide account data. Please check my tests for v8: https://github.com/OCA/l10n-spain/blob/8.0/l10n_es_account_financial_report/tests/test_journal_entries_report.py, that adds all the needed records.

account_journal_report/tests/test_account_journal_report.py Outdated
from openerp.tests.common import TransactionCase


class TestAccountJournalReport(TransactionCase):
Copy link
Member

@pedrobaeza pedrobaeza Apr 28, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use SavepointCase instead, that is more efficient. You need to switch setUp method, but only that. You can see an example here: https://github.com/OCA/l10n-spain/blob/8.0/l10n_es_account_financial_report/tests/test_journal_entries_report.py

account_journal_report/tests/test_account_journal_report.py Outdated
self.report_xlsx_name = 'account_journal_report.journal_ledger_xlsx'
self.report_title = 'Journal Ledger'

def _create_user(self, login, groups, company):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to create a new user for this, as tests are run as admin, and I see no use of this user for testing any permission.

account_journal_report/tests/test_account_journal_report.py Outdated

def test_account_journal_report(self):

wiz_id = self.wiz.create({})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Control in your wizard creation data for your test, including the journal and dates you want to print. Check https://github.com/OCA/l10n-spain/blob/8.0/l10n_es_account_financial_report/tests/test_journal_entries_report.py for my test in v8.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's not necessary because the journal and dates are established by default.

Copy link
Member

@pedrobaeza pedrobaeza May 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it's a safeguard, because you have to make your tests state-consistent, and if you depend on the default values of the wizard, maybe any change can make this fail (for example, a custom module that changes these defaults in a customer).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, the data is added.

@pedrobaeza
Copy link
Member

Please include also full Spanish translation

geluboros
geluboros reviewed May 2, 2017
View reviewed changes
account_journal_report/wizard/wizard_print_journal_ledger_view.xml Outdated
</record>

<!-- Add XLSX export button -->
<record id="account_journal_ledger_xlsx_view" model="ir.ui.view">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The export button should be added in the main view, there is no need to creat an extension view for it.

@MiquelRForgeFlow
Copy link
Contributor Author

@pedrobaeza About the translations, why should I put them? The Transifex solves this, right?

@pedrobaeza
Copy link
Member

No if you don't translate them on Transifex, and this makes at least one week difference. For initial translation upload, you should provide it with the module.

@MiquelRForgeFlow MiquelRForgeFlow force-pushed the 9.0-add-account_journal_report branch 2 times, most recently from 2518338 to df3e53e Compare May 2, 2017 14:18
pedrobaeza
pedrobaeza requested changes May 2, 2017
View reviewed changes
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are texts that are not translated yet. Can you please do it?

account_journal_report/tests/test_account_journal_report.py Outdated
super(TestAccountJournalReport, self).setUp()

self.company = self.env.ref('base.main_company')
self.g_account_user = self.env.ref('account.group_account_user')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a note, this is not needed, but it doesn't make the module fail.

@MiquelRForgeFlow
Copy link
Contributor Author

All done.

@sbidoul sbidoul added this to the 9.0 milestone May 31, 2017
JordiBForgeFlow
JordiBForgeFlow approved these changes Jun 9, 2017
View reviewed changes
Copy link
Member

@JordiBForgeFlow JordiBForgeFlow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@JordiBForgeFlow JordiBForgeFlow merged commit a275505 into OCA:9.0 Jun 9, 2017
@LoisRForgeFlow LoisRForgeFlow deleted the 9.0-add-account_journal_report branch June 9, 2017 14:20
@pedrobaeza pedrobaeza mentioned this pull request Aug 28, 2017
Closed
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geluboros geluboros geluboros left review comments

@JordiBForgeFlow JordiBForgeFlow JordiBForgeFlow approved these changes

@pedrobaeza pedrobaeza pedrobaeza approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
9.0
Development

Successfully merging this pull request may close these issues.
5 participants
@MiquelRForgeFlow @pedrobaeza @JordiBForgeFlow @geluboros @sbidoul