Merge remote-tracking branch 'origin/master' into 1636-objects-are-missing-creator-and-updator-strings

CHANGELOG.md

@@ -1,3 +1,10 @@
+04.08.2020
+* Fixed registrant verification for domain delete [#1631](https://github.com/internetee/registry/issues/1631)
+* Fixed domain transfer issue when one person was present in the same role more than once (different objects) [#1651](https://github.com/internetee/registry/issues/1651)
+
+03.08.2020
+* Fixed 0 vat issue with invoices sent to Directo [#1647](https://github.com/internetee/registry/issues/1647)
+
 17.07.2020
 * Added turemail gem for validating email addresses syntactically and on MX record level [#297](https://github.com/internetee/registry/issues/297)
 

app/controllers/registrant/domain_delete_confirms_controller.rb

@@ -24,9 +24,9 @@ class Registrant::DomainDeleteConfirmsController < RegistrantController
 
     confirmed = params[:confirmed] ? true : false
     action = if confirmed
-               @registrant_verification.domain_registrant_delete_reject!("email link #{initiator}")
-             else
                @registrant_verification.domain_registrant_delete_confirm!("email link #{initiator}")
+             else
+               @registrant_verification.domain_registrant_delete_reject!("email link #{initiator}")
              end
 
     fail_msg = t("registrant_domain_delete_#{confirmed ? 'confirmed' : 'rejected'}_failed".to_sym)
@@ -36,9 +36,9 @@ class Registrant::DomainDeleteConfirmsController < RegistrantController
     (render 'show' && return) unless action
 
     if confirmed
-      redirect_to registrant_domain_delete_confirm_path(@domain.id, confirmed: true) && return
+      redirect_to registrant_domain_delete_confirm_path(@domain.id, confirmed: true)
     else
-      redirect_to registrant_domain_delete_confirm_path(@domain.id, rejected: true) unless confirmed
+      redirect_to registrant_domain_delete_confirm_path(@domain.id, rejected: true)
     end
   end
 end

app/models/concerns/domain/transferable.rb

@@ -57,7 +57,8 @@ module Concerns::Domain::Transferable
 
   def transfer_domain_contacts(new_registrar)
     copied_ids = []
-    contacts.each do |contact|
+    domain_contacts.each do |dc|
+      contact = Contact.find(dc.contact_id)
       next if copied_ids.include?(contact.id) || contact.registrar == new_registrar
 
       if registrant_id_was == contact.id # registrant was copied previously, do not copy it again
@@ -66,7 +67,11 @@ module Concerns::Domain::Transferable
         oc = contact.transfer(new_registrar)
       end
 
-      domain_contacts.where(contact_id: contact.id).update_all({ contact_id: oc.id }) # n+1 workaround
+      if domain_contacts.find_by(contact_id: oc.id, domain_id: id, type: dc.type).present?
+        dc.destroy
+      else
+        dc.update(contact_id: oc.id)
+      end
       copied_ids << contact.id
     end
   end

app/models/concerns/invoice/book_keeping.rb

@@ -5,7 +5,7 @@ module Concerns
 
       def as_directo_json
         invoice = ActiveSupport::JSON.decode(ActiveSupport::JSON.encode(self))
-        invoice['customer_code'] = buyer.accounting_customer_code
+        invoice['customer'] = compose_directo_customer
         invoice['issue_date'] = issue_date.strftime('%Y-%m-%d')
         invoice['transaction_date'] = account_activity
                                       .bank_transaction&.paid_at&.strftime('%Y-%m-%d')
@@ -21,6 +21,14 @@ module Concerns
              subtotal, precision: 2, separator: '.'
            ) }].as_json
       end
+
+      def compose_directo_customer
+        {
+          'code': buyer.accounting_customer_code,
+          'destination': buyer_country_code,
+          'vat_reg_no': buyer_vat_no,
+        }.as_json
+      end
     end
   end
 end

app/models/concerns/registrar/book_keeping.rb

@@ -12,7 +12,7 @@ module Concerns
 
         invoice = {
           'number': 1,
-          'customer_code': accounting_customer_code,
+          'customer': compose_directo_customer,
           'language': language == 'en' ? 'ENG' : '', 'currency': activities.first.currency,
           'date': month.end_of_month.strftime('%Y-%m-%d')
         }.as_json
@@ -109,6 +109,14 @@ module Concerns
         }
       end
 
+      def compose_directo_customer
+        {
+          'code': accounting_customer_code,
+          'destination': address_country_code,
+          'vat_reg_no': vat_no,
+        }.as_json
+      end
+
       def load_price(account_activity)
         @pricelists ||= {}
         return @pricelists[account_activity.price_id] if @pricelists.key? account_activity.price_id

config/application.yml.sample

@@ -166,6 +166,7 @@ test:
   lhv_keystore: 'test/fixtures/files/keystore.jks'
   lhv_keystore_password: 'testtest'
   lhv_keystore_alias: 'testtest'
+  legal_documents_dir: 'test/fixtures/files'
 
 # Airbrake // Errbit:
 airbrake_host: "https://your-errbit-host.ee"

test/jobs/directo_invoice_forward_job_test.rb

@@ -14,6 +14,15 @@ class DirectoInvoiceForwardJobTest < ActiveSupport::TestCase
     Setting.directo_monthly_number_last = 309901
   end
 
+  def test_directo_json_sends_customer_as_hash
+    @invoice.update!(buyer_country_code: @user.address_country_code)
+
+    json_output = @invoice.as_directo_json
+    assert json_output['customer'].is_a? Hash
+    assert_equal @user.accounting_customer_code, json_output['customer']['code']
+    assert_equal @user.address_country_code, json_output['customer']['destination']
+  end
+
   def test_xml_is_include_transaction_date
     @invoice.update(total: @invoice.account_activity.bank_transaction.sum)
     @invoice.account_activity.bank_transaction.update(paid_at: Time.zone.now)

test/jobs/domain_delete_confirm_job_test.rb

@@ -1,17 +1,11 @@
 require "test_helper"
 
 class DomainDeleteConfirmJobTest < ActiveSupport::TestCase
-  def setup
+  setup do
-    super
+    @legal_doc_path = 'test/fixtures/files/legaldoc.pdf'
-
     @domain = domains(:shop)
     @new_registrant = contacts(:william)
     @user = users(:api_bestnames)
-
-    @domain.update!(pending_json: { new_registrant_id: @new_registrant.id,
-                                    new_registrant_name: @new_registrant.name,
-                                    new_registrant_email: @new_registrant.email,
-                                    current_user_id: @user.id })
   end
 
   def teardown
@@ -19,6 +13,11 @@ class DomainDeleteConfirmJobTest < ActiveSupport::TestCase
   end
 
   def test_rejected_registrant_verification_notifies_registrar
+    @domain.update!(pending_json: { new_registrant_id: @new_registrant.id,
+      new_registrant_name: @new_registrant.name,
+      new_registrant_email: @new_registrant.email,
+      current_user_id: @user.id })
+
     DomainDeleteConfirmJob.enqueue(@domain.id, RegistrantVerification::REJECTED)
 
     last_registrar_notification = @domain.registrar.notifications.last
@@ -27,10 +26,57 @@ class DomainDeleteConfirmJobTest < ActiveSupport::TestCase
   end
 
   def test_accepted_registrant_verification_notifies_registrar
+    @domain.update!(pending_json: { new_registrant_id: @new_registrant.id,
+      new_registrant_name: @new_registrant.name,
+      new_registrant_email: @new_registrant.email,
+      current_user_id: @user.id })
+
     DomainDeleteConfirmJob.enqueue(@domain.id, RegistrantVerification::CONFIRMED)
 
     last_registrar_notification = @domain.registrar.notifications.last
     assert_equal(last_registrar_notification.attached_obj_id, @domain.id)
     assert_equal(last_registrar_notification.text, 'Registrant confirmed domain deletion: shop.test')
   end
+
+  def test_marks_domain_as_pending_delete_after_acceptance
+    epp_xml = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>\n<epp>\n  <command>\n    <delete>\n" \
+    "      <delete verified=\"no\">\n        <name>#{@domain.name}</name>\n      </delete>\n    </delete>\n    <extension>\n" \
+    "      <extdata>\n        <legalDocument type=\"pdf\">#{@legal_doc_path}</legalDocument>\n" \
+    "      </extdata>\n    </extension>\n    <clTRID>20alla-1594212240</clTRID>\n  </command>\n</epp>\n"
+
+    @domain.registrant_verification_asked!(epp_xml, @user.id)
+    @domain.pending_delete!
+    @domain.reload
+
+    assert @domain.registrant_delete_confirmable?(@domain.registrant_verification_token)
+    assert_equal @user.id, @domain.pending_json['current_user_id']
+
+    DomainDeleteConfirmJob.enqueue(@domain.id, RegistrantVerification::CONFIRMED)
+    @domain.reload
+
+    assert @domain.statuses.include? DomainStatus::PENDING_DELETE
+    assert @domain.statuses.include? DomainStatus::SERVER_HOLD
+    assert_not @domain.statuses.include? DomainStatus::PENDING_DELETE_CONFIRMATION
+  end
+
+  def test_clears_pending_flags_after_delete_denial
+    epp_xml = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>\n<epp>\n  <command>\n    <delete>\n" \
+    "      <delete verified=\"no\">\n        <name>#{@domain.name}</name>\n      </delete>\n    </delete>\n    <extension>\n" \
+    "      <extdata>\n        <legalDocument type=\"pdf\">#{@legal_doc_path}</legalDocument>\n" \
+    "      </extdata>\n    </extension>\n    <clTRID>20alla-1594212240</clTRID>\n  </command>\n</epp>\n"
+
+    @domain.registrant_verification_asked!(epp_xml, @user.id)
+    @domain.pending_delete!
+    @domain.reload
+
+    assert @domain.registrant_delete_confirmable?(@domain.registrant_verification_token)
+    assert_equal @user.id, @domain.pending_json['current_user_id']
+
+    DomainDeleteConfirmJob.enqueue(@domain.id, RegistrantVerification::REJECTED)
+    @domain.reload
+
+    assert_equal ['ok'], @domain.statuses
+    assert_not @domain.statuses.include? DomainStatus::PENDING_DELETE_CONFIRMATION
+    assert_not @domain.statuses.include? DomainStatus::PENDING_DELETE
+  end
 end

test/jobs/domain_update_confirm_job_test.rb

@@ -7,6 +7,7 @@ class DomainUpdateConfirmJobTest < ActiveSupport::TestCase
     @domain = domains(:shop)
     @new_registrant = contacts(:william)
     @user = users(:api_bestnames)
+    @legal_doc_path = 'test/fixtures/files/legaldoc.pdf'
 
     @domain.update!(pending_json: { new_registrant_id: @new_registrant.id,
                                     new_registrant_name: @new_registrant.name,
@@ -33,4 +34,34 @@ class DomainUpdateConfirmJobTest < ActiveSupport::TestCase
     assert_equal(last_registrar_notification.attached_obj_id, @domain.id)
     assert_equal(last_registrar_notification.text, 'Registrant confirmed domain update: shop.test')
   end
+
+  def test_changes_domain_registrant_after_approval
+    epp_xml = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>\n<epp>\n  <command>\n    <update>\n      <update>\n        <name>#{@domain.name}</name>\n" \
+    "        <chg>\n          <registrant>#{@new_registrant.code}</registrant>\n        </chg>\n      </update>\n    </update>\n    <extension>\n      <update/>\n" \
+    "      <extdata>\n        <legalDocument type=\"pdf\">#{@legal_doc_path}</legalDocument>\n      </extdata>\n" \
+    "    </extension>\n    <clTRID>20alla-1594199756</clTRID>\n  </command>\n</epp>\n"
+    @domain.pending_json['frame'] = epp_xml
+    @domain.update(pending_json: @domain.pending_json)
+
+    @domain.reload
+    DomainUpdateConfirmJob.enqueue(@domain.id, RegistrantVerification::CONFIRMED)
+    @domain.reload
+
+    assert_equal @domain.registrant.code, @new_registrant.code
+  end
+
+  def test_clears_pending_update_after_denial
+    epp_xml = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>\n<epp>\n  <command>\n    <update>\n      <update>\n        <name>#{@domain.name}</name>\n" \
+    "        <chg>\n          <registrant>#{@new_registrant.code}</registrant>\n        </chg>\n      </update>\n    </update>\n    <extension>\n      <update/>\n" \
+    "      <extdata>\n        <legalDocument type=\"pdf\">#{@legal_doc_path}</legalDocument>\n      </extdata>\n" \
+    "    </extension>\n    <clTRID>20alla-1594199756</clTRID>\n  </command>\n</epp>\n"
+    @domain.pending_json['frame'] = epp_xml
+    @domain.update(pending_json: @domain.pending_json)
+
+    DomainUpdateConfirmJob.enqueue(@domain.id, RegistrantVerification::REJECTED)
+    @domain.reload
+
+    assert_not @domain.statuses.include? DomainStatus::PENDING_DELETE_CONFIRMATION
+    assert_not @domain.statuses.include? DomainStatus::PENDING_DELETE
+  end
 end

test/system/registrant_area/domains/domain_delete_confirms_test.rb

@@ -0,0 +1,32 @@
+require 'application_system_test_case'
+
+class DomainDeleteConfirmsTest < ApplicationSystemTestCase
+  setup do
+    @user = users(:registrant)
+    sign_in @user
+
+    @domain = domains(:shop)
+    @domain.registrant_verification_asked!('<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>\n<epp></epp>', @user.id)
+    @domain.pending_delete!
+  end
+
+  def test_enqueues_approve_job_after_verification
+    visit registrant_domain_delete_confirm_url(@domain.id, token: @domain.registrant_verification_token)
+
+    click_on 'Confirm domain delete'
+    assert_text 'Domain registrant change has successfully received.'
+
+    @domain.reload
+    assert_includes @domain.statuses, 'serverHold'
+  end
+
+  def test_enqueues_reject_job_after_verification
+    visit registrant_domain_delete_confirm_url(@domain.id, token: @domain.registrant_verification_token)
+
+    click_on 'Reject domain delete'
+    assert_text 'Domain registrant change has been rejected successfully.'
+
+    @domain.reload
+    assert_equal ['ok'], @domain.statuses
+  end
+end