-
-
Notifications
You must be signed in to change notification settings - Fork 825
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#1204 Data import for custom membership fields is done as tex… #15122
dev/core#1204 Data import for custom membership fields is done as tex… #15122
Conversation
…t Instead of value
(Standard links)
|
@sushantpaste can we get a test on this CRM_Member_Import_Parser_MembershipTest has some to use as a starting point and there is a CustomDataTrait that will be helpful |
@@ -666,6 +666,17 @@ public function membership_format_params($params, &$values, $create = FALSE) { | |||
} | |||
} | |||
} | |||
if ($type == 'Autocomplete-Select') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sushantpaste @eileenmcnaughton
This looks like repetitive code and could be centralised in one place without having to do fixes in all components separately.
https://github.com/civicrm/civicrm-core/blob/master/CRM/Event/Import/Parser/Participant.php#L527
https://github.com/civicrm/civicrm-core/blob/master/CRM/Contribute/Import/Parser/Contribution.php#L666
so that all components correctly format custom data for all field types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yashodha I totally agree!
@sushantpaste @eileenmcnaughton This looks like repetitive code and could be centralised in one place without having to do fixes in all components separately. so that all components correctly format custom data for all field types. |
@sushantpaste Can we centralize the code for importing contact custom fields as well since that is also doing same thing? https://github.com/civicrm/civicrm-core/blob/master/CRM/Contact/Import/Parser.php#L878 |
@sushantpaste have you written a unit test for civi before? Is there something I can do to get you up & running writing them? |
@eileenmcnaughton Haven't added any tests previously can you help me with this. I think also there is no proper document for writing test. If you can help for writing tests for this time, that would be great and help to write tests in future. |
I'm very happy to help on this @sushantpaste - the first thing I would try to do is see if you can run them locally - I'd probably focus on running the CRM_Contact_Import_Parser_ContactTest - here https://github.com/civicrm/civicrm-core/pull/15190/files#diff-560b2cafc594a807893f61844ed09d24 I usually run them through my IDE (Php Storm) but you can use the cli - some info on it is here https://docs.civicrm.org/dev/en/latest/testing/phpunit/ - with --filter you should be able to run just that one class |
This test is probably a good starting point - although createCustomGroupWithFieldOfType() doesnt' support all types yet & might need extending
|
I did start the work of writing a test here #17560 I'm going to close this for now as it needs rebasing & a test to be reviewable |
Overview
Data import for custom membership fields is done as text Instead of value.
Before
Check the field in DB, the data is imported but stored as text not value - so this doesn't show up in the UI.
After
Check the field in DB, the data is imported and stored as value - so show up in the UI.
Comments
https://lab.civicrm.org/dev/core/issues/1204