dev/event#25 Fix regression on billing name overwrite #16140
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
https://lab.civicrm.org/dev/event/issues/25 was caused by civicrm@b1b7f4e where it started overwriting the name in that function. In digging in the code I found that it seems that as long as we call this function before the processor all is good so we should call right before the processor rather than broader contamination. Also note ideally the settings bag would be used.
|
(Standard links)
|
|
I've tested this for the described issue in gitlab, multiple participant registrations, with multiple processors on a page, etc and looks fine to me. I'm not sure what the block of primary participant did before this PR, but I don't see any problem after its removal. @eileenmcnaughton Do you think there's any case I should be worried about before approving? If not, I think we're good to merge this change 👍 |
|
Merging based on @jitendrapurohit review. I'm happy from a code point of view. @eileenmcnaughton Do we need a 5.20 backport? |
|
@jitendrapurohit there is a lot of copy & paste in the participant code & stuff that did something once or is part of a house of cards... |
|
Here is the port #16140 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Fixes https://lab.civicrm.org/dev/event/issues/25 - where billing name can overwrite the first name
Before
See https://lab.civicrm.org/dev/event/issues/25 for good description
After
Doesn't overwrite first name.
Technical Details
https://lab.civicrm.org/dev/event/issues/25 was caused by b1b7f4e
where it started overwriting the name in that function.
In digging in the code I found that it seems that as long as we call this function before the processor all is
good so we should call right before the processor rather than broader contamination.
Also note ideally the settings bag would be used.
Comments
@mattwire