[dev] [Patch] Check for requied attributes

[Jan Schneider]

I finally found some time to review your patches.

Zitat von Tarjei Huse <tarjei+a_lists.phpgw at nu.no>:

> On Mon, 2004-08-02 at 06:38, Chuck Hagenbuch wrote:
>> Quoting Tarjei Huse <tarjei+a_lists.phpgw at nu.no>:
>>
>> > Hi, patch update. I've added some comments.
>> >>  >
>> >> > > The attached patch adds functionality to turbas ldapdriver to 
>> check if
>> >> > > an entry beeing added contains all the attributes needed.
>> >> > >
>> >> > > This is especially practical with regard to adding entries from other
>> >> > > applications (like imp) as they cannot tell the user that f.x. the
>> >> > > Lastname field is required. As the patch is no, it adds just a "N/A"
>> >> > > string instead.
>>
>> I'd definitely commit this if the field could contain an empty string
>> instead of
>> N/A. Can it? Or does that go counter to how LDAP handles required fields?
> I'm sorry, but it will not accept an empty string, a space is ok though.
> I have changed the patch so it's possible to set the string in a
> configfile.

But this config setting isn't used anywhere. Beside that you still refer to
n/a in all comments.

> Also, I've done my homework and modified the code so that it now checks
> to see if the attribute in question is a string or not. This makes the
> patch a bit larger (+60 LOC). I'd like input from other ldapusers on
> what ldapsyntaxes should be ok.
>
> Also, the patch now uses the existing ldapconnection instead of making
> it's own.
>
> Please comment.

You don't need to set $params['checkrequired'] in the ctor. It should be set
in every LDAP configuration anyway, and you can still test for
empty($params['checkrequired']) later in the code.

There are a few coding style errors, mostly spacing. Also, please add
complete phpdoc blocks to the methods you added, and make the inline
comments more "neutral".

Jan.

--
Do you need professional PHP or Horde consulting?
http://horde.org/consulting.php