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

Minor cleanup on import return codes #23626

Merged
merged 1 commit into from
May 31, 2022

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Minor cleanup on import return codes

Before

import was ignoring all return codes except NO_MATCH and UNPARSED_ADDRESS_WARNING however, wrangling was going on for the past version where it had to extract the error message which had been pushed onto $values

After

$returnCode only returned & handled for UNPARSED_ADDRESS_WARNING which I still need to work through. No crazy $values wrangling because it is ignored. NO_MATCH handled within import, like (almost) all the others are.

Technical Details

This has test cover - checking what is actually logged

Comments

@civibot
Copy link

civibot bot commented May 29, 2022

(Standard links)

array_unshift($values, $errorMessage);
$this->setImportStatus((int) $values[count($values) - 1], 'ERROR', $errorMessage);
return CRM_Import_Parser::ERROR;
$this->setImportStatus((int) $values[count($values) - 1], 'ERROR', $relatedNewContact['error_message']);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These could be $rowNumber - I didn't change them all cos I added that later on in the piece - but I've got a bunch of PRs that would need to be rebased if I change it in this PR - so I think it might be a post-cleanup

$this->assertEquals(CRM_Import_Parser::ERROR, $parser->import(CRM_Import_Parser::DUPLICATE_UPDATE, $values), 'Return code from parser import was not as expected');
$parser->init();
$parser->import(CRM_Import_Parser::DUPLICATE_UPDATE, $values);
$this->assertEquals(1, $dataSource->getRowCount([CRM_Import_Parser::ERROR]));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The other tests already use this method - ie check the actual import table not the return code (which there no longer is)

@eileenmcnaughton
Copy link
Contributor Author

@monishdeb if you have time to look - this is the first in the chain & then it reaches right up to #23622 - which gets us to almost where I am hoping to be for the rc

@monishdeb
Copy link
Member

Did r-run on my local, works fine. Code changes look good. test-build passes for the updated UT. Hence merging.

@monishdeb monishdeb merged commit 115f9a8 into civicrm:master May 31, 2022
@eileenmcnaughton eileenmcnaughton deleted the import_try branch May 31, 2022 05:26
@eileenmcnaughton
Copy link
Contributor Author

Thanks @monishdeb - if you are looking at others in the chain do you want me to rebase them now?

@eileenmcnaughton
Copy link
Contributor Author

the next one doesn't actually require a rebase - #23623

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