Support array of values for dates [#697842]

I've done update on date.inc and FeedsParser to have support array of values for dates. (i'm working on the alpha10)
If someone is interested in !

FeedsParser.inc

  /**
   * Helper method for buildDateField(). Build a FeedsDateTimeElement object
   * from a standard formatted node.
   */
  protected static function readDateField($node, $field_name,$index=0) {
    $field = content_fields($field_name);
    $ret = new FeedsDateTimeElement();
    if ($node->{$field_name}[$index]['date'] instanceof FeedsDateTime) {
      $ret->start = $node->{$field_name}[$index]['date'];
    }
    if ($node->{$field_name}[$index]['date2'] instanceof FeedsDateTime) {
      $ret->end = $node->{$field_name}[$index]['date2'];
    }
    return $ret;
  }

  /**
   * Build a node's date CCK field from our object.
   *
   * @param $node
   *   The node to build the date field on.
   * @param $field_name
   *   The name of the field to build.
   */
  public function buildDateField($node, $field_name,$index=0) {
    $field = content_fields($field_name);
    $oldfield = FeedsDateTimeElement::readDateField($node, $field_name,$index);
    // Merge with any preexisting objects on the field; we take precedence.
    $oldfield = $this->merge($oldfield);
    $use_start = $oldfield->start;
    $use_end = $oldfield->end;

    // Set timezone if not already in the FeedsDateTime object
    $to_tz = date_get_timezone($field['tz_handling'], date_default_timezone_name());
    $temp = new FeedsDateTime($to_tz, NULL);

    $db_tz = '';
    if ($use_start) {
      $use_start = $use_start->merge($temp);
      if (!date_timezone_is_valid($use_start->getTimezone()->getName())) {
        $use_start->setTimezone(new DateTimeZone("UTC"));
      }
      $db_tz = date_get_timezone_db($field['tz_handling'], $use_start->getTimezone()->getName());
    }
    if ($use_end) {
      $use_end = $use_end->merge($temp);
      if (!date_timezone_is_valid($use_end->getTimezone()->getName())) {
        $use_end->setTimezone(new DateTimeZone("UTC"));
      }
      if (!$db_tz) {
        $db_tz = date_get_timezone_db($field['tz_handling'], $use_end->getTimezone()->getName());
      }
    }
    if (!$db_tz) {
      return;
    }

    $db_tz = new DateTimeZone($db_tz);
    if (!isset($node->{$field_name})) {
      $node->{$field_name} = array();
    }
    if ($use_start) {
      $node->{$field_name}[$index]['timezone'] = $use_start->getTimezone()->getName();
      $node->{$field_name}[$index]['offset'] = $use_start->getOffset();
      $use_start->setTimezone($db_tz);
      $node->{$field_name}[$index]['date'] = $use_start;
      /**
       * @todo the date_type_format line could be simplified based upon a patch
       *   DO issue #259308 could affect this, follow up on at some point.
       *   Without this, all granularity info is lost.
       *   $use_start->format(date_type_format($field['type'], $use_start->granularity));
       */
      $node->{$field_name}[$index]['value'] = $use_start->format(date_type_format($field['type']));
    }
    if ($use_end) {
      // Don't ever use end to set timezone (for now)
      $node->{$field_name}[$index]['offset2'] = $use_end->getOffset();
      $use_end->setTimezone($db_tz);
      $node->{$field_name}[$index]['date2'] = $use_end;
      $node->{$field_name}[$index]['value2'] = $use_end->format(date_type_format($field['type']));
    }
  }
}

date.inc

function date_feeds_set_target($node, $target, $feed_element) {
  list($field_name, $sub_field) = split(':', $target);
  
  
  if (!($feed_element instanceof FeedsDateTimeElement)) {
    
     if (is_array($feed_element)) {
	    $index=0;
	    foreach ($feed_element as $feed_elem)
	    {
	    	    if ($sub_field == 'end') {
		      $feed_elem = new FeedsDateTimeElement(NULL, $feed_elem);
		    }
		    else {
		      $feed_elem = new FeedsDateTimeElement($feed_elem, NULL);
		    }
		    $feed_elem->buildDateField($node, $field_name, $index);
		    $index++;
	    }
      }
      else
      {
      	if ($sub_field == 'end') {
	      $feed_element = new FeedsDateTimeElement(NULL, $feed_element);
        }
	else {
	      $feed_element = new FeedsDateTimeElement($feed_element, NULL);
	}
        $feed_element->buildDateField($node, $field_name);  	
      }
  }
}

Comment	File	Size	Author
#39	feeds-date-array-697842-39.patch	11.6 KB	eiriksm
#39
#23	feeds-date-array-697842-23.patch	11.6 KB	eiriksm
#23
#22	feeds-date-array-697842-22.patch	11.55 KB	eiriksm
#22
#17	date-array-697842.patch	5.33 KB	franz
#17
#15	date-array-697842.patch	5.41 KB	franz
#15
#13	date-array-697842-13.patch	3.97 KB	eiriksm
#13
#12	datearray.patch	3.57 KB	eiriksm
#12
#8	date-array.patch	3.57 KB	eiriksm
#8
#2	feeds-6.x1.0-alpha11-FeedsParser.inc_.patch	3.76 KB	perpignan
#2
#2	feeds-6.x1.0-alpha11-date.inc_.patch	1.38 KB	perpignan
#2

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

alex_b CreditAttribution: alex_b commented 28 January 2010 at 14:45

Can you post a patch ?

http://drupal.org/patch/create

File	Size
feeds-6.x1.0-alpha11-date.inc_.patch	1.38 KB

feeds-6.x1.0-alpha11-FeedsParser.inc_.patch	3.76 KB

Comment #3

alex_b CreditAttribution: alex_b commented 28 April 2010 at 17:26

Version:	6.x-1.0-alpha10	» 6.x-1.x-dev
Category:	support	» feature
Status:	Active	» Needs review

Sorry, flew under my radar as an active support request. This is feature request that needs review.

Comment #4

alex_b CreditAttribution: alex_b commented 28 April 2010 at 17:28

Status:

Needs review

» Needs work

- date.inc patch does not apply to HEAD
- please roll one patch (cvs diff -u > example.patch from feeds/ directory)

Comment #5

hampshire CreditAttribution: hampshire commented 16 June 2010 at 14:49

The date patch does not apply to Alpha 15 either.
Does anyone have advice on how I can get this patch working with a later version of the module.

Thanks.

Comment #6

alex_b CreditAttribution: alex_b commented 17 June 2010 at 14:12

hampshire: patches are always against the branch HEAD, not cleanly applying patches need to be fixed by hand going through the code...

Comment #7

eiriksm

he/him

Norwegian Bokmål

Norway

CreditAttribution: eiriksm commented 7 April 2011 at 10:34

i did some similar editing on the 7 version. any chance this will get into the 7 version?

should i open up a new issue for the 7 version and date arrays?

Comment #8

eiriksm

he/him

Norwegian Bokmål

Norway

CreditAttribution: eiriksm commented 14 April 2011 at 12:26

Version:	6.x-1.x-dev	» 7.x-2.0-alpha3
Status:	Needs work	» Needs review

File	Size
date-array.patch	3.57 KB

Here is a patch if anyone else is interested.

Patch for 7.2 alpha 3

Comment #9

TripX CreditAttribution: TripX commented 14 April 2011 at 19:09

Can you commit this? .. or forward it that someone can commit this?

I think this is a frequent feature request.

Comment #10

TripX CreditAttribution: TripX commented 14 April 2011 at 21:17

Ahmn... there are mistakes in the code.

you wrote f.e. $eed_.... instead of $feed_....

Have you tested it?

Comment #11

eiriksm

he/him

Norwegian Bokmål

Norway

CreditAttribution: eiriksm commented 15 April 2011 at 08:37

its tested and works great. although i must admit some of the variables could have more logical values. it's called eed_... to prevent overwriting the feed_... variable if i remember correctly. some of the variables is in norwegian too, not good i guess.

Comment #12

eiriksm

he/him

Norwegian Bokmål

Norway

CreditAttribution: eiriksm commented 6 May 2011 at 08:58

File	Size
datearray.patch	3.57 KB

Here is a new patch with more sane variables. For people interested, i also had to make a patch for d6 feeds -beta10 for a another project. In here is a patch for that: #1148892: Support array of values for dates (mapping to a date multifield)

Comment #13

eiriksm

he/him

Norwegian Bokmål

Norway

CreditAttribution: eiriksm commented 12 September 2011 at 07:11

Version:

7.x-2.0-alpha3

» 7.x-2.x-dev

File	Size
date-array-697842-13.patch	3.97 KB

Made a new patch against -dev. Hopefully this will make it into the module, so i don't have to patch my updates. Using this on several sites now, and it works.

Comment #14

franz

any

Montréal

CreditAttribution: franz commented 14 September 2011 at 18:26

Status:

Needs review

» Needs work

It's not yet complete.

When merging with old data, it always took the first item (not sure if you tried start and end before).

I'm fixing it and it seems better also to replace $fieldnumber with $delta, as it makes more sense (Field key concept).

Comment #15

franz

any

Montréal

CreditAttribution: franz commented 14 September 2011 at 18:46

Status:	Needs work	» Needs review
Issue tags:		+Needs tests

File	Size
date-array-697842.patch	5.41 KB

Also, I think we need to work on a test for this.

Comment #16

eiriksm

he/him

Norwegian Bokmål

Norway

CreditAttribution: eiriksm commented 15 September 2011 at 06:01

Thanks for that. I am no programmer, so I am glad someone picked this up and looked at it. I agree with the $delta too, very logic. On the sites I am using this I am not using end date, no.

I have never written a test before, but i could look into it when i get the time.

Comment #17

franz

any

Montréal

CreditAttribution: franz commented 15 September 2011 at 11:47

File	Size
date-array-697842.patch	5.33 KB

http://drupal.org/simpletest has some explanation about automated tests, be sure to read it.

Then you can work on the existing test for Date fields on file tests/feeds_mapper_date.test .

I still hadn't change the patch to use $delta, now I did the change.

Comment #18

eiriksm

he/him

Norwegian Bokmål

Norway

CreditAttribution: eiriksm commented 28 September 2011 at 13:41

I tried a couple of times to make a test, both with the Feeds test setup and from scratch, but never managed to create a field allowing multiple values. I got my content type set up and a date field added, but couldn't figure out how to change the max allowed values. And that is pretty darn important in this test i would say.

If someone else wants to jump on creating a test for this, feel free.

Comment #19

emilcarpenter CreditAttribution: emilcarpenter commented 16 January 2012 at 01:15

Thanks eiriksm and franz!

This patch got the dates into the multivalue field...

...but the dates are displayed like 2012-01-16T01:54:18 in Views, regardless of the Short, Medium or Long display type chosen. Does anybody have an idea how to fix this?

