#3392: fix domain deletion - [MS] #3485
Conversation
…gov/manage.get.gov into ko/3392-fix-domain-deletion
Matt-Spence
changed the title
src/registrar/models/domain.py
Outdated
| cls.UNKNOWN: ("Before this domain can be used, " "you’ll need to add name server addresses."), | ||
| cls.DNS_NEEDED: ("Before this domain can be used, " "you’ll need to add name server addresses."), | ||
| cls.UNKNOWN: ("Before this domain can be used, " "you'll need to add name server addresses."), | ||
| cls.DNS_NEEDED: ("Before this domain can be used, " "you'll need to add name server addresses."), | ||
| cls.READY: "This domain has name servers and is ready for use.", | ||
| cls.ON_HOLD: ( | ||
| "This domain is administratively paused, " | ||
| "so it can’t be edited and won’t resolve in DNS. " | ||
| "so it can't be edited and won't resolve in DNS. " |
There was a problem hiding this comment.
We are actually intentionally using hatch marks here! @gabo0oo can speak to this
There was a problem hiding this comment.
@zandercymatics I may need a refresher 😅 Why do we have to use hatch marks here?
There was a problem hiding this comment.
@gabo0oo Sorry - mispoke! I meant we are intentionally not using hatch marks. This PR changes ’ => ' which I thought we were avoiding?
There was a problem hiding this comment.
Oh, yup, we should use apostrophes not hatch marks. Sometimes there are reasons why it has to be a hatch mark that are beyond my knowledge so I'm not sure if that's what's going on here. Ideally, we'll always use apostrophes but if it must be a hatch mark that's ok. I'm so glad you're noticing them now too, @zandercymatics they drive me crazy! 😄
There was a problem hiding this comment.
Good catch! I think there's something on my personal laptop that keeps replacing these when I copy/paste, not sure why.
src/registrar/admin.py
Outdated
| self.message_user( | ||
| request, | ||
| "Could not delete: An unspecified error occured", | ||
| f"Could not delete: An unspecified error occured: {e}", |
There was a problem hiding this comment.
In general you don't want to show users Errors that just say vague things like an error occurred, and you don't want to show technical error information unless it will help the user resolve the scenario. The error type should be logged before this (if not make sure it get's logged), so I wouldn't show it here and instead I would change the message saying that they need to contact engineering and notify us of the time and domain the error occurred on.
There was a problem hiding this comment.
VA has some wonderful guidance on this if you're interested: https://design.va.gov/patterns/help-users-to/recover-from-errors#how-to-structure-an-error-message
…gov/manage.get.gov into ko/3392-fix-domain-deletion
Ticket
Resolves #3392
Changes
Context for reviewers
Setup
Code Review Verification Steps
Go to MS->admin and find a domain that has ds data, nameservers, and a valid registry status that isn't "PendingDelete" (you can check this with the "registry status" button at the top of the domain admin page). Try and delete the domain and see what it reports back. It should work this time... hopefully.
As the original developer, I have
Satisfied acceptance criteria and met development standards
Ensured code standards are met (Original Developer)
Validated user-facing changes (if applicable)
As a code reviewer, I have
Reviewed, tested, and left feedback about the changes
Validated user-facing changes as a developer
Note: Multiple code reviewers can share the checklists above, a second reviewer should not make a duplicate checklist. All checks should be checked before approving, even those labeled N/A.
As a designer reviewer, I have
Verified that the changes match the design intention
Validated user-facing changes as a designer
References
Screenshots