Advertisement:

Author Topic: getHolidayRange in Subs-Calendar.php BUG and FIX  (Read 9454 times)

Offline SoLoGHoST

  • SMF Hero
  • ******
  • Posts: 1,795
  • Gender: Male
  • Real coders do not need to comment their code!
    • Dream Portal
getHolidayRange in Subs-Calendar.php BUG and FIX
« on: November 02, 2010, 03:38:14 PM »
Ok, so getting the range of holidays between '2010-01-0' and '2011-12-31' Gives me all holidays (cept for the '0004' year holidays, only gives me '0004' year holidays 1 time when it is supposed to call them twice), than it makes the $holidays key where the YEAR is inserted = 2011 for ALL holidays returned.  This is a problem, as is needs to return the CORRECT year string within the key index for $holidays.

The current function is this:
Code: [Select]
// Get all holidays within the given time range.
function getHolidayRange($low_date, $high_date)
{
global $smcFunc;

// Get the lowest and highest dates for "all years".
if (substr($low_date, 0, 4) != substr($high_date, 0, 4))
$allyear_part = 'event_date BETWEEN {date:all_year_low} AND {date:all_year_dec}
OR event_date BETWEEN {date:all_year_jan} AND {date:all_year_high}';
else
$allyear_part = 'event_date BETWEEN {date:all_year_low} AND {date:all_year_high}';

// Find some holidays... ;).
$result = $smcFunc['db_query']('', '
SELECT event_date, YEAR(event_date) AS year, title
FROM {db_prefix}calendar_holidays
WHERE event_date BETWEEN {date:low_date} AND {date:high_date}
OR ' . $allyear_part,
array(
'low_date' => $low_date,
'high_date' => $high_date,
'all_year_low' => '0004' . substr($low_date, 4),
'all_year_high' => '0004' . substr($high_date, 4),
'all_year_jan' => '0004-01-01',
'all_year_dec' => '0004-12-31',
)
);
$holidays = array();
while ($row = $smcFunc['db_fetch_assoc']($result))
{
if (substr($low_date, 0, 4) != substr($high_date, 0, 4))
$event_year = substr($row['event_date'], 5) < substr($high_date, 5) ? substr($high_date, 0, 4) : substr($low_date, 0, 4);
else
$event_year = substr($low_date, 0, 4);

$holidays[$event_year . substr($row['event_date'], 4)][] = $row['title'];
}
$smcFunc['db_free_result']($result);

return $holidays;
}

SUGGESTED CHANGE in order to fix this problem:

Code: [Select]
// Get all holidays within the given time range.
function getHolidayRange($low_date, $high_date)
{
global $smcFunc;

$low_year = (int) substr($low_date, 0, 4);
$high_year = (int) substr($high_date, 0, 4);
$diff_year = $high_year - $low_year;

// Get the lowest and highest dates for "all years".
if ($low_year != $high_year)
$allyear_part = 'event_date BETWEEN {date:all_year_low} AND {date:all_year_dec}
OR event_date BETWEEN {date:all_year_jan} AND {date:all_year_high}';
else
$allyear_part = 'event_date BETWEEN {date:all_year_low} AND {date:all_year_high}';

// Find some holidays... ;).
$result = $smcFunc['db_query']('', '
SELECT event_date, YEAR(event_date) AS year, title
FROM {db_prefix}calendar_holidays
WHERE event_date BETWEEN {date:low_date} AND {date:high_date}
OR ' . $allyear_part,
array(
'low_date' => $low_date,
'high_date' => $high_date,
'all_year_low' => '0004' . substr($low_date, 4),
'all_year_high' => '0004' . substr($high_date, 4),
'all_year_jan' => '0004-01-01',
'all_year_dec' => '0004-12-31',
)
);
$holidays = array();
$event_year = $low_year;

// We need to assign the correct year for all years, no matter what the month is!
while ($row = $smcFunc['db_fetch_assoc']($result))
{
if ($low_year != $high_year)
{
// Need to SET all 0004 Holidays for each year since these are only called once!
if (substr($row['event_date'], 0, 4) == '0004')
{
// Add it to each year!
for ($i = 0; $i <= $diff_year; $i++)
$holidays[$event_year + $i . substr($row['event_date'], 4)][] = $row['title'];
}
else
$holidays[$row['event_date']][] = $row['title'];
}
else
$holidays[$event_year . substr($row['event_date'], 4)][] = $row['title'];
}
$smcFunc['db_free_result']($result);

return $holidays;
}

You can test this out and see what I mean with the following code:
Code: [Select]
global $sourcedir;

require_once($sourcedir . '/Subs-Calendar.php');

$holiday = array();

$holiday = getHolidayRange('2010-01-0', '2011-12-31');

 ksort($holiday);

