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] [Import] move metadata calculations to a trait #15018

Merged
merged 1 commit into from
Aug 18, 2019

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Refactor to increase shared code

Before

Metadata passed around

After

Metadata retrieved from shared trait

Technical Details

A lot of the complexity of the import classes comes from complex & convoluted handling of metadata.

For reasons now obsolete the metadata is loaded & wrangled into various arrays on the first form and
then passed through storing the form in the session object to later forms (which don't have a shared parent).

We should be passing user form input around but not metadata as that can be retrieved as needed (and
cached for performance). This moves couple of functions out to the metadata and adds a couple of calls to them

  • more could be done but it would be hard going. Am also open to breaking this into 2 PRs so the first commit can be merged as a straight extraction per
    reviewer preference

Comments

@civibot
Copy link

civibot bot commented Aug 12, 2019

(Standard links)

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 I've updated this to be much closer to a straight extraction for easier review - I've kept the helper functions but removed the calls to them for now

This metadata needs to be shared between multiple import forms. They do not have a common parent.

Curently this is done via calculating once & passing via quickform. However, the formats
are convoluted & hard to read.

Information that is user generated should be calculated per form - not so much metadata
@eileenmcnaughton
Copy link
Contributor Author

eileenmcnaughton commented Aug 16, 2019

The removed commit is still retained in the WIP pr #15034 but doing an r-run you will hit #15048

//CRM-5125
//supporting import for contact subtypes
$csType = NULL;
if ($this->getContactSubType()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added functions to avoid enotices here

$csType = NULL;
if ($this->getContactSubType()) {
//custom fields for sub type
$subTypeFields = CRM_Core_BAO_CustomField::getFieldsForImport($this->getContactSubType());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

and here

*
* @return array
*/
protected function getRelationships(): array {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

And this is just a subset that is also useful elsewhere as it's own fn

@seamuslee001
Copy link
Contributor

(CiviCRM Review Template WORD-1.2)

@seamuslee001 seamuslee001 merged commit bdc9aa3 into civicrm:master Aug 18, 2019
@seamuslee001 seamuslee001 deleted the metadata branch August 18, 2019 03:36
@eileenmcnaughton
Copy link
Contributor Author

thanks @seamuslee001 this unblocks me on writing tests for the bug I'm chasing down

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