Admin: cancel force delete should not remove deleteCandidate status

[vohmar]

/admin/domains/id/edit

cancel force delete function should remove only the statuses that were set on schedule force delete. So deleteCandidate status should not be removed when force delete is canceled even if the status was set due to forceDelete process running to an end. Removing deleteCandidate status must be conscious decision as this is exceptional action.

Furthermore as deleteCandidate status represents domain waiting to be released and not being owned any more by previous registrant cancelling (nor setting for that matter) forceDelete should not be possible until deleteCandidate status is set.

[artur-intech]

cancel force delete function should remove only the statuses that were set on schedule force delete

How can I set those statuses on force deletion? There is only a drop-down with template selection.
How about other statuses?

registry/app/models/concerns/domain/force_delete.rb

Line 42 in b7a999f

def cancel_force_delete

[vohmar]

Force delete it self is label/name for a procedure. It is set of statuses. By applying force delete one actually adds these statuses to a domain: serverRenewProhibited, serverTransferProhibited, serverUpdateProhibited, pendingDelete, serverManualInzone + serverForceDelete. So by canceling force delete procedure no other statuses should be automatically removed.

[artur-intech]

1. Why the following statuses are removed on "force delete" action?

    statuses.delete(DomainStatus::CLIENT_DELETE_PROHIBITED)
    statuses.delete(DomainStatus::SERVER_DELETE_PROHIBITED)
    statuses.delete(DomainStatus::PENDING_UPDATE)
    statuses.delete(DomainStatus::PENDING_TRANSFER)
    statuses.delete(DomainStatus::PENDING_RENEW)
    statuses.delete(DomainStatus::PENDING_CREATE)

    statuses.delete(DomainStatus::FORCE_DELETE)
    statuses.delete(DomainStatus::SERVER_RENEW_PROHIBITED)
    statuses.delete(DomainStatus::SERVER_TRANSFER_PROHIBITED)
    statuses.delete(DomainStatus::SERVER_UPDATE_PROHIBITED)
    statuses.delete(DomainStatus::SERVER_MANUAL_INZONE)
    statuses.delete(DomainStatus::PENDING_DELETE)

registry/app/models/concerns/domain/force_delete.rb

Line 12 in b7a999f

def schedule_force_delete

2. SERVER_MANUAL_INZONE is only set if:
   

   registry/app/models/concerns/domain/force_delete.rb

   Line 34 in b7a999f

   if (statuses & [DomainStatus::SERVER_HOLD, DomainStatus::CLIENT_HOLD]).empty?

   
   is it expected? This condition is not mentioned above.

3. Why domain validation is turned off for the given actions? (both on applying and cancelling)

4. Why previous states are saved in domains.statuses_backup if the logic is just to set/remove statuses mentioned above

5. self.force_delete_at = (Time.zone.now + (Setting.redemption_grace_period.days + 1.day)).utc.beginning_of_day

Domain.where('force_delete_at <= ?', Time.zone.now.end_of_day.utc).each

Why utc and +1 day are needed here in general, and why force_delete_at is set in UTC in particular? Could you explain the logic?

[vohmar]

1. starting force delete procedure means there is something wrong with current data associated to the domain registration. Therefore we freeze the current set of data and stop any pending processes. As it is administrative delete procedure we also remove deleteProhibited statuses. The lower set is a bit trickier - as these are the statuses applied by forceDelete I would imagine that duplicate statuses are removed first to make the application process simpler. Not completely sure here.

2. yes the domain is kept in zone only if it was not already removed from zone at the time forceDelete was applied. This is expected behavior and you are correct this should have been mentioned above as well.

3. validation is off because we do not want the ability to apply or remove the status dependent on the quality of data of the domain. Similarly to regular epp domain/contact delete request.

4. The domain might have had number of statuses set before forceDelete was applied - a lot of these statuses are removed on forceDelete application. We want to restore the previous domain state when we stop forceDelete process. So there should be a step on cancel force delete step that restores these statuses from the statuses_backup.

5. like with most of period calculations we have we do not take the running day when domain was registered, or in this case forceDelete was applied into account and start counting from the first full day hence +1. Why utc is specifically forced in this situation is I would guess developers decisions - all the dates should be stored in db similarly using the same tz in our case that is UTC.

[artur-intech]

we do not want the ability to apply or remove the status dependent on the quality of data of the domain

Not sure I understand this part

[artur-intech]

I think it makes sense to rename statuses_backup to something more meaningful, say to statuses_before_force_delete (given "force delete" is the only part where that column is used).

I noticed our REST API /repp/v1/domains returns that column as well. What is the benefit of this?

[vohmar]

not sure about the necessity of returning backed up statuses with repp api. may be removed.

[artur-intech]

Can I solve #812 along with this ticket?

[artur-intech]

How can I complete this ticket, which says

cancel force delete should not remove deleteCandidate status

if #812 says that

Force Deleted domains do not get deleteCandidate status

?

So if a status wasn't initially set, there is nothing to keep :)

Or you mean when deleteCandidate status is set by CRON?

[vohmar]

• deleteCandidate status should always be set by the same cron- i do not see the reason to have separate process to set deleteCandidate for regular expiration, domain:delete requests and force delete procedures. The cron task is supposed to set the status by looking at both delete_at and force_delete_at dates on basis which ever comes first.

[artur-intech]

Why

stop any pending processes

=

