Currently the mapping form modifies mappings by adding/removing mappings and adding/removing unique flags. This makes it very inflexible as adding form actions requires writing functions akin to FeedsProcessor::addMapping() and using the form in a different context such as modifying mappings on a source requires us to replicate functions like the aforementioned FeedsProcessor::addMapping().

Goal: rewrite mapping form so that every submission sends the full mapping configuration.



This task is a prerequisite to #651478: Mapping on import.

Assigned:Unassigned» adityakg

Status:Active» Needs review

Setting this to needs review, it is tackled in #651478: Mapping on import-comment#39

Status:Needs review» Needs work

O, sorry, I just see this now - could you please break out this patch? We need to review and commit this separately, #651478 is just getting too complex.

Status:Needs work» Needs review
new5.98 KB

Hi alex,

Here is the patch. Basically what it does:
- Add $form['mappings'] in the mapping_form so it will be submitted to $form_state['values']['mappings']
- Replace addMapping(), removeMapping() and setUnique() in the mapping_form
- FeedsProcessor::addMapping is still called in the case that the FeedsProcessor wants to do anything else beside saving it to $config['mappings']

new5.98 KB

new5.94 KB

Sorry, please ignore the patch above, see this instead. I missed out changing some part when copying from #651478 patch.

Status:Needs review» Needs work

O, very nice.

We should be able to get away without any addMapping() calls: FeedsDataProcessor should implement addConfig() and setConfg(), iterate through all mapping settings and create fields for any 'new' mappings (if ($new == 'new') { ...)

Status:Needs work» Needs review
new9.83 KB

Removed addMapping() from FeedsDataProcessor and put that code inside loops in the addConfig and setConfig functions.

Assigned:adityakg» alex_b
new9.34 KB

Great to see this moving.

- Broke out duplicate code in setConfig() and addConfig() in FeedsDataProcessor
- Removed all references to addMapping()
- Consolidated $form['mappings'] and $form['#mappings']

Testing right now.

Status:Needs review» Fixed

This is committed. Thank you.

Status:Fixed» Closed (fixed)

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