Complete Synchronization functionality for existing reference values

Created on 24 January 2017, about 8 years ago
Updated 11 May 2023, almost 2 years ago

There's a backend in place for syncing existing references. When you're on the edit form, or as an action in the dropdown button on the listing page, you should be able to synchronize a single corresponding reference config entity.

When that happens, it should loop through every entity that the corresponding reference applies to and create any missing corresponding references on the other end. Most of the functionality needed to do this is in place already.

πŸ“Œ Task
Status

Needs work

Version

5.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States bmcclure West Virginia

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

    Thanks @webflo great work! Came across πŸ› Update existing references on save Needs work should we perhaps incorporate that here?

  • πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

    Closed πŸ› Update existing references on save Needs work as duplicate of this!

    So what's needed here are tests and then it's ready for the go :)

  • πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

    @webflo could you perhaps provide the patch from #19 as MR for easier review?
    Anyone able to add the required tests so we can finish this?

  • First commit to issue fork.
  • Open on Drupal.org β†’
    Core: 10.0.7 + Environment: PHP 8.1 & MySQL 8
    9:21
    9:21
    Queueing
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.0.7 + Environment: PHP 8.1 & MySQL 8
    last update over 1 year ago
    2 pass
  • πŸ‡§πŸ‡ͺBelgium dieterholvoet Brussels

    I started a merge request based on 2846318-18.patch and I fixed a couple UI-related issues:

    • A 'Synchronize' local task already existed, but wasn't appearing on the edit form.
    • I added an entity operation to synchronize a CER.
    • The sync form currently deletes the CER, I changed this to actually synchronize it.
  • πŸ‡§πŸ‡ͺBelgium dieterholvoet Brussels
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.0.7 + Environment: PHP 8.1 & MySQL 8
    last update over 1 year ago
    2 pass
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.4 + Environment: PHP 8.1 & MySQL 8
    last update over 1 year ago
    2 pass
  • Status changed to Needs review over 1 year ago
  • πŸ‡«πŸ‡·France dqd London | N.Y.C | Paris | Hamburg | Berlin
  • πŸ‡«πŸ‡·France dqd London | N.Y.C | Paris | Hamburg | Berlin

    And has #22 πŸ“Œ Complete Synchronization functionality for existing reference values Needs work question for tests been addressed yet?

  • πŸ‡«πŸ‡·France dqd London | N.Y.C | Paris | Hamburg | Berlin

    TBH the whole "update on re-save" group of issues became a very confusing clutter of different issues with different patch attempts and is hard to follow for users. We need to clarify this. There was a patch already in the previously marked as duplicate issue linked to a patch from here which seemed to work in this time. Found a comment of me reviewing it 1,2 years ago. Strange enough. Then we closed the other one and moved over here and started to comment on this previous patch and that it is out of scope? Can somebody clarify where this previous issue has been moved to (comment #17 πŸ“Œ Complete Synchronization functionality for existing reference values Needs work ) and why we refactored the way to solve this here. We need a clear update on the issue summary of somebody who really oversees this whole clutter.

  • πŸ‡«πŸ‡·France dqd London | N.Y.C | Paris | Hamburg | Berlin

    Review of latest MR (14)

    Apart from that I initiated a new Drupal 10 test instance on lap (on the road atm) with latest CER dev and tried to resave related content before and after adding the merge #14 as patch and it seem to work after applying the MR as patch without errors like expected. All previous content has been cross linked correctly on batch re-save. But I have no time & resources to further investigate any underlying flaws or drawbacks yet. But at least: It does not break anything obviously and it adds the awaited functionality. So far my review.

  • πŸ‡ΊπŸ‡ΈUnited States mortona2k Seattle

    @diqidoq messaged me on slack, so I'll clarify my thoughts here. When I read "Complete synchronization", I imagine it would fix all unsynced fields. I admit, the description talks about syncing a single item, so moving the patches to another issue may have been premature. However, I feel like having a way to re-sync all items would be a better way of handling this problem. Apologies for causing any confusion.

  • πŸ‡ΊπŸ‡¦Ukraine vlad.dancer Kyiv

    I've tested MR (14) too. Works good!
    But it took "ages" to sync 170K entities (and we have even more for other cer instances). I tested it on DigitalOcean premium amd ssd.
    My workaround was to increase number of items per batch operation:

      public function submitForm(array &$form, FormStateInterface $form_state) {
        $batch = new BatchBuilder();
        $batch->setTitle(t('Synchronizing the values for corresponding reference %label', [
          '%label' => $this->entity->label()
        ]));
    
        $idsToSync = $this->getEntityIdsToSync($this->entity);
        $i=0;
        foreach ($idsToSync as $entityTypeId => $ids) {
          foreach ($ids as $id) {
            $to[] = [$entityTypeId, $id];
            $i++;
            if ($i % 20 == 0) {
              $batch->addOperation([$this, 'doSyncEntity'], [$to]);
              $to = [];
            }
          }
        }
    
        batch_set($batch->toArray());
    
        $this->messenger()->addStatus($this->t('Corresponding reference %label has been synchronized.', [
          '%label' => $this->entity->label(),
        ]));
      }
    
      public function doSyncEntity(array $entities): void {
        foreach ($entities as [$entityTypeId, $entityId]) {
          $entity = $this->entityTypeManager
            ->getStorage($entityTypeId)
            ->load($entityId);
    
          cer_sync_corresponding_references($entity);
        }
      }
    

    What do you think about making chunk size adjustible?

    Also I think in our case we know more or less how many nodes we need to sync (between X and Y days) so I'm thinking to write VBO plugin β†’ to use views then for filtering and avoiding full cer re-sync.

  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.2.1 + Environment: PHP 8.1 & MySQL 8
    last update about 1 year ago
    2 pass
  • πŸ‡ΊπŸ‡ΈUnited States mortona2k Seattle

    I added a redirect to the CER edit form for the item being synchronized when accessed via the tab.

    When accessed via the operation link in the CER listing, it adds a destination url parameter, so it redirects to the list.

    The code looks good to me and is working well.

    I tried to drop in @vlad.dancer's code to increase the batch size, but it changes the output in the queue to say "completed 4/4". I like the idea of a bigger default, but I think we should take it further and polish the queue so it displays the number of items in progress, with a message of the summary.

    I think we should set the batch size in CER config, and a vbo plugin would be a nice addition to handle your case of filtering out items before processing.

    I would set this to RTBC, but it looks like tests are needed before committing.

  • πŸ‡«πŸ‡·France dqd London | N.Y.C | Paris | Hamburg | Berlin

    Will test if this MR works with πŸ“Œ Automated Drupal 11 compatibility fixes for cer RTBC or if this can be merged. Would be good if we can reach a certain RTBC and commit state since parts of CER are just unusable right now without this patch in a per-existing (not new) project environment.

  • πŸ‡¨πŸ‡¦Canada chrisck BC, Canada

    Just reporting back that the MR !14 is working great for me, but I have nowhere near the number of entities as @vlad.dancer.

  • πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

    Happy to merge this, once it has tests and is reviewed by experienced members of the community. Won't have the time to review this myself in the near future.

  • πŸ‡ΊπŸ‡ΈUnited States SocialNicheGuru

    It does not apply to cer 5.0.0-beta4

  • Pipeline finished with Failed
    2 months ago
    Total: 163s
    #404301
Production build 0.71.5 2024