Skip to content

Conversation

@OlegPhenomenon
Copy link
Contributor

@OlegPhenomenon OlegPhenomenon commented May 16, 2025

Close #2788

This commit refines the contact duplication checks and notification messages for domain creation and updates, ensuring consistency and addressing a bug in domain updates.

Key Changes:

  • Consistent Duplicate Contact Handling:

    • The check_for_cross_role_duplicates, remove_duplicate_contacts, and duplicate_contact? methods are now more closely aligned between DomainCreate and DomainUpdate interactors.
    • DomainUpdate now correctly uses admin_contact_ids= and tech_contact_ids= to assign filtered contacts. This resolves a PG::UniqueViolation error that occurred when trying to re-associate existing contacts using _attributes= methods.
    • The duplicate_contact? method in DomainCreate was updated to match DomainUpdate, primarily checking for semantic duplicates based on attributes (name, ident, email, phone) rather than also including a contact.code check, which is more suitable for cross-role duplication.
  • Standardized Notification Messages:

    • The notify_about_removed_duplicates method in both interactors now generates a more concise message: ". [Role] contact [CODE] was discarded as duplicate;" for each discarded contact.
    • This message is appended to domain.skipped_domain_contacts_validation.
  • EPP & REPP Response Updates:

    • The message method in Repp::V1::DomainsController now correctly appends domain.skipped_domain_contacts_validation to the "Command completed successfully" message, ensuring it appears in REPP JSON responses for both create and update.
    • The EPP XML views (app/views/epp/domains/create.xml.builder and app/views/epp/domains/success.xml.builder) are updated to dynamically include domain.skipped_domain_contacts_validation in the <msg> tag.
  • Model Attribute:

    • Added skipped_domain_contacts_validation as a string attribute to the Domain model to store these notification messages.
  • Test Refinements (Implicit):

    • The related test suite for domain updates (test/integration/epp/domain/base_test.rb) was being updated to reflect these logic changes and assert the correct notification messages.

These changes improve the robustness and consistency of contact management during domain operations, providing clearer feedback to users about discarded duplicate contacts.

@vohmar
Copy link
Contributor

vohmar commented Jun 3, 2025

It works fine on domain creation, but there are issue with domain update.

  1. adding duplicate contact object to admin or tech will result with the message about discarding the duplicate object, but in reality it is added. On next update the old ones are removed but if i try to add another the new ones are added again. So you end up with something like this:
image image image

and now records about any of this in the history:
image

  1. changing an existing contact to be identical with another object linked to the same domain will result with unsupported contect type error when trying to add another contact object to the same role. If adding contact to a different role then warning about duplicates is shown, but the duplicate contact is not removed
image

@vohmar vohmar assigned OlegPhenomenon and unassigned vohmar Jun 3, 2025
This commit refines the contact duplication checks and notification messages for domain creation and updates, ensuring consistency and addressing a bug in domain updates.

**Key Changes:**

*   **Consistent Duplicate Contact Handling:**
    *   The `check_for_cross_role_duplicates`, `remove_duplicate_contacts`, and `duplicate_contact?` methods are now more closely aligned between `DomainCreate` and `DomainUpdate` interactors.
    *   `DomainUpdate` now correctly uses `admin_contact_ids=` and `tech_contact_ids=` to assign filtered contacts. This resolves a `PG::UniqueViolation` error that occurred when trying to re-associate existing contacts using `_attributes=` methods.
    *   The `duplicate_contact?` method in `DomainCreate` was updated to match `DomainUpdate`, primarily checking for semantic duplicates based on attributes (name, ident, email, phone) rather than also including a `contact.code` check, which is more suitable for cross-role duplication.

*   **Standardized Notification Messages:**
    *   The `notify_about_removed_duplicates` method in both interactors now generates a more concise message: ". [Role] contact [CODE] was discarded as duplicate;" for each discarded contact.
    *   This message is appended to `domain.skipped_domain_contacts_validation`.

*   **EPP & REPP Response Updates:**
    *   The `message` method in `Repp::V1::DomainsController` now correctly appends `domain.skipped_domain_contacts_validation` to the "Command completed successfully" message, ensuring it appears in REPP JSON responses for both create and update.
    *   The EPP XML views (`app/views/epp/domains/create.xml.builder` and `app/views/epp/domains/success.xml.builder`) are updated to dynamically include `domain.skipped_domain_contacts_validation` in the `<msg>` tag.

*   **Model Attribute:**
    *   Added `skipped_domain_contacts_validation` as a string attribute to the `Domain` model to store these notification messages.

*   **Test Refinements (Implicit):**
    *   The related test suite for domain updates (`test/integration/epp/domain/base_test.rb`) was being updated to reflect these logic changes and assert the correct notification messages.

These changes improve the robustness and consistency of contact management during domain operations, providing clearer feedback to users about discarded duplicate contacts.
@OlegPhenomenon OlegPhenomenon force-pushed the 2788-remove-dublicates-contacts-during-domain-creation branch from 70a5ab0 to 5224824 Compare August 11, 2025 11:16
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.

Ignore duplicate data entries for domain create, update and transfer requests

3 participants