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

[FIX][8.0] l10n_nl_xaf_auditfile_export: export periodNumber #111

Merged

Conversation

astirpe
Copy link
Member

@astirpe astirpe commented Aug 21, 2017

On an old multi-company database with a lot of companies we ran into this:

:21714:0:ERROR:SCHEMASV:SCHEMAV_CVC_TOTALDIGITS_VALID: Element '{http://www.auditfiles.nl/XAF/3.2}periodNumber': [facet 'totalDigits'] The value '3167' has more digits than are allowed ('3'). :21714:0:ERROR:SCHEMASV:SCHEMAV_CVC_DATATYPE_VALID_1_2_1: Element '{http://www.auditfiles.nl/XAF/3.2}periodNumber': '3167' is not a valid value of the atomic type '{http://www.auditfiles.nl/XAF/3.2}Nonnegativeinteger3'. :21719:0:ERROR:SCHEMASV:SCHEMAV_CVC_TOTALDIGITS_VALID: Element 

The issue is that the ID of account_period (for periodNumber) has more that three digits. This is not allowed as per the XAF Auditfile schema.

As I already proposed for version 10.0 (see #59 (comment)), I'm proposing to export the period numbers (periodNumber) using 3 digits: 1st digit is the last digit of the fiscal year, the other 2 digits are the month (Eg.: period "August 2017" is represented by number 708).

So this is the backport to 8.0 of the periodNumber representation already made for 10.0.

@hbrunn hbrunn added this to the 10.0 milestone Sep 7, 2017
@astirpe
Copy link
Member Author

astirpe commented Nov 3, 2017

@hbrunn for this PR, the milestone should be set to 8.0 (and not to 10.0)

@hbrunn hbrunn modified the milestones: 10.0, 8.0 Nov 16, 2017
Copy link

@ThijsvOers ThijsvOers left a comment

Choose a reason for hiding this comment

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

I have tested the fix Andrea made in a 8.0 environment and it works fine.

@hbrunn hbrunn merged commit 0fd5d03 into OCA:8.0 Dec 7, 2017
@astirpe astirpe deleted the 80_fix_period_l10n_nl_xaf_auditfile_export branch December 7, 2017 10:14
@luc-demeyer
Copy link

I intended to rebase my PR #119 because it conflicts with this change that got merged.
Before I can do this rebase, this change in this merge must be reverted and fixed because this solution is wrong.
Let me explain:

The period code field is a field that can be entered/modfied by the end user.
We do know that the standard accounting wizard generates period codes via the logic 'mm/yyyy'.
But users can change this and I have seen many installs where the periods are called 'yyyy-mm' (makes actually more sense than the standard way for period based sorting purposes).

I think we should keep the logic proposed by andrea but implemented properly (take the month and year of the period date_stop field in order to compose periodNumber in the XAF file.

@astirpe
Copy link
Member Author

astirpe commented Feb 3, 2018

@luc-demeyer is ok for me. Will you fix it in your PR?

@luc-demeyer
Copy link

cf. #149
As soon as this one is merged I'll rebase my PR 119.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants