Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Getting Tons of Junk Blocks When Saving Existing Entries #174

Closed
23d1 opened this issue Jan 16, 2019 · 18 comments
Closed

Getting Tons of Junk Blocks When Saving Existing Entries #174

23d1 opened this issue Jan 16, 2019 · 18 comments
Labels

Comments

@23d1
Copy link

23d1 commented Jan 16, 2019

I've come across an issue after updating to Craft 3.1 and Craft Neo 2.1.4 where saving an entry that had content prior generates a whole heap of empty blocks and duplicates of existing content. The only way to make that not happen is to remove all blocks and start over, which isn't feasible for a project I'm currently working on.

@mdominguez
Copy link

Experiencing this too...

@zroberts
Copy link

Experiencing same issue

@gregorydavidjenkins
Copy link

Same here. 4x existing blocks on save after updating to Craft 3.1 and Neo 2.1.4

@kylerits
Copy link

I'm experiencing the same issues as described above. I'm really hoping to avoid rolling back to Craft 3.0.37

@gregorydavidjenkins
Copy link

gregorydavidjenkins commented Jan 16, 2019

For what it's worth, rolling back NEO to a prior version didn't seem to help.

@ttempleton
Copy link
Contributor

This unfortunately seems to have been introduced with the official Craft 3.1 release; it was working fine with 3.1 beta 7. The 4x each block situation sounds like two block structures exist for that particular field/owner/site combination -- but there's definitely only one for each combination in my databases, so maybe it's getting loaded twice somehow... We will look into this and have a fix out as soon as possible.

@brandonkelly
Copy link
Contributor

@ttempleton Let me know if you can isolate a Craft bug introduced in 3.1.0 that’s causing this.

@brandonkelly
Copy link
Contributor

Structures gained soft-delete support in 3.1.0 (didn’t make it into any of the beta releases), so that could have something to do with it, if there are soft-deleted structures in the database and Neo is manually querying that table without accounting for the new dateDeleted column.

@kylerits
Copy link

Is there an ETA on this bug fix?

@Mosnar
Copy link

Mosnar commented Jan 16, 2019

I'm looking into this as well since it's blocking progress on a project. It looks like the main block query needs to join craft_structures to check for soft-deletes on both the outer query and subquery. Here's the corrected query that needs to be implemented.

SELECT `elements`.`id`,
       `elements`.`fieldLayoutId`,
       `elements`.`uid`,
       `elements`.`enabled`,
       `elements`.`archived`,
       `elements`.`dateCreated`,
       `elements`.`dateUpdated`,
       `elements_sites`.`slug`,
       `elements_sites`.`uri`,
       `elements_sites`.`enabled` AS `enabledForSite`,
       `neoblocks`.`fieldId`,
       `neoblocks`.`ownerId`,
       `neoblocks`.`ownerSiteId`,
       `neoblocks`.`typeId`,
       `content`.`id`             AS `contentId`,
       <my fields redacted>
       `structureelements`.`root`,
       `structureelements`.`lft`,
       `structureelements`.`rgt`,
       `structureelements`.`level`,
       `structureelements`.`structureId`
FROM (SELECT `elements`.`id` AS `elementsId`, `elements_sites`.`id` AS `elementsSitesId`, `content`.`id` AS `contentId`
      FROM `craft_elements` `elements`
             INNER JOIN `craft_neoblocks` `neoblocks` ON `neoblocks`.`id` = `elements`.`id`
             INNER JOIN `craft_elements_sites` `elements_sites` ON `elements_sites`.`elementId` = `elements`.`id`
             INNER JOIN `craft_content` `content` ON `content`.`elementId` = `elements`.`id`
             LEFT JOIN `craft_structureelements` `structureelements`
                       ON `structureelements`.`elementId` = `elements`.`id`
             **LEFT JOIN `craft_structures` `structures` ON `structureelements`.`structureId` = `structures`.`id`**
      WHERE (`neoblocks`.`fieldId` = '130')
        AND (`neoblocks`.`ownerId` = '2992')
        AND (`elements_sites`.`siteId` = 1)
        AND (`content`.`siteId` = 1)
        AND (`elements`.`archived` = FALSE)
        AND (`elements`.`dateDeleted` IS NULL)
        AND (`structures`.`dateDeleted` IS NULL)
      ORDER BY `structureelements`.`lft`, `elements`.`dateCreated` DESC) `subquery`
       INNER JOIN `craft_neoblocks` `neoblocks` ON `neoblocks`.`id` = `subquery`.`elementsId`
       INNER JOIN `craft_elements` `elements` ON `elements`.`id` = `subquery`.`elementsId`
       INNER JOIN `craft_elements_sites` `elements_sites` ON `elements_sites`.`id` = `subquery`.`elementsSitesId`
       INNER JOIN `craft_content` `content` ON `content`.`id` = `subquery`.`contentId`
       LEFT JOIN `craft_structureelements` `structureelements`
                 ON `structureelements`.`elementId` = `subquery`.`elementsId`
       **LEFT JOIN `craft_structures` structures on `structureelements`.`structureId` = `structures`.`id`
              WHERE structures.dateDeleted IS NULL**
ORDER BY `structureelements`.`lft`, `elements`.`dateCreated` DESC

@Mosnar
Copy link

Mosnar commented Jan 17, 2019

I've implemented the above fix and had success on my local install. I will submit a PR tomorrow morning for proper review. In case I die before then, this method needs to be added to the BlockQuery class:

    /**
     * @inheritdoc
     */
    protected function afterPrepare(): bool
    {
        $this->subQuery->leftJoin('{{%structures}} structures', '[[structureelements.structureId]] = [[structures.id]]');
        $this->query->leftJoin('{{%structures}} structures', '[[structureelements.structureId]] = [[structures.id]]');
        $this->query->andWhere(['structures.dateDeleted' => null]);
        $this->subQuery->andWhere(['structures.dateDeleted' => null]);
        return parent::afterPrepare();
    }

@ttempleton
Copy link
Contributor

Thanks @Mosnar, the effort is much appreciated; however, we've been investigating and found an issue where the structureId wasn't being set on a block query. That doesn't seem to have caused any problems until now, and clearly the addition of the dateDeleted column as noted by @brandonkelly has affected the returning of blocks, but I suspect at the moment that the addition of dateDeleted has just triggered a bug in Neo that already existed. In our testing so far, ensuring that the query's structureId is set appears to fix the issue.

We'll continue to test both our changes and yours, and have a release out by the end of today either way. If there are no problems with our changes, though, then we're likely to go with ours, as our changes will not break Craft 3.0 compatibility, which we'd prefer to maintain for the moment, until project config support is added to Neo 2.2.

Our fix involves updating two files. Firstly, the beforePrepare() method in elements/db/BlockQuery.php is altered by moving the block structure check out of the $isSaved check, and also removing the check for the query's ownerSiteId since it will be null for fields that do not manage their blocks on a per-site basis:

if ($isSaved)
{
	foreach (['fieldId', 'ownerId', 'ownerSiteId'] as $idProperty)
	{
		if (!$this->$idProperty)
		{
			$this->$idProperty = (new Query())
				->select([$idProperty])
				->from(['{{%neoblocks}}'])
				->where(['id' => $this->id])
				->scalar();
		}
	}
}

if (!$this->structureId && $this->fieldId && $this->ownerId)
{
	$blockStructure = Neo::$plugin->blocks->getStructure($this->fieldId, $this->ownerId, $this->ownerSiteId);

	if ($blockStructure)
	{
		$this->structureId = $blockStructure->structureId;
	}
}

Field.php is also altered in the normalizeValue() method at line 300 by setting the query's ownerSiteId if the field manages its blocks on a per-site basis:

if ($this->localizeBlocks)
{
	$query->ownerSiteId($element->siteId ?? null);
}

@mdominguez
Copy link

Don't know if this has been confirmed yet but I can tell you that the error persists in a project that's not even on Craft 3.1, but does when updating Neo to 2.1.4. Downgrading back to Neo 2.1.2 now as this is a fairly critical issue..

@ttempleton
Copy link
Contributor

@mdominguez if possible, can you please apply the changes as noted in my previous comment and let me know if that resolves the issue?

If that does not resolve the issue, can you please confirm the following:

  • Craft version
  • Whether this is a multi-site installation
  • If yes to the above, whether the affected fields are set to manage blocks on a per-site basis
  • Whether the database's neoblockstructures table has multiple rows for a given field/owner/site combination (this should not be the case but would definitely cause duplicated blocks)
  • Any third-party fields that are being used by the affected Neo fields

@ttempleton ttempleton added the bug label Jan 17, 2019
@kylerits
Copy link

The fix from @ttempleton above seems to have worked for me.

Craft version 3.1.1

@ttempleton
Copy link
Contributor

Thanks @kylerits! Everything seems fine with our testing of the structure ID fix, too, so I've just released Neo 2.1.5 and at this stage I'll close this issue. If anyone does continue to experience this issue with Craft 3.1 and Neo 2.1.5, please let us know and we'll reopen.

@mdominguez, if we don't get any further reports of this issue with Craft 3.1 / Neo 2.1.5 and you still experience issues with Neo 2.1.5, then it is likely unrelated to this issue (even if it presents in a similar way). If that's the case, can you please submit a new issue report with the information requested in my previous comment.

@gregorydavidjenkins
Copy link

gregorydavidjenkins commented Jan 17, 2019

Just a quick note to confirm that 2.1.5 fixed the issue for me. Thank you!

@mdominguez
Copy link

@ttempleton Thanks, haven't got the time yet to update the site again where it backfired. Will do that asap :) thanks for the quick updates btw!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants