N°9699 - Enable AddMeToContact without write access on Ticket

[v-dumas]

Base information

Question	Answer
Related to a Combodo ticket?	N°9699
Type of change?	Enhancement

Symptom (bug) / Objective (enhancement)

A User which cannot edit Ticket, but can edit Person, such as a Configuration Manager, currently cannot add himself to a Ticket using an Hyperlink Configurator action available on iTop Products

Proposed solution (bug and enhancement)

Stop modifying the Ticket, but directly create or delete the link object

Checklist before requesting a review

• I have performed a self-review of my code
• I have tested all changes I made on an iTop instance
• I have added a unit test, otherwise I have explained why I couldn't
• Is the PR clear and detailed enough so anyone can understand without digging in the code?

[greptile-apps]

Greptile Summary

This PR refactors AddCurrentUserToContacts() and RemoveCurrentUserFromContacts() on the Ticket class to operate directly on the lnkContactToTicket link object (via DBInsert()/DBDelete()) instead of mutating the Ticket's contacts_list attribute and calling DBUpdate(). This allows users who have write access to lnkContactToTicket but not to Ticket (e.g. Configuration Managers) to add/remove themselves as contacts.

• Both methods now return void instead of bool true, which is a breaking API change for any external caller (e.g. the Hyperlink Configurator extension) that inspects the return value.
• After DBInsert() in AddCurrentUserToContacts(), the Ticket's in-memory contacts_list cache is not updated; a second call on the same instance will pass the duplicate-guard check and attempt another DBInsert(), hitting the blocking no_duplicate uniqueness rule and throwing a CoreCannotSaveObjectException.
• In RemoveCurrentUserFromContacts(), the break that was implicit via the old return true is now missing; the foreach loop continues after DBDelete() (harmless in practice due to the uniqueness constraint, but logically incorrect).

Confidence Score: 4/5

Safe to merge in the common case, but a same-instance double-call to AddCurrentUserToContacts will throw an unhandled exception instead of silently no-oping.

The in-memory contacts_list cache is not refreshed after DBInsert(), so a second invocation on the same Ticket instance bypasses the duplicate-guard and crashes on the blocking no_duplicate uniqueness rule. This breaks the method's previously documented contract of being idempotent/error-free on duplicate additions.

datamodels/2.x/itop-tickets/datamodel.itop-tickets.xml — specifically the AddCurrentUserToContacts method and its post-insert cache state.

Important Files Changed

Filename	Overview
datamodels/2.x/itop-tickets/datamodel.itop-tickets.xml	Refactors AddCurrentUserToContacts and RemoveCurrentUserFromContacts to bypass Ticket write permission by operating directly on the lnkContactToTicket link object; the stale in-memory cache after DBInsert can break the duplicate-guard on same-instance double-calls, and the missing break after DBDelete changes loop semantics.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller
    participant Ticket
    participant lnkContactToTicket

    Note over Caller,lnkContactToTicket: AddCurrentUserToContacts (new)
    Caller->>Ticket: AddCurrentUserToContacts()
    Ticket->>Ticket: UserRights::GetContactId()
    Ticket->>Ticket: Get('contacts_list') [reads in-memory cache]
    alt Person not already in list
        Ticket->>lnkContactToTicket: MetaModel::NewObject('lnkContactToTicket')
        Ticket->>lnkContactToTicket: Set contact_id + ticket_id
        Ticket->>lnkContactToTicket: DBInsert() [requires write on lnkContactToTicket, NOT Ticket]
        Note right of lnkContactToTicket: in-memory cache NOT updated
    end

    Note over Caller,lnkContactToTicket: RemoveCurrentUserFromContacts (new)
    Caller->>Ticket: RemoveCurrentUserFromContacts()
    Ticket->>Ticket: UserRights::GetContactId()
    Ticket->>Ticket: Get('contacts_list') [reads in-memory cache]
    loop each lnk in contacts_list
        alt "lnk.contact_id == personId"
            Ticket->>lnkContactToTicket: DBDelete() [requires write on lnkContactToTicket, NOT Ticket]
            Note right of lnkContactToTicket: loop continues without break
        end
    end

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Caller
    participant Ticket
    participant lnkContactToTicket

    Note over Caller,lnkContactToTicket: AddCurrentUserToContacts (new)
    Caller->>Ticket: AddCurrentUserToContacts()
    Ticket->>Ticket: UserRights::GetContactId()
    Ticket->>Ticket: Get('contacts_list') [reads in-memory cache]
    alt Person not already in list
        Ticket->>lnkContactToTicket: MetaModel::NewObject('lnkContactToTicket')
        Ticket->>lnkContactToTicket: Set contact_id + ticket_id
        Ticket->>lnkContactToTicket: DBInsert() [requires write on lnkContactToTicket, NOT Ticket]
        Note right of lnkContactToTicket: in-memory cache NOT updated
    end

    Note over Caller,lnkContactToTicket: RemoveCurrentUserFromContacts (new)
    Caller->>Ticket: RemoveCurrentUserFromContacts()
    Ticket->>Ticket: UserRights::GetContactId()
    Ticket->>Ticket: Get('contacts_list') [reads in-memory cache]
    loop each lnk in contacts_list
        alt "lnk.contact_id == personId"
            Ticket->>lnkContactToTicket: DBDelete() [requires write on lnkContactToTicket, NOT Ticket]
            Note right of lnkContactToTicket: loop continues without break
        end
    end

Loading

_{Reviews (2): Last reviewed commit: "Remove useless revision # in XML tag" | Re-trigger Greptile}

[v-dumas]

@greptile

[Copilot]

Pull request overview

This PR addresses Combodo ticket N°9699, which enables a user who can edit Person but not Ticket (e.g., a Configuration Manager) to add or remove themselves from a Ticket's contacts list via a Hyperlink Configurator action. Previously, both operations mutated the Ticket object and called DBUpdate(), which required write access on the Ticket. The change instead creates/deletes the lnkContactToTicket link object directly, so the required access is on the link rather than on the Ticket itself.

Changes:

• AddCurrentUserToContacts now DBInsert()s a new lnkContactToTicket directly instead of adding it to the Ticket's link set and calling DBUpdate().
• RemoveCurrentUserFromContacts now DBDelete()s the matching link directly instead of removing it from the set and updating the Ticket.
• Both methods' return type changed from bool to void, docblocks were updated to note the "not under modification" precondition and concurrency caveats, and stale _revision_id attributes were dropped.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.