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

"Disable globally" not working if element isn't in primary site #1468

Closed
tricki opened this issue Jun 21, 2024 · 2 comments
Closed

"Disable globally" not working if element isn't in primary site #1468

tricki opened this issue Jun 21, 2024 · 2 comments
Assignees
Labels

Comments

@tricki
Copy link

tricki commented Jun 21, 2024

Description

We have an existing import to a non-primary site, which is working fine, except for disabling the elements that no longer exist in the import ("Disable missing elements globally"). In the log, in the last step, there's always this error: Attempt to read property "enabled" on null.

image

The error occurs on this line: https://github.com/craftcms/feed-me/blob/5.5.0/src/base/Element.php#L215

That method:

public function disable($elementIds): bool
{
    /** @var CraftElementInterface|string $class */
    $class = $this->getElementClass();
    $elementsService = Craft::$app->getElements();

    foreach ($elementIds as $elementId) {
        /** @var BaseElement $element */
        $element = $elementsService->getElementById($elementId, $class);
        if ($element->enabled) {
            $element->enabled = false;
            $elementsService->saveElement($element, true, true, Hash::get($this->feed, 'updateSearchIndexes'));
        }
    }

    return true;
}

This is trying to get the element to disable it. But because the $siteId isn't passed to getElementById (the third argument), it tries to find it in the primary site. But the problem is the elements we're importing aren't in the primary site.

I fixed it by passing "*" as the site id. Since the feed option includes the word "globally", and the same class has another method called disableForSite(), I think "*" is the right choice.

- $element = $elementsService->getElementById($elementId, $class);
+ $element = $elementsService->getElementById($elementId, $class, '*');

Steps to reproduce

  1. Create a new site with at least one element type
  2. Create an import for that site and entry type with "Disable missing elements globally" enabled.
  3. Manually create an entry
  4. Run an empty import
  5. The entry should get disabled, instead there's an error.

Additional info

  • Craft version: 4.5.13
  • PHP version: 8.1.20
  • Database driver & version: MariaDB 10.5.13
  • Plugins & versions: Feed Me 5.3.0
@i-just
Copy link
Contributor

i-just commented Jun 28, 2024

Hi, thanks for reporting and the very detailed description - much appreciated! I raised a PR for this.

@tricki
Copy link
Author

tricki commented Jul 9, 2024

Thanks for the quick fix. I just tested 5.x-dev and can confirm that disabling now works.

@tricki tricki closed this as completed Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants