From b5f07682f43b8eaa61a8825eab73a013cbbfaffe Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Thu, 5 Jul 2018 16:15:32 +0300 Subject: [PATCH 1/4] Make replace_nameservers return an array of domain names --- app/models/registrar.rb | 6 +++++ .../registrar/replace_nameservers_test.rb | 23 +++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 test/models/registrar/replace_nameservers_test.rb diff --git a/app/models/registrar.rb b/app/models/registrar.rb index 05a5ad380..54e007133 100644 --- a/app/models/registrar.rb +++ b/app/models/registrar.rb @@ -132,14 +132,20 @@ class Registrar < ActiveRecord::Base # Audit log is needed, therefore no raw SQL def replace_nameservers(hostname, new_attributes) transaction do + domain_list = [] + nameservers.where(hostname: hostname).find_each do |original_nameserver| new_nameserver = Nameserver.new new_nameserver.domain = original_nameserver.domain new_nameserver.attributes = new_attributes new_nameserver.save! + domain_list << original_nameserver.domain.name + original_nameserver.destroy! end + + domain_list end end diff --git a/test/models/registrar/replace_nameservers_test.rb b/test/models/registrar/replace_nameservers_test.rb new file mode 100644 index 000000000..c247b2860 --- /dev/null +++ b/test/models/registrar/replace_nameservers_test.rb @@ -0,0 +1,23 @@ +require 'test_helper' + +class ReplaceNameserversTest < ActiveSupport::TestCase + def setup + @registrar = registrars(:bestnames) + end + + def test_replace_nameservers_in_bulk_returns_domain_names + new_attributes = { hostname: 'ns-updated1.bestnames.test', ipv4: '192.0.3.1', + ipv6: '2001:db8::2' } + result = @registrar.replace_nameservers('ns1.bestnames.test', new_attributes) + + assert_equal(["airport.test", "shop.test"], result) + end + + def test_replace_nameservers_in_bulk_returns_empty_array_for_non_existent_base_nameserver + new_attributes = { hostname: 'ns-updated1.bestnames.test', ipv4: '192.0.3.1', + ipv6: '2001:db8::2' } + result = @registrar.replace_nameservers('ns3.bestnames.test', new_attributes) + + assert_equal([], result) + end +end From 3e81366cfc80ea8f33dbb8805eaeb0bbe2130078 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Thu, 5 Jul 2018 17:24:32 +0300 Subject: [PATCH 2/4] Add affected domain parameter to REST API response --- app/api/repp/nameservers_v1.rb | 5 +++-- app/models/registrar.rb | 2 +- doc/repp/v1/nameservers.md | 1 + test/integration/api/nameservers/put_test.rb | 6 ++++-- test/integration/registrar/bulk_change/nameserver_test.rb | 3 ++- test/models/registrar/replace_nameservers_test.rb | 2 +- 6 files changed, 12 insertions(+), 7 deletions(-) diff --git a/app/api/repp/nameservers_v1.rb b/app/api/repp/nameservers_v1.rb index d653adf7f..71fde5aba 100644 --- a/app/api/repp/nameservers_v1.rb +++ b/app/api/repp/nameservers_v1.rb @@ -29,7 +29,7 @@ module Repp } begin - current_user.registrar.replace_nameservers(hostname, new_attributes) + affected_domains = current_user.registrar.replace_nameservers(hostname, new_attributes) rescue ActiveRecord::RecordInvalid => e error!({ errors: e.record.errors.full_messages.map { |error| { title: error } } }, 400) end @@ -37,7 +37,8 @@ module Repp status 200 @response = { data: { type: 'nameserver', id: params[:data][:attributes][:hostname], - attributes: params[:data][:attributes] } } + attributes: params[:data][:attributes], + affected_domains: affected_domains } } end end end diff --git a/app/models/registrar.rb b/app/models/registrar.rb index 54e007133..de02d42a7 100644 --- a/app/models/registrar.rb +++ b/app/models/registrar.rb @@ -145,7 +145,7 @@ class Registrar < ActiveRecord::Base original_nameserver.destroy! end - domain_list + domain_list.uniq.sort end end diff --git a/doc/repp/v1/nameservers.md b/doc/repp/v1/nameservers.md index 702a0c186..c1269f92a 100644 --- a/doc/repp/v1/nameservers.md +++ b/doc/repp/v1/nameservers.md @@ -35,6 +35,7 @@ Content-Type: application/json "ipv4": ["192.0.2.1", "192.0.2.2"], "ipv6": ["2001:db8::1", "2001:db8::2"] }, + "affected_domains": ["example.com", "example.org"] } } ``` diff --git a/test/integration/api/nameservers/put_test.rb b/test/integration/api/nameservers/put_test.rb index 416510541..e840cfcfc 100644 --- a/test/integration/api/nameservers/put_test.rb +++ b/test/integration/api/nameservers/put_test.rb @@ -41,11 +41,12 @@ class APINameserversPutTest < ActionDispatch::IntegrationTest assert_equal nameserver_hash, nameservers(:metro_ns1).reload.attributes end - def test_returns_new_nameserver_record + def test_returns_new_nameserver_record_and_affected_domain request_params = { format: :json, data: { type: 'nameserver', id: 'ns1.bestnames.test', attributes: { hostname: 'ns55.bestnames.test', ipv4: ['192.0.2.55'], ipv6: ['2001:db8::55'] } } } + put '/repp/v1/registrar/nameservers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } assert_response 200 @@ -53,7 +54,8 @@ class APINameserversPutTest < ActionDispatch::IntegrationTest id: 'ns55.bestnames.test', attributes: { hostname: 'ns55.bestnames.test', ipv4: ['192.0.2.55'], - ipv6: ['2001:db8::55'] } } }), + ipv6: ['2001:db8::55'] }, + affected_domains: ["airport.test", "shop.test"] } }), JSON.parse(response.body, symbolize_names: true) end diff --git a/test/integration/registrar/bulk_change/nameserver_test.rb b/test/integration/registrar/bulk_change/nameserver_test.rb index 20fd6617d..d1dbfc012 100644 --- a/test/integration/registrar/bulk_change/nameserver_test.rb +++ b/test/integration/registrar/bulk_change/nameserver_test.rb @@ -16,7 +16,8 @@ class RegistrarAreaNameserverBulkChangeTest < ActionDispatch::IntegrationTest basic_auth: ['test_goodnames', 'testtest']) .to_return(body: { data: [{ type: 'nameserver', - id: 'new-ns.bestnames.test' + id: 'new-ns.bestnames.test', + affected_domains: ["airport.test", "shop.test"] }] }.to_json, status: 200) visit registrar_domains_url diff --git a/test/models/registrar/replace_nameservers_test.rb b/test/models/registrar/replace_nameservers_test.rb index c247b2860..5bcbb83d1 100644 --- a/test/models/registrar/replace_nameservers_test.rb +++ b/test/models/registrar/replace_nameservers_test.rb @@ -5,7 +5,7 @@ class ReplaceNameserversTest < ActiveSupport::TestCase @registrar = registrars(:bestnames) end - def test_replace_nameservers_in_bulk_returns_domain_names + def test_replace_nameservers_in_bulk_returns_sorted_domain_names new_attributes = { hostname: 'ns-updated1.bestnames.test', ipv4: '192.0.3.1', ipv6: '2001:db8::2' } result = @registrar.replace_nameservers('ns1.bestnames.test', new_attributes) From 6fb1e44ccaac9fa872b01183bf41b4898c637dd0 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Thu, 5 Jul 2018 17:53:03 +0300 Subject: [PATCH 3/4] Move affected domains to the top of the hash, update integration test --- app/api/repp/nameservers_v1.rb | 4 ++-- app/controllers/registrar/nameservers_controller.rb | 5 ++++- config/locales/registrar/nameservers.en.yml | 1 + doc/repp/v1/nameservers.md | 6 +++--- test/integration/api/nameservers/put_test.rb | 4 ++-- test/integration/registrar/bulk_change/nameserver_test.rb | 6 +++--- 6 files changed, 15 insertions(+), 11 deletions(-) diff --git a/app/api/repp/nameservers_v1.rb b/app/api/repp/nameservers_v1.rb index 71fde5aba..41a735b09 100644 --- a/app/api/repp/nameservers_v1.rb +++ b/app/api/repp/nameservers_v1.rb @@ -37,8 +37,8 @@ module Repp status 200 @response = { data: { type: 'nameserver', id: params[:data][:attributes][:hostname], - attributes: params[:data][:attributes], - affected_domains: affected_domains } } + attributes: params[:data][:attributes]}, + affected_domains: affected_domains } end end end diff --git a/app/controllers/registrar/nameservers_controller.rb b/app/controllers/registrar/nameservers_controller.rb index b6f7af829..3b70059a2 100644 --- a/app/controllers/registrar/nameservers_controller.rb +++ b/app/controllers/registrar/nameservers_controller.rb @@ -44,7 +44,10 @@ class Registrar parsed_response = JSON.parse(response.body, symbolize_names: true) if response.code == '200' - flash[:notice] = t '.replaced' + notices = [t('.replaced')] + notices << "#{t('.affected_domains')}: #{parsed_response[:affected_domains].join(', ')}" + + flash[:notice] = notices redirect_to registrar_domains_url else @api_errors = parsed_response[:errors] diff --git a/config/locales/registrar/nameservers.en.yml b/config/locales/registrar/nameservers.en.yml index fbe4c387e..6cc08f0ab 100644 --- a/config/locales/registrar/nameservers.en.yml +++ b/config/locales/registrar/nameservers.en.yml @@ -3,3 +3,4 @@ en: nameservers: update: replaced: Nameserver have been successfully replaced + affected_domains: Affected domains diff --git a/doc/repp/v1/nameservers.md b/doc/repp/v1/nameservers.md index c1269f92a..8190530d7 100644 --- a/doc/repp/v1/nameservers.md +++ b/doc/repp/v1/nameservers.md @@ -34,9 +34,9 @@ Content-Type: application/json "hostname": "new-ns1.example.com", "ipv4": ["192.0.2.1", "192.0.2.2"], "ipv6": ["2001:db8::1", "2001:db8::2"] - }, - "affected_domains": ["example.com", "example.org"] - } + } + }, + "affected_domains": ["example.com", "example.org"] } ``` diff --git a/test/integration/api/nameservers/put_test.rb b/test/integration/api/nameservers/put_test.rb index e840cfcfc..0967a1169 100644 --- a/test/integration/api/nameservers/put_test.rb +++ b/test/integration/api/nameservers/put_test.rb @@ -54,8 +54,8 @@ class APINameserversPutTest < ActionDispatch::IntegrationTest id: 'ns55.bestnames.test', attributes: { hostname: 'ns55.bestnames.test', ipv4: ['192.0.2.55'], - ipv6: ['2001:db8::55'] }, - affected_domains: ["airport.test", "shop.test"] } }), + ipv6: ['2001:db8::55'] }}, + affected_domains: ["airport.test", "shop.test"] }), JSON.parse(response.body, symbolize_names: true) end diff --git a/test/integration/registrar/bulk_change/nameserver_test.rb b/test/integration/registrar/bulk_change/nameserver_test.rb index d1dbfc012..841e68db5 100644 --- a/test/integration/registrar/bulk_change/nameserver_test.rb +++ b/test/integration/registrar/bulk_change/nameserver_test.rb @@ -16,9 +16,8 @@ class RegistrarAreaNameserverBulkChangeTest < ActionDispatch::IntegrationTest basic_auth: ['test_goodnames', 'testtest']) .to_return(body: { data: [{ type: 'nameserver', - id: 'new-ns.bestnames.test', - affected_domains: ["airport.test", "shop.test"] - }] }.to_json, status: 200) + id: 'new-ns.bestnames.test'}], + affected_domains: ["airport.test", "shop.test"]}.to_json, status: 200) visit registrar_domains_url click_link 'Bulk change' @@ -33,6 +32,7 @@ class RegistrarAreaNameserverBulkChangeTest < ActionDispatch::IntegrationTest assert_requested request_stub assert_current_path registrar_domains_path assert_text 'Nameserver have been successfully replaced' + assert_text 'Affected domains: airport.test, shop.test' end def test_fails_gracefully From 4d5dd1dae795b2da9b53f094defd07963e6041ae Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Thu, 5 Jul 2018 17:58:22 +0300 Subject: [PATCH 4/4] Fix rubocop issues --- app/api/repp/nameservers_v1.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/api/repp/nameservers_v1.rb b/app/api/repp/nameservers_v1.rb index 41a735b09..04d7d4f6a 100644 --- a/app/api/repp/nameservers_v1.rb +++ b/app/api/repp/nameservers_v1.rb @@ -37,7 +37,7 @@ module Repp status 200 @response = { data: { type: 'nameserver', id: params[:data][:attributes][:hostname], - attributes: params[:data][:attributes]}, + attributes: params[:data][:attributes] }, affected_domains: affected_domains } end end