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

Fix CRM-20488: remove restrictions on soft credit contact type. #10419

Merged
merged 3 commits into from
Jun 19, 2017

Conversation

twomice
Copy link
Contributor

@twomice twomice commented May 26, 2017

I'm going with the notion that the correct solution here is just to remove the limitation on contact type for soft credits.


@twomice
Copy link
Contributor Author

twomice commented May 30, 2017

Removing "WIP" prefix now that all checks have passed. @jitendrapurohit and @monishdeb, this is related (indirectly at least) to work by you in CRM-18102 and CRM-13981. Are you able to comment or review?

@twomice twomice changed the title WIP: Fix CRM-20488: remove restrictions on soft credit contact type. Fix CRM-20488: remove restrictions on soft credit contact type. May 30, 2017
// which will be used to constraint soft-credit contact type in formRule, CRM-13981
if (!empty($profileId[0]) && !empty($profileId[2])) {
$form->_honoreeProfileType = CRM_Core_BAO_UFGroup::getContactType($profileId[0]);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If you remove this part, I think we don't have any purpose to have preProcess() here as it seems this function was only used to set _honoreeProfileType in $form. Can we remove preProcess() from this file then ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good point. Done.

Copy link
Contributor

Choose a reason for hiding this comment

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

Further,

  1. I think we should also remove any reference to this preProcess() to avoid any fatal errors caused by this change - one I can see is here.

  2. Remove definition of _honoreeProfileType as it doesn't seem to be used elsewhere.

    jitendra$ grep -irn honoreeProfileType CRM
    CRM/Contribute/Form/Contribution.php:157:  public $_honoreeProfileType;
    

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I should have thought of that. Will get you something soon. Thanks!

@monishdeb monishdeb force-pushed the CRM-20488_soft_credit_organization branch from 621a529 to 8cf9d8b Compare June 19, 2017 19:26
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.

@twomice @jitendrapurohit I have made a minor fix i.e. removed the SC::preProcess(...) call from codebase. And tested the patch, it works fine

@monishdeb monishdeb merged commit 9ab2d4b into civicrm:master Jun 19, 2017
@jitendrapurohit
Copy link
Contributor

@monishdeb worth removing unused var highlighted in point 2 of previous comment ?

@monishdeb
Copy link
Member

Oops, submitted a PR #10532 for that. Thanks

@yashodha
Copy link
Contributor

works great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants