Deleting a translation leaves behind orphaned revisions

At this point:
although we have a row for a french translation the node is showing as untranslated on node/2/translations
And on the revisions tab you can only revert to revision 6 & 7.

1. The node is showing as untranslated as you have removed the french translation from the default revision and now the default revision (revision id 9) has only the english translation.
2. I've thought that on the revisions tab you can revert only the actual translation, but I am not sure why revision id 8 is not listed there. And it is strange that while you revert the english translation you get back the french one.

We need decide what the expected behaviour should be. i don't think deleting the current translation revision is the expected behaviour. It is a halfway house between removing all revisions that pertain to the translation and just creating a new revision without the translation.

If the translation is removed from all available revisions then the history is also lost which in some cases might be undesirable as well.

Well, if revisions are on, the totally non-destructive possibility is just creating a new revision without the translation. And then there is no delete of anything. Which is how I expected it to behave.

+1

If we remove a translation through the UI then I would expect exactly the same to happen.

It seems there are two options

1. Deleting a translation removed all revisions of the language
2. Deleting a translation creates a new revision without the language

Option 2 seems to be the better solution but goes against how Drupal works on a single language site. If there was one language and we delete it the whole entities goes, we don't just created a new revision (although we should).

There is no difference between a single or multi language site. Consider having an entity with multiple translations and deleting the translations one after another resulting into an entity with a single translation and removing it at the end deletes the whole entity.

Lets see if there are some test failures if we create a new revision when removing a translation through the UI.

Re-rolling patch and try to update testing errors in patch.

This makes it more difficult for sites to comply with GDPR as it can be difficult to remove unlawful personal data.

This makes it more difficult for sites to comply with GDPR as it can be difficult to remove unlawful personal data.

Well this depends on how you look at it. If we consider revisions as a backup, then everything is fine. We only have to ensure that the revisions cannot be accessed. If you still want to delete a specific translation from all revisions, then this is still possible even with the current issue:

$entity_type_id = 'xyz';
$entity_id = 1092;
$remove_translation_langcode = 'abc';
$entity_type_manager = \Drupal::entityTypeManager();
$entity_type = $entity_type_manager->getDefinition($entity_type_id);
$storage = $entity_type_manager->getStorage($entity_type_id);
$revisions = $storage->getQuery()
  ->condition($entity_type->getKey('id'), $entity_id)
  ->condition($entity_type->getKey('langcode'), $remove_translation_langcode)
  ->allRevisions()
  ->execute();
foreach (array_keys($revisions) as $revision_id) {
  $revision = $storage->loadRevision($revision_id);
  $revision->removeTranslation($remove_translation_langcode);
  $revision->save();
}

If desired we probably could provide an API method which will perform those steps e.g.\Drupal\Core\Entity\TranslatableRevisionableStorageInterface::removeTranslationFromAllRevisions($entity_id, $langcode)

Should we cover this in the current issue and do we really need such a method?

If we consider revisions as a backup, then everything is fine. We only have to ensure that the revisions cannot be accessed

I don't think this is the case. If the revision contains personal information that should never have been added, or the information that a user have the right to get removed, it doesn't really matter if it is published or not. To comply with GDPR the data need to be irreversible deleted. Here is an issues with a similar problem #2945956: Allow removing translations in pending revisions

straight up reroll.

I'm observing the orphaned revisions in 8.7.10 using a default language and a second language.

patch seems to help. However, what I'm really trying to figure out is why there's a new english revision when I only updated the translation.

The reason why I started looking for a patch is because I'm trying to resolve a bug with forward draft revisions in a second language when using access_unpublished the non-default language access key doesn't bypass the access , and if I force it, the english revision is shown, I debugged deeply, and came accross the orphaned revisions, and was looking into why content moderation seems to behave this way. I haven't found a patch that satisfies yet.

node_revisions only ever has the default language revision, node_field_revision has the other language and default language.

node_field_revisions has twice as many entries as node_revision

select nid, vid, langcode, revision_uid, revision_log from node_revision where nid = 7460 order by vid desc ;

+------+-------+----------+--------------+---------------------------------------------------------------------------------+
| nid  | vid   | langcode | revision_uid | revision_log                                                                    |
+------+-------+----------+--------------+---------------------------------------------------------------------------------+
| 7460 | 76668 | en       |            1 | Copy of the revision from <em class="placeholder">lun, 11/25/2019 - 05:05</em>. |
| 7460 | 76667 | en       |            1 | Copy of the revision from <em class="placeholder">Mon, 11/25/2019 - 05:05</em>. |
| 7460 | 76659 | en       |            1 | Bulk operation create draft revision french                                     |
| 7460 | 76658 | en       |            1 | Bulk operation create draft revision                                            |
| 7460 | 76657 | en       |            1 | Bulk operation create archived revision french                                  |
+------+-------+----------+--------------+---------------------------------------------------------------------------------+

select nid,vid,langcode,status, title,uid,default_langcode,content_translation_source from node_field_revision where nid =7460 order by vid desc;

+------+-------+----------+--------+-----------------------------------+-----+------------------+----------------------------+
| nid  | vid   | langcode | status | title                             | uid | default_langcode | content_translation_source |
+------+-------+----------+--------+-----------------------------------+-----+------------------+----------------------------+
| 7460 | 76668 | en       |      1 | JOSEPH_ENpub                       |   1 |                1 | und                        |
| 7460 | 76668 | fr       |      1 | JOSEPH_FR                          |   1 |                0 | en                         |
| 7460 | 76667 | en       |      1 | JOSEPH_ENpub                       |   1 |                1 | und                        |
| 7460 | 76667 | fr       |      1 | JOSEPH_FR brouillon test français  |   1 |                0 | en                         |
| 7460 | 76659 | en       |      0 | JOSEPH_ENpub                       |   1 |                1 | und                        |
| 7460 | 76659 | fr       |      0 | JOSEPH_FR                          |   1 |                0 | en                         |
| 7460 | 76658 | en       |      0 | JOSEPH_ENpub                       |   1 |                1 | und                        |
| 7460 | 76658 | fr       |      0 | JOSEPH_FR                          |   1 |                0 | en                         |
| 7460 | 76657 | en       |      0 | JOSEPH_ENpub                       |   1 |                1 | und                        |
| 7460 | 76657 | fr       |      0 | JOSEPH_FR                          |   1 |                0 | en                         |
+------+-------+----------+--------+-----------------------------------+-----+------------------+----------------------------+

found a patch that might address what I am looking for
#2725523: Add a revision_parent field to revisionable entities

After adding the patch I could not see a decrease in entries in node_revision table after deleting translation.

Also worth mentioning that this issue also creates revision pages potentially listing 0 revisions (or less than it should) as the query is made against all revisions for node (even the orphaned ones). This fixes it https://www.drupal.org/project/drupal/issues/2916172

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

Did not test

But for tests requested in #7

This makes it more difficult for sites to comply with GDPR as it can be difficult to remove unlawful personal data.

Well this depends on how you look at it. If we consider revisions as a backup, then everything is fine. We only have to ensure that the revisions cannot be accessed. If you still want to delete a specific translation from all revisions, then this is still possible even with the current issue:

$entity_type_id = 'xyz';
$entity_id = 1092;
$remove_translation_langcode = 'abc';
$entity_type_manager = \Drupal::entityTypeManager();
$entity_type = $entity_type_manager->getDefinition($entity_type_id);
$storage = $entity_type_manager->getStorage($entity_type_id);
$revisions = $storage->getQuery()
  ->condition($entity_type->getKey('id'), $entity_id)
  ->condition($entity_type->getKey('langcode'), $remove_translation_langcode)
  ->allRevisions()
  ->execute();
foreach (array_keys($revisions) as $revision_id) {
  $revision = $storage->loadRevision($revision_id);
  $revision->removeTranslation($remove_translation_langcode);
  $revision->save();
} 

This doesn't seem to be true (anymore at least). I'm currently trying to clean up translations from old revisions. Removing a translation from a revision and calling save() on it results in a new revision being created with the translation removed. Instead of the translation being removed from the loaded revision.

I haven't yet found a way in which the Entity API allows me to remove a translation from a revision. Setting $revision->setNewRevision(FALSE); doesn't make it work either.

@marcusml, you need to set syncing flag to TRUE or a new revision will be created.

https://www.drupal.org/project/drupal/issues/2803717#comment-13080838