I finally tracked down this bug we've been having in the Payment UI for a while. At admin/commerce/config/payment we display two RulesPluginUI::overviewTables with different conditions in each one. The first shows enabled payment method rule configurations and the second disabled configurations. Randomly the disabled ones stopped showing... and this led me through a whole series of debug messages until I realized it didn't have anything to do with the value (i.e. does active == FALSE) but rather with which one came first. If I put the disabled on top, they showed just fine.
Turns out it was actually a caching issue tucked into EntityAPIController::load and EntityAPIController::getDefaults. The $defaultsFiltered property was getting set to TRUE the first pass through, but it wasn't specifying for which $conditions array it was doing so. This meant that my subsequent calls weren't attempting to get default configurations.
I was able to work around this by calling EntityAPIController::resetCache between my tables, but it seems like the $defaultsFiltered property could be made an array that stored values specific to the $conditions array (based on some sort of hash or something... I got it to work in a quick test by serializing the conditions array). Otherwise we just need to have documentation that states you must reset the cache between any two function calls that load entities and expect the default module-defined rule configurations to be present.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | rules_plugin.patch | 1.15 KB | fago |
Comments
Comment #2
fagohm, it does:
Thus subsequent default entities should be served from the static cache. So I don't see what's wrong here.
Maybe, you can figure out the steps needed to reproduce that? Perhaps you can do some with some basic rules_config_load_multiple() / entity_load calls?
Comment #3
rszrama commentedHmm... not sure, fago. The steps to reproduce are basically install Drupal Commerce 7.x-1.0-alpha2 and go to the payment settings screen where we have the two overview tables. I can try this again and dump the cache between tries to see what exactly is and isn't being cached, but for some reason or another clearing the cache between building those two tables resolved the issue I was having of missing rule configurations. : ?
My hunch in the original post was basically that getDefaults() is only being called the first time and filtering to active configurations based on my $conditions. The second time I have a different set of $conditions, but I don't see any filtering being done for default configurations outside of getDefaults(). And, based on the snippet you posted and lines 195-199, that means the second set of $conditions will never be applied somehow.
I can't try this again today, but I'll keep it in the queue. At the very least, we're working now. : )
Comment #4
fagoIf you have a look at cacheGet(), there the conditions are also applied. Also the code first gets all defaults, puts them into the cache and then it applies the conditions. So I don't see what's wrong here, but obviously something is.
Comment #5
fagoI've overhauled the CRUD controller a bit during the last days, so I simplified the loading logic. Could you please try whether your problem persists with the simplified logic? (=recent code) Thanks!
Comment #6
rszrama commentedHmm... so I seem to have a new problem. Default configurations aren't being recognized at all now, with no changes at all to my implementations of hook_default_rules_configuration(). Configurations that exist in the database are visible, but any default configurations that haven't been overridden cannot be seen in the UI. Trying to debug now... but it is late and I might need to crash. ; )
Comment #7
rszrama commentedAhh, you know what, this actually did work. I no longer have to clear the cache. However, as I mentioned above, there was another bug in that my default configurations weren't being detected at all. The problem was that the overview table checks for the condition that plugin => 'reaction rule'. But for default rule configurations, this plugin property is never set even though the rules_reaction_rule() function is used in hook_default_rules_configuration() as recommended in rules.api.php:
To get my default configurations to show up, I had to manually add
$rule->plugin = 'reaction rule';after the rule is created. It seems to me that the plugin name should automatically be added by rules_plugin_factory() since it's passed as an argument to this function anyways. What do you think?Comment #8
fagoarg, I recently removed the code initializing that value for defaults. I really need to add UI tests, thanks.
Anyway, $rule->plugin duplicates $rule->plugin(), so I prefer not having it at all. Attached patch implements the condition checking using the method, does that work for you?
Comment #9
fagoComment #10
rszrama commentedPatch works for me. Yay! : )
Comment #11
fagoGreat, committed.