[Test] [Ref] [Import]Add wrapper class for importProcessor #15028
Conversation
|
(Standard links)
|
CRM/Import/ImportProcessor.php
Outdated
| * @return CRM_Contact_Import_Parser_Contact | ||
| */ | ||
| public function getImporterObject() { | ||
| $importer = new CRM_Contact_Import_Parser_Contact($this->getFieldNames()); |
There was a problem hiding this comment.
Actually we want to pass more params here but I wanted to do that as a follow up
eileenmcnaughton
force-pushed
the
import_processor
branch
2 times, most recently
from
d64c2be to
258d775
Compare
With the export cleanup creating a new sensible class & migrating code to it turned out to be a good approach. This adds a class that wraps around the Importer Object, taking the mapping_field db format of fields as a format - this is also where we wound up with the Export - using that format as it is what is saed. There is a gap as I noted in https://lab.civicrm.org/dev/core/issues/1172 around the schema's adequacy. That is not such an issue for this - in that we are not dealing with Contribution import and this is really just exposing the wrapper for the unit tests at this stage - the only change to 'live' code is a little less php v4 support - we have removed a bunch of these from the constructor of the other objects already with no observed issues
eileenmcnaughton
force-pushed
the
import_processor
branch
from
258d775 to
0b0285b
Compare
|
test this please |
|
I just pushed up a 'big picture' on how this class can simplify - https://github.com/civicrm/civicrm-core/pull/15034/files#diff-238a76c67a9fdd3201a7917ddfe7b089 - I'll pick through that for the next 'real' pr once this is merged |
| @@ -122,11 +122,11 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Contact_Import_Parser { | |||
| * @param array $mapperRelatedContactWebsiteType | |||
| */ | |||
| public function __construct( | |||
| &$mapperKeys, $mapperLocType = [], $mapperPhoneType = [], $mapperImProvider = [], $mapperRelated = [], $mapperRelatedContactType = [], $mapperRelatedContactDetails = [], $mapperRelatedContactLocType = [], $mapperRelatedContactPhoneType = [], $mapperRelatedContactImProvider = [], | |||
There was a problem hiding this comment.
@eileenmcnaughton are you sure we don't need to modify the MapperKeys?
There was a problem hiding this comment.
I'm pretty sure - I've been digging around these classes a bit & it seems to just be a phpv4 hangover - we've removed the & from a bunch already
There was a problem hiding this comment.
To work backwards - the calling class would need to do something to change them for this to make sense - but actually it is just a list of field names submitted on the form
There was a problem hiding this comment.
@seamuslee001 Specifically it's called from
CRM_Contact_Import_Form_MapField::submit
uses a variable to pass in & not again
CRM_Contact_Import_Form_DataSource::postProcess
Creates empty array to pass to constructor & then (as a non-reference) to run - just 2 lines later & not again
CRM_Contact_Import_Form_Preview::postProcessOld
This is never called.....
CRM_Contact_Import_ImportJob::runImport
The value is not used again.
Also this one of the calls to this function is called from runIncompleteImportJobs - which is never called....
|
confirmed that this is fine and that we don't need to be passing by array so merging and it adds test coverage |
|
thanks @seamuslee001 |
Overview
This adds a class to support future cleanup (along the lines of where we went with export) but at this stage it is only used by unit tests
Before
No 'logical' import class exists
After
Class exists as baby step towards retrofitting sanity
Technical Details
With the export cleanup creating a new sensible class & migrating code to it turned out to be a good approach.
This adds a class that wraps around the Importer Object, taking the mapping_field db format of fields
as a format - this is also where we wound up with the Export - using that format as it is what is saved.
There is a gap as I noted in https://lab.civicrm.org/dev/core/issues/1172 around the schema's adequacy.
That is not such an issue for this - in that we are not dealing with Contribution import and
this is really just exposing the wrapper for the unit tests at this stage but it does warrant further thought/discussion.
The only change to
'live' code is a little less php v4 support - we have removed a bunch of these from the constructor of the other objects already with no observed issues
Comments
@colemanw this picks up on your idea of how to handle making export sane - ie. treating the 'civicrm_mapping_field' table format as the preferred formatting to use