Both patches committed, thank you! I changed the parameter name to 
'attribute' though.

I should note that I have only tested these patches with an LDAP 
directory.  I have not verified that they do not break any other 
backends.



-rob

Well... I learned more than I had intended to about the Horde 
framework, but I finally figured it out...



The problem with edit issue stems from the 
Turba_AbstractObject::setValue() method.  It ignores composite fields 
just like the Turba_Driver::toDriverKeys() method.



This fixes it...



--- AbstractObject.php.orig     2006-01-27 15:30:08.000000000 -0500

+++ AbstractObject.php  2006-01-27 14:57:42.000000000 -0500

@@ -117,7 +117,8 @@

          }



          if (isset($this->driver->map[$attribute]) &&

-            is_array($this->driver->map[$attribute]))

+            is_array($this->driver->map[$attribute]) &&

+                   !isset($this->driver->map[$attribute]['attr'])) {

              return false;

          }

Can the original contributor fix the edit issue?

I applied the patch but now I got problems when updating the entry:



There was an error updating this entry: Failed to change name: (34) 
Invalid DN syntax; Old DN = cn=Griletz 
Daniel,cn=robert,cn=private-addressbook,dc=server,dc=com, New DN = , 
Root = cn=robert,cn=private-addressbook,dc=server,dc=com



i.e cn was not set for the new DN

New patch.

I extended the map-array with a 'attr' field (if anyone have a better 
name, feel free to change it):



'name' => array('fields' => array('firstname', 'lastname'),

     'format' => '%s %s',

     'attr' => 'cn'),



And then I added the following to Driver.php:

--- Driver.php.orig     2005-08-30 12:39:52.000000000 +0200

+++ Driver.php  2005-09-11 17:13:31.000000000 +0200

@@ -141,8 +141,18 @@

      {

          $fields = array();

          foreach ($hash as $key => $val) {

-            if (isset($this->map[$key]) && !is_array($this->map[$key])) {

-                $fields[$this->map[$key]] = $val;

+            if (isset($this->map[$key])) {

+                if (!is_array($this->map[$key])) {

+                    $fields[$this->map[$key]] = $val;

+                }

+                elseif ($this->map[$key]['attr']) {

+                    $fieldarray = array();

+                    foreach ($this->map[$key]['fields'] as $mapfields) {

+                       $fieldarray[] = $hash[$mapfields];

+

+                    }

+                    $fields[$this->map[$key]['attr']] = 
vsprintf($this->map[$key]['format'], $fieldarray);

+               }

              }

          }

          return $fields;

I don't like the name "tofield", but yes, that and/or field hooks is 
the direction I'd like to see this go.

What if we/you/I extended the 'map' array?



From this, today:

$map = array(

         'name' => array('fields' => array('firstname', 'lastname'),

                         'format' => '%s %s'),

         'firstname' => 'givenname',

         'lastname' => 'sn',

);



to something like:



$map = array(

         'name' => array('fields' => array('firstname', 'lastname'),

                         'format' => '%s %s',

                         'tofield' => 'cn'),

         'firstname' => 'givenname',

         'lastname' => 'sn',

);



That way one can use the "tofield" (or whatever we may call it) to map 
the data to the right field.  If this is a more valid solution, I can 
probably work out a patch.

I can see the problem you're trying to solve, but you need to look at 
enhancing composite fields to take care of this instead. The way 
you've solved this is not clean and will not be committed.

Another patch to Driver.php to make sure we can import addresses from 
imp, even when "name" is set as staed above.



--- turba/lib/Driver.php.orig   2005-08-30 11:09:44.000000000 +0200

+++ turba/lib/Driver.php        2005-08-30 12:33:41.000000000 +0200

@@ -141,8 +141,38 @@

      {

          $fields = array();

          foreach ($hash as $key => $val) {

-            if (isset($this->map[$key]) && !is_array($this->map[$key])) {

-                $fields[$this->map[$key]] = $val;

+            if (isset($this->map[$key])) {

+                if (!is_array($this->map[$key])) {

+                    $fields[$this->map[$key]] = $val;

+                }

+                else {

+                    $map = $this->map[$key];

+

+                    $mapfields = $map['fields'];

+                    $mapformat = $map['format'];

+                    $values = sscanf($val, $mapformat);

+                    // To make sure we get ALL values, even if the 
format does not specify

+                    // enough arguments...

+                    if (vsprintf($mapformat, $values) != $val) {

+                        $format .= " %[^[]]";

+                        $values = sscanf($val, $format);

+                    }

+                    // Now this is ugly! BUT...

+                    // I believe this is mos widely used for names, 
and to make sure

+                    // Firstname Middlename Lastname is parsed 
correctly (with

+                    // Middlename as part of Givenname, I had to do 
it this way.

+                    // IE. map from the end of the array.

+                    // Should this be made configureable?

+                    while ($field = array_pop($mapfields)) {

+                        if ($mapfields[0]) {

+                            $value = array_pop($values);

+                        }

+                        else {

+                            $value = join(" ", $values);

+                        }

+                        $fields[$this->map[$field]] = $value;

+                    }

+                }

              }

          }

          return $fields;

Because I prefer to separate firstname and lastname in my address 
book, I have the following in my "sources.php":



         'name' => array('fields' => array('firstname', 'lastname'),

                         'format' => '%s %s'),

         'firstname' => 'givenname',

         'lastname' => 'sn',



This leads to a problem where cn is not set when I add a new contact, 
and this again leads to an object violation.

So I added this patch, to create the cn from givenname and sn if it 
does not exist:

There should probably be some sort of checking if sn and givenname is 
set, but as they are marked as required per default, I believe this is 
taken care of elsewhere.



--- turba/lib/Driver/ldap.php.orig      2005-08-30 08:55:35.000000000 +0200

+++ turba/lib/Driver/ldap.php   2005-08-30 08:59:52.000000000 +0200

@@ -219,6 +219,11 @@

          $dn = $attributes['dn'];

          unset($attributes['dn']);



+        /* If the CN does not exist, create it from SN and GIVENNAME */

+        if (!$attributes['cn']) {

+            $attributes['cn'] = $attributes['givenname'] . " " . 
$attributes['sn'];

+        }

+

          /* Put the objectClass into the attributes array. */

          if (!is_array($this->_params['objectclass'])) {

              $attributes['objectclass'] = $this->_params['objectclass'];