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#4081 Ensure that if using the API to update an event templat… #25356

Merged
merged 1 commit into from
Jan 16, 2023

Conversation

seamuslee001
Copy link
Contributor

…e it is not converted to a non template

Overview

When using the API to update an Event template the is_template flag gets reset to be 0

Before

is_template flag is reset when updating via api

After

is_template flag is not reset

ping @demeritcowboy @eileenmcnaughton

@civibot
Copy link

civibot bot commented Jan 16, 2023

No issue was found matching the number given in the pull request title. Please check the issue number.

@civibot
Copy link

civibot bot commented Jan 16, 2023

(Standard links)

@civibot civibot bot added the master label Jan 16, 2023
@eileenmcnaughton
Copy link
Contributor

@seamuslee001 I think the suggestion I made on the gitlab is we ensure the field has a DB default & then we can remove those lines at the php layer

@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton I don't see your suggestion here https://lab.civicrm.org/dev/core/-/issues/4081 maybe there was a different gitlab issue but won't we still need these lines in case we have

is_template => 0 passed in with a template event id ? the problem here is that because is_template is not in the $params array i.e. the empty() is true then it is being cast to 0

@seamuslee001
Copy link
Contributor Author

sorry get your point now @eileenmcnaughton brain catching up, I see we have the default now set https://github.com/civicrm/civicrm-core/blob/master/xml/schema/Event/Event.xml#L751 and it was converted as of 5.49 so lets just get rid of those lines then https://github.com/civicrm/civicrm-core/blob/master/CRM/Upgrade/Incremental/php/FiveFortyNine/Event.bool.php#L19

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 thanks for the link - I agree the lines can go & then I can MOP it & the test should pass....

@eileenmcnaughton
Copy link
Contributor

& yes - there is a gitlab for the same logged by @bjendres (I haven't tracked down the number at this stage )

…e it is not converted to a non template

Remove unneded lines now that we have a sensible default value in the database
@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton done that now

@eileenmcnaughton eileenmcnaughton merged commit d69cf75 into civicrm:master Jan 16, 2023
@eileenmcnaughton eileenmcnaughton deleted the dev_core_4081 branch January 16, 2023 08:47
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.

2 participants