FIX Update accounting.lib.php function getCurrentPeriodOfFiscalYear

[josett225]

When debugging Asset, I discovered that function getCurrentPeriodOfFiscalYear is generating a wrong SQL request when playing with boundaries of fiscal year.
e.g FY2018 goes from 01/01/2018 up to 31/12/2018
If you ran the function with searching the date 31/12/2018 the following SQL request was generated =
SELECT date_start, date_end FROM llx_accounting_fiscalyear WHERE date_start <= '2018-12-31 12:00:00' AND date_end >= '2018-12-31 12:00:00' ORDER BY date_start DESC LIMIT 1
WRONG SQL RESULT : 0 ROW due to $fromtime = 2018-12-31 12:00:00

The reason is the fact that the time 12:00:00 is going over the limit of the day.
Then the result returned by the main function is totally wrong (latest FY Dates available returned 2024-01-01 and 2024-12-31)

To avoid this behavior, we need to build the right $fromtime variable = 2018-12-31 with dol_print_date($from_time,"%Y-%m-%d") instead of $db->idate($from_time)
The SQL then becomes
SELECT date_start, date_end FROM llx_accounting_fiscalyear WHERE date_start <= '2018-12-31' AND date_end >= '2018-12-31' ORDER BY date_start DESC LIMIT 1
SQL RESULT : 1 row with the right Dates values returned by the function (2018-01-01 and 2018-12-31)

[rycks]

Hello @josett225,
i'm sorry but i remember that all SQL queries must use db->idate for INSERT/UPDATE requests

[lvessiller-opendsi]

I agree with @rycks,
$db->idate() is used for SQL request.
Maybe there is a mistake in "$db->escape()" in this SQL request.
Usually db->escape() is not used for dates filters in SQL requests. (only use db->idate()).

[lvessiller-opendsi]

It's strange here, it compares the same dates for start and end date.

[josett225]

This request is a SELECT and not an INSERT/UPDATE request.

I did not spend time to try to change the request by the way.

However I am sure to make it work, we need to get a date without the time information to make it work.
Unless $db->idate is able to it, I don't understand why we cannot use dol_print_date

I will look into idate method to see what can be done.

[josett225]

Hi All,
Here is what the function idate does :
public function idate($param, $gm = 'tzserver')
{
// TODO $param should be gmt, so we should have default $gm to 'gmt' instead of default 'tzserver'
return dol_print_date($param, "%Y-%m-%d %H:%M:%S", $gm);
}

Any suggestion here to either accept the PR or to add other methods in $db as itime, idate and idatetime?

[josett225]

Another option is to extend the idate with a new param as $type = 'datetime' per default and as options 'time' and 'date'
Basically :
If $type == 'time' then return dol_print_date($param, "%H:%M:%S", $gm);
If $type == 'date' then return dol_print_date($param, "%Y-%m-%d", $gm);
else return dol_print_date($param, "%Y-%m-%d %H:%M:%S", $gm);

Thoughts?

[eldy]

Another option is to extend the idate with a new param as $type = 'datetime' per default and as options 'time' and 'date'
Basically :
If $type == 'time' then return dol_print_date($param, "%H:%M:%S", $gm);
If $type == 'date' then return dol_print_date($param, "%Y-%m-%d", $gm);
else return dol_print_date($param, "%Y-%m-%d %H:%M:%S", $gm);

Thoughts?

You must always use idate to forge a date to build sql.
If a generated SQL string does not have the good hour , you must change value of param.

Note 1: in sql 'YYYY-MM-DD' is same than YYYY-MM-DD 00:00:00' when comparing date with no time because the database truncate the hour.

Note 2: having exact hours for a full fiscal year is not interesting because if you company is in paris, and invoice was generated the 01/01/25 in tokyo, when does the invoice need to appear ? The user that entered the data want the invoice to be 01/01 (there is no hour on invoice date). But when invoice was recording, according to the company, it was the 31/12. There is no solution for this, except accepting that borders are a relative values. The machine must define a border that will be same from year to year, this is why dolibarr decided to use 12:00 as limit, so in most case, only half a day is counted in a different year, but it will depend on point of view (not a bug or counting error, just a choice to decide when we stop the year . You can choose a limit of 00:00, but in such a case, you may have less invoice in a different year but you may also have more with a full day of invoices that is counted on the next or previous year depending on difference between timezone of company (we supposevtz of server) and timezone of user that has choosed the invoice date.

What is important is that if an invoice is not found for a year period because at the limit, it is found the next year.

As this is a very difficult to understand trouble, it would be easier to analyze if something is really wrong if you enter data on demo dolibarr.org and then report the url that show the wrong information so we can analyze with real data and knowing the timezone of user, of server, of php, and of database (all of this has effect).

[josett225]

Hi @eldy
Many thanks for the clear explanation. FYI and to clarify, I am right now to deep test the Asset module. The function getCurrentPeriodOfFiscalYear is used to find the correct FY to calculate the depreciation values based on the start date (basically). I am also adding the fact that some FY could be longer/shorter than 12 months when a company wants to change the FY end date. Therefore I am using in a loop to recover the exact start/end dates of FY to calculate the right amount. I will submit a new PR soon as it works on my dev.
The issue on using a border day date as start date of depreciation has generated the issue.
Based on your comments on working on $param, I tested on the asset module to discover the date given to function getCurrentPeriodOfFiscalYear, this one is having the time 12:00:00 (Depreciation Date_start 2019-09-30 12:00:00) as you describe above.
I will try then to give a date with 00:00:00 in the asset module to fix it.