(Hope i did not do any error copy-pasting the lines instead of using Netbeans for applying the patch. I'm a beginner...)

I use Feeds 7.x-2.0-alpha4, Date-7.x-2.0-rc1 and Drupal 7.9

Comment #20

emilcarpenter CreditAttribution: emilcarpenter commented 16 January 2012 at 02:10

Nevermind the question, the dates display fine without Views. Probably some Views multi value issue.

Comment #21

timos CreditAttribution: timos commented 7 March 2012 at 07:44

Hi and thanks eiriksm and franz !
It seems work well.

will this patch include in the dev version and so on in a future release ?

Chers

Comment #22

eiriksm

he/him

Norwegian Bokmål

Norway

CreditAttribution: eiriksm commented 22 April 2012 at 14:56

File	Size
feeds-date-array-697842-22.patch	11.55 KB

Hello.

OK, so I thought it was about time to get this commited and sat down to write tests for testing importing array of dates.

Included in the attached patch is:
- Patch from #17
- Updated test for date mappers (date module now sets a 15 min increment as default, therefore breaking the field assertions in the test)
- Test for array of dates.

Since none of the parsers included in Feeds supports array of dates (at least I couldn't get that to work), i am using the Feeds X path parser plugin for tests, which is also what i use on the (several) projects I have this patch up and running live with.

Would really like to see this get commited, so hopefully someone will pick this up, and maybe tell me what is missing also.

Thanks again to the maintainers for all your great work on this module, it is such a great tool!

Comment #23

eiriksm

he/him

Norwegian Bokmål

Norway

CreditAttribution: eiriksm commented 22 April 2012 at 15:13

File	Size
feeds-date-array-697842-23.patch	11.6 KB

Updated patch with some coding standard issues, and small clarifying commenting stuff. Took the liberty while adding @param int $delta to add parameter type to the whole function comment block in question. Should by the way be done several other places, but that is a different issue i guess.

Comment #24

franz

any

Montréal

CreditAttribution: franz commented 23 April 2012 at 12:28

I don't like the idea of depending on an external module to run those tests. We can instead create a test module that provides a "fake" parser: A parser that you can set the values (including multiple values) as a configuration directly, without the need to involve fetchers or other parsers. This will also decouple this test and potentially others from the fetcher/parser code.

Comment #25

eiriksm

he/him

Norwegian Bokmål

Norway

CreditAttribution: eiriksm commented 27 April 2012 at 13:28

I agree on that. But I dont have time for that atm, so will have to postpone it for some time. However, if you (or anyone else) wants to jump on this, feel free :)

Comment #26

franz

any

Montréal

CreditAttribution: franz commented 27 April 2012 at 13:40

Status:

Needs review

» Needs work

Comment #27

Rob_Feature CreditAttribution: Rob_Feature commented 18 June 2012 at 14:18

What should the values look like for this patch to work? I applied it and I wasn't able to import multiple, but I have a feeling it's because our source CSV has the dates comma separated, which seems like a big no-no for a CSV file. Any clarity on a format expected by this patch?

Comment #28

franz

any

Montréal

CreditAttribution: franz commented 19 June 2012 at 11:37

Rob_Feature, the parser must support returning multiple value for a field. The CSV Parser alone doesn't. If you need this feature, use Feeds Tamper module and apply the tamper plugin "Explode" to this field.

Comment #29

franz

any

Montréal

CreditAttribution: franz commented 19 June 2012 at 11:40

This is because of the very nature of CSV data source, which doesn't support multiple values for a field, only through "tweaking" (in this case, a string with comma-separated values).

Comment #30

Rob_Feature CreditAttribution: Rob_Feature commented 19 June 2012 at 13:28

Wow, awesome Franz. Thanks!

Comment #31

BD3 CreditAttribution: BD3 commented 23 June 2012 at 05:41

@Rob_Feature Did you ever get this to work? Has it been committed to the latest dev? I cannot seem to get this to work but I am having trouble finding the proper module to patch.

Comment #32

Rob_Feature CreditAttribution: Rob_Feature commented 25 June 2012 at 12:17

I did get it to work with the patch here. Honestly, though, I patched before I even tried to do it properly so I'm not sure if the patch 'works' or not. There WAS a dev release after this patch was made so it may need to be re-rolled if it won't apply.

Comment #33

franz

any

Montréal

CreditAttribution: franz commented 25 June 2012 at 13:21

Patch still applies clean to latest dev. What we still need is some test changes re #24

Comment #34

BD3 CreditAttribution: BD3 commented 25 June 2012 at 16:10

Thanks @Rob_Feature and @franz. I was able to successfully apply to the latest dev (2012-Jun-20) and all seems to be working!

Comment #35

eiriksm

he/him

Norwegian Bokmål

Norway

CreditAttribution: eiriksm commented 26 June 2012 at 07:03

So, does anyone want to jump on making the tests? If people are using the patch, let's try to get it in!

Comment #36

BD3 CreditAttribution: BD3 commented 28 June 2012 at 16:42

One thing that isn't working are empty values. It will bring in the date of the import instead. There are a few patches here waiting to be tested #1107522: Framework for expected behavior when importing empty/blank values + text field fix, but they are not compatible with the patch in this issue to support multiple values.