I have a feed with event items with a start date and end date. When the end date is not in the source (i.e. no tag included in the feed), the date mapper, however, fills the end date with the date of importing.
The date mapper should instead leave the fiel blank or better, use the default value for the end date (configured in the field setting of the date field, in my case as 'blank' for the end date) if the datefield is not present.

Files: 
CommentFileSizeAuthor
#71 handle_empty_and_invalid_dates-857216-71.patch2.92 KBgnucifer
FAILED: [[SimpleTest]]: [MySQL] 3,844 pass(es), 0 fail(s), and 1 exception(s).
[ View ]
#62 handle_empty_and_invalid_dates-857216-62.patch2.91 KBgnucifer
FAILED: [[SimpleTest]]: [MySQL] 3,844 pass(es), 0 fail(s), and 1 exception(s).
[ View ]
#51 handle_empty_date-857216-51.patch622 byteswuinfo
PASSED: [[SimpleTest]]: [MySQL] 3,847 pass(es).
[ View ]
#38 re.patch786 byteswuinfo
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in re.patch.
[ View ]
#27 feed_empty_date_27.patch741 bytesyareckon
PASSED: [[SimpleTest]]: [MySQL] 3,845 pass(es).
[ View ]
#12 857216-12_empty_dates.patch801 bytesalex_b
FAILED: [[SimpleTest]]: [MySQL] 183 pass(es), 412 fail(s), and 90 exception(es).
[ View ]
#8 feeds-857216.patch343 bytesderhasi
FAILED: [[SimpleTest]]: [MySQL] 183 pass(es), 412 fail(s), and 90 exception(es).
[ View ]
#8 feeds-857216-variant2.patch379 bytesderhasi
FAILED: [[SimpleTest]]: [MySQL] 183 pass(es), 412 fail(s), and 90 exception(es).
[ View ]

Comments

To clarify, my item has dates as follows:

<item date="2010-07-01 14:00:00" subject="Event without enddate"/> - enddate becomes present date
<item date="2010-07-01 14:00:00" enddate="2010-07-01 15:00:00" subject="item with enddate"/>

I tested and the start date also falls back to the present date if no date is given.

Title:date default value when no date is presentDate defaults to import date when no date is present
Version:6.x-1.0-beta2» 6.x-1.x-dev

Thanks for the report, this behavior is clearly not ideal. I would accept a patch to fall back to field default values. OTOH I can't tell what level of effort this will take.

Status:Active» Closed (duplicate)

Actually, this issue is a duplicate of #705588: CCK field default value's not getting picked up - right?

It is related to the same issue. But, the fact that default values is not implemented, and no date is detected, the date field should be empty and not now().
If default values is implemented, I am not sure if it works for the date field, because default values only works if the field is empty.

Title:Date defaults to import date when no date is presentDate is set to now when no date is present
Status:Closed (duplicate)» Active

I did some research and this issue is a bug specific for date fields. In the function buildDateField in FeedsParser.inc dates are filled with now when no dates are found. This is the case for a date with a 'from' and an optional 'to' field, as well as a date field with only a 'from' field.

scenario
1. If I have a test file like: <item date="2010-07-01 14:00:00" subject="Event without end"/> (no enddate)
2. with mapping date->from:activity and enddate->to:activity
3. $use_end get filled however, in function buildDateField for to:activity
4. the if statement in line 332 if ($use_end) fills $node->[field_activity][0]['value2']

If I disable the if routine in line 332, the date of the node doesn't set to now.

I think this issue is caused by the date handling in FeedsParser.inc due to timezones and date granularity. I can't figure out what exactly is going on there. I suppose this needs to be solved first before we can implement default values for cck dates.

Anyone have any other insight on this? I would really like to be able to use my default from cck for this.

I have no time to look into this closer, but next steps are:

1. Fix #705588: CCK field default value's not getting picked up.
2. Make sure that date elements are not populated if no date is present on source (along the lines of what Hanno found and described in #5).

Status:Active» Needs review
Issue tags:+datetime, +CCK date
StatusFileSize
new379 bytes
FAILED: [[SimpleTest]]: [MySQL] 183 pass(es), 412 fail(s), and 90 exception(es).
[ View ]
new343 bytes
FAILED: [[SimpleTest]]: [MySQL] 183 pass(es), 412 fail(s), and 90 exception(es).
[ View ]

As I think the main problem of this issue is independent of #705588: CCK field default value's not getting picked up, I attached two solutions for this issue.

The main reason why feeds sets empty dates to 'now' is located in FeedsParser.inc on line 499

// Some PHP 5.2 version's DateTime class chokes on invalid dates.
    if (!strtotime($time)) {
      $time = 'now';
    }

So any invalid date is set to 'now'.

Both solutions do not alter FeedsDateTime, they hop in at date_feeds_set_target(), before FeedsDateTime-objects are created:

The first variant assumes that there should not be set any date or time, when the feed_element is "".

Variant 2 checks if the feed_element could be a date by checking on numeric and strtotime. If both fails, it skips setting any date.

What is the overall effect of these patches on the date field? Is it just going to be empty? I assume it does not fall back to its default value, does it?

@alex_b, you are right. It just get's empty, it does not fall back to the default value. But this is only for CCK date fields. The core "published date" falls back to "now".

But this should not be a problem for this special issue, as the expected behaviour for importing an empty date is setting no date at all.
I'd prefer patch variant 2, as there also invalid dates are set to "no date", but a date like "now" or "now +1day" is still valid.

For the cck default value issue, there is still #705588: CCK field default value's not getting picked up to solve, but as I said, this should not be part of this issue.

Status:Needs review» Reviewed & tested by the community

Ok, thanks. Will commit this in my next round of Feeds work.

Issue tags:-datetime, -CCK date
StatusFileSize
new801 bytes
FAILED: [[SimpleTest]]: [MySQL] 183 pass(es), 412 fail(s), and 90 exception(es).
[ View ]

Running tests right now.

Title:Date is set to now when no date is presentSet Date to empty if no date is present

Status:Reviewed & tested by the community» Fixed

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

The use of strtotime() in this patch is a problem when trying to process dates before 1901. Please see #987206: strtotime() in class FeedsDateTime breaks dates before 1901 for a fix.

Version:6.x-1.x-dev» 7.x-2.x-dev
Status:Closed (fixed)» Patch (to be ported)

Please apply this patch in 7.x, too. This patch is applied in 6.x, but not in the 7.x branch. I tested it, and it resolved my problem in D7.

BTW, the patch as in Comment #16 didn't work for me. My source contains '00.00.0000' for a default/empty date. (I'll post it in the issue,itself, too)

Status:Patch (to be ported)» Needs work

The current patch does not remove an existing date. I propose the following change, which copies an empty value:

<?php
    
if (is_array($feed_element)) {
      
$feed_element = $feed_element[0];
     }
+
+   
// Empty string means no time - so clear the date field!
+    if (!is_numeric($feed_element) && !strtotime($feed_element)) {
+      
$feed_element = new FeedsDateTimeElement(NULL, NULL);
+
//      return ;
+    }
+
-     if (
$sub_field == 'end') {
     elseif (
$sub_field == 'end') {
      
$feed_element = new FeedsDateTimeElement(NULL, $feed_element);
     }
?>

Subscribe

Status:Needs work» Reviewed & tested by the community

#18 works ! Please commit to the d7 branch...

Status:Reviewed & tested by the community» Needs work

Please do not commit #18. It uses the strtotime() function which unnecessarily limits the capabilities of PHP date handling.

The strtotime function is meant to convert date strings to valid unix timestamps; it is not a way to validate a date string. As the PHP documentation says: "The valid range of a timestamp is typically from Fri, 13 Dec 1901 20:45:54 UTC to Tue, 19 Jan 2038 03:14:07 UTC." This means that importing dates outside this range will cause strtotime() to return false, even when the string is a perfectly valid date that Drupal is able to handle. i have already proposed a better solution to validate the date string in #987206: strtotime() in class FeedsDateTime breaks dates before 1901.

@marcvangend, with all respect, both issues handle different problems. For a lot of people 1900+ dates are satisfying, and this issues fixes a problem. Handling historical dates is also a problem, which is treated in your other issue. They both conflict, I know. But you shouldn't prohibit improvements for other people. In the end, it is up to maintainer to solved conflicts of interest.

@johnv: there is no conflict here. I'm not saying your problem shouldn't be fixed, I'm only saying that it should be fixed correctly. Date strings can be validated without using strtotime. Everybody wins.

#18 not working for me (but thanks for the clue anyway!).

I needed a very quick solution and the following works for me although I dare say not for everyone or in every situation:

function date_feeds_set_target($source, $entity, $target, $feed_element) {
list($field_name, $sub_field) = explode(':', $target, 2);
if (!($feed_element instanceof FeedsDateTimeElement)) {
if (is_array($feed_element)) {
$feed_element = $feed_element[0];
}
if (empty($feed_element)) {
$entity->$field_name = '';
return;
}
else if ($sub_field == 'end') {
$feed_element = new FeedsDateTimeElement(NULL, $feed_element);
}
else {
$feed_element = new FeedsDateTimeElement($feed_element, NULL);
}
}
$feed_element->buildDateField($entity, $field_name);
}

A second issue I discovered was with a date field that has only a year in it. FeedsDateTime doesn't seem to recognise 'YYYY' as a valid date format and builds a default object instead, changing all my year date fields to 1970. Needing to fix this quick I have implemented a workaround in my export to rewrite the year value as "YYYY-01" which gets imported and displayed correctly.

Apologies that this comment a) does not offer a great solution and b) goes off-topic to broach another issue altogether but these workarounds might help someone in the interim.

Does this patch "support" choosing either a fixed date or now() as the default value for empty date fields, on a per-field/content-type basis?

StatusFileSize
new741 bytes
PASSED: [[SimpleTest]]: [MySQL] 3,845 pass(es).
[ View ]

Rolled #18 into a git patch format.

Taxoman, no it changes the behavior, it doesn't allow switching between behaviors at all.

With #27 I can import empty date fields. The field is empty even if its default value is now(), which is the desired behavior in my case.

its' ok form me with #27 on 7.2 dev version.
thanks.

Ok, so one can use the approach in http://drupal.org/node/987206#comment-3970744 instead of strtotime() and then this patch is good to go?

Maybe we should create a method validate_date() or something like that and use it both places. Too bad idea?

Other than that, the patch worked for me.

#27 looks good

Version:7.x-2.x-dev» 7.x-2.0-alpha4
Component:Code» Feeds Import
Assigned:Unassigned» wuinfo
Status:Needs work» Reviewed & tested by the community

Commit to the next release please. Sometime, it is not wanted to convert empty string to now().

I have a customer who want to import Expense Disclosure data from csv to Drupal node with Feeds module 7.x-2.0-alpha4.

Travel start date and travel end date was imported as Date format.

But, sometime, there is expense without travel start and end date. The fields were left empty in that case. But every time we import such data, date was set to current date. It is not what we wanted at all.

Commit the patch at #27 can make sure those empty string or date with wrong format was set to null and it is more predictable and needed.

Version:7.x-2.0-alpha4» 7.x-2.x-dev
Component:Feeds Import» Code
Assigned:wuinfo» Unassigned
Status:Reviewed & tested by the community» Needs work

The patch needs work before it's committed, so just saying "please commit" doesn't help it get there. Do not change versions and components, and only assign this to yourself if you're doing some work on it.

success for me with #27

#27 patch is working. I have applied this patch to my project http://www.ohrc.on.ca . Until this piece code was committed, I have to apply this patch again whenever there is new version of feeds module.

Again, this patch is working. It may not solve all the outstanding default value issue. It solved the date data type.

franz, would you please let me know what work is needed?

See #30 and related issue linked. Using strtotime() is not recommend for checking if a date is valid (although it works most of the time).

StatusFileSize
new786 bytes
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in re.patch.
[ View ]

Thanks franz. How about this one? I have taken out the strtotime().

Status:Needs work» Needs review

Status:Needs review» Needs work

Thanks, wuinfo, but I forgot we shouldn't change the behavior, but rather provide options for it following #1107522: Framework for expected behavior when importing empty/blank values + text field fix. Now I see each mapper will have it's own logic for this, but nevertheless, we should get #860748: Config for mappers? finished to proper fix this issue. Just need to fix the tests there, if someone can work on it. =)

Title:Set Date to empty if no date is presentBehavior on importing empty/NULL/invalid dates

I´m having the same problem as #25 with date columns in CSV that only contain YYYY. Does the patch cover that also?

Ok, updated to 2.x-dev and see that there is still an issue with importing date columns in csv that only contain YYYY.

Assigned:Unassigned» twistor

This is a very straight forward problem. Feeds is not validating the date correctly. Issues of setting defaults and handling empty behavior are outside the scope of this issue. Changing behavior is fine in this case, because the current behavior is a bug.

#38 is close, we need more tests.

#38 worked for me.

#38 worked for me, too. Thanks!

Another thumbs-up for #38. Thanks.

#38 works here also - thanks!

#38 worked for me also. Thanks.

Assigned:twistor» Unassigned
Status:Needs work» Reviewed & tested by the community

Change the status, pending for commit.

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new622 bytes
PASSED: [[SimpleTest]]: [MySQL] 3,847 pass(es).
[ View ]

Updated the patch, so it can now be applied to 7x-2.x again.

#51 works for me
Any plans to commit this? It would be very useful, many people subscribed for this issue.

Was this committed?

Priority:Normal» Major

I answered my own question. This was not committed. This actually kills feeds for me and is propagating current dates all over my posts.

I would like to say #51 worked for me perfectly

#51 worked for importing new nodes.

But:
In my case I had to update nodes that have date-fields filled with data

Eg. a museum with 7 date-fields for every weekday presenting it´s opening time on each day - in one case the muesum is no longer open on Mondays, so there should be no data in this field "Monday" any more.

When I update nodes with a blank field "Monday" the data still remains there.

4kant, I think this might fall on my early comment #40. On the patch itself, can we get dummy sources posted here so we can test? A simple CSV with a couple of test cases would do.

Patch in #51 worked for me.

Thanks

@4kant: Your requirement is to erase the "Monday" data when the field is empty in CSV?

This small patch works fine for what I consider sensible defaults. If there is nothing in the field I would expect it to be empty and not today's date. If you want today's date Feeds Tamper should be involved.

As for wiping out existing data, does Feeds do this currently with text fields or other field types? If so, then certainly date should behave the same.

As for wiping out existing data, does Feeds do this currently with text fields or other field types? If so, then certainly date should behave the same.

I can't say if it currently does, but it really should in both cases. I have battled lot's of issues with customer-solution because of feeds inconsistent behavior when it comes to default values for empty sources and purging of existing values. It's frustrating this is still an issue (after almost 3 years!) since it's really not a very complex problem to solve.

Another inconsistency is the handling of multiple values. Currently the date-mapper only handles single-value date imports. This differs from text that handles multiple (as all mappers should). There is really no technical reason why it should be so (I wrote code to fix this in feedapi for drupal 6), so it puzzles me why.

StatusFileSize
new2.91 KB
FAILED: [[SimpleTest]]: [MySQL] 3,844 pass(es), 0 fail(s), and 1 exception(s).
[ View ]

This patch needs more testing (did not have time to write tests), but I think it might be a petty good approach of solving the problem with empty or invalid dates.

Status:Needs review» Needs work

The last submitted patch, handle_empty_and_invalid_dates-857216-62.patch, failed testing.

The test fail as a consequence of me removing:

-    // Some PHP 5.2 version's DateTime class chokes on invalid dates.
-    if (!strtotime($time)) {
-      $time = 'now';
-    }

I think this is bad practice though. Sweeping errors under the carpet in the constructor is to fundamentally alter the behavior of the extended class (DateTime), plus strtotime have a limited range(?) and dates before 1901 will be set to 'now'.

Status:Reviewed & tested by the community» Needs work

+++ b/plugins/FeedsParser.incundefined
@@ -602,11 +603,6 @@ class FeedsDateTime extends DateTime {
-    // Some PHP 5.2 version's DateTime class chokes on invalid dates.
-    if (!strtotime($time)) {
-      $time = 'now';
-    }
-

According to http://www.php.net/manual/en/datetime.construct.php, it is default behavior if there is nothing pass to the constructor of DateTime.

Agree: It is not the best way to prevent invalid date time string to the constructor, but it is a necessary step to avoid errors during import. And it is not the good place to fix current issue.

That's beside the point. If you pass NULL or 'now' to PHP's DateTime you get the current date, nothing wrong with that. If you pass an invalid date (like for expample '2010-20-12' in the test) you get an exception.

If you pass '2010-20-12' to FeedsDateTime you don't, you get an object for the current date, which is fundamentally changing the behavior of DateTime (in a bad way). As mentioned in my comment above you will also be unable use pre-1901 dates because strtotime is used.

Manually tested patch at #62

After apply the patch:

An AJAX HTTP error occurred. HTTP Result Code: 200 Debugging information follows. Path: /batch?id=5472&op=do StatusText: OK ResponseText: Fatal error: Call to a member function buildDateField() on a non-object in .

Status:Reviewed & tested by the community» Needs work

@presleyd at #59,

As for wiping out existing data, does Feeds do this currently with text fields or other field types? If so, then certainly date should behave the same.

Text did not wiping out existing data

Eg. a museum with 7 date-fields for every weekday presenting it´s opening time on each day - in one case the muesum is no longer open on Mondays, so there should be no data in this field "Monday" any more.

Feeds do this currently with text fields, text field is not wiping out the existing data if it is empty. Check out this issue for your request: #1107522: Framework for expected behavior when importing empty/blank values + text field fix .

@franz

4kant, I think this might fall on my early comment #40. On the patch itself, can we get dummy sources posted here so we can test? A simple CSV with a couple of test cases would do.

Patch at #51 did not change the behavior for empty date field. Actually, it fixes it. This issue is slightly different from the issue you have listed in #40 #1107522: Framework for expected behavior when importing empty/blank values + text field fix .

I think we are safe to commit patch at #51 and close this issue.

Status:Needs work» Reviewed & tested by the community

Perhaps I'm mistaken, but wouldn't the patch in #51 set the date to 'now' on empty or invalid values?

Status:Needs work» Reviewed & tested by the community
StatusFileSize
new2.92 KB
FAILED: [[SimpleTest]]: [MySQL] 3,844 pass(es), 0 fail(s), and 1 exception(s).
[ View ]

Forgot to bail out on the exception, here's a new patch that I think will solves the issue in #67.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, handle_empty_and_invalid_dates-857216-71.patch, failed testing.

Status:Needs work» Reviewed & tested by the community

I reviewed the code again and the approach in #51 seems sound. From what I can tell will not set any unexpected 'now'-values (as I first thought). I might still prefer my patch though since it gets rid of the strtotime-usage and only parses the date once. For me in #51 is also pretty hard to follow. It's not super-trivial to figure out what BuildDateField will perform on "new FeedsDateTimeElement(NULL, NULL)", and some change in any of the methods involved in the call could later on introduce new bugs or unexpected behaviors. #51 is probably safer to apply right now, since it doesn't change the way FeedsDateTime works so less potential for side-effects, short term at least.

Ok, so just to clarify, this is RTBC for patch in #51, right?

Status:Needs work» Fixed

Committed. Thanks.

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue summary:View changes
Status:Closed (fixed)» Needs work

Sorry to reopen this, but for me this is not fixed.

This patch in #51 only addresses the date:start field but not the date:end subfield:

diff --git a/mappers/date.inc b/mappers/date.inc
index 3de6a69..a96143a 100644
--- a/mappers/date.inc
+++ b/mappers/date.inc
@@ -51,7 +51,10 @@ function date_feeds_set_target($source, $entity, $target, $feed_element) {
     if (is_array($feed_element)) {
       $feed_element = $feed_element[0];
     }
-    if ($sub_field == 'end') {
+    if (empty($feed_element) || !is_numeric($feed_element) && !date_create($feed_element)) {
+      $feed_element = new FeedsDateTimeElement(NULL, NULL);
+    }
+    elseif ($sub_field == 'end') {
       $feed_element = new FeedsDateTimeElement(NULL, $feed_element);
     }
     else {

I came across this issue because it is not working as desired for me. If an :END field is empty/NULL/invalid it gets set to the :START value. Is this the desired behaviour?

I wish that empty subfields also stay empty:

-      $feed_element = new FeedsDateTimeElement(NULL, $feed_element);
+      $feed_element = new FeedsDateTimeElement(NULL, NULL);

Is it just me?