Activemodel not validating object throws wrong error

[teadur]

Expected return is Activemodel errors shown in EPP.

Contact object validation fails:

irb(main):009:0> a.validate
USER MSG: ACTIVEMODEL: Contact [ident] Required parameter missing - ident
USER MSG: ACTIVEMODEL: Contact [ident_type] is missing
USER MSG: ACTIVEMODEL: Contact [ident_type] Object status prohibits operation: ident_type of contact CID:XXXX:xxxx is invalid
USER MSG: ACTIVEMODEL: Contact [ident] Ident country code is not valid, should be in ISO_3166-1 alpha 2 format
USER MSG: ACTIVERECORD: Contact #117517 ["Ident Required parameter missing - ident", "Ident Ident country code is not valid, should be in ISO_3166-1 alpha 2 format", "Ident type is missing", "Ident type Object status prohibits operation: ident_type of contact CID:XXXX:xxxx is invalid"] []
=> false

When user wants to update that object over epp non clear error is thrown:

Request XML is

   <command>

          <update>

                 <contact:update xmlns:contact="https://epp.tld.ee/schema/contact-ee-1.1.xsd">

                       <contact:id>CID:XXXXX:xxxxx</contact:id>

                       <contact:chg>

                              <contact:postalInfo>

                                     <contact:name>New Contact name</contact:name>

                              </contact:postalInfo>

                              <contact:voice>+327.12345/contact:voice>

                              <contact:email>email@domain.tld</contact:email>

                       </contact:chg>

                 </contact:update>

          </update>

          <extension>

                 <eis:extdata xmlns:eis="https://epp.tld.ee/schema/eis-1.0.xsd">

                       <eis:ident cc="CH" type="org">CHE-XXXXX</eis:ident>

                 </eis:extdata>

          </extension>

          <clTRID>XXXX:xxxx:784991809977051</clTRID>

   </command>

Response XML is

<epp xmlns="https://epp.tld.ee/schema/epp-ee-1.0.xsd" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"

   xsi:schemaLocation="lib/schemas/epp-ee-1.0.xsd">

   <response>

          <result code="1">

                 <msg lang="en">handle_errors was executed when there were actually no errors</msg>

          </result>

          <trID>

                 <clTRID>XXXX:xxxx:784991809977051</clTRID>

                 <svTRID>ccReg-5352543179</svTRID>

          </trID>

   </response>

[artur-intech]

For some reason, the value of "ident" column from EPP request ("CHE-XXXXX") is not set on EPP contact:update. As I understand the code misses at.merge!(ident: ident_frame.text)
here

registry/app/models/epp/contact.rb

Line 187 in d3b1a23

at.merge!(ident_type: ident_frame.attr('type'))

but I need a confirmation

[artur-intech]

@vohmar Also I need business rules, describing a case, when a user sends EPP contact:update request with the following part:
<eis:ident cc="CH" type="org">CHE-XXXXX</eis:ident>

[vohmar]

Rule 1: ident cannot be updated
Rule 2: ident_cc and ident_type can be updated if ident set does not validate and validates as a result of the update

[vohmar]

error code for invalid update request trying to change ident data

2308 "Data management policy violation" + validation rule based specification ie.
2308 "Data management policy violation: update of ident not allowed, please consider creating new contact object."

other possible specifications
update of ident_cc not allowed, please consider creating new contact object.
update of ident_type not allowed, please consider creating new contact object.

[artur-intech]

Given that "ident" column cannot be updated, perhaps it's worth mentioning that in https://github.com/internetee/registry/blob/master/doc/epp/contact.md?

[artur-intech]

Ident modification must be completely retested.

[vohmar]

