From 2e9acedc901d948047ab9208c4f4e4a80cc17c00 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Sat, 7 Apr 2018 15:18:10 +0300 Subject: [PATCH 01/30] Improve test name #790 --- test/integration/epp/domain/transfer/request_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/epp/domain/transfer/request_test.rb b/test/integration/epp/domain/transfer/request_test.rb index f3ea644fc..996296257 100644 --- a/test/integration/epp/domain/transfer/request_test.rb +++ b/test/integration/epp/domain/transfer/request_test.rb @@ -75,7 +75,7 @@ class EppDomainTransferRequestTest < ActionDispatch::IntegrationTest assert_equal '2304', Nokogiri::XML(response.body).at_css('result')[:code] end - def test_discarded_domain + def test_discarded_domain_cannot_be_transferred @domain.update!(statuses: [DomainStatus::DELETE_CANDIDATE]) post '/epp/command/transfer', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=api_goodnames' } From 00a30fc019a058e5cbaa6db7190b6afdc18359fa Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Sat, 7 Apr 2018 16:21:07 +0300 Subject: [PATCH 02/30] Remove unneeded attribute alias #790 --- app/models/concerns/domain/deletable.rb | 4 ---- app/models/domain.rb | 2 +- app/presenters/domain_presenter.rb | 2 +- spec/models/domain_spec.rb | 6 +++--- spec/presenters/domain_presenter_spec.rb | 4 ++-- 5 files changed, 7 insertions(+), 11 deletions(-) diff --git a/app/models/concerns/domain/deletable.rb b/app/models/concerns/domain/deletable.rb index 86c296d88..204d924ba 100644 --- a/app/models/concerns/domain/deletable.rb +++ b/app/models/concerns/domain/deletable.rb @@ -1,10 +1,6 @@ module Concerns::Domain::Deletable extend ActiveSupport::Concern - included do - alias_attribute :delete_time, :delete_at - end - def discard statuses << DomainStatus::DELETE_CANDIDATE save diff --git a/app/models/domain.rb b/app/models/domain.rb index 9eef0f30f..a9fec134e 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -684,7 +684,7 @@ class Domain < ActiveRecord::Base end def self.delete_candidates - where("#{attribute_alias(:delete_time)} < ?", Time.zone.now) + where('delete_at < ?', Time.zone.now) end def self.uses_zone?(zone) diff --git a/app/presenters/domain_presenter.rb b/app/presenters/domain_presenter.rb index aac3c1527..2ecb82922 100644 --- a/app/presenters/domain_presenter.rb +++ b/app/presenters/domain_presenter.rb @@ -30,7 +30,7 @@ class DomainPresenter end def delete_date - view.l(domain.delete_time, format: :date) if domain.delete_time + view.l(domain.delete_at, format: :date) if domain.delete_at end def force_delete_date diff --git a/spec/models/domain_spec.rb b/spec/models/domain_spec.rb index cc1fe52a2..6c09ef7ed 100644 --- a/spec/models/domain_spec.rb +++ b/spec/models/domain_spec.rb @@ -877,9 +877,9 @@ RSpec.describe Domain do create(:zone, origin: 'ee') - create(:domain, id: 1, delete_time: Time.zone.parse('04.07.2010 23:59')) - create(:domain, id: 2, delete_time: Time.zone.parse('05.07.2010 00:00')) - create(:domain, id: 3, delete_time: Time.zone.parse('05.07.2010 00:01')) + create(:domain, id: 1, delete_at: Time.zone.parse('04.07.2010 23:59')) + create(:domain, id: 2, delete_at: Time.zone.parse('05.07.2010 00:00')) + create(:domain, id: 3, delete_at: Time.zone.parse('05.07.2010 00:01')) end it 'returns domains with delete time in the past' do diff --git a/spec/presenters/domain_presenter_spec.rb b/spec/presenters/domain_presenter_spec.rb index 3726960b4..d0cef5e6d 100644 --- a/spec/presenters/domain_presenter_spec.rb +++ b/spec/presenters/domain_presenter_spec.rb @@ -44,7 +44,7 @@ RSpec.describe DomainPresenter do subject(:delete_date) { presenter.delete_date } context 'when present' do - let(:domain) { instance_double(Domain, delete_time: '05.07.2010') } + let(:domain) { instance_double(Domain, delete_at: '05.07.2010') } it 'returns localized date' do expect(view).to receive(:l).with('05.07.2010', format: :date).and_return('delete date') @@ -53,7 +53,7 @@ RSpec.describe DomainPresenter do end context 'when absent' do - let(:domain) { instance_double(Domain, delete_time: nil) } + let(:domain) { instance_double(Domain, delete_at: nil) } specify { expect(delete_date).to be_nil } end From e776d09f9d99e4cc33ca8cd7666646510e0a0e67 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Sun, 8 Apr 2018 00:36:00 +0300 Subject: [PATCH 03/30] Improve domain discard - Extract rake task domain:discard - Remove background job when keeping a domain #790 --- app/models/concerns/domain/deletable.rb | 31 +++++++++++++ app/models/domain.rb | 11 ----- app/models/domain_cron.rb | 16 ------- app/models/que_job.rb | 3 ++ config/schedule.rb | 4 ++ lib/tasks/domain.rake | 5 +++ spec/models/domain_spec.rb | 16 ------- .../epp/domain/transfer/request_test.rb | 2 +- test/integration/tasks/discard_domain_test.rb | 43 +++++++++++++++++++ test/models/domain/deletable_test.rb | 19 +++++++- 10 files changed, 104 insertions(+), 46 deletions(-) create mode 100644 app/models/que_job.rb create mode 100644 lib/tasks/domain.rake create mode 100644 test/integration/tasks/discard_domain_test.rb diff --git a/app/models/concerns/domain/deletable.rb b/app/models/concerns/domain/deletable.rb index 204d924ba..6075ef8f7 100644 --- a/app/models/concerns/domain/deletable.rb +++ b/app/models/concerns/domain/deletable.rb @@ -1,12 +1,43 @@ module Concerns::Domain::Deletable extend ActiveSupport::Concern + class_methods do + def discard_domains + domains = where('delete_at < ? AND ? != ALL(statuses) AND ? != ALL(statuses)', + Time.zone.now, + DomainStatus::SERVER_DELETE_PROHIBITED, + DomainStatus::DELETE_CANDIDATE) + + domains.map(&:discard) + end + end + def discard statuses << DomainStatus::DELETE_CANDIDATE + # We don't validate deliberately since nobody is interested in fixing discarded domain + save(validate: false) + delete_later + logger.info "Domain #{name} (ID: #{id}) is scheduled to be deleted" + end + + def keep + statuses.delete(DomainStatus::DELETE_CANDIDATE) save + do_not_delete_later end def discarded? statuses.include?(DomainStatus::DELETE_CANDIDATE) end + + private + + def delete_later + run_at = rand(((24 * 60) - (DateTime.now.hour * 60 + DateTime.now.minute))).minutes.from_now + DomainDeleteJob.enqueue(id, run_at: run_at) + end + + def do_not_delete_later + QueJob.find_by!("args->>0 = '#{id}'").delete + end end diff --git a/app/models/domain.rb b/app/models/domain.rb index a9fec134e..1425fc1e1 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -281,13 +281,6 @@ class Domain < ActiveRecord::Base true end - def delete_candidateable? - return false if delete_at > Time.zone.now - return false if statuses.include?(DomainStatus::DELETE_CANDIDATE) - return false if statuses.include?(DomainStatus::SERVER_DELETE_PROHIBITED) - true - end - def renewable? if Setting.days_to_renew_domain_before_expire != 0 # if you can renew domain at days_to_renew before domain expiration @@ -683,10 +676,6 @@ class Domain < ActiveRecord::Base where("#{attribute_alias(:outzone_time)} < ?", Time.zone.now) end - def self.delete_candidates - where('delete_at < ?', Time.zone.now) - end - def self.uses_zone?(zone) exists?(["name ILIKE ?", "%.#{zone.origin}"]) end diff --git a/app/models/domain_cron.rb b/app/models/domain_cron.rb index dcd99328a..6ca502b00 100644 --- a/app/models/domain_cron.rb +++ b/app/models/domain_cron.rb @@ -104,22 +104,6 @@ class DomainCron c = 0 - domains = Domain.delete_candidates - - domains.each do |domain| - next unless domain.delete_candidateable? - - domain.statuses << DomainStatus::DELETE_CANDIDATE - - # If domain successfully saved, add it to delete schedule - if domain.save(validate: false) - ::PaperTrail.whodunnit = "cron - #{__method__}" - DomainDeleteJob.enqueue(domain.id, run_at: rand(((24*60) - (DateTime.now.hour * 60 + DateTime.now.minute))).minutes.from_now) - STDOUT << "#{Time.zone.now.utc} Domain.destroy_delete_candidates: job added by deleteCandidate status ##{domain.id} (#{domain.name})\n" unless Rails.env.test? - c += 1 - end - end - Domain.where('force_delete_at <= ?', Time.zone.now.end_of_day.utc).each do |x| DomainDeleteJob.enqueue(x.id, run_at: rand(((24*60) - (DateTime.now.hour * 60 + DateTime.now.minute))).minutes.from_now) STDOUT << "#{Time.zone.now.utc} DomainCron.destroy_delete_candidates: job added by force delete time ##{x.id} (#{x.name})\n" unless Rails.env.test? diff --git a/app/models/que_job.rb b/app/models/que_job.rb new file mode 100644 index 000000000..d08e65d10 --- /dev/null +++ b/app/models/que_job.rb @@ -0,0 +1,3 @@ +class QueJob < ActiveRecord::Base # To be able to remove existing jobs + self.primary_key = 'job_id' +end diff --git a/config/schedule.rb b/config/schedule.rb index a136ba5c3..dd5741dd0 100644 --- a/config/schedule.rb +++ b/config/schedule.rb @@ -61,6 +61,10 @@ if @cron_group == 'registry' every :day, at: '19:00pm' do runner 'Directo.send_receipts' end if @environment == 'production' + + every 42.minutes do + rake 'domain:discard' + end end every 10.minutes do diff --git a/lib/tasks/domain.rake b/lib/tasks/domain.rake new file mode 100644 index 000000000..f59a76141 --- /dev/null +++ b/lib/tasks/domain.rake @@ -0,0 +1,5 @@ +namespace :domain do + task discard: :environment do + Domain.discard_domains + end +end diff --git a/spec/models/domain_spec.rb b/spec/models/domain_spec.rb index 6c09ef7ed..4f1d5d6a1 100644 --- a/spec/models/domain_spec.rb +++ b/spec/models/domain_spec.rb @@ -871,22 +871,6 @@ RSpec.describe Domain do end end - describe '::delete_candidates', db: true do - before :example do - travel_to Time.zone.parse('05.07.2010 00:00') - - create(:zone, origin: 'ee') - - create(:domain, id: 1, delete_at: Time.zone.parse('04.07.2010 23:59')) - create(:domain, id: 2, delete_at: Time.zone.parse('05.07.2010 00:00')) - create(:domain, id: 3, delete_at: Time.zone.parse('05.07.2010 00:01')) - end - - it 'returns domains with delete time in the past' do - expect(described_class.delete_candidates.ids).to eq([1]) - end - end - describe '::uses_zone?', db: true do let!(:zone) { create(:zone, origin: 'domain.tld') } diff --git a/test/integration/epp/domain/transfer/request_test.rb b/test/integration/epp/domain/transfer/request_test.rb index 996296257..aef189104 100644 --- a/test/integration/epp/domain/transfer/request_test.rb +++ b/test/integration/epp/domain/transfer/request_test.rb @@ -76,7 +76,7 @@ class EppDomainTransferRequestTest < ActionDispatch::IntegrationTest end def test_discarded_domain_cannot_be_transferred - @domain.update!(statuses: [DomainStatus::DELETE_CANDIDATE]) + @domain.discard post '/epp/command/transfer', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=api_goodnames' } @domain.reload diff --git a/test/integration/tasks/discard_domain_test.rb b/test/integration/tasks/discard_domain_test.rb new file mode 100644 index 000000000..0a341765b --- /dev/null +++ b/test/integration/tasks/discard_domain_test.rb @@ -0,0 +1,43 @@ +require 'test_helper' + +class DiscardDomainTaskTest < ActiveSupport::TestCase + setup do + travel_to Time.zone.parse('2010-07-05 08:00') + @domain = domains(:shop) + end + + def test_discard_domains_with_past_delete_at + @domain.update!(delete_at: Time.zone.parse('2010-07-05 07:59')) + Rake::Task['domain:discard'].execute + @domain.reload + assert @domain.discarded? + end + + def test_ignore_domains_with_delete_at_in_the_future_or_now + @domain.update!(delete_at: Time.zone.parse('2010-07-05 08:00')) + Rake::Task['domain:discard'].execute + @domain.reload + refute @domain.discarded? + end + + def test_ignore_already_discarded_domains + @domain.update!(delete_at: Time.zone.parse('2010-07-05 07:59')) + @domain.discard + + job_count = lambda do + QueJob.where("args->>0 = '#{@domain.id}'", job_class: DomainDeleteJob.name).count + end + + assert_no_difference job_count, 'A domain should not be discarded again' do + Rake::Task['domain:discard'].execute + end + end + + def test_ignore_domains_with_server_delete_prohibited_status + @domain.update!(delete_at: Time.zone.parse('2010-07-05 07:59'), + statuses: [DomainStatus::SERVER_DELETE_PROHIBITED]) + Rake::Task['domain:discard'].execute + @domain.reload + refute @domain.discarded? + end +end diff --git a/test/models/domain/deletable_test.rb b/test/models/domain/deletable_test.rb index dbec7bc7f..5e41968d7 100644 --- a/test/models/domain/deletable_test.rb +++ b/test/models/domain/deletable_test.rb @@ -5,10 +5,25 @@ class DomainDeletableTest < ActiveSupport::TestCase @domain = domains(:shop) end - def test_discard - refute @domain.discarded? + def test_discard_domain @domain.discard @domain.reload + assert QueJob.find_by("args->>0 = '#{@domain.id}'", job_class: DomainDeleteJob.name) assert @domain.discarded? end + + def test_discard_invalid_domain + domain = domains(:invalid) + domain.discard + domain.reload + assert domain.discarded?, 'a domain should be discarded' + end + + def test_keep_domain + @domain.discard + @domain.keep + @domain.reload + assert_nil QueJob.find_by("args->>0 = '#{@domain.id}'", job_class: DomainDeleteJob.name) + refute @domain.discarded? + end end From a932d43dda65cd89c4f0ac5e316795f18133d5fd Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Sun, 8 Apr 2018 15:45:01 +0300 Subject: [PATCH 04/30] Improve UI #790 --- app/views/admin/domains/edit.html.erb | 24 +++++++++++++----------- config/locales/admin/domains.en.yml | 1 - 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/app/views/admin/domains/edit.html.erb b/app/views/admin/domains/edit.html.erb index 3f831e86b..6593f431b 100644 --- a/app/views/admin/domains/edit.html.erb +++ b/app/views/admin/domains/edit.html.erb @@ -1,20 +1,22 @@ <% domain = DomainPresenter.new(domain: @domain, view: self) %> -
-
-

- Edit: <%= domain.name %> -

-
-
-

+ + +

+
-
<%= render 'form' %> <%= render 'force_delete_dialog', domain: @domain, templates: force_delete_templates %> diff --git a/config/locales/admin/domains.en.yml b/config/locales/admin/domains.en.yml index 87c2c9c6a..aa574e8a3 100644 --- a/config/locales/admin/domains.en.yml +++ b/config/locales/admin/domains.en.yml @@ -14,7 +14,6 @@ en: edit: add_new_status_btn: Add new status - back_btn: Back to domain force_delete_dialog: title: Force delete From 4dc77235542dece7e80ece28bd2808fc71430f5a Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Sun, 8 Apr 2018 15:53:29 +0300 Subject: [PATCH 05/30] Remove dead code #790 --- app/controllers/admin/domains_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/admin/domains_controller.rb b/app/controllers/admin/domains_controller.rb index 576b5f8e1..3cf4577fc 100644 --- a/app/controllers/admin/domains_controller.rb +++ b/app/controllers/admin/domains_controller.rb @@ -1,7 +1,7 @@ module Admin class DomainsController < BaseController load_and_authorize_resource - before_action :set_domain, only: [:show, :edit, :update, :zonefile] + before_action :set_domain, only: [:show, :edit, :update] helper_method :force_delete_templates # rubocop: disable Metrics/PerceivedComplexity From 9e46222e6bf5b2a3398f03b16b81a42124ebf0c9 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Sun, 8 Apr 2018 15:54:42 +0300 Subject: [PATCH 06/30] Remove unused routes #790 --- config/routes.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index 9caeef4a2..883cad6a8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -186,7 +186,7 @@ Rails.application.routes.draw do match 'forward', via: [:post, :get] end - resources :domains do + resources :domains, except: %i[new create destroy] do resources :domain_versions, controller: 'domains', action: 'versions' resources :pending_updates resources :pending_deletes From b4ed8f6c8200fbaa3f5112144a5478c2890e5fbe Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Sun, 8 Apr 2018 16:01:00 +0300 Subject: [PATCH 07/30] Remove double load #790 --- app/controllers/admin/domains_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/domains_controller.rb b/app/controllers/admin/domains_controller.rb index 3cf4577fc..f2e78aaf0 100644 --- a/app/controllers/admin/domains_controller.rb +++ b/app/controllers/admin/domains_controller.rb @@ -1,7 +1,7 @@ module Admin class DomainsController < BaseController - load_and_authorize_resource - before_action :set_domain, only: [:show, :edit, :update] + before_action :set_domain, only: %i[show edit update] + authorize_resource helper_method :force_delete_templates # rubocop: disable Metrics/PerceivedComplexity From 90a4e11ce4057ed92c13571d5e495b613bf53155 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Sun, 8 Apr 2018 16:07:08 +0300 Subject: [PATCH 08/30] Improve readability #790 --- app/controllers/admin/domains_controller.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/admin/domains_controller.rb b/app/controllers/admin/domains_controller.rb index f2e78aaf0..9237d61af 100644 --- a/app/controllers/admin/domains_controller.rb +++ b/app/controllers/admin/domains_controller.rb @@ -40,7 +40,8 @@ module Admin # rubocop: enable Metrics/AbcSize def show - @domain.valid? + # Validation is needed to warn users + @domain.validate end def edit From cc4a2448442854c782db296815e9483ef6656ae1 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Sun, 8 Apr 2018 18:52:32 +0300 Subject: [PATCH 09/30] Enable to keep a domain #790 --- app/controllers/admin/domains_controller.rb | 7 ++++++- app/models/domain_status.rb | 3 ++- app/presenters/domain_presenter.rb | 9 +++++++++ app/views/admin/domains/edit.html.erb | 5 +++-- config/locales/admin/domains.en.yml | 5 +++++ config/routes.rb | 4 ++++ test/integration/admin/domains/details_test.rb | 10 ++++++++++ 7 files changed, 39 insertions(+), 4 deletions(-) diff --git a/app/controllers/admin/domains_controller.rb b/app/controllers/admin/domains_controller.rb index 9237d61af..0718d600a 100644 --- a/app/controllers/admin/domains_controller.rb +++ b/app/controllers/admin/domains_controller.rb @@ -1,6 +1,6 @@ module Admin class DomainsController < BaseController - before_action :set_domain, only: %i[show edit update] + before_action :set_domain, only: %i[show edit update keep] authorize_resource helper_method :force_delete_templates @@ -68,6 +68,11 @@ module Admin @versions = @domain.versions end + def keep + @domain.keep + redirect_to edit_admin_domain_url(@domain), notice: t('.kept') + end + private def set_domain diff --git a/app/models/domain_status.rb b/app/models/domain_status.rb index 4da6a4a3c..3c4e09beb 100644 --- a/app/models/domain_status.rb +++ b/app/models/domain_status.rb @@ -175,7 +175,8 @@ class DomainStatus < ActiveRecord::Base PENDING_RENEW, PENDING_TRANSFER, PENDING_UPDATE, - PENDING_DELETE_CONFIRMATION + PENDING_DELETE_CONFIRMATION, + DELETE_CANDIDATE, ] end end diff --git a/app/presenters/domain_presenter.rb b/app/presenters/domain_presenter.rb index 2ecb82922..11d767203 100644 --- a/app/presenters/domain_presenter.rb +++ b/app/presenters/domain_presenter.rb @@ -67,6 +67,15 @@ class DomainPresenter end end + def keep_btn + return unless domain.discarded? + + view.link_to view.t('admin.domains.edit.keep_btn'), view.keep_admin_domain_path(@domain), + method: :patch, + data: { confirm: view.t('admin.domains.edit.keep_btn_confirm') }, + class: 'btn btn-default' + end + private attr_reader :domain diff --git a/app/views/admin/domains/edit.html.erb b/app/views/admin/domains/edit.html.erb index 6593f431b..24735734c 100644 --- a/app/views/admin/domains/edit.html.erb +++ b/app/views/admin/domains/edit.html.erb @@ -7,12 +7,13 @@