🇮🇱Israel @yakoub

here is my code to delete previously published draft translation .
i welcome your feedback if indeed this creates problem in reverting .
for each revision along the stack three options :

1. there exists other affected translations, then only remove the intended language and sync revision
2. there aren't other affected, then delete the whole revision
3. if the target deleted translation is also original language then we can not delete it since it holds the untranslatable fields

namespace Drupal\content_administration\Form;

use Drupal\node\Form\NodeRevisionDeleteForm;
use Drupal\Core\Form\FormStateInterface;
use Drupal\node\NodeInterface;
use Drupal\views\Views;
use Drupal\Core\Entity\EntityChangesDetectionTrait;
use Drupal\content_administration\WasPublishedTrait;
use Drupal\Core\Language\LanguageInterface;

class NodeDraftStackForm extends NodeRevisionDeleteForm {

  use EntityChangesDetectionTrait;
  use WasPublishedTrait;

  public function getFormId() {
    return 'node_draft_stack_confirm';
  }

  public function getQuestion() {
    return $this->t('Are you sure to delete all unpublished revisions?');
  }

  public function buildForm(array $form, FormStateInterface $form_state, ?NodeInterface $node_revision = NULL, ?LanguageInterface $target_language = NULL) {
    $this->target_langcode = $target_language->getId();
    $this->wasPublishedStack($node_revision);
    $form = parent::buildForm($form, $form_state, $node_revision);
    return $form;
  }

  /**
   * {@inheritdoc}
   */
  public function submitForm(array &$form, FormStateInterface $form_state) {
    $revisions = $this->loadRevisionStack();
    $node_published_default = NULL;
    $languages = \Drupal::languageManager()->getLanguages();
    $target_language_name = $languages[$this->target_langcode]->getName();
    $messages = [];
    foreach ($revisions as $node_revision) {
      $affected = 0;
      foreach ($languages as $code => $language) {
        if ($this->target_langcode != $code && $node_revision->hasTranslation($code)) {
          if ($node_revision->getTranslation($code)->revision_translation_affected->value) {
            $affected++;
          }
        }
      }
      if ($affected > 0) {
        $node_revision_target = $node_revision->getTranslation($this->target_langcode);
        if ($node_revision_target->isDefaultTranslation()) {
          $this->resetOriginal($node_revision, $messages);
        }
        else {
          $this->removeTranslation($node_revision, $messages);
        }
      }
      else {
        $this->deleteRevision($node_revision, $messages);
      }
      $this->logMessages($messages, $node_revision, $target_language_name);
    }
    $this->redirectSubmit($form_state);
  }

  protected function loadRevisionStack() {
    $query = $this->nodeStorage->getQuery();
    $query
      ->accessCheck(FALSE)
      ->allRevisions();
    $query
      ->condition('nid', $this->revision->id())
      ->condition('vid', [$this->stack_published, $this->stack_top], 'BETWEEN')
      ->condition('langcode', $this->target_langcode)
      ->condition('status', 0);
    $vids = $query->execute();
    return $this->nodeStorage->loadMultipleRevisions(array_keys($vids));
  }

  protected function deleteRevision($node_revision, &$messages) {
    $this->nodeStorage
      ->deleteRevision($node_revision->getRevisionId());
    $messages['log'] = '@type: deleted @language revision %revision.';
    $messages['status'] = 'Revision %revision has been deleted.';
  }

  protected function removeTranslation($node_revision, &$messages) {
    $node_revision->removeTranslation($this->target_langcode);
    $node_revision->setSyncing(TRUE);
    $node_revision->save();
    $messages['log'] = '@type: removed translation @language from revision %revision.';
    $messages['status'] = 'Translation @language has been removed from %revision';
  }

  protected function resetOriginal($node_revision, &$messages) {
    $node_published_default = $this->nodeStorage->loadRevision($this->stack_published);
    foreach ($this->getResetFieldnames($node_revision) as $fieldname) {
      $field_value = $node_published_default->get($fieldname)->getValue();
      $node_revision->set($fieldname, $field_value);
    }
    $node_revision->setRevisionTranslationAffected(FALSE);
    $node_revision->setSyncing(TRUE);
    $node_revision->save();

    $messages['log'] =
      '@type: reset @language from revision %revision to published state.';
    $messages['status'] =
      'Original @language from %revision has been reset to published state';
  }

  protected function logMessages(&$messages, $node_revision, $target_language_name) {
    $this->logger('content')->info($messages['log'], [
      '@type' => $node_revision->bundle(),
      '@language' => $target_language_name,
      '%revision' => $node_revision->getRevisionId()
    ]);
    $this->messenger()
      ->addStatus($this->t($messages['status'], [
        '%revision' => $node_revision->getRevisionId(),
        '@language' => $target_language_name,
      ]));
  }

  protected function getResetFieldnames(NodeInterface $node) {
    static $reset_fields = [];
    if (!$reset_fields) {
      $skip_fields = $this->getFieldsToSkipFromTranslationChangesCheck($node);
      $skip_fields[] = 'changed';
      $reset_fields = array_diff(array_keys($node->getFieldDefinitions()), $skip_fields);
    }
    return $reset_fields;
  }

}

the root problem here is with the delete translation operation .

