[Freeipa-devel] [PATCH]Extending user plugin with employeenumber field

[Misnyovszki Adam]

On Tue, 25 Mar 2014 14:31:15 +0100
Petr Vobornik <pvoborni at redhat.com> wrote:

> On 21.3.2014 11:00, Misnyovszki Adam wrote:
> > On Fri, 21 Mar 2014 10:13:55 +0100
> > Misnyovszki Adam <amisnyov at redhat.com> wrote:
> >
> >> On Fri, 21 Feb 2014 16:06:27 +0100
> >> Petr Vobornik <pvoborni at redhat.com> wrote:
> >>
> >>> On 21.2.2014 15:45, Adam Misnyovszki wrote:
> >>>> Hi,
> >>>> According to http://tools.ietf.org/html/rfc2798 ipa client and
> >>>> web ui extended with employeenumber field.
> >>>>
> >>>> https://fedorahosted.org/freeipa/ticket/4165
> >>>>
> >>>> Question is, that should we extend user with other fields which
> >>>> are in the RFC, (carLicense, departmentNumber, employeeType, etc)
> >>>> if we already touched this code?
> >>>>
> >>>> Thanks
> >>>> Adam
> >>>>
> >>>
> >>>
> >>> +        Int('employeenumber?',
> >>> +            label=_('Employee ID'),
> >>> +            minvalue=1,
> >>> +        ),
> >>>
> >>>
> >>> Why Int and different label? IMO it should be Str and 'Employee
> >>> Number'
> >>>
> >>> 2.4. Employee Number
> >>>
> >>>      Numeric or alphanumeric identifier assigned to a person,
> >>> typically based on order of hire or association with an
> >>> organization. Single valued.
> >>>
> >>>       ( 2.16.840.1.113730.3.1.3
> >>>         NAME 'employeeNumber'
> >>>         DESC 'numerically identifies an employee within an
> >>> organization' EQUALITY caseIgnoreMatch
> >>>         SUBSTR caseIgnoreSubstringsMatch
> >>>         SYNTAX 1.3.6.1.4.1.1466.115.121.1.15
> >>>         SINGLE-VALUE )
> >>>
> >> Hi,
> >> fixed, also some other fields added. Note, that according to the
> >> rfc,
> 
> >> licence plate field should be multivalue, should I cange that(it
> >> is an existing field).
> 
> yes
> 
> 
> >> Also, should I write test cases(especially for
> >> preferredlanguage)?
> 
> Testing new functionality helps.
> 
> >> Greets
> >> Adam
> >
> > self NACK,
> > VERSION bump because API change
> 
> It requires another rebase.
> 
> >
> > Greets
> > Adam
> >
> 
> 1) Is there a reason to have label 'Employee ID' instead of 'Employee 
> Number' which is in RFC 2798?
> 
> +            label=_('Employee ID'),
> 
> 
> 2) Department number seems to be multivalued as well:
>      ( 2.16.840.1.113730.3.1.2
>        NAME 'departmentNumber'
>        DESC 'identifies a department within an organization'
>        EQUALITY caseIgnoreMatch
>        SUBSTR caseIgnoreSubstringsMatch
>        SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 )
> 
> 3) The regex for preferredlanguage:
> 
>    +            pattern='^[a-zA-Z]{1,8}[-[a-zA-Z]{1,8}]?$',
> 
> doesn't match the expression in RFC 2068. It's only part of it.
> 
> 
>            Accept-Language = "Accept-Language" ":"
>                              1#( language-range [ ";" "q" "=" qvalue
> ] )
> 
>            language-range  = ( ( 1*8ALPHA *( "-" 1*8ALPHA ) ) | "*" )
> 
> http://tools.ietf.org/html/rfc2068#section-14.4
> 
> RFC 2798 ( http://tools.ietf.org/html/rfc2798#section-2.7 ) says that 
> you should omit only the `"Accept-Language" ":"` sequence.
> 
> 

See the updates in the attached patch.
Greets
Adam
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-amisnyov-0005-4-Extending-user-plugin-with-inetOrgPerson-fields.patch
Type: text/x-patch
Size: 11760 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140325/3d2f2eec/attachment.bin>