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

CRM-21764: Recurring Events without Price Set fail to save #11837

Merged
merged 1 commit into from
Mar 22, 2018

Conversation

agilewarealok
Copy link
Contributor

@agilewarealok agilewarealok commented Mar 20, 2018

Overview

Steps to reproduce:

  1. Create a recurring event with a bit of recurrence
  2. Edit a specific event, update any field, such as the event description text field
  3. Save, select "only this event"

Before

Nothing happens, Event is not really saved. CRM/Core/Page/AJAX/RecurringEntity.php::updateModeAndPriceSet fails because it returns an empty array, causing the save action to fail without any errors or notices.

After

We update price set only if we have any which results in expected response from updateModeAndPriceSet of CRM/Core/Page/AJAX/RecurringEntity.php. Now event details are getting saved as expected.

Agileware Ref: CIVICRM-831


@seamuslee001
Copy link
Contributor

Jenkins test this please

@eileenmcnaughton
Copy link
Contributor

test this please

@jusfreeman
Copy link
Contributor

@eileenmcnaughton good to merge?

@eileenmcnaughton
Copy link
Contributor

I haven't had time to test yet :-(

I did spot one thing I think @agilewarealok needs to fix - can you address the file permissions - you shouldn't be changing them. I use

git config core.filemode false

for that - either globally or in the repo

@agilewarealok
Copy link
Contributor Author

@eileenmcnaughton

Sure I've updated the permissions for this PR and will take care of it in future.

Thanks!

@eileenmcnaughton
Copy link
Contributor

I test this & replicated the bug & verified this fixed it & the changes make sense. Obviously I would prefer to have a unit test but I realise this is a quick turn around fix on a regression & appreciate the fact you did that. I'd also prefer to use a one-line api call over the DAO->find - but I don't think that's a blocker.

Merging - thank you @agilewarealok & @jusfreeman

@eileenmcnaughton eileenmcnaughton merged commit f3ffb29 into civicrm:5.0 Mar 22, 2018
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.

5 participants