Add optional disabled campaign penalty for create matcher - #534
Conversation
| * @return float | ||
| * Penalty added to the contact's rating. | ||
| */ | ||
| public function adjustRatingOfDisabledCampaign($campaign_id, &$contact_probability) { |
There was a problem hiding this comment.
I think this could be private, couldn't it?
Can you add PHP type hints to the method signature? For $campaign_id I'd prefer to not allow strings.
|
|
||
| if (!array_key_exists($campaign_id, $campaign_is_active)) { | ||
| try { | ||
| $campaign = civicrm_api3('Campaign', 'getsingle', ['id' => $campaign_id]); |
There was a problem hiding this comment.
For new code we should use APIv4. You could limit the selected field to is_active.
| $campaign = civicrm_api3('Campaign', 'getsingle', ['id' => $campaign_id]); | ||
| } | ||
| catch (Exception $ex) { | ||
| $this->logMessage("Campaign [{$campaign_id}] could not be loaded, disabled-campaign penalty skipped.", 'warn'); |
There was a problem hiding this comment.
I'm wondering if the hyphen in disabled-campaign is correct English...
| public function testCampaignMatcherAppliesDisabledCampaignPenalty(): void { | ||
| $activity_type_id_used_in_matcher_config = 4; | ||
| try { | ||
| civicrm_api3('OptionValue', 'getsingle', [ |
There was a problem hiding this comment.
This is duplicated code from testCampaignMatcherBasic(). What about adding a setUp() method where is checked whether the activity type ID exists exists and if not, create an activity type (using APIv4)?
| /** | ||
| * Disabled campaigns can reduce the suggestion score if configured. | ||
| */ | ||
| public function testCampaignMatcherAppliesDisabledCampaignPenalty(): void { |
There was a problem hiding this comment.
👍 for the test. A test that checks whether the penalty is NOT applied if the campaign is active would be great.
dontub
added
status:needs work
3 participants
Refs #133
Summary
disabled_campaign_penaltyconfig value for the create campaign matcherTesting
phpandcvare not available in PATH