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-21054 - Fix is_create_activities param for fetch_activities job #10846

Merged
merged 2 commits into from
Aug 12, 2017

Conversation

jitendrapurohit
Copy link
Contributor

@jitendrapurohit jitendrapurohit commented Aug 10, 2017

Overview

Fixes is_create_activities param for fetch_activities

Before

Process Activities job execution results into notice error.

After

Job executes correctly without errors.

Technical Details

is_create_activities param was introduced in #9655, but it missed the initialization of param in processActivities() function. This PR adds those params with specs as done for bounces.


api/v3/Job.php Outdated
*/
function _civicrm_api3_job_fetch_activities_spec(&$params) {
$params['is_create_activities'] = array(
'api.default' => 0,
Copy link
Contributor Author

@jitendrapurohit jitendrapurohit Aug 10, 2017

Choose a reason for hiding this comment

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

@eileenmcnaughton should this value be 1 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes - I think you are right! Will fix

Copy link
Contributor

Choose a reason for hiding this comment

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

No, just thinking - I think it should not be a param but rather be always set in the _fetch_activities function - since there is no point calling that fn without creating activities

@jitendrapurohit jitendrapurohit changed the title Fix is_create_activities param for fetch_activities CRM-21054 - Fix is_create_activities param for fetch_activities job Aug 10, 2017
@eileenmcnaughton
Copy link
Contributor

@jitendrapurohit ie I think this line

CRM_Utils_Mail_EmailProcessor::processActivities($params['is_create_activities']);

should likely be

CRM_Utils_Mail_EmailProcessor::processActivities(TRUE);

https://github.com/jitendrapurohit/civicrm-core/blob/ab26285769791e987e0d2eb7ce70202edf46a59e/api/v3/Job.php#L407

@jitendrapurohit
Copy link
Contributor Author

jitendrapurohit commented Aug 12, 2017

agree with the change and updated the PR. Thanks @eileenmcnaughton

Update: Just modified the uninitialized variable to TRUE instead of passing through the params. This function is only called from fetch_activities job api, so seems safe to me.

@eileenmcnaughton
Copy link
Contributor

Ok - I grepped & I agree that this is only called from that function so I'm merging

@eileenmcnaughton eileenmcnaughton merged commit 2c2409c into civicrm:master Aug 12, 2017
@jitendrapurohit jitendrapurohit deleted the CRM-21054 branch August 21, 2017 03:17
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