foreach($holiday as $date => $hday)
{
   // gets the title of the holiday!
   foreach($hday as $title)
       echo $title . '<br />';

  // gets the date of the holiday format Y-m-d!
  // e.g. 2010-12-25 = Christmas!
   echo $date . '<br /><br />';
}

The above code will give you all holidays for the year 2011 and 2010 (cept for the holidays with a substr(event_date, 0, 4) as they will only appear once for the year with the substr($high_date, 0, 4)), however they will ALL have the same year (2011) attached to them, this is using the current SMF getHolidayRange function, replacing this function with my function will give you all calendar holidays for every month of every year having the CORRECT year string attached to the keys of the $holiday array.

The problem is that although SMF's function will return the holidays between the range of $low_date and $high_date, the year gets overwritten to be the last year (substr($high_date, 0, 4)), within the index of $holidays array, my fixed function code will assign the CORRECT year string to each $holiday array's index value and also fix the problem of getting the year of 0004 holidays (which are CONSTANT holidays) to all years in the range.

Not sure if the getEventRange and/or getBirthdayRange functions have the same bug or not, haven't tested those functions, as I only needed to get Holidays, but I would recommend checking these functions as well.  Although, this isn't really a Major bug in SMF, it is misleading and still a BUG, because if you place a range in this month that exceeds the difference of more than 12 months, holidays returned will obtain the WRONG year value and there will be DUPLICATE/INVALID holidays returned for the SAME YEAR (substr($high_date, 0, 4))

Cheers :)
« Last Edit: November 03, 2010, 02:53:48 AM by SoLoGHoST »

Offline Norv

  • SMF Friend
  • SMF Super Hero
  • *
  • Posts: 18,313
  • Blue Wolf
Re: getHolidayRange in Subs-Calendar.php BUG and FIX
« Reply #1 on: November 03, 2010, 06:15:12 PM »
Thank you for the report. :)

I will try to look into it ASAP.
To-do lists are for deferral. The more things you write down the later they're doneā€¦ until you have 100s of lists of things you don't do.
File a security report | Developers' Blog | Bug Tracker

Also known as Norv on D* | Norv N. on G+ | Norv on Github

Offline SoLoGHoST

  • SMF Hero
  • ******
  • Posts: 1,795
  • Gender: Male
  • Real coders do not need to comment their code!
    • Dream Portal
Re: getHolidayRange in Subs-Calendar.php BUG and FIX
« Reply #2 on: November 05, 2010, 12:38:37 AM »
Sure thing, was quite a surprise to me stumbling onto this itsy bitsy bug!  Glad to be of some help :)

Offline SoLoGHoST

  • SMF Hero
  • ******
  • Posts: 1,795
  • Gender: Male
  • Real coders do not need to comment their code!
    • Dream Portal
Re: getHolidayRange in Subs-Calendar.php BUG and FIX
« Reply #3 on: November 08, 2010, 11:45:31 PM »
Ok, I actually overlooked the database BETWEEN statements that currently only work for a difference of 1 year, so updated the BETWEEN db statement to work for more than 1 year:

Code: [Select]
// Get all holidays within the given time range.
function getHolidayRange($low_date, $high_date)
{
global $smcFunc;

$low_year = (int) substr($low_date, 0, 4);
$high_year = (int) substr($high_date, 0, 4);
$diff_year = $high_year - $low_year;

// Get the lowest and highest dates for "all years".
if ($low_year != $high_year)
{
if ($diff_year > 1)
// More than 1 year difference, we need them all.
$allyear_part = 'event_date BETWEEN {date:all_year_jan} AND {date:all_year_dec}';
else
// This would work for a difference of 1 year ONLY!
$allyear_part = 'event_date BETWEEN {date:all_year_low} AND {date:all_year_dec}
OR event_date BETWEEN {date:all_year_jan} AND {date:all_year_high}';
}
else
$allyear_part = 'event_date BETWEEN {date:all_year_low} AND {date:all_year_high}';

// Find some holidays... ;).
$result = $smcFunc['db_query']('', '
SELECT event_date, YEAR(event_date) AS year, title
FROM {db_prefix}calendar_holidays
WHERE event_date BETWEEN {date:low_date} AND {date:high_date}
OR ' . $allyear_part,
array(
'low_date' => $low_date,
'high_date' => $high_date,
'all_year_low' => '0004' . substr($low_date, 4),
'all_year_high' => '0004' . substr($high_date, 4),
'all_year_jan' => '0004-01-01',
'all_year_dec' => '0004-12-31',
)
);
$holidays = array();
$event_year = $low_year;

// We need to assign the correct year for all years, no matter what the month is!
while ($row = $smcFunc['db_fetch_assoc']($result))
{
if ($low_year != $high_year)
{
// Need to SET all 0004 Holidays for each year since these are only called once!
if (substr($row['event_date'], 0, 4) == '0004')
{
// Add it to each year!
for ($i = 0; $i <= $diff_year; $i++)
$holidays[$event_year + $i . substr($row['event_date'], 4)][] = $row['title'];
}
else
$holidays[$row['event_date']][] = $row['title'];
}
else
$holidays[$event_year . substr($row['event_date'], 4)][] = $row['title'];
}
$smcFunc['db_free_result']($result);

return $holidays;
}

