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-21004 only remove html snippet task if setting enabled #10802

Merged
merged 2 commits into from
Aug 3, 2017

Conversation

lcdservices
Copy link
Contributor

@lcdservices lcdservices commented Aug 2, 2017

Overview

Enable profile copy for mixed use profiles when html snippet is disabled.

Technical Details

Fixes a regression since the "accept profile submissions from external sites" setting was added. The html snippet action was getting removed twice if that setting was turned off, which resulted in the copy action also being removed.


@monishdeb
Copy link
Member

Jenkin test this please

@monishdeb
Copy link
Member

@lcdservices alternatively this could be implemented as https://gist.github.com/monishdeb/f37ae22c27c410acb3db082d8421bc29 as HTML Form Snippet action link is only added if that setting is enabled, see

Apart from that it's working fine

@lcdservices
Copy link
Contributor Author

@monishdeb agreed -- I think that's better. It accounts for other circumstances where the html snippet action may not be present. updated the PR.

Copy link
Member

@monishdeb monishdeb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested working fine. @colemanw / @eileenmcnaughton can you please review and merge this PR?

Thanks!

@eileenmcnaughton
Copy link
Contributor

Looks like @lcdservices & @monishdeb have agreed the fix here & my role is just to click go

@eileenmcnaughton eileenmcnaughton merged commit 0c369e8 into civicrm:master Aug 3, 2017
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