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

Cannot override an option in feedOptions with a value of false #1352

Closed
trevor-cuffe opened this issue Jul 20, 2023 · 3 comments
Closed

Cannot override an option in feedOptions with a value of false #1352

trevor-cuffe opened this issue Jul 20, 2023 · 3 comments
Labels

Comments

@trevor-cuffe
Copy link

trevor-cuffe commented Jul 20, 2023

Description

In my feed-me.php config file, when setting a property for a specific feed in feedOptions, if I set the value to false, the value is not able to override the default value. This is specifically happening with compareContent, but I believe would happen with any other property if trying to set to false, null, 0, or other "empty" values.

Here is my config file:

return [
   '*' => [
       'cache' => 0,
       'feedOptions' => [
           '4' => [
               'compareContent' => false
           ]
       ]
   ]
];

The problem occurs in the getConfig method in craft\feedme\services\Service.php

    if ($feedId) {
        $configFeedItem = Hash::get($settings, 'feedOptions.' . $feedId . '.' . $key);

        if ($configFeedItem) {
            $configItem = $configFeedItem;
        }
    }

The problem is, if ($configFeedItem) will evaluate to false if the config option is set to a value of false, null, or 0, and will not be allowed to override the existing $configFeedItem variable. There needs to be an additional check to see if the property is set, so that a false or 0 value can be allowed.

I have only tested with compareContent, but I believe the same thing would happen if setting queueMaxRetry or backupLimit to 0, for example.

Steps to reproduce

  1. Create feed-me config file
  2. Set up feedOptions for an existing feed
  3. choose a property whose default value is true (eg. compareContent or logging), and set its value to false in the feedOptions
  4. Run the feed

Additional info

  • Craft version: 4.4.15
  • PHP version: 8.0.8
  • Database driver & version: MySQL 5.7.34
  • Plugins & versions: Feed Me 5.2.0

Tasks

Preview Give feedback
No tasks being tracked yet.
@trevor-cuffe
Copy link
Author

Testing locally, I believe changing the if condition on line 35 to the following will correct this issue:

if (isset($configFeedItem))

This allows values of 0 or false to override default settings, but ignores anything set to null or not defined.

@trevor-cuffe
Copy link
Author

Should be fixed by pending pull request #1353

@angrybrad
Copy link
Member

Thanks for that - went with d85c719 as it was safe to do with v4 - will also merge into v5.

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