Right now, configured tamper plugins can be overwritten easily, because the primary key is only based on the id. From the current configuration logic I would suggest to also use importer, source and plugin_id (whould require changes to the load routines) or to automatically assign a more complex id using the items noted above.

Comments

digi24’s picture

digi24 commented
Status: Active » Closed (fixed)

sorry, just noticed my mistake

digi24’s picture

digi24 commented
Status: Closed (fixed) » Needs review

Just saw how machine_name works, so my suggestion would be to add the same logic to the manually entered Ids.

Suggestion attached:


Index: feeds_tamper_ui/feeds_tamper_ui.admin.inc
===================================================================
--- feeds_tamper_ui/feeds_tamper_ui.admin.inc   (revision 2382)
+++ feeds_tamper_ui/feeds_tamper_ui.admin.inc   (working copy)
@@ -252,6 +252,9 @@
     }
     else {
       unset($form_state['values'][$plugin_id]['id']);
+      $form_state['storage']['id'] = $form_state['importer']->id .
+      '_' . feeds_tamper_make_machine($form_state['source']) .
+      '_' . $form_state['storage']['id'];
     }
   }
 }
twistor’s picture

twistor commented
Assigned: Unassigned » twistor
Status: Needs review » Needs work

Naming conventions do need some work. Also, the generated id's will cause problems sometimes when the generated names are too simplistic. Try adding two plugins of type change_case:uppercase. In most cases, that would be pointless anyway but the names need to be checked for conflict on creation and maybe append a number on the end. I'm definitely open to ideas. I like the idea of automatic generation for machine names but the fact that it doesn't work well across the board makes it messy.

twistor’s picture

twistor commented
Status: Needs work » Fixed

This is fixed. All machine names are converted to importer_id-source_element-plugin_id. Also, if an id conflicts, an error message is shown.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.