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

dev/core#1806 - Fix import of "Radio"-style custom fields using option "label" #17632

Merged
merged 1 commit into from
Jun 16, 2020

Conversation

seamuslee001
Copy link
Contributor

@seamuslee001 seamuslee001 commented Jun 16, 2020

Overview

This fixes a regression when using the contribution importer with certain custom-data.

Custom fields of type "Radio" or "Select" have a list of acceptable options, and every acceptable option has a "value" (OptionValue.value) and "label" (OptionValue.label). During import, each input must be checked against the list of acceptable options.

See: https://lab.civicrm.org/dev/core/-/issues/1806

Before

When matching the imported datum to the list of acceptable options, it matches only by "value" (OptionValue.value).

After

When matching an imported datum to the list of acceptable options, it matches on either "value" or "label" (OptionValue.value or OptionValue.label). This reproduces the older behavior.

ping @eileenmcnaughton @totten

@civibot
Copy link

civibot bot commented Jun 16, 2020

(Standard links)

@civibot civibot bot added the 5.27 label Jun 16, 2020
@@ -671,6 +671,7 @@ private function deprecatedFormatParams($params, &$values, $create = FALSE, $onD
}
}
}
continue;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stops it it getting caught in the default case

@totten totten changed the title dev/core#1806 Fix issue with importing radio custom data field using … dev/core#1806 - Fix import of "Radio"-style custom fields using option "label" Jun 16, 2020
@totten
Copy link
Member

totten commented Jun 16, 2020

I've been r-running a bit on 5.25, 5.27 (without patch), and 5.27 (with patch) -- with two different import files. (The files include nearly identical contribution records, except the column for custom_6` aka "How long have you been a donor?" -- one file references items by value; the other by label.) As expected, the patch restores support for matching by label.

I must admit - from a structural/programmatic perspective, matching by value-only makes a lot of sense... but from a usability perspective, it is much more forgiving/useful for the importer to be flexible about matching on either.

@seamuslee001
Copy link
Contributor Author

I'm going to merge this based on @totten 's comment and that this has passed unit tests

@seamuslee001 seamuslee001 merged commit 6329872 into civicrm:5.27 Jun 16, 2020
@seamuslee001 seamuslee001 deleted the dev_core_1806 branch June 16, 2020 08:13
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