diff --git a/app/controllers/admin/disputes_controller.rb b/app/controllers/admin/disputes_controller.rb index 03b4d4895..450576aa3 100644 --- a/app/controllers/admin/disputes_controller.rb +++ b/app/controllers/admin/disputes_controller.rb @@ -44,7 +44,7 @@ module Admin # DELETE /admin/disputes/1 def delete - @dispute.close + @dispute.close(initiator: 'Admin') redirect_to admin_disputes_url, notice: 'Dispute was successfully closed.' end diff --git a/app/controllers/registrant/domain_update_confirms_controller.rb b/app/controllers/registrant/domain_update_confirms_controller.rb index 0b964ae2b..0e4f2a582 100644 --- a/app/controllers/registrant/domain_update_confirms_controller.rb +++ b/app/controllers/registrant/domain_update_confirms_controller.rb @@ -31,10 +31,7 @@ class Registrant::DomainUpdateConfirmsController < RegistrantController end elsif params[:confirmed] if @registrant_verification.domain_registrant_change_confirm!("email link, #{initiator}") - if @domain.disputed? - dispute = Dispute.active.find_by(domain_name: @domain.name) - dispute.close - end + Dispute.close_by_domain(@domain.name) if @domain.disputed? flash[:notice] = t(:registrant_domain_verification_confirmed) redirect_to registrant_domain_update_confirm_path(@domain.id, confirmed: true) diff --git a/app/jobs/dispute_status_update_job.rb b/app/jobs/dispute_status_update_job.rb index 736b65236..2a3f3445a 100644 --- a/app/jobs/dispute_status_update_job.rb +++ b/app/jobs/dispute_status_update_job.rb @@ -13,7 +13,7 @@ class DisputeStatusUpdateJob < Que::Job end def close_disputes - disputes = Dispute.where(closed: false).where('expires_at < ?', Time.zone.today).all + disputes = Dispute.where(closed: nil).where('expires_at < ?', Time.zone.today).all Rails.logger.info "DisputeStatusUpdateJob - Found #{disputes.count} closable disputes" disputes.each do |dispute| process_dispute(dispute, closing: true) @@ -21,7 +21,7 @@ class DisputeStatusUpdateJob < Que::Job end def activate_disputes - disputes = Dispute.where(closed: false, starts_at: Time.zone.today).all + disputes = Dispute.where(closed: nil, starts_at: Time.zone.today).all Rails.logger.info "DisputeStatusUpdateJob - Found #{disputes.count} activatable disputes" disputes.each do |dispute| @@ -31,7 +31,7 @@ class DisputeStatusUpdateJob < Que::Job def process_dispute(dispute, closing: false) intent = closing ? 'close' : 'activate' - success = closing ? dispute.close : dispute.generate_data + success = closing ? dispute.close(initiator: 'Job') : dispute.generate_data create_backlog_entry(dispute: dispute, intent: intent, successful: success) end diff --git a/app/models/dispute.rb b/app/models/dispute.rb index 7aaca65b1..7434c8e26 100644 --- a/app/models/dispute.rb +++ b/app/models/dispute.rb @@ -10,9 +10,9 @@ class Dispute < ApplicationRecord scope :expired, -> { where('expires_at < ?', Time.zone.today) } scope :active, lambda { - where('starts_at <= ? AND expires_at >= ? AND closed = false', Time.zone.today, Time.zone.today) + where('starts_at <= ? AND expires_at >= ? AND closed IS NULL', Time.zone.today, Time.zone.today) } - scope :closed, -> { where(closed: true) } + scope :closed, -> { where.not(closed: nil) } attr_readonly :domain_name @@ -24,7 +24,7 @@ class Dispute < ApplicationRecord dispute = Dispute.active.find_by(domain_name: domain_name) return false unless dispute - dispute.close + dispute.close(initiator: 'Registrant') end def self.valid_auth?(domain_name, password) @@ -52,8 +52,8 @@ class Dispute < ApplicationRecord wr.save end - def close - return false unless update(closed: true) + def close(initiator: 'Unknown') + return false unless update(closed: Time.zone.now, initiator: initiator) return if Dispute.active.where(domain_name: domain_name).any? domain&.unmark_as_disputed @@ -129,7 +129,7 @@ class Dispute < ApplicationRecord end def validate_domain_name_period_uniqueness - existing_dispute = Dispute.unscoped.where(domain_name: domain_name, closed: false) + existing_dispute = Dispute.unscoped.where(domain_name: domain_name, closed: nil) .where('expires_at >= ?', starts_at) existing_dispute = existing_dispute.where.not(id: id) unless new_record? diff --git a/app/views/admin/disputes/index.html.erb b/app/views/admin/disputes/index.html.erb index 3a72e7d41..c4bd094a9 100644 --- a/app/views/admin/disputes/index.html.erb +++ b/app/views/admin/disputes/index.html.erb @@ -125,13 +125,13 @@ <%= sort_link(@q, 'name') %> - <%= sort_link(@q, 'password') %> + <%= sort_link(@q, 'Initiator') %> <%= sort_link(@q, 'starts_at') %> - <%= sort_link(@q, 'expires_at') %> + <%= sort_link(@q, 'Expired/Closed At') %> <%= sort_link(@q, 'comment') %> @@ -145,13 +145,13 @@ <%= x.domain_name %> - <%= x.password %> + <%= x.initiator %> <%= x.starts_at %> - <%= x.expires_at %> + <%= x.closed %> <%= x.comment %> diff --git a/db/migrate/20200518104105_add_closed_date_time_and_updator_to_dispute.rb b/db/migrate/20200518104105_add_closed_date_time_and_updator_to_dispute.rb new file mode 100644 index 000000000..1aae02e06 --- /dev/null +++ b/db/migrate/20200518104105_add_closed_date_time_and_updator_to_dispute.rb @@ -0,0 +1,19 @@ +class AddClosedDateTimeAndUpdatorToDispute < ActiveRecord::Migration[5.2] + def up + rename_column :disputes, :closed, :closed_boolean + add_column :disputes, :closed, :datetime + execute 'UPDATE disputes SET closed = updated_at WHERE closed_boolean = true' + execute 'UPDATE disputes SET closed = NULL WHERE closed_boolean = false' + remove_column :disputes, :closed_boolean + add_column :disputes, :initiator, :string + end + + def down + rename_column :disputes, :closed, :closed_datetime + add_column :disputes, :closed, :boolean, null: false, default: false + execute 'UPDATE disputes SET closed = true WHERE closed_datetime != NULL' + execute 'UPDATE disputes SET closed = false WHERE closed_datetime = NULL' + remove_column :disputes, :closed_datetime + remove_column :disputes, :initiator + end +end diff --git a/db/structure.sql b/db/structure.sql index ff2a625d9..4104b2db5 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -1,6 +1,6 @@ --- --- PostgreSQL database dump --- +--- +--- PostgreSQL database dump +--- SET statement_timeout = 0; SET lock_timeout = 0; @@ -605,9 +605,10 @@ CREATE TABLE public.disputes ( expires_at date NOT NULL, starts_at date NOT NULL, comment text, - closed boolean DEFAULT false NOT NULL, created_at timestamp without time zone NOT NULL, - updated_at timestamp without time zone NOT NULL + updated_at timestamp without time zone NOT NULL, + closed timestamp without time zone, + initiator character varying ); @@ -4520,5 +4521,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20200417075720'), ('20200421093637'), ('20200505103316'), -('20200505150413'); +('20200505150413'), +('20200518104105'); + diff --git a/test/fixtures/disputes.yml b/test/fixtures/disputes.yml index 3e6b882f4..a999fa0a1 100644 --- a/test/fixtures/disputes.yml +++ b/test/fixtures/disputes.yml @@ -3,22 +3,20 @@ active: password: active-001 starts_at: <%= Date.parse '2010-07-05' %> expires_at: <%= Date.parse '2013-07-05' %> - closed: false future: domain_name: future-dispute.test password: active-001 starts_at: <%= Date.parse '2010-10-05' %> expires_at: <%= Date.parse '2013-10-05' %> - closed: false expired: domain_name: expired-dispute.test password: active-001 starts_at: <%= Date.parse '2010-07-05' %> expires_at: <%= Date.parse '2013-07-05' %> - closed: true + closed: <%= Date.parse '2013-07-05' %> closed: domain_name: closed_dispute.test password: active-001 starts_at: <%= Date.parse '2010-07-05' %> expires_at: <%= Date.parse '2013-07-05' %> - closed: true + closed: <%= Date.parse '2013-07-05' %> diff --git a/test/integration/admin_area/disputes_test.rb b/test/integration/admin_area/disputes_test.rb index cfda9d23d..b829e3b49 100644 --- a/test/integration/admin_area/disputes_test.rb +++ b/test/integration/admin_area/disputes_test.rb @@ -18,7 +18,7 @@ class AdminDisputesSystemTest < ApplicationSystemTestCase assert_nil Dispute.active.find_by(domain_name: 'disputed.test') visit admin_disputes_path - click_on 'New domain dispute' + click_on 'New disputed domain' fill_in 'Domain name', with: 'disputed.test' fill_in 'Password', with: '1234' @@ -34,7 +34,7 @@ class AdminDisputesSystemTest < ApplicationSystemTestCase assert_nil Dispute.active.find_by(domain_name: 'disputed.test') visit admin_disputes_path - click_on 'New domain dispute' + click_on 'New disputed domain' fill_in 'Domain name', with: 'disputed.test' fill_in 'Password', with: '1234' @@ -65,7 +65,7 @@ class AdminDisputesSystemTest < ApplicationSystemTestCase def test_can_not_create_overlapping_dispute visit admin_disputes_path - click_on 'New domain dispute' + click_on 'New disputed domain' fill_in 'Domain name', with: 'active-dispute.test' fill_in 'Starts at', with: @dispute.starts_at + 1.day