Project60 / org.project60.banking

CiviCRM banking extension
18 stars 38 forks source link

Extension crashes if another option value named "import" exists #346

Closed MarcMichalsky closed 2 years ago

MarcMichalsky commented 3 years ago

This API call returns not only the option value for the desired banking plugin type but also other option values with the name "import". https://github.com/Project60/org.project60.banking/blob/ea20fd3a5cd5df44f7acf0754039ee97c5aef569/extension/CRM/Banking/BAO/PluginInstance.php#L54-L57 The parameter group_id does not exist and is ignored so the call returns all option values that match the name.

array (
  'is_error' => 0,
  'version' => 3,
  'count' => 2,
  'values' => 
  array (
    1049 => 
    array (
      'id' => '1049',
      'option_group_id' => '62',
      'label' => 'Some other option value with the same name',
      'value' => '4',
      'name' => 'import',
      'filter' => '0',
      'is_default' => '0',
      'weight' => '4',
      'is_optgroup' => '0',
      'is_reserved' => '0',
      'is_active' => '1',
    ),
    1040 => 
    array (
      'id' => '1040',
      'option_group_id' => '123',
      'label' => 'Import plugin',
      'value' => '1',
      'name' => 'import',
      'filter' => '0',
      'is_default' => '0',
      'weight' => '10',
      'is_optgroup' => '0',
      'is_reserved' => '0',
      'is_active' => '1',
    ),
  ),
)

Because the result array contains more than one value, the following api call BankingPluginInstance.get cannot populate the 'plugin_type_id' parameter with an id and therefore returns all banking plugins – not only the importers. https://github.com/Project60/org.project60.banking/blob/ea20fd3a5cd5df44f7acf0754039ee97c5aef569/extension/CRM/Banking/BAO/PluginInstance.php#L59-L62

This behavior leads to an error when opening the import page. There, the plugin types get instantiated and called on $class::does_import_files(). Because this method does not exists in plugin classes which are not importers, the extension crashes. https://github.com/Project60/org.project60.banking/blob/ea20fd3a5cd5df44f7acf0754039ee97c5aef569/extension/CRM/Banking/Page/Import.php#L99-L100 Screenshot 2021-09-21 at 18-01-39 Error CiviCRM Sandbox on Drupal

Many thanks to @sebalis and @detsieber who found and solved this problem together with me.

PR is coming! 😉

MarcMichalsky commented 3 years ago

If you don't believe me, just try it yourself: Create an option value of any type and name it import. Then try to open the import page.