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-21672 fix intra-rc regression, fatal when deleting participants #11538

Merged
merged 1 commit into from
Jan 18, 2018

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Jan 17, 2018

Overview

Follow up fix to recent merged change. Fixes fatal when deleting a participant

Before

Fatal error prevents deleting a participant

After

Screen works

Technical Details

The issue is the addField function relies on the form having a specific action. Instead of just not adding the field when a form is used in delete rather than view/create mode it fatals. I've never been terribly sold on re-using view/create forms for delete & I'm not convinced addField should fail so hard. However, here I've fixed it by doing an early return based on form action.

Comments

This was caused by a patch merged in the last few days


@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 FYI - just a regression we both missed - in case it triggers us to be a bit more wary of addField() (@colemanw )

@eileenmcnaughton eileenmcnaughton force-pushed the regress branch 2 times, most recently from 5144733 to 53b2809 Compare January 17, 2018 22:23
Change-Id: Iccbd17d53f07a93c74ce46e9539d1768019a2532
@seamuslee001
Copy link
Contributor

Changes make sense to me and can understand reasoning for them. Code is clean and passes unit tests

@eileenmcnaughton
Copy link
Contributor Author

Thanks @seamuslee001 merging - good to fix regressions ASAP before the work they create is duplicated

@eileenmcnaughton eileenmcnaughton merged commit 91da8b6 into civicrm:master Jan 18, 2018
@eileenmcnaughton eileenmcnaughton deleted the regress branch January 18, 2018 00:19
@mlutfy mlutfy added this to the 4.7.31 milestone Feb 9, 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.

4 participants