On 21.3.2014 11:00, Misnyovszki Adam wrote:
On Fri, 21 Mar 2014 10:13:55 +0100
Misnyovszki Adam <amisn...@redhat.com> wrote:

On Fri, 21 Feb 2014 16:06:27 +0100
Petr Vobornik <pvobo...@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.


--
Petr Vobornik

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to