N°9527 - Avoid multiple notifications on user mentionned on ticket with children - #991
N°9527 - Avoid multiple notifications on user mentionned on ticket with children#991v-dumas wants to merge 1 commit into
Conversation
|
| Filename | Overview |
|---|---|
| datamodels/2.x/itop-tickets/datamodel.itop-tickets.xml | Cleans mention metadata from parent log HTML before propagating the entry to child tickets. |
Reviews (2): Last reviewed commit: "N°9527 - Multi notifications when user m..." | Re-trigger Greptile
| && MetaModel::IsValidAttCode($sChildClass, $sChildAttCode) && MetaModel::GetAttributeDef($sChildClass, $sChildAttCode) instanceof AttributeCaseLog | ||
| && (!utils::IsNullOrEmptyString($this->Get($sParentAttCode)->GetModifiedEntry('html')))) { | ||
| $aChildEntries[$sChildAttCode] = Dict::Format('Class:'.$sParentClass.'/Method:UpdateChildTicketWith:'.$sParentAttCode, $this->GetKey(), $this->Get('ref')).$this->Get($sParentAttCode)->GetModifiedEntry('html'); | ||
| $sCleanedEntry = preg_replace('/<a\s*([^>]*)data-object-class="([^"]*)"\s.*data-object-key="([^"]*)"\s*([^>]*)>@/Ui','<a\1\4>',$this->Get($sParentAttCode)->GetModifiedEntry('html')); |
There was a problem hiding this comment.
The @ is hardcoded here. But this value comes from the array configuration parameter mentions.allowed_classes which can hold multiple mention triggering character (@, #, ? etc..) and may be interpreted not as character but as joker or placeholder
There was a problem hiding this comment.
Moreover, this is removing the data-object-class and data-object-key metadata which may be used by other features, you should not remove them.
Maybe the real fix (but it has to be discussed) would be that a mention adds a dedicated HTML metadata in addition to data-object-class / data-object-key which should be the one triggering the mention notification. That way you could stip this one without impacting other features.
There was a problem hiding this comment.
Pull request overview
Prevents duplicate mention notifications when parent-ticket log entries propagate to children.
Changes:
- Removes mention metadata from copied
@links. - Updates the method comment wording.
Suppressed comments (2)
datamodels/2.x/itop-tickets/datamodel.itop-tickets.xml:386
- The
.*can backtrack past the current closing>into later anchors. For example, an object link followed on the same line by an@Personmention can be matched as one span, causing this replacement to delete the first link and all intervening visible content. Keep attribute matching inside the current opening tag (or parse the HTML structurally).
$sCleanedEntry = preg_replace('/<a\s*([^>]*)data-object-class="([^"]*)"\s.*data-object-key="([^"]*)"\s*([^>]*)>@/Ui','<a\1\4>',$this->Get($sParentAttCode)->GetModifiedEntry('html'));
datamodels/2.x/itop-tickets/datamodel.itop-tickets.xml:386
- The new sanitization behavior has no regression test, although this method already has dedicated coverage in
tests/php-unit-tests/unitary-tests/datamodels/2.x/itop-tickets/UpdateChildTicketLogTest.php. Add a parent entry containing a Person mention and multiple children, then assert copied entries are no longer returned byutils::GetMentionedObjectsFromText; include an ordinary object link before the mention to guard against cross-anchor content loss.
$sCleanedEntry = preg_replace('/<a\s*([^>]*)data-object-class="([^"]*)"\s.*data-object-key="([^"]*)"\s*([^>]*)>@/Ui','<a\1\4>',$this->Get($sParentAttCode)->GetModifiedEntry('html'));
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * Remove the Person associated with the current user from the contacts_list of this Ticket | ||
| * No error if there is no associated Person or if the Person is not in the list |
Molkobain
left a comment
Molkobain
left a comment
There was a problem hiding this comment.
See comment in Steffunky thread
|
There is another PR which aims to replace the regexp with a Xpath, as a result, I will stop this one until we have agreed on a common strategy |
Base information
Symptom (bug) / Objective (enhancement)
When a Person is mentioned on a parent Ticket within a log entry, that log entry is copied on the children tickets as is, which result in multiple notification of the same person for the same topic
Proposed solution (bug and enhancement)
The idea is to clean the copied log entry, removing the part which trigger the TriggerOnMention
Checklist before requesting a review