- Issue created by @dalin
- First commit to issue fork.
- 🇺🇸United States johnle
Great job on the patch! Here are some bugs that I've updated to resolve from the previous patch
* Sometimes the revisions are not all deleted when a term is removed. I've just re-order the revision so that the main revision is deleted last that way it would not stop the revision from being deleted.
* SetSyncing still update the modified date to the most recent dates of the active revision on all languages, causing an issue with the content nove allowing to save with a message that it has been modified by another user. To fix this, used the code from the https://www.drupal.org/project/preserve_changed → to keep the modified dates untouched if we are not creating a revision.
* Minor coding standard fixes and comments. - Status changed to Needs work
over 1 year ago 10:09pm 31 July 2023 - 🇨🇦Canada dalin Guelph, 🇨🇦, 🌍
-
+++ b/entity_reference_purger.module @@ -9,8 +9,16 @@ use Drupal\Core\Entity\EntityInterface; +function preserve_changed_field_info_alter(array &$info): void {
This should be `entity_reference_purger_field_info_alter()`
-
+++ b/entity_reference_purger.module @@ -9,8 +9,16 @@ use Drupal\Core\Entity\EntityInterface; + $info['changed']['class'] = PreservedChangedItem::class;
What happens if both entity_reference_purger and preserve_changes modules are installed? Should ERP just require PC as a dependency?
-
+++ b/entity_reference_purger.module @@ -105,8 +113,7 @@ function entity_reference_purger_entity_delete(EntityInterface $entity) { + \Drupal::service('entity_reference_purger.process_entity_reference_items')->processentities($parent_entity, $field_name, $delta, $field_item);
Should be `processEntities()`
-
+++ b/src/Plugin/QueueWorker/EntityReferencePurgerWorker.php @@ -64,12 +74,12 @@ class EntityReferencePurgerWorker extends QueueWorkerBase implements ContainerFa + $this->entityReferencePurgerService->processentities($parent_entity, $field_name, $delta, $field_item);
Here too
-
+++ b/src/ProcessEntityReferenceItems.php @@ -0,0 +1,106 @@ + // Move the current revision last on the array so that it does not break + // the not stop revision from being able to stop saving due to no longer + // being the main revision active.
This comment doesn't quite make sense. Maybe:
The current revision can't be deleted until the rest have, so move it to the end of the array.
-
+++ b/src/ProcessEntityReferenceItems.php @@ -0,0 +1,106 @@ + $terms = $translated_entity->get($field_name)->getValue(); + foreach ($terms as $key => $tid) { + if ($tid['target_id'] === $field_item->target_id) { + $translated_entity->get($field_name)->removeItem($key); + $translated_entity->setSyncing(TRUE); + $translated_entity->changed->preserve = TRUE; + $translated_entity->save(); + } + }
This duplicated code should be moved into a protected method.
-
- 🇺🇸United States johnle
@dalin
Thanks for the code review, I'll get all recommended change in place, one thing I am kinda struggling on is when adding a dependencies to preserve_change module, I've ran into the following issue:
* When applying a composer patch, it did not install the require preserve_change change module, likely due to it running the composer install on the module first, and then later on patching it with the composer changes.+++ b/composer.json @@ -21,6 +21,7 @@ "source": "https://git.drupalcode.org/project/entity_reference_purger" }, "require": { - "drupal/core": "^8.8.0 || ^9.0 || ^10" + "drupal/core": "^8.8.0 || ^9.0 || ^10", + "drupal/preserve_changed": "8.8.0 || ^9.0 || ^10" } }
* Adding the dependencies in the .info.yml file, seem to give conflict. I've tried the suggestion of adding an additional line break, but still no luck.
+++ b/entity_reference_purger.info.yml @@ -2,3 +2,8 @@ name: 'Entity Reference Purger' description: 'Removes orphaned entity references when an entity is deleted.' type: module core_version_requirement: ^8.8.0 || ^9 || ^10 +dependencies: + - preserve_changed:preserve_changed
I think for the time being it might be easier to add the changedItem alter to the module, but use maybe a different key instead so it doesn't conflict with the preserve_change module properties key, what do you think?
- 🇨🇦Canada dalin Guelph, 🇨🇦, 🌍
@johnle
> When applying a composer patch, it did not install the require preserve_change change module, likely due to it running the composer install on the module first, and then later on patching it with the composer changes.
Yes, that's a limitation with Composer Patches that can't be avoided.
https://docs.cweagans.net/composer-patches/troubleshooting/non-patchable...
But I wonder if you could get around that by using an issue fork (see the info at the top of this page) rather than a .patch file. - 🇺🇸United States johnle
Thanks, I will try pushing the changes to once https://git.drupalcode.org/ is stable again.
- Merge request !1Issue #3348323: Entity Reference Purger is not workflow aware → (Open) created by johnle
- last update
over 1 year ago Fetch Error - 🇺🇸United States johnle
Ok I was able to get this working that resolve the 2 issues above
Added this above the drupal.org patckagelist{ "type": "composer", "url": "https://packages.drupal.org/8" },
with:
{ "type": "package", "package": { "name": "drupal/entity_reference_purger", "type": "drupal-module", "version": "dev-3348323-entity-reference-purger", "source": { "url": "https://git.drupalcode.org/issue/entity_reference_purger-3348323.git", "type": "git", "reference": "3348323-entity-reference-purger" }, "require": { "drupal/preserve_changed": "^2.0@alpha" } } }, .......
And ran:
composer require drupal/entity_reference_purger:dev-3348323-entity-reference-purger
It seem to installed the require dependencies. I'll need to retest this again with the replacement of the dependencies and the queuing bug that was reported from the patch from my team.
- last update
over 1 year ago Composer require failure - last update
over 1 year ago Composer require failure - 🇬🇧United Kingdom scott_euser
Updating issue summary to match the code which gets all revisions rather than just 'forward' revisions. Since the MR already covers the scenario of reverting to an older revision as well.