Calculate VAT button handles the use previous periods flag in one way (by passing in a different date_from in the context and letting the chart of taxes do things normally), and the Breakdown button does it the other way (it passes the flag to the chart of taxes wizard and then the chart of taxes handles it).
My understanding is this is down to project history where originally we were dealing with periods so the chart of taxes had to deal explicitly with the possibility of including previous periods, but nowadays the chart of taxes report takes a date range, so this can be handled by passing in a different date_from (like the Calculate VAT button does) in the VAT Breakdown button handler.
Nowadays however the VAT Breakdown button could feed the corrected date_from into the Chart of Taxes report, just like the Calculate VAT button does.
As it stands there will be two chunks of code doing the same thing with the same meaning, and this is asking for problems, and I have two code paths to review to ensure they are doing things consistently and ponder "if they were inconsistent, which one would be right?".
As it happens, we get the same results (at least with the O1851 case), but it has already caused me problems trying to unify the way the CoT report is configured for the two; wondering why we're doing different things with the chart of taxes parameters in the two different buttons has made a refactoring that should have taken in the order of 15 minutes and with a very low risk of regression to one that's more complicated with a need to test things I don't yet understand. As a result I have postponed that refactoring until later when we can properly verify that both code paths do the right thing and then lose the chart of taxes version of it.
That refactoring was to make one method, used by the Calculate VAT button handler and the View VAT Breakdown button handler, that gives the appropriate Chart of Taxes parameters, so any changes to that only have to be made in one place and we only have to understand the one code path.
Calculate VAT button handles the use previous periods flag in one way (by passing in a different date_from in the context and letting the chart of taxes do things normally), and the Breakdown button does it the other way (it passes the flag to the chart of taxes wizard and then the chart of taxes handles it).
My understanding is this is down to project history where originally we were dealing with periods so the chart of taxes had to deal explicitly with the possibility of including previous periods, but nowadays the chart of taxes report takes a date range, so this can be handled by passing in a different date_from (like the Calculate VAT button does) in the VAT Breakdown button handler.
Nowadays however the VAT Breakdown button could feed the corrected date_from into the Chart of Taxes report, just like the Calculate VAT button does.
As it stands there will be two chunks of code doing the same thing with the same meaning, and this is asking for problems, and I have two code paths to review to ensure they are doing things consistently and ponder "if they were inconsistent, which one would be right?".
As it happens, we get the same results (at least with the O1851 case), but it has already caused me problems trying to unify the way the CoT report is configured for the two; wondering why we're doing different things with the chart of taxes parameters in the two different buttons has made a refactoring that should have taken in the order of 15 minutes and with a very low risk of regression to one that's more complicated with a need to test things I don't yet understand. As a result I have postponed that refactoring until later when we can properly verify that both code paths do the right thing and then lose the chart of taxes version of it.
That refactoring was to make one method, used by the Calculate VAT button handler and the View VAT Breakdown button handler, that gives the appropriate Chart of Taxes parameters, so any changes to that only have to be made in one place and we only have to understand the one code path.