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

Extend import option handling to prefix, suffix, language, communication_style #23535

Merged
merged 1 commit into from
May 24, 2022

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Extend import option handling to prefix, suffix, language, communication_style

This extends a pattern thoroughly tested for gender to
other single-option-value fields & removes duplicate handling.

Test cover in

CRM_Contact_Import_Parser_ContactTest::testPrefixLabel
CRM_Contact_Import_Parser_ContactTest::testPreferredLanguageImport

Before

Complicated handling

After

Leverages metadata based handling thoroughly tested for gender

Technical Details

Comments

@civibot
Copy link

civibot bot commented May 21, 2022

(Standard links)

@civibot civibot bot added the master label May 21, 2022
@@ -775,16 +775,6 @@ protected static function contactTrash($contact): bool {
*
*/
public static function resolveDefaults(&$defaults, $reverse = FALSE) {
// Hack for birth_date.
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 function is only called from the import parser. We already added extensive cover for birth_date handling so this can go along with the options

@@ -482,25 +485,6 @@ public function import($onDuplicate, &$values) {
//now we create new contact in update/fill mode also.
$contactID = NULL;
if ($createNewContact || ($this->_updateWithId)) {
// @todo - there are multiple places where formatting is done that need consolidation.
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 was added in conjuction with CRM_Contact_Import_Parser_ContactTest::testPreferredLanguageImport as a generic way to handle preferred_language labels. The same is already being achieved in the getTransformedFieldValue so this is now obsolete

@eileenmcnaughton eileenmcnaughton force-pushed the import_prefix branch 2 times, most recently from d8e2829 to 092ac7e Compare May 23, 2022 21:05
…ion_style

This extends a pattern thoroughly tested for gender to
other single-option-value fields & removes duplicate handling.

Test cover in

`CRM_Contact_Import_Parser_ContactTest::testPrefixLabel`
`CRM_Contact_Import_Parser_ContactTest::testPreferredLanguageImport`
@eileenmcnaughton
Copy link
Contributor Author

@colemanw this is passing post rebase now - are you able to merge? I haven't added additional tests because there was existing cover of the specific affected fields and the generic handling

@monishdeb
Copy link
Member

Did r-run on local and works fine, rightful change to cleanup the cruft code. Updated unit test passes too.

@monishdeb monishdeb merged commit 2212f90 into civicrm:master May 24, 2022
@eileenmcnaughton eileenmcnaughton deleted the import_prefix branch May 24, 2022 06:13
@eileenmcnaughton
Copy link
Contributor Author

Yay - thanks @monishdeb - now I have to rebase some others :-)

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