Skip to content

Conversation

CydeWeys
Copy link
Member

@CydeWeys CydeWeys commented Sep 16, 2025

This implements the first part of Minimum Data Set phase 3, wherein we delete all contact data. This action is necessary to leave a permanent record on the domain (in the form of a domain history entry) documenting when the contacts were removed by the administrative user.

Then, after this has finished removing all contact assocations, we can simply empty out or drop the Contact/ContactHistory tables and associated join tables.


This change is Reviewable

@CydeWeys CydeWeys force-pushed the remove-contact-assocs branch 6 times, most recently from 98b9c10 to 20ab873 Compare September 23, 2025 18:19
@CydeWeys CydeWeys requested a review from jicelhay September 23, 2025 18:20
This implements the first part of Minimum Data Set phase 3, wherein we delete
all contact data. This action is necessary to leave a permanent record on the
domain (in the form of a domain history entry) documenting when the contacts
were removed by the administrative user.

Then, after this has finished removing all contact assocations, we can simply
empty out or drop the Contact/ContactHistory tables and associated join tables.
@CydeWeys CydeWeys force-pushed the remove-contact-assocs branch from 20ab873 to f8998fb Compare September 23, 2025 18:51
Copy link
Collaborator

@jicelhay jicelhay left a comment

Choose a reason for hiding this comment

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

@jicelhay reviewed 1 of 4 files at r1, all commit messages.
Reviewable status: 1 of 7 files reviewed, 1 unresolved discussion


core/src/main/java/google/registry/batch/RemoveAllDomainContactsAction.java line 195 at r7 (raw file):

    for (DesignatedContact designatedContact : domain.getContacts()) {
      @Nullable Contact contact = contacts.get(designatedContact.getContactKey());
      if (contact == null) {

is there any AI for domains that have this data?

Copy link
Member Author

@CydeWeys CydeWeys left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 of 7 files reviewed, 1 unresolved discussion (waiting on @jicelhay)


core/src/main/java/google/registry/batch/RemoveAllDomainContactsAction.java line 195 at r7 (raw file):

Previously, jicelhay (Juan Celhay) wrote…

is there any AI for domains that have this data?

This is a state in the system that shouldn't exist -- indeed foreign keys at the DB schema layer should probably ensure that it doesn't. And if it does exist, then it's not fixable via EPP, as the domain update flow would run into the same problem.

So I think the best we can do here is, in the unlikely event of this scenario, log it, and then go in and fix it manually. I suspect we won't have any occurrences of this, though. (But if I didn't explicitly handle the situation, then sb.append(...) below could run into an NPE.

Copy link
Collaborator

@jicelhay jicelhay left a comment

Choose a reason for hiding this comment

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

@jicelhay reviewed 1 of 4 files at r1, 1 of 3 files at r4, 4 of 4 files at r7.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @CydeWeys)

@CydeWeys CydeWeys enabled auto-merge September 24, 2025 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants