[dev] [Patch] Check for requied attributes

[Tarjei Huse]

On søn, 2004-08-15 at 17:45, Jan Schneider wrote:
> I finally found some time to review your patches.
Thanks.
> 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.
Ok, I've removed the comment and made sure the confsetting is used. 

> > 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.
ok, changed.

> 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".
Done. 

I didn't make any changes to the config patch, but I attached it anyway.

Regards,
Tarjei
> Jan.
> 
> --
> Do you need professional PHP or Horde consulting?
> http://horde.org/consulting.php
-- 
Med vennlig hilsen
Tarjei Huse
Rådgiver
Bergfald & Co AS
Telefon: 23 00 05 90
Mobiltelefon: 920 63 413
www.bergfald.no
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ldap.php.checkrequired.v3.patch
Type: text/x-patch
Size: 4998 bytes
Desc: not available
Url : http://lists.horde.org/archives/dev/attachments/20040817/6150a981/ldap.php.checkrequired.v3.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ldap.checkrequired.config.v2.patch
Type: text/x-patch
Size: 1132 bytes
Desc: not available
Url : http://lists.horde.org/archives/dev/attachments/20040817/6150a981/ldap.checkrequired.config.v2.bin