• contact update to a contact that had no ident_type and ident_cc values was successful with respective values of "birthday" and "ee" - birthday is not allowed in case of ee (not a valid Estonian identity format) 2005
• contact update to a contact that had no ident type, valid date as an ident value and ident_cc as ee resulted in wrong error message 2308 on setting type to "birthday". Should have been 2005 (not valid Estonian identity)
• contact update to a contact that had no ident type, valid date as an ident value and ident_cc as ch resulted in error 2308 on setting type to "birthday" . Should have been 1000
• contact update to a contact that had no ident type, invalid date as an ident value and ident_cc as ch resulted in error 2308 on setting type to "birthday". Should have been 2005 (not valid birthday format)
• contact update to a contact that had no ident type, invalid date as an ident value and ident_cc as ee resulted in error 2308 on setting type to "birthday". Should have been 2005 (not valid Estonian identity format / not valid birthday format)
• contact update to a contact that had no ident_cc, valid date as an ident value and ident_type as birthday was successful with ident_cc values of "ee". Should have been 2005 (not valid Estonian identity format)
• contact update to a contact that had invalid ident_cc, valid date as an ident value and ident_type as birthday was successful with ident_cc value of "ee". Should have been 2005 (not valid Estonian identity format)
• contact update to a contact that had invalid ident_type, valid date as an ident value and ident_cc as ee was successful with ident_type value of "birthday". Should have been 2005 (not valid Estonian identity format)
• contact update to a contact that had no ident_cc, valid ident and ident_type as priv resulted in error 2308 on setting ident_cc to "EE". Should have been 1000
• contact update to a contact that had no ident_cc, valid ident and ident_type as priv resulted in error 2308 on setting ident_cc to "US". Should have been 1000
• contact update to a contact that had invalid ident_cc, valid ident and ident_type as priv resulted in error 2308 on setting ident_cc to "EE". Should have been 1000
• contact update to a contact that had invalid ident_cc, invalid ident and ident_type as priv resulted in error 2308 on setting ident_cc to "EE". Should have been 2005 (not valid Estonian identity format)
• contact update to a contact that had no ident_cc, invalid ident and ident_type as priv resulted in error 2308 on setting ident_cc to "EE". Should have been 2005 (not valid Estonian identity format)
• same problems as with ident_type priv apply to ident_type org as well

all the validation applied to contact create must apply to contact update as well (at the resulting object)

[artur-intech]

@vohmar
Please provide the following for № 12:

• EPP request
• Expected behaviour

Should it still allow updating ident? Is it possible to update (set) ident if a contact has none? I am not sure I understand what should happen in such case.

Also, what is the expected behaviour for the case when:

• Contact has ident with value "test", but no ident_type and ident_country_code (therefore it is invalid)
• User sends EPP contact:update request with ident value of "another-ident"
  ?

[artur-intech]

Why ident_country_code is not validated if ident_type is "birthday"?

https://github.com/internetee/registry/blob/master/app/models/contact.rb#L32

[vohmar]

@artur-beljajev
No12: https://staging-adm.infra.tld.ee:444/admin/epp_logs/46588
Expected behavior is as said in the error reports error 2005 as the current ident code is not valid EE personal identity code.
As specified in rule 1 the ident code cannot be updated - that means value of ident element can not be updated under any circumstance.
Rule 2 specifies that ident_cc and ident_type can be updated in case of invalid ident and the update results with valid one
So in the case where country code and type are missing and user attempts to update ident code the result is error 2308
If ident element is emtpy then the only way to fix the contact is to create a new object and replace the current one because of rule 1

ident_country_code was not initially required if the ident_type was set as birthday. Currently the country_code is always required and thus it should also be validated - so this is a bug.

[vohmar]

No 6: error messages still need improvements

case1: contact:update ident - 2308: Invalid ident. Only ident type and country can be updated in case of invalid ident. Please create new contact object to update ident code.

case2: contact:update ident type/cc (ident data set validates) - 2308: Ident update is not allowed. Consider creating new contact object.

[artur-intech]

№ 6:
errors.update_disallowed What needs to be improved here? I see exactly the same message
errors.wrong_ident This error is displayed when user provides wrong <ident></ident> value in EPP request (I would use just "wrong" here to distinguish it from invalid by validating format of something). Why you want a user to create new contact? Why he cannot just provide correct <ident> (the same as in the db)

[vohmar]

contact update:

case1: ident data set validates on an existing contact object - update of ident data not allowed. Attempt to update results "2308: Ident update is not allowed. Consider creating new contact object."

case2: ident data set is not valid on an existing contact object - update of ident_cc and ident_type is allowed in case the resulting data set validates
case2.1: attept to update ident code results in error (ident code cannot be changed in any condition) "2308: Only ident type and country can be updated in case of invalid ident. Please create new contact object to update ident code."
case2.*: different cases with handling updates of ident_country_code and ident_type of an invalid contact.

At the same time the general principle of detecting actual change still applies - if update includes data that matches the data already in db (in reality nothing is changed) it is not considered as attempt to change data and cannot result in error. For example

contact object with valid ident data set in registry
contact update with <ident type="same as in db", cc="same as in db">same as in db
results with code 1000

[vohmar]

• ident set is valid : if contact:update query includes eis:ident element with the same data as already in the system error 2308 is returned. Code 1000 is correct as nothing is changed with the ident data because of this query.

[artur-intech]

Fixed, spec added

[vohmar]

this branch includes changes from branch #231. As #231 is not ready I cannot merge this to master.

[vohmar]

FYI: merges reverted from master with this ticket:
#605
#586
#575

[artur-intech]

I reverted your revert. See PR