statuses.delete(DomainStatus::CLIENT_DELETE_PROHIBITED)
statuses.delete(DomainStatus::SERVER_DELETE_PROHIBITED)

?

what is pending if any of those statuses is set?

5. What is the expected time of domains.force_delete_at column after "force delete" operation?
   This test passes: https://gist.github.com/artur-beljajev/d068e9af07836a135bfd5c4c9778472d
   Current implementation sample:

irb(main):010:0> d.force_delete_at = (Time.zone.now + (Setting.redemption_grace_period.days + 1.day)).utc.beginning_of_day
=> 2018-05-21 00:00:00 UTC
irb(main):011:0> d.save(validate: false)
irb(main):012:0> d.reload
irb(main):013:0> d.force_delete_at
=> Mon, 21 May 2018 03:00:00 EEST +03:00
irb(main):014:0> d.force_delete_at_before_type_cast
=> "2018-05-21 00:00:00"

force_delete_at in application time zone
force_delete_at_before_type_cast value in the database (UTC)

[artur-intech]

not sure about the necessity of returning backed up statuses with repp api. may be removed.

This requires new API version. Though, I am not sure if there is anyone who actually relies on this. Perhaps we can follow pragmatic approach and to just change it?

[artur-intech]

I see some restrictions connected to forceDelete status. Could you explain that part in detail?

registry/app/models/domain.rb

Line 134 in 96e2136

return unless force_delete_scheduled?

[artur-intech]

Can domains.statuses column's value change after "force delete" is initialised. i.e. is it needed to merge domains.statuses_backup with statuses at the moment of cancelling "force delete"?

[vohmar]

1. *_delete_prohibited statuses have to be removed because force delete is administrative delete procedure. The domain has to be deleted if the process is not canceled in 30 days.
2. force_delete_at value is calculated by adding length of redemption grace period and 1 day at time 00:00 (ie it is now 24.04.2018 10:14 - force_delete_at would be 24.05.2018 00:00 (+31days)) . The same way as delete_at value is calculated on domain.delete epp request.
3. REPP API is used by registrars - but in this case i believe we can make an exception and not add new API version
4. I am not sure I can understand the code correctly here
5. Yes admin can change list of statuses applied with force_delete - ie take the domain outzone or remove some prohibited statuses to allow registrant to correct invalid data. But these changes are usually directly connected to fixing the reason force delete process was applied. On canceling the process I cannot see the reason for merging any statuses. Remove all the statuses, restore statuses from backup and optionally recalculate outzone and delete dates and set statuses accordingly

[artur-intech]

I am not sure I can understand the code correctly here

registry/app/models/domain.rb

Line 134 in 96e2136

return unless force_delete_scheduled?

It effectively prohibits any manipulation with a domain if it has "force delete" operation scheduled.
Extracted to #870

[vohmar]

ForceDelete by itself should not prohibit anything - there are *prohibited statuses for that. deleteCandidate prohibits any change over EPP/REPP

[vohmar]

• Still possible to cancel force delete while deleteCandidate status is set
• Canceling forcedelete also removed deleteCandidate status from the domain

test process:

1. find a domain with deletecandidate status set and remove that status
2. apply force delete process to that domain
3. run DomainCron.destroy_delete_candidates to reset deletecandidate status
4. cancel forcedelete process - force delete canceled, deletecandidate removed

expected result:

• not possible to cancel force delete while deletecandidate status is set
• to cancel force delete, admin should first remove deletecandidate status and only then is able to cancel the force delete process

[artur-intech]

First you asked for:

cancelling forceDelete should not be possible until deleteCandidate status is set.

I have implemented it and the following tests pass:

registry/test/models/domain/force_delete_test.rb

Line 76 in c2d5b7c

def test_cancelling_force_delete_on_a_discarded_domain

registry/test/models/domain/force_delete_test.rb

Line 85 in c2d5b7c

def test_cancelling_force_delete_requires_a_domain_to_be_discarded

So now it's not possible to cancel "force delete" operation unless a domain has DELETE_CANDIDATE status, so If a domain has DELETE_CANDIDATE, it can be cancelled.

now you ask for the opposite (at least how I understood it).

not possible to cancel force delete while deletecandidate status is set

Can you clarify?

[vohmar]

• deletecandidate basically prohibits any further change to the domain except deletion
• to make any change including stopping forceDelete procedure one must first remove deletecandidate status

[vohmar]

test results:

• was still able to add and remove forcedelete or any other status for that matter even though deleteCandidate status was set

[artur-intech]

Now you cannot cancel "force delete" procedure if a domain has deleteCandidate status.

[vohmar]

can still add force delete while domain has deletecandidate status set. adding forcedelete to delete candidate, then removing delete candidate status to cancel forcedelete results with domain with reapplied deletecandidate status but no que job. Setting delete candidate status is a separate process so it should not be applied by anything else like canceling force delete process for example. (this would not be a problem if changing statuses by admin or registrar was disabled while deletecandidate status is applied.

[vohmar]

problem with cancelforcedelete restoring deletecandidate status if it was set before forcedelete was added is still there. The status is restored, but no que job is created. It would still be preferable if it was not possible to make any status changes on a domain while it has deletecandidate status set. But removing forcedelete should not set deletecandidate status on a domain.

[artur-intech]

Now you cannot schedule "force delete" procedure if a domain has deleteCandidate status.

Same check during cancellation is removed as being useless (if you cannot start the process, there is obviously nothing to end).

[artur-intech]

Please retest.