Rewrite tag deletion #2564
Rewrite tag deletion #2564
Conversation
'deleteAll()' doesn't trigger the 'afterDelete' event which we need for updating the search index.
| public function removeTagFromSentence($tagId, $sentenceId) { | ||
| $entity = $this->find() | ||
| ->where(['tag_id' => $tagId, 'sentence_id' => $sentenceId]) | ||
| ->first(); |
There was a problem hiding this comment.
This won’t work for tags that have been applied more than once on the same sentence. As far as I remember, this only happens when Horus merges two sentences having the same tag. See by yourself:
select count(*) as c, tag_id, sentence_id from tags_sentences group by tag_id, sentence_id having c > 1;
+---+--------+-------------+
| c | tag_id | sentence_id |
+---+--------+-------------+
| 2 | 2 | 482049 |
| 2 | 5 | 2337281 |
| 2 | 5 | 3214286 |
...
+---+--------+-------------+
4024 rows in set (1.481 sec)There was a problem hiding this comment.
Just to explain this mess. At the time Horus was created, we were aware of these "duplicate" tags and we (well, I) chose to adapt the way tags are displayed (in the sentence page and in the "sentences tagged as x" listing) to hide these duplicates (as opposed to deleting them) in order not to loose the information that two users had tagged that sentence. Looking back, I think it was a mistake because it produced a number of new bugs and added no real value in the end.
There was a problem hiding this comment.
Thanks for reminding me of this Horus bug.
Due to a bug in Horus there may be more than one tagId-sentenceId pair in the database.
This PR closes #2563.
The problem was that the table method for removing a tag used
deleteAll()which doesn't trigger theafterDeleteevent. But we use this event to update the attributes of the search index.