Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Import] [Ref] Iterate through the mapping of fields #23384

Merged
merged 1 commit into from
May 6, 2022

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented May 5, 2022

Overview

[Import] [Ref] Iterate through the mapping of fields

Before

The process for mapping fields is crazy complex.....

The form layer has to take the submitted value for 'mapper' and split it into 8 arrays ie

$fields = ['first_name', 'last_name', 'email', NULL];
$location_types = [NULL, NULL, 1, NULL];
$relatedContactFieldName = [NULL, NULL, NULL, 'email'];
$relatedContactLocationType = [NULL, NULL, NULL, 1];

These arrays are passed to the field constructer and tinkered with a bit in init before being accessed in getActiveFieldMapping to determine how the row of values maps to a $params array

After

We already have a function that interprets what the mapper means into the format we use in civicrm_field_mapping so use that function to interpret rather than array-o-rama. The only thing we need to change is to interpret 'email with no location type' to 'email with location_type of "Primary" - everything else is already done in this function

Technical Details

The change involves updating tests to pass in the mapper - rather than doing weird contortions to imitate the forms.

We have good test cover - including a test specifically merged in preparation for this (testMapFields)

Comments

With this change most of the construct function & init is obsolete as well as a lot of code on the form layer - I've left that cleanup as a follow up for legibility

@civibot
Copy link

civibot bot commented May 5, 2022

(Standard links)

@civibot civibot bot added the master label May 5, 2022
@eileenmcnaughton eileenmcnaughton changed the title [Import] [Ref] Iterate through the mapping of fields rather than a 'c… [Import] [Ref] Iterate through the mapping of fields May 5, 2022
@eileenmcnaughton eileenmcnaughton force-pushed the import_active branch 4 times, most recently from 6e6f771 to 08e0163 Compare May 6, 2022 00:55
…ount'

The mapper holds an array of mappings for each of the active fields.
The active field count holds a count of this array - switching to
a foreach loop means we can stop calculating extra stuff.

This PR doesn't start ripping that out - for legibility reasons - but it
will take out a whole lotta code....
@eileenmcnaughton
Copy link
Contributor Author

@monishdeb if you get a chance to look this would be the highest priority of the import ones

@monishdeb
Copy link
Member

@monishdeb monishdeb merged commit 3ce7c1b into civicrm:master May 6, 2022
@eileenmcnaughton eileenmcnaughton deleted the import_active branch May 6, 2022 21:07
@eileenmcnaughton
Copy link
Contributor Author

Yay thanks @monishdeb

eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request May 6, 2022
This is the first round of follow up cleanup from civicrm#23384

These properties are no longer being used
so don't need to be passed in or wrangled
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants