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-19836 Enable activity create from replies to CiviMail. #9655

Merged
merged 1 commit into from
Mar 9, 2017

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Jan 9, 2017

This relies on a parameter being added to the api call:

civicrm_api3('Job', 'fetch_bounces', array('is_create_activities' => TRUE));


* @param array $params
*/
function _civicrm_api3_job_fetch_bounces_spec(&$params) {
$params['is_create_activities'] = array(
Copy link
Member

Choose a reason for hiding this comment

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

I like the grammar of "is create activities" but what about "can I haz activities?"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no you may not! but if you look at the line below this comment, that is why I'm sure the var will always be set.

I'll fix the data block

@@ -48,14 +48,14 @@ class CRM_Utils_Mail_EmailProcessor {
* Always returns true (for the api). at a later stage we should
* fix this to return true on success / false on failure etc.
*/
public static function processBounces() {
public static function processBounces($is_create_activities) {
Copy link
Member

Choose a reason for hiding this comment

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

this variable needs to go in the docblock

api/v3/Job.php Outdated
@@ -358,7 +358,7 @@ function civicrm_api3_job_fetch_bounces($params) {
if (!$lock->isAcquired()) {
return civicrm_api3_create_error('Could not acquire lock, another EmailProcessor process is running');
}
if (!CRM_Utils_Mail_EmailProcessor::processBounces()) {
if (!CRM_Utils_Mail_EmailProcessor::processBounces($params['is_create_activities'])) {
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure this array key will always be set?

This relies on a parameter being added to the api call:

civicrm_api3('Job', 'fetch_bounces', array('is_create_activities' => TRUE));
@eileenmcnaughton
Copy link
Contributor Author

@colemanw I fixed the data block. I did a bit of clean up on the fantasy return value. processBounces is only called from the api, which handles the possibility a function that never returns false might someday feel like it. I put it out of it's misery

@colemanw
Copy link
Member

colemanw commented Mar 9, 2017

I think the can_haz_activities param might better belong in the options array rather than params but whatever, that's a mess we'll clean up in api4.

@colemanw colemanw merged commit 7515d91 into civicrm:master Mar 9, 2017
@colemanw colemanw deleted the activities branch March 9, 2017 15:19
monishdeb pushed a commit to monishdeb/civicrm-core that referenced this pull request May 2, 2017
CRM-19836 Enable activity create from replies to CiviMail.
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.

3 participants