Date Custom Fields patch.

undergrid · Post by **undergrid** » 25 Feb 2005, 01:01

I've recently had the need to create a date custom field that allows proper filtering (between date, before date, after date etc) and I've seen issues in bugs.mantis.org requesting the same sort of thing (#5056 and #4935 for example) so I've had a go.

The following patch is a first pass at implementing the feature for mantis and it apears to work pretty well, although there may be subtle bugs I havn't came accross yet. Hopefully we might get this added to the main distro at some point, it would save me patching it in every time I need to update works version of mantis.

The patch applies to the current head in CVS, just look for the extra Date type when definine custom field.

Opinons and suggestions are welcome.

Edit: Patch URL removed, see posts below.

Post by **vboctor** » 25 Feb 2005, 11:56

That is a needed feature, thanks for implementing it. If you can post a zip file that includes the modified files, I can apply it. I use Windows and don't have a reliable way of applying proper cvs patches. Always got errors from the Windows patch utility when I tried before.

Regards,
Victor
http://www.futureware.biz

Guest · Post by **Guest** » 25 Feb 2005, 12:38

Theres a bug in the diff patch whereby date pre 09-09-2001 (ish) can be incorrectly included in filtered results (earlier dates have a lower number of digits when the number of seconds since 1970 is stored in a string, adding 0 to the value before doing the comparison forces SQL to evaluate the value as an integer, solving the problem).

Since I can't update the diff while at work (cant get CVS to work via a proxy) I've deleted the diff file. Get the updated files in the zip file:

http://www.undergrid.org.uk/mantis/mant ... -patch.zip

I think thats what your after Victor.

undergrid · Post by **undergrid** » 25 Feb 2005, 12:39

guest wrote:Theres a bug in the diff patch whereby date pre 09-09-2001 (ish) can be incorrectly included in filtered results (earlier dates have a lower number of digits when the number of seconds since 1970 is stored in a string, adding 0 to the value before doing the comparison forces SQL to evaluate the value as an integer, solving the problem).

Since I can't update the diff while at work (cant get CVS to work via a proxy) I've deleted the diff file. Get the updated files in the zip file:

http://www.undergrid.org.uk/mantis/mant ... -patch.zip

I think thats what your after Victor.

Forgot to log in, that is me above :P

Post by **vboctor** » 25 Feb 2005, 12:43

I downloaded the zip file, but I am unable to extract it. It seems to be corrupt.

Regards,
Victor
http://www.futureware.biz

undergrid · Post by **undergrid** » 25 Feb 2005, 13:04

vboctor wrote:I downloaded the zip file, but I am unable to extract it. It seems to be corrupt.

Regards,
Victor
http://www.futureware.biz

Yeah, looks like the upload got cut off before it was complete, no idea why. I've re-uploaded the file and it should be fine now, you may need to clear your browser cache to get the new version.

Post by **vboctor** » 25 Feb 2005, 13:14

Why did you remove the patch file? It will be useful if another developer is applying your changes.

I also noticed that your changes were against 0.19.0-CVS, which is very different than what we have now in CVS. Your best bet to get this in soon is to update it against the latest CVS. Then include both the patch and the modified files in a zip file here. It would even be better if you add the patch to the issues in the bugtracker and refer to it from here.

It is after mid-night now, I better go to sleep.

Regards,
Victor
http://www.futureware.biz

Guest · Post by **Guest** » 25 Feb 2005, 14:49

vboctor wrote:Why did you remove the patch file? It will be useful if another developer is applying your changes.

I also noticed that your changes were against 0.19.0-CVS, which is very different than what we have now in CVS. Your best bet to get this in soon is to update it against the latest CVS. Then include both the patch and the modified files in a zip file here. It would even be better if you add the patch to the issues in the bugtracker and refer to it from here.

It is after mid-night now, I better go to sleep.

Regards,
Victor
http://www.futureware.biz

I deleted the diff because it has a bug, and since I cant do a proper diff at work (because of the proxy) I cant replace it right now, I'll upload an updated diff when I get home.

As for the version, I'm a little confused about how you came to the conclusion its against 0.19.0-CVS, I do have an old source tree in a directory called that but the files in the zip are against the CVS head at the time I posted. I've just checked the version numbers against the cvs repository and while jlatour has updated a few of the files recently, they are certainly not as old as 0.19.0-CVS...

undergrid · Post by **undergrid** » 25 Feb 2005, 14:49

^^ forgot to log in again.

Post by **vboctor** » 25 Feb 2005, 22:07

The first file I attempted to merge was config_defaults_inc.php which had the version set to 0.19.0-CVS. From memory, I think there was some relatively old changes that were not in your version of the file. I didn't check other files.

Regards,
Victor
http://www.futureware.biz

undergrid · Post by **undergrid** » 25 Feb 2005, 22:43

Don't know how I managed to get an old config_defaults in there..

Anyway I've just uploaded a zip to #5056 that contains both a context diff of the changes plus complete copies of the files I modified, its also got a few fixes for things I came accross today. I've checked this set of files three times now so hopefully its all up to date.

I'll cross my fingers...

Post by **vboctor** » 26 Feb 2005, 01:32

I applied the patch to CVS. Following are my comments:

Issues that I already fixed:
- Make sure notices are on. There was a couple of notices that were introduced by your changes.
- Do not put a space before semi-colons.
- Put spaces inside (). For example, if ( i == 0 ).
- Use XHTML rather than HTML (eg: <br />, attributes must be in double quotes, ..etc)
- print_year_range_option_list() was starting from year 0.
- On Or After doesnâ€™t work properly if the filter date is one day less than the issues date. There are some related issues. The way I fixed that was to use $t_one_day + 1 or â€“ 1 as appropriate.

Issues that I recommend you fix and submit as a separate patch:
- The year range to be displayed should be dependent on the defined custom fields. Maybe we should make use of the possible values to define the date range or the year range.
- The current technique of handling year range, will not work when year 2006 starts and you are updating issues where the year is set to 2005.
- Remove the $g_forward_year_count configuration variable.
- More testing on the filtering to make sure that it works for boundary dates (dates just before or after the date of an issue).

Regards,
Victor
http://www.futureware.biz

Guest · Post by **Guest** » 26 Feb 2005, 14:55

Appologies for all the extra modifications you had to do Victor, I guess after years of C coding I've got used to doing stuff in a certain way. I've already examined the changes you've made and will be more carefull in future.

I'm working on the other issues you mentioned, I guess you'd want me to add the patch to #5056 when I'm done?

Two queries though. First I can't actually replicate the issue you mention with On Or After with the code I submitted, indeed its not one of the searches that uses the $t_one_day variable. I can see the issue you might mean, adding a complete day to an On Or Before type search would include items with the following day, but the value of $t_one_day is already 1 second short of a full day (DATE + $t_one_day = DATE 23:59:59). As I understand it, the modification you made means that anything with a timestamp on DATE 23:59:59 (not possible with the interface I know) wont be included in an On Or Before or Between search. Could you clarify if I've missed something here?

Second, the year handling doesn't seem to be a problem. With the code in CVS, when you update a bug with an old date (say 2004) the year field should have a range from the current value to the current year plus $g_forward_year_count,ie 2004 to 2009. As an example I have a record dated 1973 which gives me a selection range from 1973 to 2009. Granted you can't select earlier dates, but if you don't touch the values when updating an issue, they shouldn't change in the db.

btw, someones messed around with http://www.mantisbt.org, in case you hadn't already noticed.

undergrid · Post by **undergrid** » 26 Feb 2005, 14:56

Guess what I forgot to do.... I've checked the remember me box this time :)

undergrid · Post by **undergrid** » 28 Feb 2005, 12:10

Actually forget that first query, I think I see where the problem is but its with the After search rather then the On or After. Theres an additional +1 near the end of the following line (around line 572 in filters_api.php, in the CUSTOM_FIELD_DATE_AFTER clause of the switch).

$t_custom_where_clause = '(( ' . $t_table_name . '.field_id = ' . $t_cfid . ' AND (' . $t_table_name . '.value+0) > ' . ($t_filter['custom_fields'][$t_cfid][1]+1) . ')' ;

Removing that +1 would probably be a more correct fix that subtracting 1 from the $t_one_day value.

Still can't find any problem with the second issue.