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

dev/core#766 fix failure to save custom data on template-based-event #19308

Merged
merged 1 commit into from
Jan 7, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

https://lab.civicrm.org/dev/core/-/issues/766

Before

custom data fails to save
image

After

saves

Technical Details

I decided that this was not a regression but that the fix was trying to format custom data in a way that was needed for the custom data retrieved from the template defaults but not for submitted custom data so I moved it

Comments

@civibot civibot bot added the master label Jan 2, 2021
@civibot
Copy link

civibot bot commented Jan 2, 2021

(Standard links)

@mattwire mattwire merged commit b97d805 into civicrm:master Jan 7, 2021
@seamuslee001 seamuslee001 deleted the ev branch January 7, 2021 10:33
@agileware-justin
Copy link
Contributor

This change caused regression, https://lab.civicrm.org/dev/event/-/issues/58

@eileenmcnaughton
Copy link
Contributor Author

Dang whackamole

@agileware-justin
Copy link
Contributor

And have re-opened https://lab.civicrm.org/dev/core/-/issues/766 as well, sorry about that.

@eileenmcnaughton
Copy link
Contributor Author

Yeah - we tend to lower the bar re tests for regressions - in part because of the rush & in part because the person doing the fix under pressure is generally not the person who broke it (this PR was perceived as a regression & hence worked on but ultimately it wasn't clear it was) - however we definitely have to make sure we fix this with a test - even if it delays it by a release given the history

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.

3 participants