I have a website with a special workflow where the status (published or not) property of nodes i set to translatable, so that nodes can be published in their original language, but not yet published in another one.
I stumbled upon a problem where some translations get unindexed even if they are published. Actually they are untracked in the search_api_item table, so that I can't even index them, except if I rebuild the tracker of my index (drush search-api:rebuild-tracker).
I suspect it comes from the fact that ContentEntity::loadMultiple() will not load unpublished translations, because of the way hasTranslation() method works: only return true if the translation is published.
Unfortunately, I failed to find an easily reproducible scenario. At some time in the life of my production site, some indexed translations get untracked, and I know (thanks to mail() debug calls in production code) that it happens after hasTranslation() returned false (I still wonder why, because it seems the translations where published... some cache?). It happened multiple times in last weeks but I don't know exactly which event triggered the bug, most of the time it just works.
Anyway, the current code looks wrong to me and should be fixed IMHO : load() is not supposed to return NULL is status is unpublished. It doesn't work like that elsewhere in the Drupal code.
Do not call hasTranslation() to detect if a content has a (maybe unpublished) translation.
Patch is coming.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 3257728-2.patch | 923 bytes | gaëlg |
Comments
Comment #2
gaëlgHere's the patch. I'll apply it to my project and see if the bug comes again after some time.
Comment #3
drunken monkeyThanks for reporting this issue and providing a patch!
This is neither what it says in the
hasTranslation()method’s documentation, nor how its code looks. From what I see, it will only returnFALSEif a translation is completely missing or has thestatusTRANSLATION_REMOVED. (Here,statusrefers to the key in$this->translations, not the node’sstatusfield.)So, if whatever code you use to keep the unpublished translation from showing up sets that
$this->translations[$langcode]['status']key toTRANSLATION_REMOVEDto achieve this, it seems to misuse it and problems should be expected. Otherwise, I don’t see how that call would returnFALSE.So, I think I’ll need more explanation before considering to commit this. The general rule is that exceptions should not be used for code control, so I definitely prefer checking for the translation’s existence before calling
getTranslation(). (Also, I find it a bit strange that Core would returnFALSEinhasTranslation()for some cases in whichgetTranslation()wouldn’t throw an exception – not very intuitive. Still, unless there is a real case where using the APIs properly results in unindexed entities, I’d leave the code as-is.Also, this would need test coverage before being committed. Maybe if you write a test demonstrating the problem it would also be easier to assess it.
Comment #4
gaëlgThank you for your feedback. We are months later now, and I don't have much time to dig into the code again. But what I can say is that I still had my bug after some time, even with my patch, and I finally found out that it was related to very strange problems with the hosting server. We eventually hosted the website on another hosting service and have not seen the bug since then.
So I guess this issue was a wrong track, I probably misunderstood the status property as you said. Sorry for the inconvenience and thank you again!