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

Remove always-true if #24410

Merged
merged 1 commit into from
Aug 30, 2022
Merged

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Remove always-true if

Before

Both of these ifs are always true

https://github.com/civicrm/civicrm-core/compare/master...eileenmcnaughton:civicrm-core:import_cont?expand=1#diff-a223445bfabeca3421466f7add0f896b2f7126287cbbd3124665781336f086d9L347-L349

the first is because the id is already loaded earlier - or aborted if not possible

In addition the if isUpdateExisting has these lines https://github.com/civicrm/civicrm-core/compare/master...eileenmcnaughton:civicrm-core:import_cont?expand=1#diff-a223445bfabeca3421466f7add0f896b2f7126287cbbd3124665781336f086d9L395-L404 - but they also occur outside the statement

After

Simplified - plus a couple of extraneous assignments

Technical Details

Comments

@colemanw should be straight forward

@civibot
Copy link

civibot bot commented Aug 29, 2022

(Standard links)

@@ -342,74 +342,50 @@ public function import(array $values): void {
$this->deprecatedFormatParams($paramValues, $formatted);

if ($this->isUpdateExisting()) {
//fix for CRM-2219 - Update Contribution
// onDuplicate == CRM_Import_Parser::DUPLICATE_UPDATE
if (!empty($paramValues['id'])) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@eileenmcnaughton just wondering what if a user had chosen to do an update import but only was matching on transaction_id field not on the contribution.id field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@seamuslee001 - it is resolved back here -

$existingContribution = $this->lookupContribution($entityKeyedParams['Contribution']);

@eileenmcnaughton
Copy link
Contributor Author

Fail relates here - will fix .testPledgeImport

@eileenmcnaughton eileenmcnaughton merged commit 02fcadf into civicrm:master Aug 30, 2022
@eileenmcnaughton eileenmcnaughton deleted the import_cont branch August 30, 2022 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants