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

Regression: Fix delete event participant fatal error and display name if no email #13113

Merged
merged 1 commit into from Nov 16, 2018

Conversation

mattwire
Copy link
Contributor

Overview

Bizarrely, this #12775 seems to cause delete participant to fail because it tries to build the contribution search form before deleting and it can't create the cancel_reason field in "delete" context!

To reproduce search for an event participant (with no email address) and select the "delete" option.

Before

  • The delete "popup" doesn't load and there is a fatal error.
  • If the contact has no email address the name of the contact is not displayed in the title!

After

  • The delete "popup" loads and allows you to continue and delete the participant.
  • If the contact has no email address the contact name is still displayed in the title!

Technical Details

The event participant delete function was shared with the "edit" function on the core controller. When editing, the contribution form needs to be built and displayed - we don't need to do this for delete participant! Fix is to add a new delete action that only calls the required actions - should be a performance improvement too as we're not building a form we don't need.

Failure to display name is a slight aside, but found whilst debugging, if the contactID is set but has no email address the function assignContactEmailDetails() returns nothing. We add an additional call to call the contact API and get the display_name if it was not found in the email call.

Comments

Regression in master. But this should also improve code.

@alifrumin Is this something you could test for me? @eileenmcnaughton ping as it was your PR that introduced this regression I think! Though there's no way I'd have expected this side-effect.

@civibot
Copy link

civibot bot commented Nov 16, 2018

(Standard links)

@civibot civibot bot added the master label Nov 16, 2018
@alifrumin
Copy link
Contributor

I think this is ready to be merged.

I tested this by:

  1. Going to the CiviCRM Navigation Menu->Search->Find Participants and searching for participants.
  2. Selecting a Participant from the list who did not have an email address (Ivanov, Roland)
  3. Clicking delete for that participant.

Result on a site with out this pr:

  1. A network error message is displayed.
  2. The Participant record is not deleted.

networkerror

Result on the Jenkins build with this pr:

  1. that participant record is deleted
  2. The "Find Participants" search results are refreshed and no longer show that participant,
  3. A "Record Deleted" message is displayed

see screenshot below:

recorddeleted

@eileenmcnaughton eileenmcnaughton merged commit fba6012 into civicrm:master Nov 16, 2018
@eileenmcnaughton
Copy link
Contributor

thanks @alifrumin

@mattwire mattwire deleted the fix_delete_participant branch March 1, 2019 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants