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-21069 Fix crash when event_id is not set #10865

Merged
merged 2 commits into from
Aug 28, 2017

Conversation

mattwire
Copy link
Contributor

@mattwire mattwire commented Aug 16, 2017

Overview

If the user has an event registration but the event has been deleted CiviCRM crashes with a CRM_Core_Error::fatal because the event search still tries to lookup the event details. This patch checks the event_id is set, and skips the registration if it is not.

Before

CiviCRM crashes with CRM_Core_Error::fatal()

After

CiviCRM shows all event registrations that have valid events.

Technical Details

It is unlikely to happen unless data has been modified directly in the database.


@eileenmcnaughton
Copy link
Contributor

test this please

@eileenmcnaughton
Copy link
Contributor

(looks like an unrelated fail)

@MegaphoneJon
Copy link
Contributor

I just reviewed this and confirm that it makes sense both at a code level, and that I was able to replicate the bug and confirm it's fixed by this patch.

FWIW, foreign keys/cascading deletes mean that this scenario really shouldn't ever happen. I had to disable foreign key checks to replicate this bug - but better in than out, I suppose.

@eileenmcnaughton
Copy link
Contributor

This makes me uncomfortable - handling data errors rather than forcing people to fix it. @colemanw @totten what are your thoughts? One one hand this is safe & fixes an experienced problem. On the other hand I feel like if people mess up their data we shouldn't 'just work anyway'.

@mattwire
Copy link
Contributor Author

@eileenmcnaughton I totally agree there shouldn't be bad data, but sometimes there is! In this case the impact without this patch is you cannot access the contact event tab at all, as it dies with a "fatal" and no reason why. With the patch the system continues working, without displaying the corrupt event registrations. I've just updated the patch to write a warning to the log file if this happens - so a sysadmin is made aware it should be fixed.

@eileenmcnaughton
Copy link
Contributor

ok this is a pretty rare event & while not ideal it does help in some circumstance - I think enough time has gone on it now - merging

@eileenmcnaughton eileenmcnaughton merged commit 82f6a75 into civicrm:master Aug 28, 2017
@mattwire mattwire deleted the CRM-21069_check_eventid branch September 5, 2017 14:16
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.

4 participants