for me in project which i worked on, we have to chosen to implement different translation delete operation which pops out the whole stack of draft revision for a given language translation until it reaches the published latest affected translation or continue to the first ever revision

alternatively if we do decide to go the path of changing getActive (which i think the better approach)
then you can add condition here :
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...
after it checks $revision->wasDefaultRevision() && !$revision->isDefaultRevision()
set it is to null and it will return the latest revision .

  if ($entity instanceof TranslatableRevisionableInterface && $entity->isTranslatable()) {
      /** @var \Drupal\Core\Entity\TranslatableRevisionableStorageInterface $storage */
      $revision_id = $storage->getLatestTranslationAffectedRevisionId($entity->id(), $langcode);

      // If the latest translation-affecting revision was a default revision, it
      // is fine to load the latest revision instead, because in this case the
      // latest revision, regardless of it being default or pending, will always
      // contain the most up-to-date values for the specified translation. This
      // provides a BC behavior when the route is defined by a module always
      // expecting the latest revision to be loaded and to be the default
      // revision. In this particular case the latest revision is always going
      // to be the default revision, since pending revisions would not be
      // supported.
      $revision = $revision_id ? $this->loadRevision($entity, $revision_id) : NULL;
      if (!$revision || ($revision->wasDefaultRevision() && !$revision->isDefaultRevision())) {
        $revision = NULL;
      }
    }

the `_access_content_translation_manage` is used in multiple routes under `Drupal\content_translation\Routing\ContentTranslationRouteSubscriber`
changing the loaded revisions like you suggest will apply also to operations `edit` and `delete` which is not correct

if we do decide that indeed `load_latest_revision` is not working correct, then the change needs to happen at route definition by changing the ParamConverter from 'entity:node' to 'entity_latest:node' which will be custom converter extending `Drupal\Core\ParamConverter\EntityRevisionParamConverter` .

however, it seems to me that the problem not in behavior of getActive but the wrong implementation of revision-delete which creates new revision instead of actually deleting the revision trail like a pop stack operation.

the access handler should not decide on which revision to load, this is decided by entity repository getActive method .
the whole purpose of getActive is to make such decisions, so it is wrong to say getActive is not latestRevision because sometimes it is equal and sometimes not .
when getActive decides it must divert from the latestRevision, it is making this decision for good reason and any other function must respect this decision .

what you suggest is this : getActive have decided NOT to return the latest revision, but after you have already defined your route to call getActive then you override its decision and do enforce loading the latest revision .

you must not debate what access manager should result in, you need to debate what getActive needs to return under given scenario .
your description text that evolves around access checking makes it confusing to understand the underlying issue .

The access manager should use the latest revision when looking for existing translations, not the "active" revision, as already done by ContentTranslationController::add()

the referred to add method calls

$storage->getLatestTranslationAffectedRevisionId

and not the latest revision as in your code

storage->getLatestRevisionId

https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/conte...

the add method comment explains why this check is needed and it is very different reason than what you describe here

otherwise the initial form values may not be up-to-date.

i believe there is no logical connection between the add method and what you are trying to resolve, completely different scenarios .

maybe consider my comment once again ?
https://www.drupal.org/project/drupal/issues/2352009#comment-12814040 🐛 [pp-3] Bubbling of elements' max-age to the page's headers and the page cache Postponed

//This means we may need to update access records,
// flush some caches containing the entity or perform other operations we
// cannot possibly know about.

if we go back to this comment, may i suggest to add new method to `Drupal\group\Plugin\Group\Relation\GroupRelationInterface`
which takes care of invoking exactly the kind of hooks required for each entity instead of calling $entity->save() ?

released in beta3

i am refactoring revision save logic, once that done i will fix the phpcs .

current 2.0.x branch is ready for testing .
do you require backporting to 1.0.x ?

current branch 2.0.x has working version, but :
given the eck route for adding content is shared among different entity types, then the method of defining tabs will not work when one entity has translations pack enabled while another entity is not even defined as translatable .
the solution instead of tabs is to add link for entity operations to dedicated add content route which loads the translations pack .
this should be simple solution, but given i have fulltime job then it will need to wait several days maybe until the weekend .

this is working version, but the code still ugly and need fixing
https://git.drupalcode.org/project/translations_pack/-/commit/cb24240f81...

i refactored code to use entity handler, this way i can now continue to write custom handler for eck entities .
https://git.drupalcode.org/project/translations_pack/-/commit/a278a804df...

looks good .
i am also working on support for inline entity form .

it will be done this weekend .
it is just a routing derivative .

you are correct .
i am working on it .
the add new content route is defined in

src/Routing/TranslationsPackRouteSubscriber.php

i need to add support for eck or maybe define a new plugin type .

this module supports all entities .
simply enable content translation for your eck entities under path : admin/config/regional/content-language
and make sure to check "translations pack" like in the screenshot .
https://www.drupal.org/files/project-images/Screenshot%20from%202023-05-... →

new version symfony diverge in use of parameterbag->all(KEY)

It means that with a class like \Gmagick, the code should not include a use \Gmagick; line.

where does that happen in my code ? i still don't understand .

@jayprakash.kushwah thank you for you help .
but i don't think the lines removing the use statements are correct .
can you explain the issue with : Non-namespaced classes/interfaces/traits should not be referenced with use statements

have you tested your patch ? it takes more than version value to make it compatible with 10 .