fix #346 | wrong parameter name for option group id

[MarcMichalsky]

fix #346

Thanks to @sebalis and @Detsieber for working this out with me! 💪

It looks like the entire call to OptionGroup.get for receiving the option group id is unnecessary, so we removed that. https://github.com/Project60/org.project60.banking/blob/ea20fd3a5cd5df44f7acf0754039ee97c5aef569/extension/CRM/Banking/BAO/PluginInstance.php#L50-L52

We changed the OptionValue.get call to an OptionValue.getsingle call to make sure it can only return one value. Also we renamed the group_id parameter to option_group_id and passed the hardcoded string for the plugin classes.

// find the correct plugin type
$import_plugin_type = civicrm_api3('OptionValue', 'getsingle', array(
    'name'     => $type_name,
    'option_group_id' => 'civicrm_banking.plugin_classes'));

[jensschuppe]

Since getsingle throws an exception on failure (other than get), this might introduce different behavior. Let's keep get although the ID is only present when there's only one result.

[MarcMichalsky]

Since getsingle throws an exception on failure (other than get), this might introduce different behavior. Let's keep get although the ID is only present when there's only one result.

Hi @jensschuppe, keeping the get call could still lead to the same error as described in #346. If there is more than one result, the result array will not contain the ID referenced in this line: https://github.com/Project60/org.project60.banking/blob/4312dfffdb000c3139eb1df7c948056042e05d65/extension/CRM/Banking/BAO/PluginInstance.php#L57 The call to OptionValue then returns all option values with that name, which crashes the import page.

Of course, if there is more than one result, the getsingle call will also produce an error. But maybe the error message is easier to interpret then.

[MarcMichalsky]

Okay, I should read your comments more carefully. You just wrote that:

Let's keep get although the ID is only present when there's only one result.

Your decision. ☺️

[jensschuppe]

You are, of course, correct with all that, but @bjendres didn't want an exception being thrown. I also think that this error is theoretical because there can't be two option values with the same name in the same option group. At least that's not desired at all.

[MarcMichalsky]

I also think that this error is theoretical because there can't be two option values with the same name in the same option group.

Yes, that's right.

[bjendres]

thanks @MarcMichalsky