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

[REF] minor code refactor on import - Pass ProcessorObject into loadSavedMapping & use it to load the formName #15068

Merged
merged 1 commit into from
Aug 22, 2019

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Small change towards a larger cleanup

Before

Long list of params

After

Still a long list of params but $formName replaced by processor which can later replace all the rest.

Technical Details

After a few more steps we can eliminate most of this function & use the processor - baby step towards #15034

Comments

@seamuslee001

@civibot
Copy link

civibot bot commented Aug 18, 2019

(Standard links)

@civibot civibot bot added the master label Aug 18, 2019
@@ -396,12 +400,16 @@ public function buildQuickForm() {
$formName = 'document.forms.' . $this->_name;
//used to warn for mismatch column count or mismatch mapping
CRM_Core_Session::singleton()->setStatus(NULL);
$processor = new CRM_Import_ImportProcessor();
$processor->setMappingID($savedMappingID);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not actually using mappingId & metadata yet but it seemed like the 'right sized bite' to include these sets in this pr

@eileenmcnaughton eileenmcnaughton force-pushed the mapping_test branch 2 times, most recently from f19a2d1 to 0405fa6 Compare August 18, 2019 08:59
@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 this is passing now - can you merge it - I'm keen to get the full processor class in (this brings in more but not all) so I can start switching to it with tests.

eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Aug 19, 2019
This adds the full ImportProcessor class from the WIP pr civicrm#15034 - minus the functions that
replace loadSavedMapping) along with tests. It does NOT start to use the new class as yet.

Note that civicrm#15068 will need rebasing if this is merged first & vice versa

The goal is to bring it into use along with extending tests but this is 'safe' in that no funcitonal code is changed
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Aug 19, 2019
This adds the full ImportProcessor class from the WIP pr civicrm#15034 - minus the functions that
replace loadSavedMapping) along with tests. It does NOT start to use the new class as yet.

Note that civicrm#15068 will need rebasing if this is merged first & vice versa

The goal is to bring it into use along with extending tests but this is 'safe' in that no funcitonal code is changed
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Aug 19, 2019
This adds the full ImportProcessor class from the WIP pr civicrm#15034 - minus the functions that
replace loadSavedMapping) along with tests. It does NOT start to use the new class as yet.

Note that civicrm#15068 will need rebasing if this is merged first & vice versa

The goal is to bring it into use along with extending tests but this is 'safe' in that no funcitonal code is changed
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Aug 19, 2019
This adds the full ImportProcessor class from the WIP pr civicrm#15034 - minus the functions that
replace loadSavedMapping) along with tests. It does NOT start to use the new class as yet.

Note that civicrm#15068 will need rebasing if this is merged first & vice versa

The goal is to bring it into use along with extending tests but this is 'safe' in that no funcitonal code is changed
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Aug 19, 2019
This adds the full ImportProcessor class from the WIP pr civicrm#15034 - minus the functions that
replace loadSavedMapping) along with tests. It does NOT start to use the new class as yet.

Note that civicrm#15068 will need rebasing if this is merged first & vice versa

The goal is to bring it into use along with extending tests but this is 'safe' in that no funcitonal code is changed
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Aug 19, 2019
This adds the full ImportProcessor class from the WIP pr civicrm#15034 - minus the functions that
replace loadSavedMapping) along with tests. It does NOT start to use the new class as yet.

Note that civicrm#15068 will need rebasing if this is merged first & vice versa

The goal is to bring it into use along with extending tests but this is 'safe' in that no funcitonal code is changed
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Aug 19, 2019
This adds the full ImportProcessor class from the WIP pr civicrm#15034 - minus the functions that
replace loadSavedMapping) along with tests. It does NOT start to use the new class as yet.

Note that civicrm#15068 will need rebasing if this is merged first & vice versa

The goal is to bring it into use along with extending tests but this is 'safe' in that no funcitonal code is changed
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Aug 19, 2019
This adds the full ImportProcessor class from the WIP pr civicrm#15034 - minus the functions that
replace loadSavedMapping) along with tests. It does NOT start to use the new class as yet.

Note that civicrm#15068 will need rebasing if this is merged first & vice versa

The goal is to bring it into use along with extending tests but this is 'safe' in that no funcitonal code is changed
After a few more steps we can eliminate most of this function & use the processor but this is jusut a very small
change....
@colemanw
Copy link
Member

Merging based on code review & passing tests.

@colemanw colemanw merged commit 5c15019 into civicrm:master Aug 22, 2019
@colemanw colemanw deleted the mapping_test branch August 22, 2019 00:41
@eileenmcnaughton
Copy link
Contributor Author

thanks @colemanw

magnolia61 pushed a commit to magnolia61/civicrm-core that referenced this pull request Oct 13, 2019
This adds the full ImportProcessor class from the WIP pr civicrm#15034 - minus the functions that
replace loadSavedMapping) along with tests. It does NOT start to use the new class as yet.

Note that civicrm#15068 will need rebasing if this is merged first & vice versa

The goal is to bring it into use along with extending tests but this is 'safe' in that no funcitonal code is changed
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