Sorry bout that, because we are getting more than 1 year we'll need all 0004 holidays from 0004-01-01 to 0004-12-31

Cheers :)

Offline Joshua Dickerson

  • SMF Friend
  • SMF Super Hero
  • *
  • Posts: 12,775
  • Gender: Male
    • joshuaadickerson on GitHub
    • joshuaadickerson on LinkedIn
Re: getHolidayRange in Subs-Calendar.php BUG and FIX
« Reply #4 on: April 30, 2012, 03:32:27 PM »
Norv, did you ever look in to this?
Need help? See the wiki. Want to help SMF? See the wiki!

Did you know you can help develop SMF? See us on Github.

How have you bettered the world today?

Offline Arantor

  • Resident Overthinker
  • SMF Friend
  • SMF Legend
  • *
  • Posts: 68,051
    • Arantor on GitHub
Re: getHolidayRange in Subs-Calendar.php BUG and FIX
« Reply #5 on: December 30, 2013, 10:48:24 PM »
This is one of those things that I'm not entirely convinced is a bug. It's a bug, but it's only a bug when it's used in a way different to how it was originally designed which means it's not really a bug. A car is a great vehicle until you try and drive it on a pond, then it won't be :P

Consider: this never manifests inside SMF normally. It only manifests because you have a range whereby the 'yearly' holidays are being pulled more than once - the code is really designed for the 3 months' range the calendar gets, which relies on the way annual holidays are stored essentially as an exception.

I guess I'm surprised anyone noticed, though I'm curious to know when you'd ever need more than a single year's worth of holidays at once outside the admin panel... I'm also concerned that this might actually have some side effects that need to be dealt with.
To assume is to hope that those who came before had the presence of mind and capacity to implement the dreams of those who would come after.

You either die a hero or live long enough to see yourself become the villain. It seems you have chosen which, and now I must do the same.

Offline SoLoGHoST

  • SMF Hero
  • ******
  • Posts: 1,795
  • Gender: Male
  • Real coders do not need to comment their code!
    • Dream Portal
Re: getHolidayRange in Subs-Calendar.php BUG and FIX
« Reply #6 on: December 31, 2013, 12:40:53 AM »
This could be ignored since this is not technically a bug within SMF and how SMF uses that function.  But it's just a bug within the function itself, as it implies that it gets a holiday range between 2 specific dates, but really only gets a holiday range for 1 year.  But the bug is that it allows you to change the year part within the low and high dates, thus not giving you a proper return value when those years are different.

Offline Arantor

  • Resident Overthinker
  • SMF Friend
  • SMF Legend
  • *
  • Posts: 68,051
    • Arantor on GitHub
Re: getHolidayRange in Subs-Calendar.php BUG and FIX
« Reply #7 on: December 31, 2013, 12:44:52 AM »
Oh yes, I can see what the bug *is*, that part isn't a problem. My problem with this fix is, I'm not sure there aren't side effects with it. I haven't done enough testing on it or played with it enough yet to be comfortable about it working as 'expected'.
To assume is to hope that those who came before had the presence of mind and capacity to implement the dreams of those who would come after.

You either die a hero or live long enough to see yourself become the villain. It seems you have chosen which, and now I must do the same.

Offline SoLoGHoST

  • SMF Hero
  • ******
  • Posts: 1,795
  • Gender: Male
  • Real coders do not need to comment their code!
    • Dream Portal
Re: getHolidayRange in Subs-Calendar.php BUG and FIX
« Reply #8 on: December 31, 2013, 12:52:49 AM »
No problem, I had to modify the Calendar Module with this change, which allows you to grab more than 3 months of holidays, in fact it allows you to grab any number of months (prev. or next from the current month).  I used this edit within the function and seems to work fine.  Module located here:  http://dream-portal.net/index.php?topic=833.0  I also changed other functions used for the SMF Calendar for better, overall, performance.

Offline Arantor

  • Resident Overthinker
  • SMF Friend
  • SMF Legend
  • *
  • Posts: 68,051
    • Arantor on GitHub
Re: getHolidayRange in Subs-Calendar.php BUG and FIX
« Reply #9 on: December 31, 2013, 01:07:58 AM »
Nothing says better performance like proper benchmarks showing the conditions under which improvement was obtained.
To assume is to hope that those who came before had the presence of mind and capacity to implement the dreams of those who would come after.

You either die a hero or live long enough to see yourself become the villain. It seems you have chosen which, and now I must do the same.