Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#2823: update delete domain process - [MS] #3185

Merged
merged 35 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
7cf8b8a
Delete contacts and subdomains on delete domain
Matt-Spence Nov 22, 2024
6891f5c
Rework delete from epp
Matt-Spence Nov 26, 2024
ede01e3
Merge remote-tracking branch 'origin/main' into ms/2823-update-delete…
Matt-Spence Nov 26, 2024
b5e4f8b
update deletion process and tests
Matt-Spence Dec 3, 2024
27868a0
minor fixes to tests
Matt-Spence Dec 4, 2024
9437b73
minor test fix
Matt-Spence Dec 4, 2024
74eeae1
Merge remote-tracking branch 'origin/main' into ms/2823-update-delete…
Matt-Spence Dec 4, 2024
89253a1
linter fixes
Matt-Spence Dec 4, 2024
f25bb9b
include hostname in error messages for shared hosts
Matt-Spence Dec 4, 2024
dad4226
add back in less console noise decorator
Matt-Spence Dec 4, 2024
6fdb763
admin fix
Matt-Spence Dec 4, 2024
e8fdf0c
revert accidental admin change
Matt-Spence Dec 5, 2024
3f79b56
temp test changes
Matt-Spence Dec 5, 2024
2e84171
fix a test
Matt-Spence Dec 5, 2024
43484ec
Merge remote-tracking branch 'origin/main' into ms/2823-update-delete…
Matt-Spence Dec 5, 2024
aaaa4f2
fix broken test
Matt-Spence Dec 5, 2024
8b473d5
add error message to registry errors
Matt-Spence Dec 5, 2024
3dbafb5
up log level
Matt-Spence Dec 5, 2024
a9710da
more debugging
Matt-Spence Dec 5, 2024
5e7823a
more debugging
Matt-Spence Dec 5, 2024
e87c4f7
use update function to delete hosts
Matt-Spence Dec 9, 2024
1a9b671
consolidate delete domain function
Matt-Spence Dec 11, 2024
e0eb70a
working _delete_domain
dave-kennedy-ecs Dec 11, 2024
533e577
refactor delete to use same logic as nameserver setter
Matt-Spence Dec 11, 2024
51da456
fix tests
Matt-Spence Dec 12, 2024
1fb8b22
pushing changes to pull on another device
Matt-Spence Dec 12, 2024
63c2b79
clean up
Matt-Spence Dec 12, 2024
21007ce
review changes
Matt-Spence Dec 13, 2024
32789fa
review changes and linting
Matt-Spence Dec 13, 2024
db4f9c5
Merge branch 'ms/2823-update-delete-domain-process' of https://github…
Matt-Spence Dec 13, 2024
bb2072d
log exception in nameserver setter.
Matt-Spence Dec 13, 2024
f2d7be1
Merge remote-tracking branch 'origin' into ms/2823-update-delete-doma…
Matt-Spence Dec 13, 2024
0178d57
linter fixes
Matt-Spence Dec 16, 2024
f039f31
linter fixes
Matt-Spence Dec 16, 2024
8016dd9
Merge branch 'main' into ms/2823-update-delete-domain-process
Matt-Spence Dec 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/registrar/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -2927,7 +2927,7 @@ def do_delete_domain(self, request, obj):
message = "Cannot connect to the registry"
if not err.is_connection_error():
# If nothing is found, will default to returned err
message = error_messages[err.code]
message = error_messages.get(err.code, err)
self.message_user(request, f"Error deleting this Domain: {message}", messages.ERROR)
except TransitionNotAllowed:
if obj.state == Domain.State.DELETED:
Expand Down
9 changes: 8 additions & 1 deletion src/registrar/models/domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -750,7 +750,12 @@ def nameservers(self, hosts: list[tuple[str, list]]):

successTotalNameservers = len(oldNameservers) - deleteCount + addToDomainCount

self._delete_hosts_if_not_used(hostsToDelete=deleted_values)
try:
self._delete_hosts_if_not_used(hostsToDelete=deleted_values)
except:
# the error will be logged in the erring function and we don't
# need this part to succeed in order to continue.s
pass
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would suggest logging the error + some minor grammar things in the comment, but other than that looks good


if successTotalNameservers < 2:
try:
Expand Down Expand Up @@ -1065,6 +1070,8 @@ def _delete_domain(self):
if responseCode != ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY:
raise NameserverError(code=nsErrorCodes.BAD_DATA)

# addAndRemoveHostsFromDomain removes the hosts from the domain object,
# but we still need to delete the object themselves
self._delete_hosts_if_not_used(hostsToDelete=deleted_values)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Nitpick / optional) A comment explaining as to why delete hosts is called after 1062 would be a nice to have but not needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea


logger.debug("Deleting non-registrant contacts for %s", self.name)
Expand Down
1 change: 0 additions & 1 deletion src/registrar/tests/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -1676,7 +1676,6 @@ def mockUpdateHostCommands(self, _request, cleaned):
def mockDeleteHostCommands(self, _request, cleaned):
host = getattr(_request, "name", None)
if "sharedhost.com" in host:
print("raising registry error")
raise RegistryError(code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION, note="ns1.sharedhost.com")
return MagicMock(
res_data=[self.mockDataHostChange],
Expand Down