diff --git a/app/interactions/actions/contact_update.rb b/app/interactions/actions/contact_update.rb index 0cf76d116..d9a136dd3 100644 --- a/app/interactions/actions/contact_update.rb +++ b/app/interactions/actions/contact_update.rb @@ -15,7 +15,7 @@ module Actions maybe_update_statuses maybe_update_ident if ident.present? maybe_attach_legal_doc - maybe_change_email + maybe_change_email if new_attributes[:email].present? maybe_filtering_old_failed_records commit end diff --git a/app/models/depp/contact.rb b/app/models/depp/contact.rb index 7deb36562..48e49d5e4 100644 --- a/app/models/depp/contact.rb +++ b/app/models/depp/contact.rb @@ -16,6 +16,8 @@ module Depp ['Birthday', 'birthday'] ] + validates :phone, e164: true, phone: true + class << self attr_reader :epp_xml, :user @@ -144,6 +146,8 @@ module Depp end def save + return false unless valid? + hash = { id: { value: code }, postalInfo: { @@ -166,13 +170,15 @@ module Depp hash[:id] = nil if code.blank? create_xml = Depp::Contact.epp_xml.create(hash, extension_xml(:create)) - data = Depp::Contact.user.request(create_xml) self.id = data.css('id').text handle_errors(data) end + # rubocop:disable Metrics/MethodLength def update_attributes(params) + return false unless valid? + self.ident_country_code = params[:ident_country_code] self.ident_type = params[:ident_type] self.ident = params[:ident] @@ -220,6 +226,7 @@ module Depp data = Depp::Contact.user.request(update_xml) handle_errors(data) end + # rubocop:enable Metrics/MethodLength def delete delete_xml = Contact.epp_xml.delete( diff --git a/app/views/registrar/contacts/form/_general.haml b/app/views/registrar/contacts/form/_general.haml index 77443903d..5f1c90098 100644 --- a/app/views/registrar/contacts/form/_general.haml +++ b/app/views/registrar/contacts/form/_general.haml @@ -56,7 +56,8 @@ .col-md-3.control-label = f.label :email, t(:email) + '*' .col-md-7 - = f.email_field :email, class: 'form-control', required: true + = f.email_field :email, class: 'form-control', required: true, + pattern: "[a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,4}$" .form-group .col-md-3.control-label diff --git a/config/locales/en.yml b/config/locales/en.yml index 069f997ce..31947350d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -377,6 +377,11 @@ en: activemodel: errors: models: + 'depp/contact': + attributes: + phone: + invalid: "Phone number must be in +XXX.YYYYYYY format" + too_long: "Phone number is too long" 'depp/user': attributes: base: diff --git a/lib/validators/e164_validator.rb b/lib/validators/e164_validator.rb index e5807e585..72e805fd9 100644 --- a/lib/validators/e164_validator.rb +++ b/lib/validators/e164_validator.rb @@ -5,7 +5,7 @@ class E164Validator < ActiveModel::EachValidator length_validator.validate(record) format_validator = ActiveModel::Validations:: - FormatValidator.new(with: /\+[0-9]{1,3}\.[0-9]{1,14}?/, + FormatValidator.new(with: /\A\+[0-9]{1,3}\.[0-9]{1,14}\z/, attributes: attribute) format_validator.validate(record) end diff --git a/test/helpers/phone_format_helper_test.rb b/test/helpers/phone_format_helper_test.rb new file mode 100644 index 000000000..45ee80300 --- /dev/null +++ b/test/helpers/phone_format_helper_test.rb @@ -0,0 +1,37 @@ +module PhoneFormatHelperTest + # https://en.wikipedia.org/wiki/E.164 + def assert_phone_format(contact) + contact.phone = '+.1' + assert contact.invalid? + + contact.phone = '+123.' + assert contact.invalid? + + contact.phone = '+1.123456789123456' + assert contact.invalid? + + contact.phone = '+134.1234567891234' + assert contact.invalid? + + contact.phone = '+000.1' + assert contact.invalid? + + contact.phone = '+123.0' + assert contact.invalid? + + contact.phone = '+1.2' + assert contact.valid? + + contact.phone = '+123.4' + assert contact.valid? + + contact.phone = '+1.12345678912345' + assert contact.valid? + + contact.phone = '+134.123456789123' + assert contact.valid? + + contact.phone = '+134.00000000' + assert contact.invalid? + end +end \ No newline at end of file diff --git a/test/models/contact_test.rb b/test/models/contact_test.rb index bd4e1c0c1..d258d7fb4 100644 --- a/test/models/contact_test.rb +++ b/test/models/contact_test.rb @@ -1,6 +1,9 @@ require 'test_helper' +require 'helpers/phone_format_helper_test' class ContactTest < ActiveJob::TestCase + include PhoneFormatHelperTest + setup do @contact = contacts(:john) @old_validation_type = Truemail.configure.default_validation_type @@ -102,39 +105,9 @@ class ContactTest < ActiveJob::TestCase assert contact.invalid? end - # https://en.wikipedia.org/wiki/E.164 def test_validates_phone_format contact = valid_contact - - contact.phone = '+.1' - assert contact.invalid? - - contact.phone = '+123.' - assert contact.invalid? - - contact.phone = '+1.123456789123456' - assert contact.invalid? - - contact.phone = '+134.1234567891234' - assert contact.invalid? - - contact.phone = '+000.1' - assert contact.invalid? - - contact.phone = '+123.0' - assert contact.invalid? - - contact.phone = '+1.2' - assert contact.valid? - - contact.phone = '+123.4' - assert contact.valid? - - contact.phone = '+1.12345678912345' - assert contact.valid? - - contact.phone = '+134.123456789123' - assert contact.valid? + assert_phone_format(contact) end def test_valid_without_address_when_address_processing_id_disabled diff --git a/test/models/depp_contact_test.rb b/test/models/depp_contact_test.rb new file mode 100644 index 000000000..12f7c3b4f --- /dev/null +++ b/test/models/depp_contact_test.rb @@ -0,0 +1,14 @@ +require 'test_helper' +require 'helpers/phone_format_helper_test' + +class DeppContactTest < ActiveSupport::TestCase + include PhoneFormatHelperTest + + setup do + @depp_contact = Depp::Contact.new + end + + def test_validates_phone_format + assert_phone_format(@depp_contact) + end +end \ No newline at end of file diff --git a/test/system/registrar_area/contact_test.rb b/test/system/registrar_area/contact_test.rb new file mode 100644 index 000000000..49726029b --- /dev/null +++ b/test/system/registrar_area/contact_test.rb @@ -0,0 +1,45 @@ +require 'application_system_test_case' + +class RegistrarAreaContactTest < ApplicationSystemTestCase + setup do + @registrar = registrars(:bestnames) + @contact = contacts(:john) + sign_in users(:api_bestnames) + end + + def test_creates_contact_with_invalid_phone + visit registrar_contacts_path + click_on 'New' + + fill_in 'Ident', with: @contact.ident + fill_in 'Name', with: @contact.name + fill_in 'E-mail', with: @contact.email + fill_in 'Phone', with: '372' + click_on 'Create' + + assert_text 'Phone number must be in +XXX.YYYYYYY format' + end + + def test_updates_contact_with_invalid_phone + depp_contact = Depp::Contact.new( + id: @contact.id, + name: @contact.name, + code: @contact.code, + email: @contact.email, + phone: @contact.phone, + ident: @contact.ident, + ident_type: @contact.ident_type, + ident_country_code: @contact.ident_country_code) + + Spy.on(Depp::Contact, :find_by_id).and_return(depp_contact) + + visit edit_registrar_contact_path(depp_contact.code) + + assert_text "Edit: #{depp_contact.name}" + + fill_in 'Phone', with: '372' + click_on 'Save' + + assert_text 'Phone number must be in +XXX.YYYYYYY format' + end +end