Extend fiscal year relative options to better match other periods

[eileenmcnaughton]

Overview

Adds relative date handling for fiscal_year.previous_before, fiscal_year.previous_2, fiscal_year.previous_3, fiscal_year.previous_n at the function level and adds unit tests (but does not expose)

Before

CRM_Utils_Date::relativeToAbsolute cannot parse previous.before et al

After

CRM_Utils_Date::relativeToAbsolute can parse previous.before et al, unit tests

Technical Details

This comes out of my effort to review #11592 and to see if it is mergeable. In digging I concluded I would NOT be comfortable merging additional handling options without unit tests (I'm undecided on adding new options to the core option value set & I note there is an extension proposal in the mix).

I do agree it's logical for the CRM_Utils_Date::relativeToAbsolute function to handle the same options for fiscal_year as for others and I added 3 of the proposed options with unit tests :

(based on a current date of ie. in May 2018 & financial year end 30 June)

fiscal_year.previous_before - the fiscal year before the previous one - 1 Jul 2015 to 30 June 2016
fiscal_year.previous_2 - the 2 fiscal years before this one - ie. 1 Jul 2015 to 30 June 2017
fiscal_year.previous_3 - the 3 fiscal years before this one - 1 Jul 2014 to 30 June 2017

(fiscal_year.previous_205 could also be handled by the code loop but I only test to previous_4)

Comments

I think this is mergeable as a partial of #11592 and because it enhances the code by adding unit tests. However, I will only keep it open for a month if not merged as it is a reviewer's commit.

I don't feel comfortable adding more options without more unit tests.

[eileenmcnaughton]

@MegaphoneJon @magnolia61 if one of you is able to review this it does at least part of the closed PR (& adds tests)

[monishdeb]

Seems like I need to close my PR #11663 as current UT covers it all.

[eileenmcnaughton]

oh dear - more work duplication! This is why I make such a fuss about our overblown PR queue!

I'm not sure I covered all your cases. I cut it down because they were all so confusing I felt I could only handle a few at once. Are you up for reviewing this one @monishdeb

[magnolia61]

Hello Eileen, I tested your PR for previous_n.fiscal year and can confirm it works as expected.
What would be the best way forward from here to also add the other fiscal_year variants? (this_n, current, earlier, greater, less).
Would you want to merge the PR you created, and shall I change #11592 to use your approach (including tests)?

[eileenmcnaughton]

@magnolia61 that sounds like a plan - leave the upgrade lines to the end since that will go stale easily

[magnolia61]

Not sure if this is related but I guess so. "This Fiscal Year" and "Previous Fiscal Year" are gone from the UI. The only one that is working is "Next Fiscal year", and that is the only one where $relativeTermSuffix is not involved. I opened an issue at: https://lab.civicrm.org/dev/core/issues/438

[magnolia61]

Fun fact: making the relative date filter use parameter 'n' for how many months, years, etc broke the regular one. Quick fix":

Change the option value
this.fiscal_year (This fiscal Year)
to
this_0.fiscal_year (This Fiscal year)

But this doesnt make existing smartgroups and reports work of course.