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

Use eventID rather than the object in completeTransaction #18358

Merged
merged 1 commit into from
Sep 5, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

This allows us to stop passing it in as an object as a follow up. It potentially does an extra query but this is
mitigated by

  1. it only does a look up if the contribution does not already have a source (rare)
  2. it uses a pseudoconstant so would only look up a given event once
  • we can later remove the whole loading of event and just get the id from the participant record

Before

$objects['event']

passed around

After

just the id passed around

Technical Details

Comments

This allows us to stop passing it in as an object as a follow up. It potentially does an extra query but this is
mitigated by
1) it only does a look up if the contribution does not already have a source (rare)
2) it uses a pseudoconstant so would only look up a given event once

- we can later remove the whole loading of event and just get the id from the participant record
@civibot
Copy link

civibot bot commented Sep 4, 2020

(Standard links)

@civibot civibot bot added the master label Sep 4, 2020
@seamuslee001
Copy link
Contributor

This looks fine to me

@seamuslee001 seamuslee001 merged commit b89e8c1 into civicrm:master Sep 5, 2020
@seamuslee001 seamuslee001 deleted the event branch September 5, 2020 01:16
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Sep 5, 2020
This is a follow on to civicrm#18358 - after thinking
more I realised that I had already established this code is rarely reached and by
doing a very small query when it is we can avoid loading the whole event to pass in

Same test applies...
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Sep 5, 2020
This is a follow on to civicrm#18358 - after thinking
more I realised that I had already established this code is rarely reached and by
doing a very small query when it is we can avoid loading the whole event to pass in

Same test applies...
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